Merge pull request #64172 from nextcloud/perf/short-circuit-aliases

perf: Speed up fetching services from container

Author: come-nc

lib/private/AppFramework/DependencyInjection/DIContainer.php

@@ -309,27 +309,16 @@ public function has($id): bool {
 	 */
 	#[\Override]
 	protected function query(string $name, bool $autoload = true, array $chain = []): mixed {
+		$name = $this->resolveAlias($name);
 		if ($name === 'AppName' || $name === 'appName') {
 			return $this->appName;
 		}
 
-		$isServerClass = str_starts_with($name, 'OCP\\') || str_starts_with($name, 'OC\\');
-		if ($isServerClass && !$this->has($name)) {
-			return $this->server->query($name, $autoload, $chain);
-		}
-
-		try {
-			return $this->queryNoFallback($name, $chain);
-		} catch (QueryException $firstException) {
-			try {
-				return $this->server->query($name, $autoload, $chain);
-			} catch (QueryException $secondException) {
-				if ($firstException->getCode() === 1) {
-					throw $secondException;
-				}
-				throw $firstException;
-			}
+		$result = $this->queryNoFallback($name, $chain);
+		if ($result !== null) {
+			return $result;
 		}
+		return $this->server->query($name, $autoload, $chain);
 	}
 
 	/**
@@ -340,6 +329,7 @@ protected function query(string $name, bool $autoload = true, array $chain = [])
 	 * @internal
 	 */
 	public function queryNoFallback($name, array $chain) {
+		$name = $this->resolveAlias($name);
 		if (isset($this->container[$name])) {
 			return $this->container[$name];
 		} elseif ($this->appName === 'settings' && str_starts_with($name, 'OC\\Settings\\')) {
@@ -353,8 +343,6 @@ public function queryNoFallback($name, array $chain) {
 			/* AppFramework services are scoped to the application */
 			return parent::query($name, chain: $chain);
 		}
-
-		throw new QueryException('Could not resolve ' . $name . '!'
-			. ' Class can not be instantiated', 1);
+		return null;
 	}
 }

lib/private/AppFramework/Utility/SimpleContainer.php

@@ -32,6 +32,9 @@ class SimpleContainer implements ArrayAccess, ContainerInterface, IContainer {
 
 	protected Container $container;
 
+	/** @var array<string,string> */
+	private array $aliases = [];
+
 	public function __construct() {
 		$this->container = new Container();
 	}
@@ -49,7 +52,7 @@ public function get(string $id): mixed {
 	#[\Override]
 	public function has(string $id): bool {
 		// If a service is no registered but is an existing class, we can probably load it
-		return isset($this->container[$id]) || class_exists($id);
+		return isset($this->aliases[$id]) || isset($this->container[$id]) || class_exists($id);
 	}
 
 	/**
@@ -152,6 +155,7 @@ public function resolve(string $name, array $chain = []): mixed {
 	 * @param list<class-string> $chain
 	 */
 	protected function query(string $name, bool $autoload = true, array $chain = []): mixed {
+		$name = $this->resolveAlias($name);
 		if (isset($this->container[$name])) {
 			return $this->container[$name];
 		}
@@ -195,6 +199,9 @@ public function registerService(string $name, Closure $closure, bool $shared = t
 		if (isset($this->container[$name])) {
 			unset($this->container[$name]);
 		}
+		if (isset($this->aliases[$name])) {
+			unset($this->aliases[$name]);
+		}
 		if ($shared) {
 			$this->container[$name] = $wrapped;
 		} else {
@@ -210,11 +217,14 @@ public function registerService(string $name, Closure $closure, bool $shared = t
 	 * @param string $target the target that should be resolved instead
 	 */
 	public function registerAlias(string $alias, string $target): void {
-		$this->registerService(
-			$alias,
-			static fn (ContainerInterface $container): mixed => $container->get($target),
-			false,
-		);
+		$this->aliases[$alias] = $target;
+	}
+
+	protected function resolveAlias(string $name) : string {
+		while (isset($this->aliases[$name])) {
+			$name = $this->aliases[$name];
+		}
+		return $name;
 	}
 
 	protected function registerDeprecatedAlias(string $alias, string $target): void {

lib/private/ServerContainer.php

@@ -123,23 +123,16 @@ public function has($id, bool $noRecursion = false): bool {
 	 */
 	#[\Override]
 	protected function query(string $name, bool $autoload = true, array $chain = []): mixed {
+		$name = $this->resolveAlias($name);
 		if (str_starts_with($name, 'OCA\\')) {
-			// Skip server container query for app namespace classes
-			if (isset($this->container[$name])) {
-				return $this->container[$name];
-			}
-			// Continue with general autoloading
-			// In case the service starts with OCA\ we try to find the service in
-			// the apps container first.
+			// In case the service starts with OCA\ we try to find the service in the apps container.
 			if (($appContainer = $this->getAppContainerForService($name)) !== null) {
-				try {
-					return $appContainer->queryNoFallback($name, $chain);
-				} catch (QueryException $e) {
-					// Didn't find the service or the respective app container
-					// In this case the service won't be part of the core container,
-					// so we can throw directly
-					throw $e;
+				$result = $appContainer->queryNoFallback($name, $chain);
+				if ($result !== null) {
+					return $result;
 				}
+				throw new QueryException('Could not resolve ' . $name . '!'
+					. ' Class can not be instantiated', 1);
 			}
 		}
 

tests/lib/Settings/DeclarativeManagerTest.php

@@ -583,7 +583,8 @@ public function testSetValueWithHandler(): void {
 			->method('setValue')
 			->with('test_field_2', 'some password', $this->adminUser);
 
-		\OC::$server->registerService('OCA\\Testing\\Settings\\DeclarativeForm', fn () => $form, false);
+		$appContainer = \OC::$server->getAppContainerForService('OCA\\Testing\\Settings\\DeclarativeForm');
+		$appContainer->registerService('OCA\\Testing\\Settings\\DeclarativeForm', fn () => $form, false);
 
 		$context = $this->createMock(RegistrationContext::class);
 		$context->expects(self::atLeastOnce())
@@ -616,7 +617,8 @@ public function testGetValueWithHandler(): void {
 			->with('test_field_2', $this->adminUser)
 			->willReturn('very secret password');
 
-		\OC::$server->registerService('OCA\\Testing\\Settings\\DeclarativeForm', fn () => $form, false);
+		$appContainer = \OC::$server->getAppContainerForService('OCA\\Testing\\Settings\\DeclarativeForm');
+		$appContainer->registerService('OCA\\Testing\\Settings\\DeclarativeForm', fn () => $form, false);
 
 		$context = $this->createMock(RegistrationContext::class);
 		$context->expects(self::atLeastOnce())