Skip to content

refactor: Cleanup application namespace handling #59977

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
come-nc wants to merge 4 commits into
base: master
Choose a base branch
from
Open

refactor: Cleanup application namespace handling #59977

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
55f40b8
fix: Remove weird tinkering with namespaces and dead code
come-nc Apr 28, 2026
e42832d
fix: Add proper methods in IAppManager for namespace handling
come-nc Apr 28, 2026
7df7d5c
chore: Update psalm baseline
come-nc Apr 28, 2026
2095622
fix: Fix debug mode and tests mocking IAppManager
come-nc Apr 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions apps/files/tests/Controller/ViewControllerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ protected function setUp(): void {
$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);
Expand Down
8 changes: 0 additions & 8 deletions build/psalm-baseline.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3726,14 +3726,6 @@
<code><![CDATA[setPassword]]></code>
</InternalMethod>
</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>
Expand Down
41 changes: 41 additions & 0 deletions lib/private/App/AppManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -1160,4 +1163,42 @@ public function isUpgradeRequired(string $appId): bool {
public function isAppCompatible(string $serverVersion, array $appInfo, bool $ignoreMax = false): bool {
return count($this->dependencyAnalyzer->analyzeServerVersion($serverVersion, $appInfo, $ignoreMax)) === 0;
}

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);
Comment on lines +1179 to +1180
Copy link
Copy Markdown
Member

@ChristophWurst ChristophWurst Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was wondering if that logic makes sense for app IDs with more than one word but this code just moved 👍

}

return $topNamespace . $this->namespaceCache[$appId];
}

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;
}
}
37 changes: 10 additions & 27 deletions lib/private/AppFramework/App.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,46 +30,29 @@
* 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
* @param string $appId the app id
* @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];
}

$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);
$appManager = Server::get(IAppManager::class);
$namespace = $appManager->getAppNamespace($appId);
if ($topNamespace !== 'OCA\\') {
return $topNamespace . substr($namespace, strlen('OCA\\'));
}

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

/**
Expand Down
8 changes: 4 additions & 4 deletions lib/private/AppFramework/Bootstrap/Coordinator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ private function registerApps(array $appIds): void {
if ($appId === 'core') {
$appNameSpace = 'OC\\Core';
} else {
$appNameSpace = App::buildAppNamespace($appId);
$appNameSpace = $this->appManager->getAppNamespace($appId);
}
$applicationClassName = $appNameSpace . '\\AppInfo\\Application';

Expand Down Expand Up @@ -147,7 +147,7 @@ public function bootApp(string $appId): void {
}
$this->bootedApps[$appId] = true;

$appNameSpace = App::buildAppNamespace($appId);
$appNameSpace = $this->appManager->getAppNamespace($appId);
$applicationClassName = $appNameSpace . '\\AppInfo\\Application';
if (!class_exists($applicationClassName)) {
// Nothing to boot
Expand Down Expand Up @@ -181,8 +181,8 @@ public function bootApp(string $appId): void {
$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);
Expand Down
5 changes: 4 additions & 1 deletion lib/private/AppFramework/DependencyInjection/DIContainer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@
class DIContainer extends SimpleContainer implements IAppContainer {
private array $middleWares = [];
private ServerContainer $server;
private IAppManager $appManager;

public function __construct(
protected string $appName,
Expand All @@ -93,6 +94,7 @@ public function __construct(
$server = \OC::$server;
}
$this->server = $server;
$this->appManager = $this->server->get(IAppManager::class);
$this->server->registerAppContainer($this->appName, $this);

// aliases
Expand Down Expand Up @@ -357,6 +359,7 @@ public function query(string $name, bool $autoload = true, array $chain = []): m

/**
* @param string $name
* @param list<class-string> $chain
* @return mixed
* @throws QueryException if the query could not be resolved
*/
Expand All @@ -369,7 +372,7 @@ public function queryNoFallback($name, array $chain) {
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\\')
Expand Down
2 changes: 1 addition & 1 deletion lib/private/DB/MigrationService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public function __construct(
} 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';

Expand Down
4 changes: 2 additions & 2 deletions lib/private/Route/Router.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ private function getAttributeRoutes(string $app): array {
} catch (AppPathNotFoundException) {
return [];
}
$appNameSpace = App::buildAppNamespace($app);
$appNameSpace = $this->appManager->getAppNamespace($app);
}

if (!file_exists($appControllerPath)) {
Expand Down Expand Up @@ -550,7 +550,7 @@ private function setupRoutes($routes, $appName) {
}

private function getApplicationClass(string $appName) {
$appNameSpace = App::buildAppNamespace($appName);
$appNameSpace = $this->appManager->getAppNamespace($appName);

$applicationClassName = $appNameSpace . '\\AppInfo\\Application';

Expand Down
32 changes: 12 additions & 20 deletions lib/private/ServerContainer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
/** @var DIContainer[] */
protected $appContainers;

/** @var string[] */
/** @var array<string,true> */
protected $hasNoAppContainer;

/** @var string[] */
Expand All @@ -45,17 +45,15 @@
* @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;
}

/**
* @param string $appName
* @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;
}

/**
Expand All @@ -64,8 +62,8 @@
* @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();
Expand All @@ -77,18 +75,21 @@
* @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();

Check failure on line 89 in lib/private/ServerContainer.php

View workflow job for this annotation

GitHub Actions / static-code-analysis-security

TaintedCallable 

lib/private/ServerContainer.php:89:17: TaintedCallable: Detected tainted text (see https://psalm.dev/243)
if (isset($this->appContainers[$namespace])) {
$this->appContainers[$namespace]->offsetSet($applicationClassName, $app);
/** @psalm-suppress NoValue false-positive (see comment above) */
return $this->appContainers[$namespace];
}
}
Expand Down Expand Up @@ -141,15 +142,6 @@
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);
Expand All @@ -166,8 +158,8 @@
}

try {
[,$namespace,] = explode('\\', $id);
return $this->getAppContainer(strtolower($namespace), $namespace);
[,$namespace,] = explode('\\', $id, 3);
return $this->getAppContainer('OCA\\' . $namespace);
} catch (QueryException $e) {
return null;
}
Expand Down
2 changes: 1 addition & 1 deletion lib/private/legacy/OC_App.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public static function registerAutoloading(string $app, string $path, bool $forc
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')) {
Expand Down
14 changes: 14 additions & 0 deletions lib/public/App/IAppManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,4 +373,18 @@ public function isUpgradeRequired(string $appId): bool;
* @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;
Comment on lines +376 to +389
Copy link
Copy Markdown
Member

@ChristophWurst ChristophWurst Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new public API -> needs docs

}
4 changes: 2 additions & 2 deletions lib/public/AppFramework/App.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -57,7 +58,6 @@ public function __construct(string $appName, array $urlParams = []) {
$setUpViaQuery = false;

$classNameParts = explode('\\', trim($applicationClassName, '\\'));

foreach ($e->getTrace() as $step) {
if (isset($step['class'], $step['function'], $step['args'][0])
&& $step['class'] === ServerContainer::class
Expand All @@ -68,7 +68,7 @@ public function __construct(string $appName, array $urlParams = []) {
} 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])
Expand Down
5 changes: 5 additions & 0 deletions tests/lib/AppFramework/Bootstrap/CoordinatorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ protected function setUp(): void {
$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,
Expand Down
2 changes: 1 addition & 1 deletion tests/lib/InfoXmlTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public function testClasses($app): void {
\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);
Expand Down
Loading