From 03f5effc75b448361651858cbf32976af489067f Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Sun, 28 Sep 2025 13:12:29 +0200 Subject: [PATCH] refactor(dependency-injection): Port away from query where possible Signed-off-by: Carl Schwan --- apps/settings/lib/AppInfo/Application.php | 14 +-- build/psalm-baseline.xml | 5 - lib/private/AppFramework/App.php | 5 +- .../AppFramework/Bootstrap/Coordinator.php | 4 +- .../DependencyInjection/DIContainer.php | 31 ++++--- .../AppFramework/Utility/SimpleContainer.php | 92 +++++++++++-------- lib/private/Calendar/Resource/Manager.php | 2 +- lib/private/Calendar/Room/Manager.php | 2 +- .../Resources/ProviderManager.php | 5 +- lib/private/DB/MigrationService.php | 4 +- lib/private/InitialStateService.php | 5 +- lib/private/Server.php | 3 +- lib/private/ServerContainer.php | 17 +++- lib/private/Streamer.php | 4 +- lib/private/Support/Subscription/Registry.php | 5 +- lib/public/AppFramework/App.php | 6 +- lib/public/IContainer.php | 6 +- .../Bootstrap/CoordinatorTest.php | 6 +- .../DependencyInjection/DIContainerTest.php | 5 +- .../DIIntergrationTests.php | 16 ++-- .../Utility/SimpleContainerTest.php | 81 ++++++++-------- tests/lib/Calendar/Resource/ManagerTest.php | 32 +++---- tests/lib/Calendar/Room/ManagerTest.php | 16 ++-- .../Resources/ProviderManagerTest.php | 16 ++-- tests/lib/ServerTest.php | 12 +-- .../lib/Support/Subscription/RegistryTest.php | 2 +- tests/lib/Traits/EncryptionTrait.php | 6 +- 27 files changed, 205 insertions(+), 197 deletions(-) diff --git a/apps/settings/lib/AppInfo/Application.php b/apps/settings/lib/AppInfo/Application.php index de007a6978f..f5831e257a7 100644 --- a/apps/settings/lib/AppInfo/Application.php +++ b/apps/settings/lib/AppInfo/Application.php @@ -137,20 +137,20 @@ class Application extends App implements IBootstrap { */ $context->registerService(IProvider::class, function (IAppContainer $appContainer) { /** @var IServerContainer $serverContainer */ - $serverContainer = $appContainer->query(IServerContainer::class); - return $serverContainer->query(IProvider::class); + $serverContainer = $appContainer->get(IServerContainer::class); + return $serverContainer->get(IProvider::class); }); $context->registerService(IManager::class, function (IAppContainer $appContainer) { /** @var IServerContainer $serverContainer */ - $serverContainer = $appContainer->query(IServerContainer::class); + $serverContainer = $appContainer->get(IServerContainer::class); return $serverContainer->getSettingsManager(); }); $context->registerService(NewUserMailHelper::class, function (IAppContainer $appContainer) { /** @var Server $server */ - $server = $appContainer->query(IServerContainer::class); + $server = $appContainer->get(IServerContainer::class); /** @var Defaults $defaults */ - $defaults = $server->query(Defaults::class); + $defaults = $server->get(Defaults::class); return new NewUserMailHelper( $defaults, @@ -234,7 +234,7 @@ class Application extends App implements IBootstrap { */ public function onChangePassword(array $parameters) { /** @var Hooks $hooks */ - $hooks = $this->getContainer()->query(Hooks::class); + $hooks = $this->getContainer()->get(Hooks::class); $hooks->onChangePassword($parameters['uid']); } @@ -251,7 +251,7 @@ class Application extends App implements IBootstrap { } /** @var Hooks $hooks */ - $hooks = $this->getContainer()->query(Hooks::class); + $hooks = $this->getContainer()->get(Hooks::class); $hooks->onChangeEmail($parameters['user'], $parameters['old_value']); } } diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 8f95f1415a1..f5330fa8913 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -3745,11 +3745,6 @@ - - - - - getDN(X509::DN_OPENSSL)['CN']]]> diff --git a/lib/private/AppFramework/App.php b/lib/private/AppFramework/App.php index 7bf32852209..f4942155ff2 100644 --- a/lib/private/AppFramework/App.php +++ b/lib/private/AppFramework/App.php @@ -21,6 +21,7 @@ use OCP\Diagnostics\IEventLogger; use OCP\HintException; use OCP\IRequest; use OCP\Profiler\IProfiler; +use Psr\Container\ContainerExceptionInterface; /** * Entry point for every request in your app. You can consider this as your @@ -117,7 +118,7 @@ class App { // first try $controllerName then go for \OCA\AppName\Controller\$controllerName try { $controller = $container->get($controllerName); - } catch (QueryException $e) { + } catch (ContainerExceptionInterface) { if (str_contains($controllerName, '\\Controller\\')) { // This is from a global registered app route that is not enabled. [/*OC(A)*/, $app, /* Controller/Name*/] = explode('\\', $controllerName, 3); @@ -130,7 +131,7 @@ class App { $appNameSpace = self::buildAppNamespace($appName); } $controllerName = $appNameSpace . '\\Controller\\' . $controllerName; - $controller = $container->query($controllerName); + $controller = $container->get($controllerName); } $eventLogger->end('app:controller:load'); diff --git a/lib/private/AppFramework/Bootstrap/Coordinator.php b/lib/private/AppFramework/Bootstrap/Coordinator.php index a31dd6a05e1..f675f3b2872 100644 --- a/lib/private/AppFramework/Bootstrap/Coordinator.php +++ b/lib/private/AppFramework/Bootstrap/Coordinator.php @@ -99,7 +99,7 @@ class Coordinator { $this->eventLogger->start("bootstrap:register_app:$appId:application", "Load `Application` instance for $appId"); try { /** @var IBootstrap&App $application */ - $application = $this->serverContainer->query($applicationClassName); + $application = $this->serverContainer->get($applicationClassName); $apps[$appId] = $application; } catch (ContainerExceptionInterface $e) { // Weird, but ok @@ -163,7 +163,7 @@ class Coordinator { $this->eventLogger->start('bootstrap:boot_app:' . $appId, "Call `Application::boot` for $appId"); try { /** @var App $application */ - $application = $this->serverContainer->query($applicationClassName); + $application = $this->serverContainer->get($applicationClassName); if ($application instanceof IBootstrap) { /** @var BootContext $context */ $context = new BootContext($application->getContainer()); diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php index 76261fe6b92..680c7ab40a2 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -60,6 +60,7 @@ use OCP\ISession; use OCP\IURLGenerator; use OCP\IUserSession; use OCP\Security\Ip\IRemoteAddress; +use Psr\Container\ContainerExceptionInterface; use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; @@ -263,7 +264,7 @@ class DIContainer extends SimpleContainer implements IAppContainer { * @return string the name of your application */ public function getAppName() { - return $this->query('appName'); + return $this->get('appName'); } /** @@ -293,8 +294,8 @@ class DIContainer extends SimpleContainer implements IAppContainer { * @param string $serviceName e.g. 'OCA\Files\Capabilities' */ public function registerCapability($serviceName) { - $this->query('OC\CapabilitiesManager')->registerCapability(function () use ($serviceName) { - return $this->query($serviceName); + $this->get('OC\CapabilitiesManager')->registerCapability(function () use ($serviceName) { + return $this->get($serviceName); }); } @@ -322,10 +323,10 @@ class DIContainer extends SimpleContainer implements IAppContainer { try { return $this->queryNoFallback($name); - } catch (QueryException $firstException) { + } catch (ContainerExceptionInterface $firstException) { try { return $this->getServer()->query($name, $autoload); - } catch (QueryException $secondException) { + } catch (ContainerExceptionInterface $secondException) { if ($firstException->getCode() === 1) { throw $secondException; } @@ -334,28 +335,30 @@ class DIContainer extends SimpleContainer implements IAppContainer { } } + public function get(string $id): mixed { + return $this->query($id); + } + /** - * @param string $name - * @return mixed * @throws QueryException if the query could not be resolved */ - public function queryNoFallback($name) { + public function queryNoFallback(string $name): mixed { $name = $this->sanitizeName($name); - if ($this->offsetExists($name)) { - return parent::query($name); + if ($this->has($name)) { + return parent::get($name); } elseif ($this->appName === 'settings' && str_starts_with($name, 'OC\\Settings\\')) { - return parent::query($name); + return parent::get($name); } elseif ($this->appName === 'core' && str_starts_with($name, 'OC\\Core\\')) { - return parent::query($name); + return parent::get($name); } elseif (str_starts_with($name, \OC\AppFramework\App::buildAppNamespace($this->appName) . '\\')) { - return parent::query($name); + return parent::get($name); } elseif ( str_starts_with($name, 'OC\\AppFramework\\Services\\') || str_starts_with($name, 'OC\\AppFramework\\Middleware\\') ) { /* AppFramework services are scoped to the application */ - return parent::query($name); + return parent::get($name); } throw new QueryException('Could not resolve ' . $name . '!' diff --git a/lib/private/AppFramework/Utility/SimpleContainer.php b/lib/private/AppFramework/Utility/SimpleContainer.php index 0db3bfc1c77..87b590d6bc1 100644 --- a/lib/private/AppFramework/Utility/SimpleContainer.php +++ b/lib/private/AppFramework/Utility/SimpleContainer.php @@ -10,6 +10,7 @@ namespace OC\AppFramework\Utility; use ArrayAccess; use Closure; use OCP\AppFramework\QueryException; +use OCP\AppFramework\Services\IAppConfig; use OCP\IContainer; use Pimple\Container; use Psr\Container\ContainerExceptionInterface; @@ -42,7 +43,16 @@ class SimpleContainer implements ArrayAccess, ContainerInterface, IContainer { * @psalm-return (S is class-string ? T : mixed) */ public function get(string $id): mixed { - return $this->query($id); + $name = $this->sanitizeName($id); + if (isset($this->container[$name])) { + return $this->container[$name]; + } + + $object = $this->resolve($name); + $this->registerService($name, function () use ($object) { + return $object; + }); + return $object; } public function has(string $id): bool { @@ -76,40 +86,50 @@ class SimpleContainer implements ArrayAccess, ContainerInterface, IContainer { private function buildClassConstructorParameters(\ReflectionMethod $constructor): array { return array_map(function (ReflectionParameter $parameter) { $parameterType = $parameter->getType(); - $resolveName = $parameter->getName(); + if (!($parameterType instanceof ReflectionNamedType)) { + return $this->get($resolveName); + } + // try to find out if it is a class or a simple parameter - if ($parameterType !== null && ($parameterType instanceof ReflectionNamedType) && !$parameterType->isBuiltin()) { + if (!$parameterType->isBuiltin()) { $resolveName = $parameterType->getName(); } try { - $builtIn = $parameterType !== null && ($parameterType instanceof ReflectionNamedType) - && $parameterType->isBuiltin(); - return $this->query($resolveName, !$builtIn); + // Try special variable names like $appName or $user with basic built-in types + if ($parameterType->isBuiltin()) { + if (isset($this->container[$resolveName])) { + return $this->container[$resolveName]; + } + throw new QueryException("Could not resolve variable with name " . $resolveName . ' and build-in type ' . $parameterType->getName()); + } else { + return $this->get($resolveName); + } } catch (ContainerExceptionInterface $e) { // Service not found, use the default value when available if ($parameter->isDefaultValueAvailable()) { return $parameter->getDefaultValue(); } - if ($parameterType !== null && ($parameterType instanceof ReflectionNamedType) && !$parameterType->isBuiltin()) { - $resolveName = $parameter->getName(); - try { - return $this->query($resolveName); - } catch (ContainerExceptionInterface $e2) { - // Pass null if typed and nullable - if ($parameter->allowsNull() && ($parameterType instanceof ReflectionNamedType)) { - return null; - } - - // don't lose the error we got while trying to query by type - throw new QueryException($e->getMessage(), (int)$e->getCode(), $e); - } + if ($parameterType->isBuiltin()) { + throw $e; } - throw $e; + // Try to get the class from its type + $resolveName = $parameter->getName(); + try { + return $this->get($resolveName); + } catch (ContainerExceptionInterface $e2) { + // Pass null if typed and nullable + if ($parameter->allowsNull()) { + return null; + } + + // don't lose the error we got while trying to query by type + throw new QueryException($e->getMessage(), (int)$e->getCode(), $e2); + } } }, $constructor->getParameters()); } @@ -131,19 +151,15 @@ class SimpleContainer implements ArrayAccess, ContainerInterface, IContainer { } public function query(string $name, bool $autoload = true) { + if ($autoload) { + return $this->get($name); + } + $name = $this->sanitizeName($name); if (isset($this->container[$name])) { return $this->container[$name]; } - if ($autoload) { - $object = $this->resolve($name); - $this->registerService($name, function () use ($object) { - return $object; - }); - return $object; - } - throw new QueryNotFoundException('Could not resolve ' . $name . '!'); } @@ -151,7 +167,7 @@ class SimpleContainer implements ArrayAccess, ContainerInterface, IContainer { * @param string $name * @param mixed $value */ - public function registerParameter($name, $value) { + public function registerParameter(string $name, mixed $value): void { $this[$name] = $value; } @@ -164,7 +180,7 @@ class SimpleContainer implements ArrayAccess, ContainerInterface, IContainer { * @param Closure $closure the closure to be called on service creation * @param bool $shared */ - public function registerService($name, Closure $closure, $shared = true) { + public function registerService(string $name, Closure $closure, bool $shared = true): void { $wrapped = function () use ($closure) { return $closure($this); }; @@ -186,7 +202,7 @@ class SimpleContainer implements ArrayAccess, ContainerInterface, IContainer { * @param string $alias the alias that should be registered * @param string $target the target that should be resolved instead */ - public function registerAlias($alias, $target): void { + public function registerAlias(string $alias, string $target): void { $this->registerService($alias, function (ContainerInterface $container) use ($target): mixed { return $container->get($target); }, false); @@ -207,11 +223,7 @@ class SimpleContainer implements ArrayAccess, ContainerInterface, IContainer { }, false); } - /** - * @param string $name - * @return string - */ - protected function sanitizeName($name) { + protected function sanitizeName(string $name): string { if (isset($name[0]) && $name[0] === '\\') { return ltrim($name, '\\'); } @@ -221,8 +233,8 @@ class SimpleContainer implements ArrayAccess, ContainerInterface, IContainer { /** * @deprecated 20.0.0 use \Psr\Container\ContainerInterface::has */ - public function offsetExists($id): bool { - return $this->container->offsetExists($id); + public function offsetExists($offset): bool { + return $this->container->offsetExists($offset); } /** @@ -230,8 +242,8 @@ class SimpleContainer implements ArrayAccess, ContainerInterface, IContainer { * @return mixed */ #[\ReturnTypeWillChange] - public function offsetGet($id) { - return $this->container->offsetGet($id); + public function offsetGet($offset) { + return $this->container->offsetGet($offset); } /** diff --git a/lib/private/Calendar/Resource/Manager.php b/lib/private/Calendar/Resource/Manager.php index db04e6a648a..cc1904b2958 100644 --- a/lib/private/Calendar/Resource/Manager.php +++ b/lib/private/Calendar/Resource/Manager.php @@ -80,7 +80,7 @@ class Manager implements IManager { continue; } - $this->initializedBackends[$backend] = $this->server->query($backend); + $this->initializedBackends[$backend] = $this->server->get($backend); } return array_values($this->initializedBackends); diff --git a/lib/private/Calendar/Room/Manager.php b/lib/private/Calendar/Room/Manager.php index 65897010f2a..d7c5b8267e6 100644 --- a/lib/private/Calendar/Room/Manager.php +++ b/lib/private/Calendar/Room/Manager.php @@ -87,7 +87,7 @@ class Manager implements IManager { * The backend might have services injected that can't be build from the * server container. */ - $this->initializedBackends[$backend] = $this->server->query($backend); + $this->initializedBackends[$backend] = $this->server->get($backend); } return array_values($this->initializedBackends); diff --git a/lib/private/Collaboration/Resources/ProviderManager.php b/lib/private/Collaboration/Resources/ProviderManager.php index 0ce4ae7155a..cfe1ee130ae 100644 --- a/lib/private/Collaboration/Resources/ProviderManager.php +++ b/lib/private/Collaboration/Resources/ProviderManager.php @@ -12,6 +12,7 @@ use OCP\AppFramework\QueryException; use OCP\Collaboration\Resources\IProvider; use OCP\Collaboration\Resources\IProviderManager; use OCP\IServerContainer; +use Psr\Container\ContainerExceptionInterface; use Psr\Log\LoggerInterface; class ProviderManager implements IProviderManager { @@ -31,8 +32,8 @@ class ProviderManager implements IProviderManager { if ($this->providers !== []) { foreach ($this->providers as $provider) { try { - $this->providerInstances[] = $this->serverContainer->query($provider); - } catch (QueryException $e) { + $this->providerInstances[] = $this->serverContainer->get($provider); + } catch (ContainerExceptionInterface $e) { $this->logger->error("Could not query resource provider $provider: " . $e->getMessage(), [ 'exception' => $e, ]); diff --git a/lib/private/DB/MigrationService.php b/lib/private/DB/MigrationService.php index 51afd7c09c5..0ce95e95c25 100644 --- a/lib/private/DB/MigrationService.php +++ b/lib/private/DB/MigrationService.php @@ -24,6 +24,7 @@ use OCP\IDBConnection; use OCP\Migration\IMigrationStep; use OCP\Migration\IOutput; use OCP\Server; +use Psr\Container\ContainerExceptionInterface; use Psr\Log\LoggerInterface; class MigrationService { @@ -479,7 +480,8 @@ class MigrationService { if (!$s instanceof IMigrationStep) { throw new \InvalidArgumentException('Not a valid migration'); } - } catch (QueryException $e) { + } catch (ContainerExceptionInterface $e) { + echo $e->getTraceAsString() . $e->getMessage() . PHP_EOL; if (class_exists($class)) { $s = new $class(); } else { diff --git a/lib/private/InitialStateService.php b/lib/private/InitialStateService.php index 300aa238397..6289252eba3 100644 --- a/lib/private/InitialStateService.php +++ b/lib/private/InitialStateService.php @@ -13,6 +13,7 @@ use OC\AppFramework\Bootstrap\Coordinator; use OCP\AppFramework\QueryException; use OCP\AppFramework\Services\InitialStateProvider; use OCP\IInitialStateService; +use Psr\Container\ContainerExceptionInterface; use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; @@ -92,8 +93,8 @@ class InitialStateService implements IInitialStateService { $initialStates = $context->getInitialStates(); foreach ($initialStates as $initialState) { try { - $provider = $this->container->query($initialState->getService()); - } catch (QueryException $e) { + $provider = $this->container->get($initialState->getService()); + } catch (ContainerExceptionInterface $e) { // Log an continue. We can be fault tolerant here. $this->logger->error('Could not load initial state provider dynamically: ' . $e->getMessage(), [ 'exception' => $e, diff --git a/lib/private/Server.php b/lib/private/Server.php index 2804acbbb79..0488d14b064 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -257,6 +257,7 @@ class Server extends ServerContainer implements IServerContainer { */ public function __construct($webRoot, \OC\Config $config) { parent::__construct(); + $this->webRoot = $webRoot; // To find out if we are running from CLI or not @@ -575,7 +576,7 @@ class Server extends ServerContainer implements IServerContainer { $this->registerService(IFactory::class, function (Server $c) { return new \OC\L10N\Factory( $c->get(\OCP\IConfig::class), - $c->getRequest(), + $c->get(Request::class), $c->get(IUserSession::class), $c->get(ICacheFactory::class), \OC::$SERVERROOT, diff --git a/lib/private/ServerContainer.php b/lib/private/ServerContainer.php index b5bcbdaeb6f..0915a4ba9fa 100644 --- a/lib/private/ServerContainer.php +++ b/lib/private/ServerContainer.php @@ -12,6 +12,7 @@ use OC\AppFramework\App; use OC\AppFramework\DependencyInjection\DIContainer; use OC\AppFramework\Utility\SimpleContainer; use OCP\AppFramework\QueryException; +use Psr\Container\ContainerExceptionInterface; use function explode; use function strtolower; @@ -125,7 +126,7 @@ class ServerContainer extends SimpleContainer { // Skip server container query for app namespace classes try { return parent::query($name, false); - } catch (QueryException $e) { + } catch (ContainerExceptionInterface $e) { // Continue with general autoloading then } // In case the service starts with OCA\ we try to find the service in @@ -133,7 +134,7 @@ class ServerContainer extends SimpleContainer { if (($appContainer = $this->getAppContainerForService($name)) !== null) { try { return $appContainer->queryNoFallback($name); - } catch (QueryException $e) { + } catch (ContainerExceptionInterface $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 @@ -145,13 +146,21 @@ class ServerContainer extends SimpleContainer { try { $appContainer = $this->getAppContainer(strtolower($segments[1]), $segments[1]); return $appContainer->queryNoFallback($name); - } catch (QueryException $e) { + } catch (ContainerExceptionInterface $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); + if ($autoload) { + return parent::get($name); + } else { + return parent::query($name, false); + } + } + + public function get(string $id): mixed { + return $this->query($id); } /** diff --git a/lib/private/Streamer.php b/lib/private/Streamer.php index 7ebb66bf03a..0d066a3c760 100644 --- a/lib/private/Streamer.php +++ b/lib/private/Streamer.php @@ -116,13 +116,13 @@ class Streamer { // prevent absolute dirs $internalDir = ltrim($internalDir, '/'); - $userFolder = \OC::$server->get(IRootFolder::class)->get(Filesystem::getRoot()); + $userFolder = \OCP\Server::get(IRootFolder::class)->get(Filesystem::getRoot()); /** @var Folder $dirNode */ $dirNode = $userFolder->get($dir); $files = $dirNode->getDirectoryListing(); /** @var LoggerInterface $logger */ - $logger = \OC::$server->query(LoggerInterface::class); + $logger = \OCP\Server::get(LoggerInterface::class); foreach ($files as $file) { if ($file instanceof File) { try { diff --git a/lib/private/Support/Subscription/Registry.php b/lib/private/Support/Subscription/Registry.php index 34f24d1d026..29b7363114c 100644 --- a/lib/private/Support/Subscription/Registry.php +++ b/lib/private/Support/Subscription/Registry.php @@ -18,6 +18,7 @@ use OCP\Support\Subscription\Exception\AlreadyRegisteredException; use OCP\Support\Subscription\IRegistry; use OCP\Support\Subscription\ISubscription; use OCP\Support\Subscription\ISupportedApps; +use Psr\Container\ContainerExceptionInterface; use Psr\Log\LoggerInterface; class Registry implements IRegistry { @@ -54,8 +55,8 @@ class Registry implements IRegistry { private function getSubscription(): ?ISubscription { if ($this->subscription === null && $this->subscriptionService !== null) { try { - $this->subscription = $this->container->query($this->subscriptionService); - } catch (QueryException $e) { + $this->subscription = $this->container->get($this->subscriptionService); + } catch (ContainerExceptionInterface) { // Ignore this } } diff --git a/lib/public/AppFramework/App.php b/lib/public/AppFramework/App.php index c00fde47418..2bef186fd67 100644 --- a/lib/public/AppFramework/App.php +++ b/lib/public/AppFramework/App.php @@ -52,7 +52,7 @@ class App { if ($runIsSetupDirectly) { $applicationClassName = get_class($this); - $e = new \RuntimeException('App class ' . $applicationClassName . ' is not setup via query() but directly'); + $e = new \RuntimeException('App class ' . $applicationClassName . ' is not setup via get() but directly'); $setUpViaQuery = false; $classNameParts = explode('\\', trim($applicationClassName, '\\')); @@ -122,8 +122,8 @@ class App { * parent::__construct('tasks', $params); * * $this->getContainer()->registerService('PageController', function(IAppContainer $c){ - * $a = $c->query('API'); - * $r = $c->query('Request'); + * $a = $c->get('API'); + * $r = $c->get('Request'); * return new PageController($a, $r); * }); * } diff --git a/lib/public/IContainer.php b/lib/public/IContainer.php index 9cf1196ef2e..6b3f5a38c78 100644 --- a/lib/public/IContainer.php +++ b/lib/public/IContainer.php @@ -68,7 +68,7 @@ interface IContainer extends ContainerInterface { * @since 6.0.0 * @deprecated 20.0.0 use \OCP\AppFramework\Bootstrap\IRegistrationContext::registerParameter */ - public function registerParameter($name, $value); + public function registerParameter(string $name, mixed $value): void; /** * A service is registered in the container where a closure is passed in which will actually @@ -84,7 +84,7 @@ interface IContainer extends ContainerInterface { * @since 6.0.0 * @deprecated 20.0.0 use \OCP\AppFramework\Bootstrap\IRegistrationContext::registerService */ - public function registerService($name, Closure $closure, $shared = true); + public function registerService(string $name, Closure $closure, bool $shared = true): void; /** * Shortcut for returning a service from a service under a different key, @@ -95,5 +95,5 @@ interface IContainer extends ContainerInterface { * @since 8.2.0 * @deprecated 20.0.0 use \OCP\AppFramework\Bootstrap\IRegistrationContext::registerServiceAlias */ - public function registerAlias($alias, $target); + public function registerAlias(string $alias, string $target): void; } diff --git a/tests/lib/AppFramework/Bootstrap/CoordinatorTest.php b/tests/lib/AppFramework/Bootstrap/CoordinatorTest.php index 0eeddb2173a..caa601c24da 100644 --- a/tests/lib/AppFramework/Bootstrap/CoordinatorTest.php +++ b/tests/lib/AppFramework/Bootstrap/CoordinatorTest.php @@ -76,7 +76,7 @@ class CoordinatorTest extends TestCase { public function testBootAppNotLoadable(): void { $appId = 'settings'; $this->serverContainer->expects($this->once()) - ->method('query') + ->method('get') ->with(Application::class) ->willThrowException(new QueryException('')); $this->logger->expects($this->once()) @@ -89,7 +89,7 @@ class CoordinatorTest extends TestCase { $appId = 'settings'; $mockApp = $this->createMock(Application::class); $this->serverContainer->expects($this->once()) - ->method('query') + ->method('get') ->with(Application::class) ->willReturn($mockApp); @@ -110,7 +110,7 @@ class CoordinatorTest extends TestCase { } }; $this->serverContainer->expects($this->once()) - ->method('query') + ->method('get') ->with(Application::class) ->willReturn($mockApp); diff --git a/tests/lib/AppFramework/DependencyInjection/DIContainerTest.php b/tests/lib/AppFramework/DependencyInjection/DIContainerTest.php index 31188b12f14..6d4a67f88eb 100644 --- a/tests/lib/AppFramework/DependencyInjection/DIContainerTest.php +++ b/tests/lib/AppFramework/DependencyInjection/DIContainerTest.php @@ -19,6 +19,7 @@ use OCP\AppFramework\QueryException; use OCP\IConfig; use OCP\IRequestId; use PHPUnit\Framework\MockObject\MockObject; +use Psr\Container\ContainerExceptionInterface; /** * @group DB @@ -138,7 +139,7 @@ class DIContainerTest extends \Test\TestCase { } public function testInvalidAppClass(): void { - $this->expectException(QueryException::class); - $this->container->query('\OCA\Name\Foo'); + $this->expectException(ContainerExceptionInterface::class); + $this->container->get('\OCA\Name\Foo'); } } diff --git a/tests/lib/AppFramework/DependencyInjection/DIIntergrationTests.php b/tests/lib/AppFramework/DependencyInjection/DIIntergrationTests.php index 219fd5134ae..fb45d1b05ed 100644 --- a/tests/lib/AppFramework/DependencyInjection/DIIntergrationTests.php +++ b/tests/lib/AppFramework/DependencyInjection/DIIntergrationTests.php @@ -54,12 +54,12 @@ class DIIntergrationTests extends TestCase { $this->server->registerService(ClassB::class, function (SimpleContainer $c) { return new ClassB( - $c->query(Interface1::class) + $c->get(Interface1::class) ); }); /** @var ClassB $res */ - $res = $this->container->query(ClassB::class); + $res = $this->container->get(ClassB::class); $this->assertSame(ClassA1::class, get_class($res->interface1)); } @@ -70,12 +70,12 @@ class DIIntergrationTests extends TestCase { $this->container->registerService(ClassB::class, function (SimpleContainer $c) { return new ClassB( - $c->query(Interface1::class) + $c->get(Interface1::class) ); }); /** @var ClassB $res */ - $res = $this->container->query(ClassB::class); + $res = $this->container->get(ClassB::class); $this->assertSame(ClassA1::class, get_class($res->interface1)); } @@ -90,12 +90,12 @@ class DIIntergrationTests extends TestCase { $this->container->registerService(ClassB::class, function (SimpleContainer $c) { return new ClassB( - $c->query(Interface1::class) + $c->get(Interface1::class) ); }); /** @var ClassB $res */ - $res = $this->container->query(ClassB::class); + $res = $this->container->get(ClassB::class); $this->assertSame(ClassA2::class, get_class($res->interface1)); } @@ -110,12 +110,12 @@ class DIIntergrationTests extends TestCase { $this->server->registerService(ClassB::class, function (SimpleContainer $c) { return new ClassB( - $c->query(Interface1::class) + $c->get(Interface1::class) ); }); /** @var ClassB $res */ - $res = $this->container->query(ClassB::class); + $res = $this->container->get(ClassB::class); $this->assertSame(ClassA1::class, get_class($res->interface1)); } } diff --git a/tests/lib/AppFramework/Utility/SimpleContainerTest.php b/tests/lib/AppFramework/Utility/SimpleContainerTest.php index 33800c7376f..12d6b7893a2 100644 --- a/tests/lib/AppFramework/Utility/SimpleContainerTest.php +++ b/tests/lib/AppFramework/Utility/SimpleContainerTest.php @@ -12,6 +12,8 @@ namespace Test\AppFramework\Utility; use OC\AppFramework\Utility\SimpleContainer; use OCP\AppFramework\QueryException; +use Psr\Container\ContainerExceptionInterface; +use Psr\Container\ContainerInterface; use Psr\Container\NotFoundExceptionInterface; interface TestInterface { @@ -66,75 +68,66 @@ class SimpleContainerTest extends \Test\TestCase { $this->container = new SimpleContainer(); } - - public function testRegister(): void { $this->container->registerParameter('test', 'abc'); - $this->assertEquals('abc', $this->container->query('test')); + $this->assertEquals('abc', $this->container->get('test')); } - /** * Test querying a class that is not registered without autoload enabled */ public function testNothingRegistered(): void { try { - $this->container->query('something really hard', false); + $this->container->get('something really hard', false); $this->fail('Expected `QueryException` exception was not thrown'); } catch (\Throwable $exception) { - $this->assertInstanceOf(QueryException::class, $exception); + $this->assertInstanceOf(ContainerExceptionInterface::class, $exception); $this->assertInstanceOf(NotFoundExceptionInterface::class, $exception); } } - /** * Test querying a class that is not registered with autoload enabled */ public function testNothingRegistered_autoload(): void { try { - $this->container->query('something really hard'); + $this->container->get('something really hard'); $this->fail('Expected `QueryException` exception was not thrown'); } catch (\Throwable $exception) { - $this->assertInstanceOf(QueryException::class, $exception); + $this->assertInstanceOf(ContainerExceptionInterface::class, $exception); $this->assertInstanceOf(NotFoundExceptionInterface::class, $exception); } } - - public function testNotAClass(): void { - $this->expectException(QueryException::class); + $this->expectException(ContainerExceptionInterface::class); - $this->container->query('Test\AppFramework\Utility\TestInterface'); + $this->container->get('Test\AppFramework\Utility\TestInterface'); } - public function testNoConstructorClass(): void { - $object = $this->container->query('Test\AppFramework\Utility\ClassEmptyConstructor'); + $object = $this->container->get('Test\AppFramework\Utility\ClassEmptyConstructor'); $this->assertTrue($object instanceof ClassEmptyConstructor); } - public function testInstancesOnlyOnce(): void { - $object = $this->container->query('Test\AppFramework\Utility\ClassEmptyConstructor'); - $object2 = $this->container->query('Test\AppFramework\Utility\ClassEmptyConstructor'); + $object = $this->container->get('Test\AppFramework\Utility\ClassEmptyConstructor'); + $object2 = $this->container->get('Test\AppFramework\Utility\ClassEmptyConstructor'); $this->assertSame($object, $object2); } public function testConstructorSimple(): void { $this->container->registerParameter('test', 'abc'); - $object = $this->container->query( + $object = $this->container->get( 'Test\AppFramework\Utility\ClassSimpleConstructor' ); $this->assertTrue($object instanceof ClassSimpleConstructor); $this->assertEquals('abc', $object->test); } - public function testConstructorComplex(): void { $this->container->registerParameter('test', 'abc'); - $object = $this->container->query( + $object = $this->container->get( 'Test\AppFramework\Utility\ClassComplexConstructor' ); $this->assertTrue($object instanceof ClassComplexConstructor); @@ -142,14 +135,13 @@ class SimpleContainerTest extends \Test\TestCase { $this->assertEquals('abc', $object->test); } - public function testConstructorComplexInterface(): void { $this->container->registerParameter('test', 'abc'); $this->container->registerService( - 'Test\AppFramework\Utility\IInterfaceConstructor', function ($c) { - return $c->query('Test\AppFramework\Utility\ClassSimpleConstructor'); + 'Test\AppFramework\Utility\IInterfaceConstructor', function (ContainerInterface $c) { + return $c->get('Test\AppFramework\Utility\ClassSimpleConstructor'); }); - $object = $this->container->query( + $object = $this->container->get( 'Test\AppFramework\Utility\ClassInterfaceConstructor' ); $this->assertTrue($object instanceof ClassInterfaceConstructor); @@ -157,17 +149,16 @@ class SimpleContainerTest extends \Test\TestCase { $this->assertEquals('abc', $object->test); } - public function testOverrideService(): void { $this->container->registerService( - 'Test\AppFramework\Utility\IInterfaceConstructor', function ($c) { - return $c->query('Test\AppFramework\Utility\ClassSimpleConstructor'); + 'Test\AppFramework\Utility\IInterfaceConstructor', function (ContainerInterface $c) { + return $c->get('Test\AppFramework\Utility\ClassSimpleConstructor'); }); $this->container->registerService( - 'Test\AppFramework\Utility\IInterfaceConstructor', function ($c) { - return $c->query('Test\AppFramework\Utility\ClassEmptyConstructor'); + 'Test\AppFramework\Utility\IInterfaceConstructor', function (ContainerInterface $c) { + return $c->get('Test\AppFramework\Utility\ClassEmptyConstructor'); }); - $object = $this->container->query( + $object = $this->container->get( 'Test\AppFramework\Utility\IInterfaceConstructor' ); $this->assertTrue($object instanceof ClassEmptyConstructor); @@ -176,7 +167,7 @@ class SimpleContainerTest extends \Test\TestCase { public function testRegisterAliasParamter(): void { $this->container->registerParameter('test', 'abc'); $this->container->registerAlias('test1', 'test'); - $this->assertEquals('abc', $this->container->query('test1')); + $this->assertEquals('abc', $this->container->get('test1')); } public function testRegisterAliasService(): void { @@ -185,11 +176,11 @@ class SimpleContainerTest extends \Test\TestCase { }, true); $this->container->registerAlias('test1', 'test'); $this->assertSame( - $this->container->query('test'), $this->container->query('test')); + $this->container->get('test'), $this->container->get('test')); $this->assertSame( - $this->container->query('test1'), $this->container->query('test1')); + $this->container->get('test1'), $this->container->get('test1')); $this->assertSame( - $this->container->query('test'), $this->container->query('test1')); + $this->container->get('test'), $this->container->get('test1')); } public static function sanitizeNameProvider(): array { @@ -206,14 +197,14 @@ class SimpleContainerTest extends \Test\TestCase { $this->container->registerService($register, function () { return 'abc'; }); - $this->assertEquals('abc', $this->container->query($query)); + $this->assertEquals('abc', $this->container->get($query)); } public function testConstructorComplexNoTestParameterFound(): void { - $this->expectException(QueryException::class); + $this->expectException(ContainerExceptionInterface::class); - $object = $this->container->query( + $object = $this->container->get( 'Test\AppFramework\Utility\ClassComplexConstructor' ); /* Use the object to trigger DI on PHP >= 8.4 */ @@ -225,7 +216,7 @@ class SimpleContainerTest extends \Test\TestCase { return new \StdClass(); }, false); $this->assertNotSame( - $this->container->query('test'), $this->container->query('test')); + $this->container->get('test'), $this->container->get('test')); } public function testRegisterAliasFactory(): void { @@ -234,17 +225,17 @@ class SimpleContainerTest extends \Test\TestCase { }, false); $this->container->registerAlias('test1', 'test'); $this->assertNotSame( - $this->container->query('test'), $this->container->query('test')); + $this->container->get('test'), $this->container->get('test')); $this->assertNotSame( - $this->container->query('test1'), $this->container->query('test1')); + $this->container->get('test1'), $this->container->get('test1')); $this->assertNotSame( - $this->container->query('test'), $this->container->query('test1')); + $this->container->get('test'), $this->container->get('test1')); } public function testQueryUntypedNullable(): void { - $this->expectException(QueryException::class); + $this->expectException(ContainerExceptionInterface::class); - $object = $this->container->query( + $object = $this->container->get( ClassNullableUntypedConstructorArg::class ); /* Use the object to trigger DI on PHP >= 8.4 */ @@ -253,7 +244,7 @@ class SimpleContainerTest extends \Test\TestCase { public function testQueryTypedNullable(): void { /** @var ClassNullableTypedConstructorArg $service */ - $service = $this->container->query(ClassNullableTypedConstructorArg::class); + $service = $this->container->get(ClassNullableTypedConstructorArg::class); self::assertNull($service->class); } diff --git a/tests/lib/Calendar/Resource/ManagerTest.php b/tests/lib/Calendar/Resource/ManagerTest.php index 15977765da7..a701070318f 100644 --- a/tests/lib/Calendar/Resource/ManagerTest.php +++ b/tests/lib/Calendar/Resource/ManagerTest.php @@ -20,17 +20,13 @@ use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; class ManagerTest extends TestCase { - /** @var Coordinator|MockObject */ - private $coordinator; + private Coordinator&MockObject $coordinator; - /** @var IServerContainer|MockObject */ - private $server; + private IServerContainer&MockObject $server; - /** @var ResourcesRoomsUpdater|MockObject */ - private $resourcesRoomsUpdater; + private ResourcesRoomsUpdater&MockObject $resourcesRoomsUpdater; - /** @var Manager */ - private $manager; + private Manager $manager; protected function setUp(): void { parent::setUp(); @@ -52,10 +48,10 @@ class ManagerTest extends TestCase { $backend2 = $this->createMock(IBackend::class); $backend2->method('getBackendIdentifier')->willReturn('backend_2'); $this->server->expects(self::exactly(2)) - ->method('query') + ->method('get') ->willReturnMap([ - ['calendar_resource_backend1', true, $backend1,], - ['calendar_resource_backend2', true, $backend2,], + ['calendar_resource_backend1', $backend1,], + ['calendar_resource_backend2', $backend2,], ]); $this->manager->registerBackend('calendar_resource_backend1'); @@ -84,7 +80,7 @@ class ManagerTest extends TestCase { new ServiceRegistration('calendar_resource_foo', $backendClass) ]); $this->server->expects(self::once()) - ->method('query') + ->method('get') ->with($backendClass) ->willReturn($backend); @@ -97,10 +93,10 @@ class ManagerTest extends TestCase { $backend2 = $this->createMock(IBackend::class); $backend2->method('getBackendIdentifier')->willReturn('backend_2'); $this->server->expects(self::exactly(2)) - ->method('query') + ->method('get') ->willReturnMap([ - ['calendar_resource_backend1', true, $backend1,], - ['calendar_resource_backend2', true, $backend2,], + ['calendar_resource_backend1', $backend1,], + ['calendar_resource_backend2', $backend2,], ]); $this->manager->registerBackend('calendar_resource_backend1'); @@ -116,10 +112,10 @@ class ManagerTest extends TestCase { $backend2 = $this->createMock(IBackend::class); $backend2->method('getBackendIdentifier')->willReturn('backend_2'); $this->server->expects(self::exactly(2)) - ->method('query') + ->method('get') ->willReturnMap([ - ['calendar_resource_backend1', true, $backend1,], - ['calendar_resource_backend2', true, $backend2,], + ['calendar_resource_backend1', $backend1,], + ['calendar_resource_backend2', $backend2,], ]); $this->manager->registerBackend('calendar_resource_backend1'); diff --git a/tests/lib/Calendar/Room/ManagerTest.php b/tests/lib/Calendar/Room/ManagerTest.php index e0f2de970ed..63adb0abf06 100644 --- a/tests/lib/Calendar/Room/ManagerTest.php +++ b/tests/lib/Calendar/Room/ManagerTest.php @@ -52,10 +52,10 @@ class ManagerTest extends TestCase { $backend2 = $this->createMock(IBackend::class); $backend2->method('getBackendIdentifier')->willReturn('backend_2'); $this->server->expects(self::exactly(2)) - ->method('query') + ->method('get') ->willReturnMap([ - ['calendar_room_backend1', true, $backend1,], - ['calendar_room_backend2', true, $backend2,], + ['calendar_room_backend1', $backend1,], + ['calendar_room_backend2', $backend2,], ]); $this->manager->registerBackend('calendar_room_backend1'); @@ -86,7 +86,7 @@ class ManagerTest extends TestCase { new ServiceRegistration('calendar_room_foo', $backendClass) ]); $this->server->expects(self::once()) - ->method('query') + ->method('get') ->with($backendClass) ->willReturn($backend); @@ -99,10 +99,10 @@ class ManagerTest extends TestCase { $backend2 = $this->createMock(IBackend::class); $backend2->method('getBackendIdentifier')->willReturn('backend_2'); $this->server->expects(self::exactly(2)) - ->method('query') + ->method('get') ->willReturnMap([ - ['calendar_room_backend1', true, $backend1,], - ['calendar_room_backend2', true, $backend2,], + ['calendar_room_backend1', $backend1,], + ['calendar_room_backend2', $backend2,], ]); $this->manager->registerBackend('calendar_room_backend1'); @@ -118,7 +118,7 @@ class ManagerTest extends TestCase { $backend2 = $this->createMock(IBackend::class); $backend2->method('getBackendIdentifier')->willReturn('backend_2'); $this->server->expects(self::exactly(2)) - ->method('query') + ->method('get') ->willReturnMap([ ['calendar_room_backend1', true, $backend1,], ['calendar_room_backend2', true, $backend2,], diff --git a/tests/lib/Collaboration/Resources/ProviderManagerTest.php b/tests/lib/Collaboration/Resources/ProviderManagerTest.php index b063d89f06e..ebc34b9853b 100644 --- a/tests/lib/Collaboration/Resources/ProviderManagerTest.php +++ b/tests/lib/Collaboration/Resources/ProviderManagerTest.php @@ -13,16 +13,14 @@ use OCA\Files\Collaboration\Resources\ResourceProvider; use OCP\AppFramework\QueryException; use OCP\Collaboration\Resources\IProviderManager; use OCP\IServerContainer; +use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; use Test\TestCase; class ProviderManagerTest extends TestCase { - /** @var IServerContainer */ - protected $serverContainer; - /** @var LoggerInterface */ - protected $logger; - /** @var IProviderManager */ - protected $providerManager; + protected IServerContainer&MockObject $serverContainer; + protected LoggerInterface&MockObject $logger; + protected IProviderManager $providerManager; protected function setUp(): void { parent::setUp(); @@ -48,7 +46,7 @@ class ProviderManagerTest extends TestCase { public function testGetResourceProvidersValidProvider(): void { $this->serverContainer->expects($this->once()) - ->method('query') + ->method('get') ->with($this->equalTo(ResourceProvider::class)) ->willReturn($this->createMock(ResourceProvider::class)); @@ -61,7 +59,7 @@ class ProviderManagerTest extends TestCase { public function testGetResourceProvidersInvalidProvider(): void { $this->serverContainer->expects($this->once()) - ->method('query') + ->method('get') ->with($this->equalTo('InvalidResourceProvider')) ->willThrowException(new QueryException('A meaningful error message')); @@ -76,7 +74,7 @@ class ProviderManagerTest extends TestCase { public function testGetResourceProvidersValidAndInvalidProvider(): void { $this->serverContainer->expects($this->exactly(2)) - ->method('query') + ->method('get') ->willReturnCallback(function (string $service) { if ($service === 'InvalidResourceProvider') { throw new QueryException('A meaningful error message'); diff --git a/tests/lib/ServerTest.php b/tests/lib/ServerTest.php index b070641e856..2f340f53938 100644 --- a/tests/lib/ServerTest.php +++ b/tests/lib/ServerTest.php @@ -12,6 +12,7 @@ use OC\App\AppStore\Fetcher\AppFetcher; use OC\Config; use OC\Server; use OCP\Comments\ICommentsManager; +use OCP\IConfig; /** * Class Server @@ -43,14 +44,9 @@ class ServerTest extends \Test\TestCase { ]; } - /** - * - * @param string $serviceName - * @param string $instanceOf - */ #[\PHPUnit\Framework\Attributes\DataProvider('dataTestQuery')] - public function testQuery($serviceName, $instanceOf): void { - $this->assertInstanceOf($instanceOf, $this->server->query($serviceName), 'Service "' . $serviceName . '"" did not return the right class'); + public function testQuery(string $serviceName, string $instanceOf): void { + $this->assertInstanceOf($instanceOf, $this->server->get($serviceName), 'Service "' . $serviceName . '"" did not return the right class'); } public function testGetCertificateManager(): void { @@ -59,7 +55,7 @@ class ServerTest extends \Test\TestCase { } public function testOverwriteDefaultCommentsManager(): void { - $config = $this->server->getConfig(); + $config = $this->server->get(IConfig::class); $defaultManagerFactory = $config->getSystemValue('comments.managerFactory', '\OC\Comments\ManagerFactory'); $config->setSystemValue('comments.managerFactory', '\Test\Comments\FakeFactory'); diff --git a/tests/lib/Support/Subscription/RegistryTest.php b/tests/lib/Support/Subscription/RegistryTest.php index e6e83d6038b..3915218dac3 100644 --- a/tests/lib/Support/Subscription/RegistryTest.php +++ b/tests/lib/Support/Subscription/RegistryTest.php @@ -119,7 +119,7 @@ class RegistryTest extends TestCase { } public function testSubscriptionService(): void { - $this->serverContainer->method('query') + $this->serverContainer->method('get') ->with(DummySubscription::class) ->willReturn(new DummySubscription(true, false, false)); $this->registry->registerService(DummySubscription::class); diff --git a/tests/lib/Traits/EncryptionTrait.php b/tests/lib/Traits/EncryptionTrait.php index 2dc9213ff24..308b52d13b1 100644 --- a/tests/lib/Traits/EncryptionTrait.php +++ b/tests/lib/Traits/EncryptionTrait.php @@ -75,11 +75,11 @@ trait EncryptionTrait { $container = $this->encryptionApp->getContainer(); /** @var KeyManager $keyManager */ - $keyManager = $container->query(KeyManager::class); + $keyManager = $container->get(KeyManager::class); /** @var Setup $userSetup */ - $userSetup = $container->query(Setup::class); + $userSetup = $container->get(Setup::class); $userSetup->setupUser($name, $password); - $encryptionManager = $container->query(IManager::class); + $encryptionManager = $container->get(IManager::class); $this->encryptionApp->setUp($encryptionManager); $keyManager->init($name, $password); $this->invokePrivate($keyManager, 'keyUid', [$name]);