From 001648037055985cc10f38d6ba1e0e71cac49af7 Mon Sep 17 00:00:00 2001 From: Daniel Kesselberg Date: Sun, 29 Sep 2019 20:57:00 +0200 Subject: [PATCH] Decouple resource provider registration Signed-off-by: Daniel Kesselberg --- apps/files/lib/AppInfo/Application.php | 8 +- lib/composer/composer/autoload_classmap.php | 2 + lib/composer/composer/autoload_static.php | 2 + .../DependencyInjection/DIContainer.php | 1 + .../Collaboration/Resources/Manager.php | 36 ++---- .../Resources/ProviderManager.php | 70 +++++++++++ lib/private/Server.php | 1 + .../Collaboration/Resources/IManager.php | 1 + .../Resources/IProviderManager.php | 41 +++++++ .../Collaboration/Resources/ManagerTest.php | 62 ++++++++++ .../Resources/ProviderManagerTest.php | 111 ++++++++++++++++++ 11 files changed, 304 insertions(+), 31 deletions(-) create mode 100644 lib/private/Collaboration/Resources/ProviderManager.php create mode 100644 lib/public/Collaboration/Resources/IProviderManager.php create mode 100644 tests/lib/Collaboration/Resources/ManagerTest.php create mode 100644 tests/lib/Collaboration/Resources/ProviderManagerTest.php diff --git a/apps/files/lib/AppInfo/Application.php b/apps/files/lib/AppInfo/Application.php index afd9f78874..b8770caa4a 100644 --- a/apps/files/lib/AppInfo/Application.php +++ b/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 */ diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 5073081370..8d261f3495 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/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', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 5f7e8fde98..806a36e3c6 100644 --- a/lib/composer/composer/autoload_static.php +++ b/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', diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php index 5d7f60265e..1304153292 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/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); } diff --git a/lib/private/Collaboration/Resources/Manager.php b/lib/private/Collaboration/Resources/Manager.php index 0c1336bf09..82731cbdb4 100644 --- a/lib/private/Collaboration/Resources/Manager.php +++ b/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); } /** diff --git a/lib/private/Collaboration/Resources/ProviderManager.php b/lib/private/Collaboration/Resources/ProviderManager.php new file mode 100644 index 0000000000..059e42daba --- /dev/null +++ b/lib/private/Collaboration/Resources/ProviderManager.php @@ -0,0 +1,70 @@ + + * + * @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 . + * + */ + +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; + } +} diff --git a/lib/private/Server.php b/lib/private/Server.php index f49bc2364b..d16b55ac21 100644 --- a/lib/private/Server.php +++ b/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) { diff --git a/lib/public/Collaboration/Resources/IManager.php b/lib/public/Collaboration/Resources/IManager.php index 6cdd0e5a6e..69f42bbbe0 100644 --- a/lib/public/Collaboration/Resources/IManager.php +++ b/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; } diff --git a/lib/public/Collaboration/Resources/IProviderManager.php b/lib/public/Collaboration/Resources/IProviderManager.php new file mode 100644 index 0000000000..04a1212b31 --- /dev/null +++ b/lib/public/Collaboration/Resources/IProviderManager.php @@ -0,0 +1,41 @@ + + * + * @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 . + * + */ + +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; +} diff --git a/tests/lib/Collaboration/Resources/ManagerTest.php b/tests/lib/Collaboration/Resources/ManagerTest.php new file mode 100644 index 0000000000..f59c2913c8 --- /dev/null +++ b/tests/lib/Collaboration/Resources/ManagerTest.php @@ -0,0 +1,62 @@ + + * + * @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 . + * + */ + +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'); + } +} diff --git a/tests/lib/Collaboration/Resources/ProviderManagerTest.php b/tests/lib/Collaboration/Resources/ProviderManagerTest.php new file mode 100644 index 0000000000..d8bebe8fa6 --- /dev/null +++ b/tests/lib/Collaboration/Resources/ProviderManagerTest.php @@ -0,0 +1,111 @@ + + * + * @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 . + * + */ + +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); + } +}