Merge pull request #17325 from nextcloud/enh/decouple-register-resource-provider

Decouple resource provider registration

apps/files/lib/AppInfo/Application.php

@@ -41,7 +41,7 @@ use OCA\Files\Listener\LoadSidebarListener;
 use OCA\Files\Notification\Notifier;
 use OCA\Files\Service\TagService;
 use OCP\AppFramework\App;
-use OCP\Collaboration\Resources\IManager;
+use OCP\Collaboration\Resources\IProviderManager;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IContainer;
 
@@ -92,9 +92,9 @@ class Application extends App {
 		/**
 		 * Register Collaboration ResourceProvider
 		 */
-		/** @var IManager $resourceManager */
-		$resourceManager = $container->query(IManager::class);
-		$resourceManager->registerResourceProvider(ResourceProvider::class);
+		/** @var IProviderManager $providerManager */
+		$providerManager = $container->query(IProviderManager::class);
+		$providerManager->registerResourceProvider(ResourceProvider::class);
 		Listener::register($server->getEventDispatcher());
 
 		/** @var IEventDispatcher $dispatcher */

lib/composer/composer/autoload_classmap.php

@@ -125,6 +125,7 @@ return array(
     'OCP\\Collaboration\\Resources\\ICollection' => $baseDir . '/lib/public/Collaboration/Resources/ICollection.php',
     'OCP\\Collaboration\\Resources\\IManager' => $baseDir . '/lib/public/Collaboration/Resources/IManager.php',
     'OCP\\Collaboration\\Resources\\IProvider' => $baseDir . '/lib/public/Collaboration/Resources/IProvider.php',
+    'OCP\\Collaboration\\Resources\\IProviderManager' => $baseDir . '/lib/public/Collaboration/Resources/IProviderManager.php',
     'OCP\\Collaboration\\Resources\\IResource' => $baseDir . '/lib/public/Collaboration/Resources/IResource.php',
     'OCP\\Collaboration\\Resources\\ResourceException' => $baseDir . '/lib/public/Collaboration/Resources/ResourceException.php',
     'OCP\\Command\\IBus' => $baseDir . '/lib/public/Command/IBus.php',
@@ -637,6 +638,7 @@ return array(
     'OC\\Collaboration\\Resources\\Collection' => $baseDir . '/lib/private/Collaboration/Resources/Collection.php',
     'OC\\Collaboration\\Resources\\Listener' => $baseDir . '/lib/private/Collaboration/Resources/Listener.php',
     'OC\\Collaboration\\Resources\\Manager' => $baseDir . '/lib/private/Collaboration/Resources/Manager.php',
+    'OC\\Collaboration\\Resources\\ProviderManager' => $baseDir . '/lib/private/Collaboration/Resources/ProviderManager.php',
     'OC\\Collaboration\\Resources\\Resource' => $baseDir . '/lib/private/Collaboration/Resources/Resource.php',
     'OC\\Color' => $baseDir . '/lib/private/Color.php',
     'OC\\Command\\AsyncBus' => $baseDir . '/lib/private/Command/AsyncBus.php',

lib/composer/composer/autoload_static.php

@@ -154,6 +154,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OCP\\Collaboration\\Resources\\ICollection' => __DIR__ . '/../../..' . '/lib/public/Collaboration/Resources/ICollection.php',
         'OCP\\Collaboration\\Resources\\IManager' => __DIR__ . '/../../..' . '/lib/public/Collaboration/Resources/IManager.php',
         'OCP\\Collaboration\\Resources\\IProvider' => __DIR__ . '/../../..' . '/lib/public/Collaboration/Resources/IProvider.php',
+        'OCP\\Collaboration\\Resources\\IProviderManager' => __DIR__ . '/../../..' . '/lib/public/Collaboration/Resources/IProviderManager.php',
         'OCP\\Collaboration\\Resources\\IResource' => __DIR__ . '/../../..' . '/lib/public/Collaboration/Resources/IResource.php',
         'OCP\\Collaboration\\Resources\\ResourceException' => __DIR__ . '/../../..' . '/lib/public/Collaboration/Resources/ResourceException.php',
         'OCP\\Command\\IBus' => __DIR__ . '/../../..' . '/lib/public/Command/IBus.php',
@@ -666,6 +667,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OC\\Collaboration\\Resources\\Collection' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Resources/Collection.php',
         'OC\\Collaboration\\Resources\\Listener' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Resources/Listener.php',
         'OC\\Collaboration\\Resources\\Manager' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Resources/Manager.php',
+        'OC\\Collaboration\\Resources\\ProviderManager' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Resources/ProviderManager.php',
         'OC\\Collaboration\\Resources\\Resource' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Resources/Resource.php',
         'OC\\Color' => __DIR__ . '/../../..' . '/lib/private/Color.php',
         'OC\\Command\\AsyncBus' => __DIR__ . '/../../..' . '/lib/private/Command/AsyncBus.php',

lib/private/AppFramework/DependencyInjection/DIContainer.php

@@ -290,6 +290,7 @@ class DIContainer extends SimpleContainer implements IAppContainer {
 			return $dispatcher;
 		});
 
+		$this->registerAlias(\OCP\Collaboration\Resources\IProviderManager::class, OC\Collaboration\Resources\ProviderManager::class);
 		$this->registerAlias(\OCP\Collaboration\Resources\IManager::class, OC\Collaboration\Resources\Manager::class);
 	}
 

lib/private/Collaboration/Resources/Manager.php

@@ -35,6 +35,7 @@ use OCP\Collaboration\Resources\CollectionException;
 use OCP\Collaboration\Resources\ICollection;
 use OCP\Collaboration\Resources\IManager;
 use OCP\Collaboration\Resources\IProvider;
+use OCP\Collaboration\Resources\IProviderManager;
 use OCP\Collaboration\Resources\IResource;
 use OCP\Collaboration\Resources\ResourceException;
 use OCP\DB\QueryBuilder\IQueryBuilder;
@@ -50,17 +51,18 @@ class Manager implements IManager {
 
 	/** @var IDBConnection */
 	protected $connection;
+	/** @var IProviderManager */
+	protected $providerManager;
 	/** @var ILogger */
 	protected $logger;
 
 	/** @var string[] */
 	protected $providers = [];
 
-	/** @var IProvider[] */
-	protected $providerInstances = [];
 
-	public function __construct(IDBConnection $connection, ILogger $logger) {
+	public function __construct(IDBConnection $connection, IProviderManager $providerManager, ILogger $logger) {
 		$this->connection = $connection;
+		$this->providerManager = $providerManager;
 		$this->logger = $logger;
 	}
 
@@ -273,27 +275,6 @@ class Manager implements IManager {
 		return $resources;
 	}
 
-	/**
-	 * @return IProvider[]
-	 * @since 16.0.0
-	 */
-	public function getProviders(): array {
-		if (!empty($this->providers)) {
-			foreach ($this->providers as $provider) {
-				try {
-					$this->providerInstances[] = \OC::$server->query($provider);
-				} catch (QueryException $e) {
-					$this->logger->logException($e, [
-						'message' => 'Error when instantiating resource provider'
-					]);
-				}
-			}
-			$this->providers = [];
-		}
-
-		return $this->providerInstances;
-	}
-
 	/**
 	 * Get the rich object data of a resource
 	 *
@@ -302,7 +283,7 @@ class Manager implements IManager {
 	 * @since 16.0.0
 	 */
 	public function getResourceRichObject(IResource $resource): array {
-		foreach ($this->getProviders() as $provider) {
+		foreach ($this->providerManager->getResourceProviders() as $provider) {
 			if ($provider->getType() === $resource->getType()) {
 				try {
 					return $provider->getResourceRichObject($resource);
@@ -329,7 +310,7 @@ class Manager implements IManager {
 		}
 
 		$access = false;
-		foreach ($this->getProviders() as $provider) {
+		foreach ($this->providerManager->getResourceProviders() as $provider) {
 			if ($provider->getType() === $resource->getType()) {
 				try {
 					if ($provider->canAccessResource($resource, $user)) {
@@ -532,7 +513,8 @@ class Manager implements IManager {
 	 * @param string $provider
 	 */
 	public function registerResourceProvider(string $provider): void {
-		$this->providers[] = $provider;
+		$this->logger->debug('\OC\Collaboration\Resources\Manager::registerResourceProvider is deprecated', ['provider' => $provider]);
+		$this->providerManager->registerResourceProvider($provider);
 	}
 
 	/**

lib/private/Collaboration/Resources/ProviderManager.php

@@ -0,0 +1,70 @@
+<?php
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2019 Daniel Kesselberg <mail@danielkesselberg.de>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OC\Collaboration\Resources;
+
+use OCP\AppFramework\QueryException;
+use OCP\Collaboration\Resources\IProvider;
+use OCP\Collaboration\Resources\IProviderManager;
+use OCP\ILogger;
+use OCP\IServerContainer;
+
+class ProviderManager implements IProviderManager {
+
+	/** @var string[] */
+	protected $providers = [];
+
+	/** @var IProvider[] */
+	protected $providerInstances = [];
+
+	/** @var IServerContainer */
+	protected $serverContainer;
+
+	/** @var ILogger */
+	protected $logger;
+
+	public function __construct(IServerContainer $serverContainer, ILogger $logger) {
+		$this->serverContainer = $serverContainer;
+		$this->logger = $logger;
+	}
+
+	public function getResourceProviders(): array {
+		if ($this->providers !== []) {
+			foreach ($this->providers as $provider) {
+				try {
+					$this->providerInstances[] = $this->serverContainer->query($provider);
+				} catch (QueryException $e) {
+					$this->logger->logException($e, [
+						'message' => "Could not query resource provider $provider: " . $e->getMessage()
+					]);
+				}
+			}
+			$this->providers = [];
+		}
+
+		return $this->providerInstances;
+	}
+
+	public function registerResourceProvider(string $provider): void {
+		$this->providers[] = $provider;
+	}
+}

lib/private/Server.php

@@ -1086,6 +1086,7 @@ class Server extends ServerContainer implements IServerContainer {
 
 		$this->registerAlias(\OCP\Collaboration\AutoComplete\IManager::class, \OC\Collaboration\AutoComplete\Manager::class);
 
+		$this->registerAlias(\OCP\Collaboration\Resources\IProviderManager::class, \OC\Collaboration\Resources\ProviderManager::class);
 		$this->registerAlias(\OCP\Collaboration\Resources\IManager::class, \OC\Collaboration\Resources\Manager::class);
 
 		$this->registerService('SettingsManager', function (Server $c) {

lib/public/Collaboration/Resources/IManager.php

@@ -121,6 +121,7 @@ interface IManager extends IProvider {
 	/**
 	 * @param string $provider
 	 * @since 16.0.0
+	 * @deprecated 18.0.0 Use IProviderManager::registerResourceProvider instead
 	 */
 	public function registerResourceProvider(string $provider): void;
 }

lib/public/Collaboration/Resources/IProviderManager.php

@@ -0,0 +1,41 @@
+<?php
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2019 Daniel Kesselberg <mail@danielkesselberg.de>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCP\Collaboration\Resources;
+
+/**
+ * @since 18.0.0
+ */
+interface IProviderManager {
+
+	/**
+	 * @return IProvider[] list of resource providers
+	 * @since 18.0.0
+	 */
+	public function getResourceProviders(): array;
+
+	/**
+	 * @param string $provider provider's class name
+	 * @since 18.0.0
+	 */
+	public function registerResourceProvider(string $provider): void;
+}

tests/lib/Collaboration/Resources/ManagerTest.php

@@ -0,0 +1,62 @@
+<?php
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2019 Daniel Kesselberg <mail@danielkesselberg.de>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace Test\Collaboration\Resources;
+
+use OC\Collaboration\Resources\Manager;
+use OCP\Collaboration\Resources\IManager;
+use OCP\Collaboration\Resources\IProviderManager;
+use OCP\IDBConnection;
+use OCP\ILogger;
+use Test\TestCase;
+
+class ManagerTest extends TestCase {
+
+	/** @var ILogger */
+	protected $logger;
+	/** @var IProviderManager */
+	protected $providerManager;
+	/** @var IManager */
+	protected $manager;
+
+	protected function setUp(): void {
+		parent::setUp();
+
+		$this->logger = $this->createMock(ILogger::class);
+		$this->providerManager = $this->createMock(IProviderManager::class);
+
+		/** @var IDBConnection $connection */
+		$connection = $this->createMock(IDBConnection::class);
+		$this->manager = new Manager($connection, $this->providerManager, $this->logger);
+	}
+	
+	public function testRegisterResourceProvider(): void {
+		$this->logger->expects($this->once())
+			->method('debug')
+			->with($this->equalTo('\OC\Collaboration\Resources\Manager::registerResourceProvider is deprecated'), $this->equalTo(['provider' => 'AwesomeResourceProvider']));
+		$this->providerManager->expects($this->once())
+			->method('registerResourceProvider')
+			->with($this->equalTo('AwesomeResourceProvider'));
+
+		$this->manager->registerResourceProvider('AwesomeResourceProvider');
+	}
+}

tests/lib/Collaboration/Resources/ProviderManagerTest.php

@@ -0,0 +1,111 @@
+<?php
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2019 Daniel Kesselberg <mail@danielkesselberg.de>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace Test\Collaboration\Resources;
+
+use OC\Collaboration\Resources\ProviderManager;
+use OCA\Files\Collaboration\Resources\ResourceProvider;
+use OCP\AppFramework\QueryException;
+use OCP\Collaboration\Resources\IProviderManager;
+use OCP\ILogger;
+use OCP\IServerContainer;
+use Test\TestCase;
+
+class ProviderManagerTest extends TestCase {
+
+	/** @var IServerContainer */
+	protected $serverContainer;
+	/** @var ILogger */
+	protected $logger;
+	/** @var IProviderManager */
+	protected $providerManager;
+
+	protected function setUp(): void {
+		parent::setUp();
+
+		$this->serverContainer = $this->createMock(IServerContainer::class);
+		$this->logger = $this->createMock(ILogger::class);
+
+		$this->providerManager = new class($this->serverContainer, $this->logger) extends ProviderManager {
+			public function countProviders(): int {
+				return count($this->providers);
+			}
+		};
+	}
+
+	public function testRegisterResourceProvider(): void {
+		$this->providerManager->registerResourceProvider('AwesomeResourceProvider');
+		$this->assertSame(1, $this->providerManager->countProviders());
+	}
+
+	public function testGetResourceProvidersNoProvider(): void {
+		$this->assertCount(0, $this->providerManager->getResourceProviders());
+	}
+
+	public function testGetResourceProvidersValidProvider(): void {
+		$this->serverContainer->expects($this->once())
+			->method('query')
+			->with($this->equalTo(ResourceProvider::class))
+			->willReturn($this->createMock(ResourceProvider::class));
+
+		$this->providerManager->registerResourceProvider(ResourceProvider::class);
+		$resourceProviders = $this->providerManager->getResourceProviders();
+
+		$this->assertCount(1, $resourceProviders);
+		$this->assertInstanceOf(ResourceProvider::class, $resourceProviders[0]);
+	}
+
+	public function testGetResourceProvidersInvalidProvider(): void {
+		$this->serverContainer->expects($this->once())
+			->method('query')
+			->with($this->equalTo('InvalidResourceProvider'))
+			->willThrowException(new QueryException('A meaningful error message'));
+
+		$this->logger->expects($this->once())
+			->method('logException');
+
+		$this->providerManager->registerResourceProvider('InvalidResourceProvider');
+		$resourceProviders = $this->providerManager->getResourceProviders();
+
+		$this->assertCount(0, $resourceProviders);
+	}
+
+	public function testGetResourceProvidersValidAndInvalidProvider(): void {
+		$this->serverContainer->expects($this->at(0))
+			->method('query')
+			->with($this->equalTo('InvalidResourceProvider'))
+			->willThrowException(new QueryException('A meaningful error message'));
+		$this->serverContainer->expects($this->at(1))
+			->method('query')
+			->with($this->equalTo(ResourceProvider::class))
+			->willReturn($this->createMock(ResourceProvider::class));
+
+		$this->logger->expects($this->once())
+			->method('logException');
+
+		$this->providerManager->registerResourceProvider('InvalidResourceProvider');
+		$this->providerManager->registerResourceProvider(ResourceProvider::class);
+		$resourceProviders = $this->providerManager->getResourceProviders();
+
+		$this->assertCount(1, $resourceProviders);
+	}
+}