Skip to content

chore: Remove deprecated interface ArrayAccess on the container #64495

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 1 commit into
base: master
Choose a base branch
from
Open

chore: Remove deprecated interface ArrayAccess on the container #64495

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 0 additions & 6 deletions build/psalm-baseline.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3222,9 +3222,6 @@
</MoreSpecificImplementedParamType>
</file>
<file src="lib/private/AppFramework/Utility/SimpleContainer.php">
<MissingTemplateParam>
<code><![CDATA[ArrayAccess]]></code>
</MissingTemplateParam>
<RedundantCast>
<code><![CDATA[(int)$e->getCode()]]></code>
</RedundantCast>
Expand Down Expand Up @@ -4063,9 +4060,6 @@
</DeprecatedMethod>
</file>
<file src="tests/lib/TestCase.php">
<DeprecatedMethod>
<code><![CDATA[$container]]></code>
</DeprecatedMethod>
<InternalMethod>
<code><![CDATA[lockFile]]></code>
<code><![CDATA[unlockFile]]></code>
Expand Down
12 changes: 6 additions & 6 deletions lib/private/AppFramework/App.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public static function main(
$profiler->setEnabled($profiler->isEnabled() && !is_null($urlParams) && isset($urlParams['_route']) && !str_starts_with($urlParams['_route'], 'profiler.'));
if ($profiler->isEnabled()) {
Server::get(IEventLogger::class)->activate();
$profiler->add(new RoutingDataCollector($container['appName'], $controllerName, $methodName));
$profiler->add(new RoutingDataCollector($container->get('appName'), $controllerName, $methodName));
}

$eventLogger->start('app:controller:params', 'Gather controller parameters');
Expand All @@ -88,12 +88,12 @@ public static function main(
/** @var Request $request */
$request = $container->get(IRequest::class);
$request->setUrlParameters($urlParams);
} elseif (isset($container['urlParams']) && !is_null($container['urlParams'])) {
} elseif ($container->has('urlParams') && !is_null($container->get('urlParams'))) {
/** @var Request $request */
$request = $container->get(IRequest::class);
$request->setUrlParameters($container['urlParams']);
$request->setUrlParameters($container->get('urlParams'));
}
$appName = $container['appName'];
$appName = $container->get('appName');

$eventLogger->end('app:controller:params');

Expand Down Expand Up @@ -139,7 +139,7 @@ public static function main(

$eventLogger->end('app:controller:run');

$io = $container[IOutput::class];
$io = $container->get(IOutput::class);

if ($profiler->isEnabled()) {
$eventLogger->end('runtime');
Expand Down Expand Up @@ -169,7 +169,7 @@ public static function main(
$value['value'],
$expireDate,
$container->get('webRoot'),
null,
'',
$container->getServer()->get(IRequest::class)->getServerProtocol() === 'https',
true,
$sameSite
Expand Down
35 changes: 7 additions & 28 deletions lib/private/AppFramework/Utility/SimpleContainer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

namespace OC\AppFramework\Utility;

use ArrayAccess;
use Closure;
use OCP\AppFramework\QueryException;
use OCP\IContainer;
Expand All @@ -26,7 +25,7 @@
/**
* SimpleContainer is a simple implementation of a container on basis of Pimple
*/
class SimpleContainer implements ArrayAccess, ContainerInterface, IContainer {
class SimpleContainer implements ContainerInterface, IContainer {
/** @psalm-suppress ImpureStaticProperty A static property is the only way to pass the information from config to autoload */
public static bool $useLazyObjects = false;

Expand Down Expand Up @@ -137,7 +136,7 @@
public function resolve(string $name, array $chain = []): mixed {
$baseMsg = 'Could not resolve ' . $name . '!';
try {
$class = new ReflectionClass($name);

Check failure on line 139 in lib/private/AppFramework/Utility/SimpleContainer.php

View workflow job for this annotation

GitHub Actions / static-code-analysis-security

TaintedCallable 

lib/private/AppFramework/Utility/SimpleContainer.php:139:33: TaintedCallable: Detected tainted text (see https://psalm.dev/243)
if ($class->isInstantiable()) {
return $this->buildClass($class, $chain);
} else {
Expand Down Expand Up @@ -254,36 +253,16 @@
}

/**
* @deprecated 20.0.0 use \Psr\Container\ContainerInterface::has
* @internal Used by tests
*/
#[\Override]
public function offsetExists($id): bool {
return $this->container->offsetExists($id);
}

/**
* @deprecated 20.0.0 use \Psr\Container\ContainerInterface::get
* @return mixed
*/
#[\Override]
#[\ReturnTypeWillChange]
public function offsetGet($id) {
return $this->container->offsetGet($id);
public function removeFromInternalContainer(string $service): void {
unset($this->container[$service]);
}

/**
* @deprecated 20.0.0 use \OCP\IContainer::registerService
* @internal Used by server container on app containers
*/
#[\Override]
public function offsetSet($offset, $value): void {
$this->container->offsetSet($offset, $value);
}

/**
* @deprecated 20.0.0
*/
#[\Override]
public function offsetUnset($offset): void {
$this->container->offsetUnset($offset);
public function setInInternalContainer(string $service, mixed $value): void {
$this->container[$service] = $value;
}
}
4 changes: 2 additions & 2 deletions lib/private/Server.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -781,8 +781,8 @@ public function __construct(
);
});
$this->registerService(Request::class, function (ContainerInterface $c) {
if (isset($this['urlParams'])) {
$urlParams = $this['urlParams'];
if ($this->has('urlParams')) {
$urlParams = $this->get('urlParams');
} else {
$urlParams = [];
}
Expand Down
2 changes: 1 addition & 1 deletion lib/private/ServerContainer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,9 @@
$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 90 in lib/private/ServerContainer.php

View workflow job for this annotation

GitHub Actions / static-code-analysis-security

TaintedCallable 

lib/private/ServerContainer.php:90:17: TaintedCallable: Detected tainted text (see https://psalm.dev/243)
if (isset($this->appContainers[$namespace])) {
$this->appContainers[$namespace]->offsetSet($applicationClassName, $app);
$this->appContainers[$namespace]->setInInternalContainer($applicationClassName, $app);
/** @psalm-suppress NoValue false-positive (see comment above) */
return $this->appContainers[$namespace];
}
Expand Down
26 changes: 13 additions & 13 deletions tests/lib/AppFramework/AppTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,10 @@ protected function setUp(): void {
$this->controllerName = 'Controller';
$this->controllerMethod = 'method';

$this->container[$this->controllerName] = $this->controller;
$this->container[Dispatcher::class] = $this->dispatcher;
$this->container[IOutput::class] = $this->io;
$this->container['urlParams'] = ['_route' => 'not-profiler'];
$this->container->setInInternalContainer($this->controllerName, $this->controller);
$this->container->setInInternalContainer(Dispatcher::class, $this->dispatcher);
$this->container->setInInternalContainer(IOutput::class, $this->io);
$this->container->registerParameter('urlParams', ['_route' => 'not-profiler']);

$this->appPath = __DIR__ . '/../../../apps/namespacetestapp';
$infoXmlPath = $this->appPath . '/appinfo/info.xml';
Expand Down Expand Up @@ -159,9 +159,9 @@ public function testCallbackIsCalled(): void {
}

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

$return = ['HTTP/2.0 200 OK', [], [], null, new Response()];
$this->dispatcher->expects($this->once())
Expand All @@ -177,9 +177,9 @@ public function testCoreApp(): void {
}

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

$return = ['HTTP/2.0 200 OK', [], [], null, new Response()];
$this->dispatcher->expects($this->once())
Expand All @@ -195,9 +195,9 @@ public function testSettingsApp(): void {
}

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

$return = ['HTTP/2.0 200 OK', [], [], null, new Response()];
$this->dispatcher->expects($this->once())
Expand Down
56 changes: 28 additions & 28 deletions tests/lib/AppFramework/DependencyInjection/DIContainerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,30 +36,30 @@ protected function setUp(): void {
}

public function testProvidesRequest(): void {
$this->assertTrue(isset($this->container['Request']));
$this->assertTrue($this->container->has('Request'));
}

public function testProvidesMiddlewareDispatcher(): void {
$this->assertTrue(isset($this->container['MiddlewareDispatcher']));
$this->assertTrue($this->container->has('MiddlewareDispatcher'));
}

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

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

public function testMiddlewareDispatcherIncludesSecurityMiddleware(): void {
$this->container['Request'] = new Request(
$this->container->registerService('Request', fn () => new Request(
['method' => 'GET'],
$this->createMock(IRequestId::class),
$this->createMock(IConfig::class)
);
$dispatcher = $this->container['MiddlewareDispatcher'];
));
$dispatcher = $this->container->get('MiddlewareDispatcher');
$middlewares = $dispatcher->getMiddlewares();

$found = false;
Expand All @@ -74,29 +74,29 @@ public function testMiddlewareDispatcherIncludesSecurityMiddleware(): void {

public function testMiddlewareDispatcherIncludesBootstrapMiddlewares(): void {
$coordinator = $this->createMock(Coordinator::class);
$this->container[Coordinator::class] = $coordinator;
$this->container['Request'] = $this->createMock(Request::class);
$this->container->registerService(Coordinator::class, fn () => $coordinator);
$this->container->registerService('Request', fn () => $this->createMock(Request::class));
$registrationContext = $this->createMock(RegistrationContext::class);
$registrationContext->method('getMiddlewareRegistrations')
->willReturn([
new MiddlewareRegistration($this->container['appName'], 'foo', false),
new MiddlewareRegistration($this->container->get('appName'), 'foo', false),
new MiddlewareRegistration('otherapp', 'bar', false),
]);
$this->container['foo'] = new class extends Middleware {
};
$this->container['bar'] = new class extends Middleware {
};
$this->container->registerService('foo', fn () => new class extends Middleware {
});
$this->container->registerService('bar', fn () => new class extends Middleware {
});
$coordinator->method('getRegistrationContext')->willReturn($registrationContext);

$dispatcher = $this->container['MiddlewareDispatcher'];
$dispatcher = $this->container->get('MiddlewareDispatcher');

$middlewares = $dispatcher->getMiddlewares();
self::assertNotEmpty($middlewares);
foreach ($middlewares as $middleware) {
if ($middleware === $this->container['bar']) {
if ($middleware === $this->container->get('bar')) {
$this->fail('Container must not register this middleware');
}
if ($middleware === $this->container['foo']) {
if ($middleware === $this->container->get('foo')) {
// It is done
return;
}
Expand All @@ -106,29 +106,29 @@ public function testMiddlewareDispatcherIncludesBootstrapMiddlewares(): void {

public function testMiddlewareDispatcherIncludesGlobalBootstrapMiddlewares(): void {
$coordinator = $this->createMock(Coordinator::class);
$this->container[Coordinator::class] = $coordinator;
$this->container['Request'] = $this->createMock(Request::class);
$this->container->registerService(Coordinator::class, fn () => $coordinator);
$this->container->registerService('Request', fn () => $this->createMock(Request::class));
$registrationContext = $this->createMock(RegistrationContext::class);
$registrationContext->method('getMiddlewareRegistrations')
->willReturn([
new MiddlewareRegistration('otherapp', 'foo', true),
new MiddlewareRegistration('otherapp', 'bar', false),
]);
$this->container['foo'] = new class extends Middleware {
};
$this->container['bar'] = new class extends Middleware {
};
$this->container->registerService('foo', fn () => new class extends Middleware {
});
$this->container->registerService('bar', fn () => new class extends Middleware {
});
$coordinator->method('getRegistrationContext')->willReturn($registrationContext);

$dispatcher = $this->container['MiddlewareDispatcher'];
$dispatcher = $this->container->get('MiddlewareDispatcher');

$middlewares = $dispatcher->getMiddlewares();
self::assertNotEmpty($middlewares);
foreach ($middlewares as $middleware) {
if ($middleware === $this->container['bar']) {
if ($middleware === $this->container->get('bar')) {
$this->fail('Container must not register this middleware');
}
if ($middleware === $this->container['foo']) {
if ($middleware === $this->container->get('foo')) {
// It is done
return;
}
Expand Down
19 changes: 10 additions & 9 deletions tests/lib/TestCase.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ public function restoreService(string $name): bool {
} else {
// The service was not registered before the test override.
// Remove the test registration so the container returns to its prior state.
unset($container[$name]);
/** @psalm-suppress InternalMethod */
$container->removeFromInternalContainer($name);
}

unset($this->services[$name]);
Expand Down Expand Up @@ -368,14 +369,14 @@ public static function tearDownAfterClass(): void {
self::tearDownAfterClassCleanStrayLocks();

// Ensure we start with fresh instances of some classes to reduce side-effects between tests
/** @psalm-suppress DeprecatedMethod */
unset(\OC::$server[Factory::class]);
/** @psalm-suppress DeprecatedMethod */
unset(\OC::$server[AppFetcher::class]);
/** @psalm-suppress DeprecatedMethod */
unset(\OC::$server[Installer::class]);
/** @psalm-suppress DeprecatedMethod */
unset(\OC::$server[Updater::class]);
/** @psalm-suppress InternalMethod */
\OC::$server->removeFromInternalContainer(Factory::class);
/** @psalm-suppress InternalMethod */
\OC::$server->removeFromInternalContainer(AppFetcher::class);
/** @psalm-suppress InternalMethod */
\OC::$server->removeFromInternalContainer(Installer::class);
/** @psalm-suppress InternalMethod */
\OC::$server->removeFromInternalContainer(Updater::class);

/** @var SetupManager $setupManager */
$setupManager = Server::get(SetupManager::class);
Expand Down
Loading