Skip to content
Prev Previous commit
Next Next commit
fix: Fix issues and tests in DIContainer and friends
Some tests related to MiddlewareDispatcher are still failing.

Signed-off-by: Côme Chilliet <[email protected]>
  • Loading branch information
come-nc committed Jul 8, 2025
commit ab310ce93836b01c01d7e286d6ae6650d9148620
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@
use OCP\Files\NotFoundException;
use OCP\IGroupManager;
use OCP\IRequest;
use OCP\IServerContainer;
use OCP\IUserManager;
use OCP\Share\Exceptions\GenericShareException;
use OCP\Share\Exceptions\ShareNotFound;
use OCP\Share\IManager as ShareManager;
use OCP\Share\IShare;
use Psr\Container\ContainerInterface;

/**
* @psalm-import-type Files_SharingDeletedShare from ResponseDefinitions
Expand All @@ -44,7 +44,7 @@ public function __construct(
private IGroupManager $groupManager,
private IRootFolder $rootFolder,
private IAppManager $appManager,
private IServerContainer $serverContainer,
private ContainerInterface $serverContainer,
) {
parent::__construct($appName, $request);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
use OCP\INavigationManager;
use OCP\IRequest;
use OCP\IServerContainer;
use OCP\ISession;
use OCP\IURLGenerator;
use OCP\IUserSession;
use OCP\Security\Ip\IRemoteAddress;
Expand Down Expand Up @@ -110,8 +111,8 @@ public function __construct(string $appName, array $urlParams = [], ?ServerConta
$this->registerDeprecatedAlias(IAppContainer::class, ContainerInterface::class);

// commonly used attributes
$this->registerService('userId', function (ContainerInterface $c): string {
return $c->get(IUserSession::class)->getSession()->get('user_id');
$this->registerService('userId', function (ContainerInterface $c): ?string {
return $c->get(ISession::class)->get('user_id');
});

$this->registerService('webRoot', function (ContainerInterface $c): string {
Expand Down Expand Up @@ -359,6 +360,9 @@ public function queryNoFallback($name) {
return parent::query($name);
} elseif (str_starts_with($name, \OC\AppFramework\App::buildAppNamespace($this->appName) . '\\')) {
return parent::query($name);
} elseif (str_starts_with($name, 'OC\\AppFramework\\Services\\')) {
/* AppFramework services are scoped to the application */
return parent::query($name);
}

