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 <christoph@winzerhof-wurst.at>

diff --git a/lib/private/AppFramework/Bootstrap/Coordinator.php b/lib/private/AppFramework/Bootstrap/Coordinator.php
index 085e7460da6d5df2d4cd52f07711af767504ac1c..fb01012a46f66459ff2285a1bb64ddbcb32f2031 100644 (file)
--- 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 23eee9c6e33e063d909bc569f5d893ab7c27d96e..15b1cfa51e8170bd6392b9efc961b94eca403f2f 100644 (file)
--- 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 ae4350ca5cca2cf0a5f95f352b11cbb9c967ea8d..61bbbbc969b9c5429a2173861547d3df73b2217a 100644 (file)
--- 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 6909ad94e7fc84512db488f68421116c4d746e97..c12e5eeb15063f0e6b43af138f9327432865c70e 100644 (file)
--- 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
                );
        }