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>

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 {

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;
 	}
 }

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 = [];
 	}
 
 	/**

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
 		);
 	}