Merge pull request #886 from nextcloud/capabilities_error_proof

Make the capabilities manager more error proof

lib/private/CapabilitiesManager.php

@@ -22,15 +22,22 @@
 namespace OC;
 
 
+use OCP\AppFramework\QueryException;
 use OCP\Capabilities\ICapability;
+use OCP\ILogger;
 
 class CapabilitiesManager {
 
-	/**
-	 * @var \Closure[]
-	 */
+	/** @var \Closure[] */
 	private $capabilities = array();
 
+	/** @var ILogger */
+	private $logger;
+
+	public function __construct(ILogger $logger) {
+		$this->logger = $logger;
+	}
+
 	/**
 	 * Get an array of al the capabilities that are registered at this manager
      *
@@ -40,7 +47,13 @@ class CapabilitiesManager {
 	public function getCapabilities() {
 		$capabilities = [];
 		foreach($this->capabilities as $capability) {
-			$c = $capability();
+			try {
+				$c = $capability();
+			} catch (QueryException $e) {
+				$this->logger->error('CapabilitiesManager: {message}', ['app' => 'core', 'message' => $e->getMessage()]);
+				continue;
+			}
+
 			if ($c instanceof ICapability) {
 				$capabilities = array_replace_recursive($capabilities, $c->getCapabilities());
 			} else {

lib/private/Server.php

@@ -629,7 +629,7 @@ class Server extends ServerContainer implements IServerContainer {
 			return new Manager();
 		});
 		$this->registerService('CapabilitiesManager', function (Server $c) {
-			$manager = new \OC\CapabilitiesManager();
+			$manager = new \OC\CapabilitiesManager($c->getLogger());
 			$manager->registerCapability(function () use ($c) {
 				return new \OC\OCS\CoreCapabilities($c->getConfig());
 			});

tests/lib/CapabilitiesManagerTest.php

@@ -21,14 +21,29 @@
 
 namespace Test;
 
+use OC\CapabilitiesManager;
+use OCP\AppFramework\QueryException;
+use OCP\Capabilities\ICapability;
+use OCP\ILogger;
+
 class CapabilitiesManagerTest extends TestCase {
 
+	/** @var CapabilitiesManager */
+	private $manager;
+
+	/** @var ILogger */
+	private $logger;
+
+	public function setUp() {
+		$this->logger = $this->getMockBuilder('OCP\ILogger')->getMock();
+		$this->manager = new CapabilitiesManager($this->logger);
+	}
+
 	/**
 	 * Test no capabilities
 	 */
 	public function testNoCapabilities() {
-		$manager = new \OC\CapabilitiesManager();
-		$res = $manager->getCapabilities();
+		$res = $this->manager->getCapabilities();
 		$this->assertEmpty($res);
 	}
 
@@ -36,13 +51,11 @@ class CapabilitiesManagerTest extends TestCase {
 	 * Test a valid capabilitie
 	 */
 	public function testValidCapability() {
-		$manager = new \OC\CapabilitiesManager();
-
-		$manager->registerCapability(function() {
+		$this->manager->registerCapability(function() {
 			return new SimpleCapability();
 		});
 
-		$res = $manager->getCapabilities();
+		$res = $this->manager->getCapabilities();
 		$this->assertEquals(['foo' => 1], $res);
 	}
 
@@ -52,13 +65,11 @@ class CapabilitiesManagerTest extends TestCase {
 	 * @expectedExceptionMessage The given Capability (Test\NoCapability) does not implement the ICapability interface
 	 */
 	public function testNoICapability() {
-		$manager = new \OC\CapabilitiesManager();
-
-		$manager->registerCapability(function() {
+		$this->manager->registerCapability(function() {
 			return new NoCapability();
 		});
 
-		$res = $manager->getCapabilities();
+		$res = $this->manager->getCapabilities();
 		$this->assertEquals([], $res);
 	}
 
@@ -66,19 +77,17 @@ class CapabilitiesManagerTest extends TestCase {
 	 * Test a bunch of merged Capabilities
 	 */
 	public function testMergedCapabilities() {
-		$manager = new \OC\CapabilitiesManager();
-
-		$manager->registerCapability(function() {
+		$this->manager->registerCapability(function() {
 			return new SimpleCapability();
 		});
-		$manager->registerCapability(function() {
+		$this->manager->registerCapability(function() {
 			return new SimpleCapability2();
 		});
-		$manager->registerCapability(function() {
+		$this->manager->registerCapability(function() {
 			return new SimpleCapability3();
 		});
 
-		$res = $manager->getCapabilities();
+		$res = $this->manager->getCapabilities();
 		$expected = [
 			'foo' => 1,
 			'bar' => [
@@ -94,16 +103,14 @@ class CapabilitiesManagerTest extends TestCase {
 	 * Test deep identical capabilities
 	 */
 	public function testDeepIdenticalCapabilities() {
-		$manager = new \OC\CapabilitiesManager();
-
-		$manager->registerCapability(function() {
+		$this->manager->registerCapability(function() {
 			return new DeepCapability();
 		});
-		$manager->registerCapability(function() {
+		$this->manager->registerCapability(function() {
 			return new DeepCapability();
 		});
 
-		$res = $manager->getCapabilities();
+		$res = $this->manager->getCapabilities();
 		$expected = [
 			'foo' => [
 				'bar' => [
@@ -114,9 +121,22 @@ class CapabilitiesManagerTest extends TestCase {
 		
 		$this->assertEquals($expected, $res);
 	}
+
+	public function testInvalidCapability() {
+		$this->manager->registerCapability(function () {
+			throw new QueryException();
+		});
+
+		$this->logger->expects($this->once())
+			->method('error');
+
+		$res = $this->manager->getCapabilities();
+
+		$this->assertEquals([], $res);
+	}
 }
 
-class SimpleCapability implements \OCP\Capabilities\ICapability {
+class SimpleCapability implements ICapability {
 	public function getCapabilities() {
 		return [
 			'foo' => 1
@@ -124,7 +144,7 @@ class SimpleCapability implements \OCP\Capabilities\ICapability {
 	}
 }
 
-class SimpleCapability2 implements \OCP\Capabilities\ICapability {
+class SimpleCapability2 implements ICapability {
 	public function getCapabilities() {
 		return [
 			'bar' => ['x' => 1]
@@ -132,7 +152,7 @@ class SimpleCapability2 implements \OCP\Capabilities\ICapability {
 	}
 }
 
-class SimpleCapability3 implements \OCP\Capabilities\ICapability {
+class SimpleCapability3 implements ICapability {
 	public function getCapabilities() {
 		return [
 			'bar' => ['y' => 2]
@@ -148,7 +168,7 @@ class NoCapability {
 	}
 }
 
-class DeepCapability implements \OCP\Capabilities\ICapability {
+class DeepCapability implements ICapability {
 	public function getCapabilities() {
 		return [
 			'foo' => [
@@ -159,4 +179,3 @@ class DeepCapability implements \OCP\Capabilities\ICapability {
 		];
 	}
 }
-