Merge pull request #59977 from nextcloud/fix/clean-app-namespace-handling

refactor: Cleanup application namespace handling

apps/files/tests/Controller/ViewControllerTest.php

@@ -104,6 +104,10 @@ class ViewControllerTest extends TestCase {
 		$this->appManager->expects($this->any())
 			->method('isAppLoaded')
 			->willReturn(true);
+		$this->appManager->expects($this->any())
+			->method('getAppNamespace')
+			->with('files')
+			->willReturn('OCA\\Files');
 
 		$this->cacheFactory = $this->createMock(ICacheFactory::class);
 		$this->logger = $this->createMock(LoggerInterface::class);

build/psalm-baseline.xml

@@ -4140,14 +4140,6 @@
       <code><![CDATA[inet_ntop(substr($binary, -4))]]></code>
     </FalsableReturnStatement>
   </file>
-  <file src="lib/private/ServerContainer.php">
-    <InvalidPropertyAssignmentValue>
-      <code><![CDATA[$this->hasNoAppContainer]]></code>
-    </InvalidPropertyAssignmentValue>
-    <NoValue>
-      <code><![CDATA[return $this->appContainers[$namespace];]]></code>
-    </NoValue>
-  </file>
   <file src="lib/private/Session/Internal.php">
     <MoreSpecificImplementedParamType>
       <code><![CDATA[$value]]></code>

lib/private/App/AppManager.php

@@ -76,6 +76,9 @@ class AppManager implements IAppManager {
 	/** @var array<string, true> */
 	private array $loadedApps = [];
 
+	/** @var string[] */
+	private $namespaceCache = [];
+
 	private ?AppConfig $appConfig = null;
 	private ?IURLGenerator $urlGenerator = null;
 	private ?INavigationManager $navigationManager = null;
@@ -1197,4 +1200,44 @@ class AppManager implements IAppManager {
 	public function isAppCompatible(string $serverVersion, array $appInfo, bool $ignoreMax = false): bool {
 		return count($this->dependencyAnalyzer->analyzeServerVersion($serverVersion, $appInfo, $ignoreMax)) === 0;
 	}
+
+	#[\Override]
+	public function getAppNamespace(string $appId): string {
+		$topNamespace = 'OCA\\';
+
+		// Hit the cache!
+		if (isset($this->namespaceCache[$appId])) {
+			return $topNamespace . $this->namespaceCache[$appId];
+		}
+
+		$appInfo = $this->getAppInfo($appId);
+		if (isset($appInfo['namespace'])) {
+			$this->namespaceCache[$appId] = trim($appInfo['namespace']);
+		} else {
+			// If the tag is not found, fall back to uppercasing the first letter
+			$this->namespaceCache[$appId] = ucfirst($appId);
+		}
+
+		return $topNamespace . $this->namespaceCache[$appId];
+	}
+
+	#[\Override]
+	public function getAppFromNamespace(string $className): ?string {
+		$topNamespace = 'OCA\\';
+
+		if (str_starts_with($className, 'OC\\Core')) {
+			return 'core';
+		}
+		if (!str_starts_with($className, $topNamespace)) {
+			return null;
+		}
+
+		foreach ($this->namespaceCache as $appId => $namespace) {
+			if (str_starts_with($className, $topNamespace . $namespace . '\\')) {
+				return $appId;
+			}
+		}
+
+		return null;
+	}
 }

lib/private/AppFramework/App.php

@@ -30,9 +30,6 @@ use OCP\Server;
  * Handles all the dependency injection, controllers and output flow
  */
 class App {
-	/** @var string[] */
-	private static $nameSpaceCache = [];
-
 	/**
 	 * Turns an app id into a namespace by either reading the appinfo.xml's
 	 * namespace tag or uppercasing the appid's first letter
@@ -40,36 +37,22 @@ class App {
 	 * @param string $topNamespace the namespace which should be prepended to
 	 *                             the transformed app id, defaults to OCA\
 	 * @return string the starting namespace for the app
+	 * @deprecated 34.0.0 use IAppManager::getAppNamespace
 	 */
 	public static function buildAppNamespace(string $appId, string $topNamespace = 'OCA\\'): string {
-		// Hit the cache!
-		if (isset(self::$nameSpaceCache[$appId])) {
-			return $topNamespace . self::$nameSpaceCache[$appId];
+		$appManager = Server::get(IAppManager::class);
+		$namespace = $appManager->getAppNamespace($appId);
+		if ($topNamespace !== 'OCA\\') {
+			return $topNamespace . substr($namespace, strlen('OCA\\'));
 		}
-
-		$appInfo = Server::get(IAppManager::class)->getAppInfo($appId);
-		if (isset($appInfo['namespace'])) {
-			self::$nameSpaceCache[$appId] = trim($appInfo['namespace']);
-		} else {
-			// if the tag is not found, fall back to uppercasing the first letter
-			self::$nameSpaceCache[$appId] = ucfirst($appId);
-		}
-
-		return $topNamespace . self::$nameSpaceCache[$appId];
+		return $namespace;
 	}
 
+	/**
+	 * @deprecated 34.0.0 use IAppManager::getAppFromNamespace
+	 */
 	public static function getAppIdForClass(string $className, string $topNamespace = 'OCA\\'): ?string {
-		if (!str_starts_with($className, $topNamespace)) {
-			return null;
-		}
-
-		foreach (self::$nameSpaceCache as $appId => $namespace) {
-			if (str_starts_with($className, $topNamespace . $namespace . '\\')) {
-				return $appId;
-			}
-		}
-
-		return null;
+		return Server::get(IAppManager::class)->getAppFromNamespace($className);
 	}
 
 	/**

lib/private/AppFramework/Bootstrap/Coordinator.php

@@ -90,7 +90,7 @@ class Coordinator {
 			if ($appId === 'core') {
 				$appNameSpace = 'OC\\Core';
 			} else {
-				$appNameSpace = App::buildAppNamespace($appId);
+				$appNameSpace = $this->appManager->getAppNamespace($appId);
 			}
 			$applicationClassName = $appNameSpace . '\\AppInfo\\Application';
 
@@ -147,7 +147,7 @@ class Coordinator {
 		}
 		$this->bootedApps[$appId] = true;
 
-		$appNameSpace = App::buildAppNamespace($appId);
+		$appNameSpace = $this->appManager->getAppNamespace($appId);
 		$applicationClassName = $appNameSpace . '\\AppInfo\\Application';
 		if (!class_exists($applicationClassName)) {
 			// Nothing to boot
@@ -181,8 +181,8 @@ class Coordinator {
 		$this->eventLogger->end('bootstrap:boot_app:' . $appId);
 	}
 
-	public function isBootable(string $appId) {
-		$appNameSpace = App::buildAppNamespace($appId);
+	public function isBootable(string $appId): bool {
+		$appNameSpace = $this->appManager->getAppNamespace($appId);
 		$applicationClassName = $appNameSpace . '\\AppInfo\\Application';
 		return class_exists($applicationClassName)
 			&& in_array(IBootstrap::class, class_implements($applicationClassName), true);

lib/private/AppFramework/DependencyInjection/DIContainer.php

@@ -76,6 +76,7 @@ use Psr\Log\LoggerInterface;
 class DIContainer extends SimpleContainer implements IAppContainer {
 	private array $middleWares = [];
 	private ServerContainer $server;
+	private IAppManager $appManager;
 
 	public function __construct(
 		protected string $appName,
@@ -93,6 +94,7 @@ class DIContainer extends SimpleContainer implements IAppContainer {
 			$server = \OC::$server;
 		}
 		$this->server = $server;
+		$this->appManager = $this->server->get(IAppManager::class);
 		$this->server->registerAppContainer($this->appName, $this);
 
 		// aliases
@@ -363,6 +365,7 @@ class DIContainer extends SimpleContainer implements IAppContainer {
 
 	/**
 	 * @param string $name
+	 * @param list<class-string> $chain
 	 * @return mixed
 	 * @throws QueryException if the query could not be resolved
 	 */
@@ -375,7 +378,7 @@ class DIContainer extends SimpleContainer implements IAppContainer {
 			return parent::query($name, chain: $chain);
 		} elseif ($this->appName === 'core' && str_starts_with($name, 'OC\\Core\\')) {
 			return parent::query($name, chain: $chain);
-		} elseif (str_starts_with($name, App::buildAppNamespace($this->appName) . '\\')) {
+		} elseif (str_starts_with($name, $this->appManager->getAppNamespace($this->appName) . '\\')) {
 			return parent::query($name, chain: $chain);
 		} elseif (
 			str_starts_with($name, 'OC\\AppFramework\\Services\\')

lib/private/DB/MigrationService.php

@@ -63,7 +63,7 @@ class MigrationService {
 		} else {
 			$appManager = Server::get(IAppManager::class);
 			$appPath = $appManager->getAppPath($this->appName);
-			$namespace = App::buildAppNamespace($this->appName);
+			$namespace = $appManager->getAppNamespace($this->appName);
 			$this->migrationsPath = "$appPath/lib/Migration";
 			$this->migrationsNamespace = $namespace . '\\Migration';
 

lib/private/Route/Router.php

@@ -483,7 +483,7 @@ class Router implements IRouter {
 			} catch (AppPathNotFoundException) {
 				return [];
 			}
-			$appNameSpace = App::buildAppNamespace($app);
+			$appNameSpace = $this->appManager->getAppNamespace($app);
 		}
 
 		if (!file_exists($appControllerPath)) {
@@ -553,7 +553,7 @@ class Router implements IRouter {
 	}
 
 	private function getApplicationClass(string $appName) {
-		$appNameSpace = App::buildAppNamespace($appName);
+		$appNameSpace = $this->appManager->getAppNamespace($appName);
 
 		$applicationClassName = $appNameSpace . '\\AppInfo\\Application';
 

lib/private/ServerContainer.php

@@ -24,7 +24,7 @@ class ServerContainer extends SimpleContainer {
 	/** @var DIContainer[] */
 	protected $appContainers;
 
-	/** @var string[] */
+	/** @var array<string,true> */
 	protected $hasNoAppContainer;
 
 	/** @var string[] */
@@ -45,9 +45,7 @@ class ServerContainer extends SimpleContainer {
 	 * @param string $appNamespace
 	 */
 	public function registerNamespace(string $appName, string $appNamespace): void {
-		// Cut of OCA\ and lowercase
-		$appNamespace = strtolower(substr($appNamespace, strrpos($appNamespace, '\\') + 1));
-		$this->namespaces[$appNamespace] = $appName;
+		$this->namespaces[strtolower($appNamespace)] = $appName;
 	}
 
 	/**
@@ -55,7 +53,7 @@ class ServerContainer extends SimpleContainer {
 	 * @param DIContainer $container
 	 */
 	public function registerAppContainer(string $appName, DIContainer $container): void {
-		$this->appContainers[strtolower(App::buildAppNamespace($appName, ''))] = $container;
+		$this->appContainers[strtolower(App::buildAppNamespace($appName))] = $container;
 	}
 
 	/**
@@ -64,8 +62,8 @@ class ServerContainer extends SimpleContainer {
 	 * @throws QueryException
 	 */
 	public function getRegisteredAppContainer(string $appName): DIContainer {
-		if (isset($this->appContainers[strtolower(App::buildAppNamespace($appName, ''))])) {
-			return $this->appContainers[strtolower(App::buildAppNamespace($appName, ''))];
+		if (isset($this->appContainers[strtolower(App::buildAppNamespace($appName))])) {
+			return $this->appContainers[strtolower(App::buildAppNamespace($appName))];
 		}
 
 		throw new QueryException();
@@ -77,18 +75,21 @@ class ServerContainer extends SimpleContainer {
 	 * @return DIContainer
 	 * @throws QueryException
 	 */
-	protected function getAppContainer(string $namespace, string $sensitiveNamespace): DIContainer {
+	protected function getAppContainer(string $sensitiveNamespace): DIContainer {
+		$namespace = strtolower($sensitiveNamespace);
 		if (isset($this->appContainers[$namespace])) {
 			return $this->appContainers[$namespace];
 		}
 
 		if (isset($this->namespaces[$namespace])) {
 			if (!isset($this->hasNoAppContainer[$namespace])) {
-				$applicationClassName = 'OCA\\' . $sensitiveNamespace . '\\AppInfo\\Application';
+				$applicationClassName = $sensitiveNamespace . '\\AppInfo\\Application';
 				if (class_exists($applicationClassName)) {
+					/* The application constructor will register the container, see App::__construct */
 					$app = new $applicationClassName();
 					if (isset($this->appContainers[$namespace])) {
 						$this->appContainers[$namespace]->offsetSet($applicationClassName, $app);
+						/** @psalm-suppress NoValue false-positive (see comment above) */
 						return $this->appContainers[$namespace];
 					}
 				}
@@ -143,15 +144,6 @@ class ServerContainer extends SimpleContainer {
 					throw $e;
 				}
 			}
-		} elseif (str_starts_with($name, 'OC\\Settings\\') && substr_count($name, '\\') >= 3) {
-			$segments = explode('\\', $name);
-			try {
-				$appContainer = $this->getAppContainer(strtolower($segments[1]), $segments[1]);
-				return $appContainer->queryNoFallback($name, $chain);
-			} catch (QueryException $e) {
-				// Didn't find the service or the respective app container,
-				// ignore it and fall back to the core container.
-			}
 		}
 
 		return parent::query($name, $autoload, $chain);
@@ -168,8 +160,8 @@ class ServerContainer extends SimpleContainer {
 		}
 
 		try {
-			[,$namespace,] = explode('\\', $id);
-			return $this->getAppContainer(strtolower($namespace), $namespace);
+			[,$namespace,] = explode('\\', $id, 3);
+			return $this->getAppContainer('OCA\\' . $namespace);
 		} catch (QueryException $e) {
 			return null;
 		}

lib/private/legacy/OC_App.php

@@ -115,7 +115,7 @@ class OC_App {
 		self::$alreadyRegistered[$key] = true;
 
 		// Register on PSR-4 composer autoloader
-		$appNamespace = App::buildAppNamespace($app);
+		$appNamespace = Server::get(IAppManager::class)->getAppNamespace($app);
 		\OC::$server->registerNamespace($app, $appNamespace);
 
 		if (file_exists($path . '/composer/autoload.php')) {

lib/public/App/IAppManager.php

@@ -373,4 +373,18 @@ interface IAppManager {
 	 * @since 32.0.0
 	 */
 	public function isAppCompatible(string $serverVersion, array $appInfo, bool $ignoreMax = false): bool;
+
+	/**
+	 * Get the app namespace
+	 *
+	 * @since 34.0.0
+	 */
+	public function getAppNamespace(string $appId): string;
+
+	/**
+	 * Get the app id for this namespace
+	 *
+	 * @since 34.0.0
+	 */
+	public function getAppFromNamespace(string $className): ?string;
 }

lib/public/AppFramework/App.php

@@ -36,6 +36,7 @@ class App {
 	 *                             the transformed app id, defaults to OCA\
 	 * @return string the starting namespace for the app
 	 * @since 8.0.0
+	 * @deprecated 34.0.0 use IAppManager::getAppNamespace
 	 */
 	public static function buildAppNamespace(string $appId, string $topNamespace = 'OCA\\'): string {
 		return \OC\AppFramework\App::buildAppNamespace($appId, $topNamespace);
@@ -57,7 +58,6 @@ class App {
 			$setUpViaQuery = false;
 
 			$classNameParts = explode('\\', trim($applicationClassName, '\\'));
-
 			foreach ($e->getTrace() as $step) {
 				if (isset($step['class'], $step['function'], $step['args'][0])
 					&& $step['class'] === ServerContainer::class
@@ -68,7 +68,7 @@ class App {
 				} elseif (isset($step['class'], $step['function'], $step['args'][0])
 					&& $step['class'] === ServerContainer::class
 					&& $step['function'] === 'getAppContainer'
-					&& $step['args'][1] === $classNameParts[1]) {
+					&& $step['args'][0] === $classNameParts[0] . '\\' . $classNameParts[1]) {
 					$setUpViaQuery = true;
 					break;
 				} elseif (isset($step['class'], $step['function'], $step['args'][0])

tests/lib/AppFramework/Bootstrap/CoordinatorTest.php

@@ -48,6 +48,11 @@ class CoordinatorTest extends TestCase {
 		$this->eventLogger = $this->createMock(IEventLogger::class);
 		$this->logger = $this->createMock(LoggerInterface::class);
 
+		$this->appManager->expects($this->any())
+			->method('getAppNamespace')
+			->with('settings')
+			->willReturn('OCA\\Settings');
+
 		$this->coordinator = new Coordinator(
 			$this->serverContainer,
 			$this->crashReporterRegistry,

tests/lib/InfoXmlTest.php

@@ -61,7 +61,7 @@ class InfoXmlTest extends TestCase {
 		\OC_App::registerAutoloading($app, $appPath);
 
 		//Add the appcontainer
-		$applicationClassName = App::buildAppNamespace($app) . '\\AppInfo\\Application';
+		$applicationClassName = $this->appManager->getAppNamespace($app) . '\\AppInfo\\Application';
 		if (class_exists($applicationClassName)) {
 			$application = new $applicationClassName();
 			$this->addToAssertionCount(1);