throw new QueryException('Could not resolve ' . $name . '!'
Expand Down
26 changes: 5 additions & 21 deletions lib/private/Support/CrashReport/Registry.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@
* SPDX-FileCopyrightText: 2016 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OC\Support\CrashReport;

use Exception;
use OCP\AppFramework\QueryException;
use OCP\IServerContainer;
use OCP\Server;
use OCP\Support\CrashReport\ICollectBreadcrumbs;
use OCP\Support\CrashReport\IMessageReporter;
use OCP\Support\CrashReport\IRegistry;
Expand All @@ -26,17 +27,8 @@ class Registry implements IRegistry {
/** @var IReporter[] */
private $reporters = [];

/** @var IServerContainer */
private $serverContainer;

public function __construct(IServerContainer $serverContainer) {
$this->serverContainer = $serverContainer;
}

/**
* Register a reporter instance
*
* @param IReporter $reporter
*/
public function register(IReporter $reporter): void {
$this->reporters[] = $reporter;
Expand All @@ -49,10 +41,6 @@ public function registerLazy(string $class): void {
/**
* Delegate breadcrumb collection to all registered reporters
*
* @param string $message
* @param string $category
* @param array $context
*
* @since 15.0.0
*/
public function delegateBreadcrumb(string $message, string $category, array $context = []): void {
Expand All @@ -69,7 +57,6 @@ public function delegateBreadcrumb(string $message, string $category, array $con
* Delegate crash reporting to all registered reporters
*
* @param Exception|Throwable $exception
* @param array $context
*/
public function delegateReport($exception, array $context = []): void {
$this->loadLazyProviders();
Expand All @@ -82,9 +69,6 @@ public function delegateReport($exception, array $context = []): void {
/**
* Delegate a message to all reporters that implement IMessageReporter
*
* @param string $message
* @param array $context
*
* @return void
*/
public function delegateMessage(string $message, array $context = []): void {
Expand All @@ -101,13 +85,13 @@ private function loadLazyProviders(): void {
while (($class = array_shift($this->lazyReporters)) !== null) {
try {
/** @var IReporter $reporter */
$reporter = $this->serverContainer->query($class);
$reporter = Server::get($class);
} catch (QueryException $e) {
/*
* There is a circular dependency between the logger and the registry, so
* we can not inject it. Thus the static call.
*/
\OC::$server->get(LoggerInterface::class)->critical('Could not load lazy crash reporter: ' . $e->getMessage(), [
Server::get(LoggerInterface::class)->critical('Could not load lazy crash reporter: ' . $e->getMessage(), [
'exception' => $e,
]);
return;
Expand All @@ -123,7 +107,7 @@ private function loadLazyProviders(): void {
* There is a circular dependency between the logger and the registry, so
* we can not inject it. Thus the static call.
*/
\OC::$server->get(LoggerInterface::class)->critical('Could not register lazy crash reporter: ' . $e->getMessage(), [
Server::get(LoggerInterface::class)->critical('Could not register lazy crash reporter: ' . $e->getMessage(), [
'exception' => $e,
]);
}
Expand Down
12 changes: 6 additions & 6 deletions tests/lib/AppFramework/AppTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ function rrmdir($directory) {


class AppTest extends \Test\TestCase {
private $container;
private DIContainer $container;
private $io;
private $api;
private $controller;
Expand All @@ -55,8 +55,8 @@ protected function setUp(): void {
$this->controllerMethod = 'method';

$this->container[$this->controllerName] = $this->controller;
$this->container['Dispatcher'] = $this->dispatcher;
$this->container['OCP\\AppFramework\\Http\\IOutput'] = $this->io;
$this->container[Dispatcher::class] = $this->dispatcher;
$this->container[IOutput::class] = $this->io;
$this->container['urlParams'] = ['_route' => 'not-profiler'];

$this->appPath = __DIR__ . '/../../../apps/namespacetestapp';
Expand Down Expand Up @@ -165,7 +165,7 @@ public function testCallbackIsCalled(): void {
}

public function testCoreApp(): void {
$this->container['AppName'] = 'core';
$this->container['appName'] = 'core';
$this->container['OC\Core\Controller\Foo'] = $this->controller;
$this->container['urlParams'] = ['_route' => 'not-profiler'];

Expand All @@ -183,7 +183,7 @@ public function testCoreApp(): void {
}

public function testSettingsApp(): void {
$this->container['AppName'] = 'settings';
$this->container['appName'] = 'settings';
$this->container['OCA\Settings\Controller\Foo'] = $this->controller;
$this->container['urlParams'] = ['_route' => 'not-profiler'];

Expand All @@ -201,7 +201,7 @@ public function testSettingsApp(): void {
}

public function testApp(): void {
$this->container['AppName'] = 'bar';
$this->container['appName'] = 'bar';
$this->container['OCA\Bar\Controller\Foo'] = $this->controller;
$this->container['urlParams'] = ['_route' => 'not-profiler'];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@
use OCP\AppFramework\QueryException;
use OCP\IConfig;
use OCP\IRequestId;
use PHPUnit\Framework\MockObject\MockObject;

/**
* @group DB
*/
class DIContainerTest extends \Test\TestCase {
/** @var DIContainer|\PHPUnit\Framework\MockObject\MockObject */
private $container;
private DIContainer&MockObject $container;

protected function setUp(): void {
parent::setUp();
Expand All @@ -45,11 +45,13 @@ public function testProvidesMiddlewareDispatcher(): void {

public function testProvidesAppName(): void {
$this->assertTrue(isset($this->container['AppName']));
$this->assertTrue(isset($this->container['appName']));
}


public function testAppNameIsSetCorrectly(): void {
$this->assertEquals('name', $this->container['AppName']);
$this->assertEquals('name', $this->container['appName']);
}

public function testMiddlewareDispatcherIncludesSecurityMiddleware(): void {
Expand Down