From f03f88b437511ef29bad9e13c0d1a16e24c50bc8 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Tue, 14 Jul 2020 10:21:39 +0200 Subject: [PATCH] Delegate bootstrap registration lazily * Keep the registration context * Expose the context object for other components * Ensure registration is only run once Search providers are migrated for demonstration. Signed-off-by: Christoph Wurst --- .../AppFramework/Bootstrap/Coordinator.php | 33 ++++++++------- .../Bootstrap/RegistrationContext.php | 17 ++------ lib/private/Search/SearchComposer.php | 40 ++++++++----------- .../Bootstrap/CoordinatorTest.php | 6 --- 4 files changed, 38 insertions(+), 58 deletions(-) diff --git a/lib/private/AppFramework/Bootstrap/Coordinator.php b/lib/private/AppFramework/Bootstrap/Coordinator.php index 085e7460da..fb01012a46 100644 --- a/lib/private/AppFramework/Bootstrap/Coordinator.php +++ b/lib/private/AppFramework/Bootstrap/Coordinator.php @@ -25,7 +25,6 @@ declare(strict_types=1); namespace OC\AppFramework\Bootstrap; -use OC\Search\SearchComposer; use OC\Support\CrashReport\Registry; use OC_App; use OCP\AppFramework\App; @@ -34,6 +33,7 @@ use OCP\AppFramework\QueryException; use OCP\EventDispatcher\IEventDispatcher; use OCP\ILogger; use OCP\IServerContainer; +use RuntimeException; use Throwable; use function class_exists; use function class_implements; @@ -50,26 +50,28 @@ class Coordinator { /** @var IEventDispatcher */ private $eventDispatcher; - /** @var SearchComposer */ - private $searchComposer; - /** @var ILogger */ private $logger; + /** @var RegistrationContext|null */ + private $registrationContext; + public function __construct(IServerContainer $container, Registry $registry, IEventDispatcher $eventListener, - SearchComposer $searchComposer, ILogger $logger) { $this->serverContainer = $container; $this->registry = $registry; $this->eventDispatcher = $eventListener; - $this->searchComposer = $searchComposer; $this->logger = $logger; } public function runRegistration(): void { - $context = new RegistrationContext($this->logger); + if ($this->registrationContext !== null) { + throw new RuntimeException('Registration has already been run'); + } + + $this->registrationContext = new RegistrationContext($this->logger); $apps = []; foreach (OC_App::getEnabledApps() as $appId) { /* @@ -99,7 +101,7 @@ class Coordinator { continue; } try { - $application->register($context->for($appId)); + $application->register($this->registrationContext->for($appId)); } catch (Throwable $e) { $this->logger->logException($e, [ 'message' => 'Error during app service registration: ' . $e->getMessage(), @@ -113,12 +115,15 @@ class Coordinator { * Now that all register methods have been called, we can delegate the registrations * to the actual services */ - $context->delegateCapabilityRegistrations($apps); - $context->delegateCrashReporterRegistrations($apps, $this->registry); - $context->delegateEventListenerRegistrations($this->eventDispatcher); - $context->delegateContainerRegistrations($apps); - $context->delegateMiddlewareRegistrations($apps); - $context->delegateSearchProviderRegistration($apps, $this->searchComposer); + $this->registrationContext->delegateCapabilityRegistrations($apps); + $this->registrationContext->delegateCrashReporterRegistrations($apps, $this->registry); + $this->registrationContext->delegateEventListenerRegistrations($this->eventDispatcher); + $this->registrationContext->delegateContainerRegistrations($apps); + $this->registrationContext->delegateMiddlewareRegistrations($apps); + } + + public function getRegistrationContext(): ?RegistrationContext { + return $this->registrationContext; } public function bootApp(string $appId): void { diff --git a/lib/private/AppFramework/Bootstrap/RegistrationContext.php b/lib/private/AppFramework/Bootstrap/RegistrationContext.php index 23eee9c6e3..15b1cfa51e 100644 --- a/lib/private/AppFramework/Bootstrap/RegistrationContext.php +++ b/lib/private/AppFramework/Bootstrap/RegistrationContext.php @@ -26,7 +26,6 @@ declare(strict_types=1); namespace OC\AppFramework\Bootstrap; use Closure; -use OC\Search\SearchComposer; use OC\Support\CrashReport\Registry; use OCP\AppFramework\App; use OCP\AppFramework\Bootstrap\IRegistrationContext; @@ -347,19 +346,9 @@ class RegistrationContext { } /** - * @param App[] $apps + * @return array[] */ - public function delegateSearchProviderRegistration(array $apps, SearchComposer $searchComposer): void { - foreach ($this->searchProviders as $registration) { - try { - $searchComposer->registerProvider($registration['class']); - } catch (Throwable $e) { - $appId = $registration['appId']; - $this->logger->logException($e, [ - 'message' => "Error during search provider registration of $appId: " . $e->getMessage(), - 'level' => ILogger::ERROR, - ]); - } - } + public function getSearchProviders(): array { + return $this->searchProviders; } } diff --git a/lib/private/Search/SearchComposer.php b/lib/private/Search/SearchComposer.php index ae4350ca5c..61bbbbc969 100644 --- a/lib/private/Search/SearchComposer.php +++ b/lib/private/Search/SearchComposer.php @@ -26,7 +26,7 @@ declare(strict_types=1); namespace OC\Search; use InvalidArgumentException; -use OCP\AppFramework\Bootstrap\IRegistrationContext; +use OC\AppFramework\Bootstrap\Coordinator; use OCP\AppFramework\QueryException; use OCP\ILogger; use OCP\IServerContainer; @@ -57,37 +57,24 @@ use function array_map; */ class SearchComposer { - /** @var string[] */ - private $lazyProviders = []; - /** @var IProvider[] */ private $providers = []; + /** @var Coordinator */ + private $bootstrapCoordinator; + /** @var IServerContainer */ private $container; /** @var ILogger */ private $logger; - public function __construct(IServerContainer $container, + public function __construct(Coordinator $bootstrapCoordinator, + IServerContainer $container, ILogger $logger) { $this->container = $container; $this->logger = $logger; - } - - /** - * Register a search provider lazily - * - * Registers the fully-qualified class name of an implementation of an - * IProvider. The service will only be queried on demand. Apps will register - * the providers through the registration context object. - * - * @see IRegistrationContext::registerSearchProvider() - * - * @param string $class - */ - public function registerProvider(string $class): void { - $this->lazyProviders[] = $class; + $this->bootstrapCoordinator = $bootstrapCoordinator; } /** @@ -96,11 +83,17 @@ class SearchComposer { * If a provider can't be loaded we log it but the operation continues nevertheless */ private function loadLazyProviders(): void { - $classes = $this->lazyProviders; - foreach ($classes as $class) { + $context = $this->bootstrapCoordinator->getRegistrationContext(); + if ($context === null) { + // Too early, nothing registered yet + return; + } + + $registrations = $context->getSearchProviders(); + foreach ($registrations as $registration) { try { /** @var IProvider $provider */ - $provider = $this->container->query($class); + $provider = $this->container->query($registration['class']); $this->providers[$provider->getId()] = $provider; } catch (QueryException $e) { // Log an continue. We can be fault tolerant here. @@ -110,7 +103,6 @@ class SearchComposer { ]); } } - $this->lazyProviders = []; } /** diff --git a/tests/lib/AppFramework/Bootstrap/CoordinatorTest.php b/tests/lib/AppFramework/Bootstrap/CoordinatorTest.php index 6909ad94e7..c12e5eeb15 100644 --- a/tests/lib/AppFramework/Bootstrap/CoordinatorTest.php +++ b/tests/lib/AppFramework/Bootstrap/CoordinatorTest.php @@ -26,7 +26,6 @@ declare(strict_types=1); namespace lib\AppFramework\Bootstrap; use OC\AppFramework\Bootstrap\Coordinator; -use OC\Search\SearchComposer; use OC\Support\CrashReport\Registry; use OCP\App\IAppManager; use OCP\AppFramework\App; @@ -54,9 +53,6 @@ class CoordinatorTest extends TestCase { /** @var IEventDispatcher|MockObject */ private $eventDispatcher; - /** @var SearchComposer|MockObject */ - private $searchComposer; - /** @var ILogger|MockObject */ private $logger; @@ -70,14 +66,12 @@ class CoordinatorTest extends TestCase { $this->serverContainer = $this->createMock(IServerContainer::class); $this->crashReporterRegistry = $this->createMock(Registry::class); $this->eventDispatcher = $this->createMock(IEventDispatcher::class); - $this->searchComposer = $this->createMock(SearchComposer::class); $this->logger = $this->createMock(ILogger::class); $this->coordinator = new Coordinator( $this->serverContainer, $this->crashReporterRegistry, $this->eventDispatcher, - $this->searchComposer, $this->logger ); }