From 74df5ed480334415f19b4f9e60a32f18f4a73843 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Mon, 7 Sep 2026 12:56:12 +0200 Subject: [PATCH 01/10] feat: Add PersistAcrossRequests attribute Used to mark a service as reusable between request when running franken php. Signed-off-by: Carl Schwan --- lib/OC.php | 3 + lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + .../AppFramework/Utility/SimpleContainer.php | 36 ++++++- .../Attribute/PersistAcrossRequests.php | 27 ++++++ .../Utility/SimpleContainerTest.php | 28 ++++++ tests/lib/Console/CommandAdapterTest.php | 93 +++++++++++++++++++ .../Fixtures/CompletionFixtureCommand.php | 49 ++++++++++ .../Console/Fixtures/FixtureDependency.php | 15 +++ 9 files changed, 250 insertions(+), 3 deletions(-) create mode 100644 lib/public/AppFramework/Attribute/PersistAcrossRequests.php create mode 100644 tests/lib/Console/CommandAdapterTest.php create mode 100644 tests/lib/Console/Fixtures/CompletionFixtureCommand.php create mode 100644 tests/lib/Console/Fixtures/FixtureDependency.php diff --git a/lib/OC.php b/lib/OC.php index 251aa8b9a2bc1..86c10b9098fb7 100644 --- a/lib/OC.php +++ b/lib/OC.php @@ -7,6 +7,7 @@ * SPDX-License-Identifier: AGPL-3.0-only */ +use OC\AppFramework\Utility\SimpleContainer; use OC\Files\Filesystem; use OC\NavigationManager; use OC\Profiler\BuiltInProfiler; @@ -1382,6 +1383,8 @@ private static function resetStaticProperties(): void { */ public static function handleRequests(callable $handler): void { if (function_exists('frankenphp_handle_request') && isset($_SERVER['FRANKENPHP_WORKER']) && $_SERVER['FRANKENPHP_WORKER'] === '1') { + SimpleContainer::$keepPersistentServices = true; + $maxRequests = (int)($_SERVER['MAX_REQUESTS'] ?? 0); for ($nbRequests = 0; !$maxRequests || $nbRequests < $maxRequests; ++$nbRequests) { $keepRunning = \frankenphp_handle_request($handler); diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 4467383ba6a9b..a85e98f0501b1 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -106,6 +106,7 @@ 'OCP\\AppFramework\\Attribute\\ExceptionalImplementable' => $baseDir . '/lib/public/AppFramework/Attribute/ExceptionalImplementable.php', 'OCP\\AppFramework\\Attribute\\Implementable' => $baseDir . '/lib/public/AppFramework/Attribute/Implementable.php', 'OCP\\AppFramework\\Attribute\\Listenable' => $baseDir . '/lib/public/AppFramework/Attribute/Listenable.php', + 'OCP\\AppFramework\\Attribute\\PersistAcrossRequests' => $baseDir . '/lib/public/AppFramework/Attribute/PersistAcrossRequests.php', 'OCP\\AppFramework\\Attribute\\Throwable' => $baseDir . '/lib/public/AppFramework/Attribute/Throwable.php', 'OCP\\AppFramework\\AuthPublicShareController' => $baseDir . '/lib/public/AppFramework/AuthPublicShareController.php', 'OCP\\AppFramework\\Bootstrap\\IBootContext' => $baseDir . '/lib/public/AppFramework/Bootstrap/IBootContext.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 27b13860aec74..47f5465aac9cc 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -147,6 +147,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OCP\\AppFramework\\Attribute\\ExceptionalImplementable' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Attribute/ExceptionalImplementable.php', 'OCP\\AppFramework\\Attribute\\Implementable' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Attribute/Implementable.php', 'OCP\\AppFramework\\Attribute\\Listenable' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Attribute/Listenable.php', + 'OCP\\AppFramework\\Attribute\\PersistAcrossRequests' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Attribute/PersistAcrossRequests.php', 'OCP\\AppFramework\\Attribute\\Throwable' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Attribute/Throwable.php', 'OCP\\AppFramework\\AuthPublicShareController' => __DIR__ . '/../../..' . '/lib/public/AppFramework/AuthPublicShareController.php', 'OCP\\AppFramework\\Bootstrap\\IBootContext' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Bootstrap/IBootContext.php', diff --git a/lib/private/AppFramework/Utility/SimpleContainer.php b/lib/private/AppFramework/Utility/SimpleContainer.php index 6246a56ac209b..357ee8d441ee7 100644 --- a/lib/private/AppFramework/Utility/SimpleContainer.php +++ b/lib/private/AppFramework/Utility/SimpleContainer.php @@ -10,6 +10,7 @@ use ArrayAccess; use Closure; +use OCP\AppFramework\Attribute\PersistAcrossRequests; use OCP\AppFramework\QueryException; use OCP\IContainer; use Pimple\Container; @@ -30,6 +31,23 @@ class SimpleContainer implements ArrayAccess, 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; + /** @psalm-suppress ImpureStaticProperty Set once when a long-running worker (e.g. FrankenPHP) starts */ + public static bool $keepPersistentServices = false; + + /** + * @psalm-suppress ImpureStaticProperty This class has a reset method + * @var array + */ + private static array $persistentInstances = []; + + /** + * @internal + */ + public static function resetPersistentInstances(): void { + self::$persistentInstances = []; + self::$keepPersistentServices = false; + } + protected Container $container; /** @var array */ @@ -138,12 +156,24 @@ public function resolve(string $name, array $chain = []): mixed { $baseMsg = 'Could not resolve ' . $name . '!'; try { $class = new ReflectionClass($name); - if ($class->isInstantiable()) { - return $this->buildClass($class, $chain); - } else { + if (!$class->isInstantiable()) { throw new QueryException($baseMsg . ' Class can not be instantiated'); } + + $isPersistent = self::$keepPersistentServices + && !empty($class->getAttributes(PersistAcrossRequests::class)); + if ($isPersistent && isset(self::$persistentInstances[$class->getName()])) { + return self::$persistentInstances[$class->getName()]; + } + + $object = $this->buildClass($class, $chain); + + if ($isPersistent) { + self::$persistentInstances[$class->getName()] = $object; + } + + return $object; } catch (ReflectionException $e) { // Class does not exist throw new QueryNotFoundException($baseMsg . ' ' . $e->getMessage()); diff --git a/lib/public/AppFramework/Attribute/PersistAcrossRequests.php b/lib/public/AppFramework/Attribute/PersistAcrossRequests.php new file mode 100644 index 0000000000000..1b4bbe89c863a --- /dev/null +++ b/lib/public/AppFramework/Attribute/PersistAcrossRequests.php @@ -0,0 +1,27 @@ +container = new SimpleContainer(); } + #[\Override] + protected function tearDown(): void { + SimpleContainer::resetPersistentInstances(); + + parent::tearDown(); + } + public function testRegister(): void { $this->container->registerParameter('test', 'abc'); $this->assertEquals('abc', $this->container->get('test')); @@ -120,6 +132,22 @@ public function testInstancesOnlyOnce(): void { $this->assertSame($object, $object2); } + public function testPersistAcrossRequestsIgnoredByDefault(): void { + $object = $this->container->query(ClassPersistAcrossRequests::class); + $object2 = (new SimpleContainer())->query(ClassPersistAcrossRequests::class); + $this->assertNotSame($object, $object2); + } + + public function testPersistAcrossRequestsKeepsInstanceOnceEnabled(): void { + SimpleContainer::$keepPersistentServices = true; + + $object = $this->container->query(ClassPersistAcrossRequests::class); + // Simulate a new request rebuilding the whole Server container + $object2 = (new SimpleContainer())->query(ClassPersistAcrossRequests::class); + + $this->assertSame($object, $object2); + } + public function testConstructorSimple(): void { $this->container->registerParameter('test', 'abc'); $object = $this->container->get( diff --git a/tests/lib/Console/CommandAdapterTest.php b/tests/lib/Console/CommandAdapterTest.php new file mode 100644 index 0000000000000..d203105e29e88 --- /dev/null +++ b/tests/lib/Console/CommandAdapterTest.php @@ -0,0 +1,93 @@ +container = $this->createMock(ContainerInterface::class); + $this->container->method('get') + ->with(CompletionFixtureCommand::class) + ->willReturn(new CompletionFixtureCommand(new FixtureDependency())); + } + + private function createAdapter(): CommandAdapter { + return new CommandAdapter(CompletionFixtureCommand::class, null, $this->container); + } + + private function contextWithCurrentWord(string $word): CompletionContext&MockObject { + $context = $this->createMock(CompletionContext::class); + $context->method('getCurrentWord')->willReturn($word); + return $context; + } + + public function testCompleteArgumentValuesResolvesAStaticCallableDynamically(): void { + $adapter = $this->createAdapter(); + $this->assertEquals(['alpha'], $adapter->completeArgumentValues('dynamic', $this->contextWithCurrentWord('a'))); + $this->assertEquals(['alpha', 'beta', 'gamma'], $adapter->completeArgumentValues('dynamic', $this->contextWithCurrentWord(''))); + } + + public function testCompleteArgumentValuesResolvesANonStaticCallableOnAContainerResolvedInstance(): void { + $adapter = $this->createAdapter(); + $this->assertEquals(['injected-value'], $adapter->completeArgumentValues('instanceBased', $this->contextWithCurrentWord(''))); + } + + public function testCompleteArgumentValuesReturnsAStaticList(): void { + $adapter = $this->createAdapter(); + $this->assertEquals(['foo', 'bar'], $adapter->completeArgumentValues('static', $this->contextWithCurrentWord(''))); + } + + public function testCompleteArgumentValuesReturnsEmptyForUnknownArgument(): void { + $adapter = $this->createAdapter(); + $this->assertEquals([], $adapter->completeArgumentValues('does-not-exist', $this->contextWithCurrentWord(''))); + } + + public function testCompleteOptionValuesReturnsAStaticList(): void { + $adapter = $this->createAdapter(); + $this->assertEquals(['x', 'y'], $adapter->completeOptionValues('option', $this->contextWithCurrentWord(''))); + } + + public function testCompleteOptionValuesStillHardcodesOutputFormats(): void { + $adapter = $this->createAdapter(); + $this->assertEquals(['plain', 'json', 'json_pretty'], $adapter->completeOptionValues('output', $this->contextWithCurrentWord(''))); + } + + /** "occ completion" goes through Command::complete(), a separate path from completeArgumentValues() above. */ + public function testNativeCompletionResolvesAStaticCallableDynamically(): void { + $tester = new CommandCompletionTester($this->createAdapter()); + $this->assertEquals(['alpha'], $tester->complete(['a'])); + } + + public function testNativeCompletionResolvesANonStaticCallableOnAContainerResolvedInstance(): void { + $tester = new CommandCompletionTester($this->createAdapter()); + $this->assertEquals(['injected-value'], $tester->complete(['x', ''])); + } + + public function testNativeCompletionReturnsAStaticListForAnArgument(): void { + $tester = new CommandCompletionTester($this->createAdapter()); + $this->assertEquals(['foo', 'bar'], $tester->complete(['x', 'y', ''])); + } + + public function testNativeCompletionReturnsAStaticListForAnOption(): void { + $tester = new CommandCompletionTester($this->createAdapter()); + $this->assertEquals(['x', 'y'], $tester->complete(['--option', ''])); + } +} diff --git a/tests/lib/Console/Fixtures/CompletionFixtureCommand.php b/tests/lib/Console/Fixtures/CompletionFixtureCommand.php new file mode 100644 index 0000000000000..6360f1a5f8c3a --- /dev/null +++ b/tests/lib/Console/Fixtures/CompletionFixtureCommand.php @@ -0,0 +1,49 @@ + str_starts_with($v, $currentWord))); + } + + public function suggestFromInstance(string $currentWord): array { + return [$this->dependency?->getValue() ?? 'no-dependency']; + } +} diff --git a/tests/lib/Console/Fixtures/FixtureDependency.php b/tests/lib/Console/Fixtures/FixtureDependency.php new file mode 100644 index 0000000000000..6a28e8e5fd923 --- /dev/null +++ b/tests/lib/Console/Fixtures/FixtureDependency.php @@ -0,0 +1,15 @@ + Date: Mon, 7 Sep 2026 14:49:15 +0200 Subject: [PATCH 02/10] feat: Allow to invalidate a group of services Signed-off-by: Carl Schwan --- core/AppInfo/Application.php | 5 ++ .../PersistentServiceInvalidationListener.php | 34 +++++++++ lib/composer/composer/autoload_classmap.php | 4 ++ lib/composer/composer/autoload_static.php | 4 ++ lib/private/AppConfig.php | 14 ++++ .../Utility/PersistentServiceInvalidator.php | 43 +++++++++++ .../AppFramework/Utility/SimpleContainer.php | 68 ++++++++++++++++-- lib/private/Server.php | 3 + lib/private/SystemConfig.php | 14 ++++ .../Attribute/PersistAcrossRequests.php | 11 +++ .../Utility/IPersistentServiceInvalidator.php | 30 ++++++++ .../Utility/PersistentServiceGroup.php | 35 +++++++++ ...sistentServiceInvalidationListenerTest.php | 53 ++++++++++++++ tests/lib/AppConfigIntegrationTest.php | 46 ++++++++++++ .../PersistentServiceInvalidatorTest.php | 58 +++++++++++++++ .../Utility/SimpleContainerTest.php | 72 +++++++++++++++++-- tests/lib/SystemConfigTest.php | 41 +++++++++++ 17 files changed, 526 insertions(+), 9 deletions(-) create mode 100644 core/Listener/PersistentServiceInvalidationListener.php create mode 100644 lib/private/AppFramework/Utility/PersistentServiceInvalidator.php create mode 100644 lib/public/AppFramework/Utility/IPersistentServiceInvalidator.php create mode 100644 lib/public/AppFramework/Utility/PersistentServiceGroup.php create mode 100644 tests/Core/Listener/PersistentServiceInvalidationListenerTest.php create mode 100644 tests/lib/AppFramework/Utility/PersistentServiceInvalidatorTest.php diff --git a/core/AppInfo/Application.php b/core/AppInfo/Application.php index 42a3a09f2625a..067cb136cb328 100644 --- a/core/AppInfo/Application.php +++ b/core/AppInfo/Application.php @@ -24,6 +24,7 @@ use OC\Core\Listener\BeforeTemplateRenderedListener; use OC\Core\Listener\LoadAdditionalEntriesListener; use OC\Core\Listener\PasswordUpdatedListener; +use OC\Core\Listener\PersistentServiceInvalidationListener; use OC\Core\Listener\RestrictInteractionListener; use OC\Core\Notification\CoreNotifier; use OC\Core\Sharing\Permission\EditSharePermissionPreset; @@ -42,6 +43,8 @@ use OC\DirectEditing\Listeners\UserDisabledTokenCleanupListener as UserDisabledDirectEditingTokenCleanupListener; use OC\OCM\OCMDiscoveryHandler; use OC\TagManager; +use OCP\App\Events\AppDisableEvent; +use OCP\App\Events\AppEnableEvent; use OCP\AppFramework\App; use OCP\AppFramework\Bootstrap\IBootContext; use OCP\AppFramework\Bootstrap\IBootstrap; @@ -92,6 +95,8 @@ public function register(IRegistrationContext $context): void { $context->registerEventListener(BeforeTemplateRenderedEvent::class, BeforeTemplateRenderedListener::class); $context->registerEventListener(BeforeLoginTemplateRenderedEvent::class, BeforeTemplateRenderedListener::class); $context->registerEventListener(LoadAdditionalEntriesEvent::class, LoadAdditionalEntriesListener::class); + $context->registerEventListener(AppEnableEvent::class, PersistentServiceInvalidationListener::class); + $context->registerEventListener(AppDisableEvent::class, PersistentServiceInvalidationListener::class); $context->registerEventListener(RemoteWipeStarted::class, RemoteWipeActivityListener::class); $context->registerEventListener(RemoteWipeStarted::class, RemoteWipeNotificationsListener::class); $context->registerEventListener(RemoteWipeStarted::class, RemoteWipeEmailListener::class); diff --git a/core/Listener/PersistentServiceInvalidationListener.php b/core/Listener/PersistentServiceInvalidationListener.php new file mode 100644 index 0000000000000..1bae155ab03cb --- /dev/null +++ b/core/Listener/PersistentServiceInvalidationListener.php @@ -0,0 +1,34 @@ + + */ +class PersistentServiceInvalidationListener implements IEventListener { + public function __construct( + private IPersistentServiceInvalidator $invalidator, + ) { + } + + #[\Override] + public function handle(Event $event): void { + if ($event instanceof AppEnableEvent || $event instanceof AppDisableEvent) { + $this->invalidator->invalidate(PersistentServiceGroup::Apps); + } + } +} diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index a85e98f0501b1..c978eb18dc6f5 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -198,7 +198,9 @@ 'OCP\\AppFramework\\Services\\IInitialState' => $baseDir . '/lib/public/AppFramework/Services/IInitialState.php', 'OCP\\AppFramework\\Services\\InitialStateProvider' => $baseDir . '/lib/public/AppFramework/Services/InitialStateProvider.php', 'OCP\\AppFramework\\Utility\\IControllerMethodReflector' => $baseDir . '/lib/public/AppFramework/Utility/IControllerMethodReflector.php', + 'OCP\\AppFramework\\Utility\\IPersistentServiceInvalidator' => $baseDir . '/lib/public/AppFramework/Utility/IPersistentServiceInvalidator.php', 'OCP\\AppFramework\\Utility\\ITimeFactory' => $baseDir . '/lib/public/AppFramework/Utility/ITimeFactory.php', + 'OCP\\AppFramework\\Utility\\PersistentServiceGroup' => $baseDir . '/lib/public/AppFramework/Utility/PersistentServiceGroup.php', 'OCP\\App\\AppInfoDefinition' => $baseDir . '/lib/public/App/AppInfoDefinition.php', 'OCP\\App\\AppPathNotFoundException' => $baseDir . '/lib/public/App/AppPathNotFoundException.php', 'OCP\\App\\Events\\AppDisableEvent' => $baseDir . '/lib/public/App/Events/AppDisableEvent.php', @@ -1251,6 +1253,7 @@ 'OC\\AppFramework\\Services\\AppConfig' => $baseDir . '/lib/private/AppFramework/Services/AppConfig.php', 'OC\\AppFramework\\Services\\InitialState' => $baseDir . '/lib/private/AppFramework/Services/InitialState.php', 'OC\\AppFramework\\Utility\\ControllerMethodReflector' => $baseDir . '/lib/private/AppFramework/Utility/ControllerMethodReflector.php', + 'OC\\AppFramework\\Utility\\PersistentServiceInvalidator' => $baseDir . '/lib/private/AppFramework/Utility/PersistentServiceInvalidator.php', 'OC\\AppFramework\\Utility\\QueryNotFoundException' => $baseDir . '/lib/private/AppFramework/Utility/QueryNotFoundException.php', 'OC\\AppFramework\\Utility\\SimpleContainer' => $baseDir . '/lib/private/AppFramework/Utility/SimpleContainer.php', 'OC\\AppFramework\\Utility\\TimeFactory' => $baseDir . '/lib/private/AppFramework/Utility/TimeFactory.php', @@ -1638,6 +1641,7 @@ 'OC\\Core\\Listener\\FeedBackHandler' => $baseDir . '/core/Listener/FeedBackHandler.php', 'OC\\Core\\Listener\\LoadAdditionalEntriesListener' => $baseDir . '/core/Listener/LoadAdditionalEntriesListener.php', 'OC\\Core\\Listener\\PasswordUpdatedListener' => $baseDir . '/core/Listener/PasswordUpdatedListener.php', + 'OC\\Core\\Listener\\PersistentServiceInvalidationListener' => $baseDir . '/core/Listener/PersistentServiceInvalidationListener.php', 'OC\\Core\\Listener\\RestrictInteractionListener' => $baseDir . '/core/Listener/RestrictInteractionListener.php', 'OC\\Core\\Middleware\\TwoFactorMiddleware' => $baseDir . '/core/Middleware/TwoFactorMiddleware.php', 'OC\\Core\\Migrations\\Version13000Date20170705121758' => $baseDir . '/core/Migrations/Version13000Date20170705121758.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 47f5465aac9cc..a6c71dc0812ef 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -239,7 +239,9 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OCP\\AppFramework\\Services\\IInitialState' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Services/IInitialState.php', 'OCP\\AppFramework\\Services\\InitialStateProvider' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Services/InitialStateProvider.php', 'OCP\\AppFramework\\Utility\\IControllerMethodReflector' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Utility/IControllerMethodReflector.php', + 'OCP\\AppFramework\\Utility\\IPersistentServiceInvalidator' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Utility/IPersistentServiceInvalidator.php', 'OCP\\AppFramework\\Utility\\ITimeFactory' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Utility/ITimeFactory.php', + 'OCP\\AppFramework\\Utility\\PersistentServiceGroup' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Utility/PersistentServiceGroup.php', 'OCP\\App\\AppInfoDefinition' => __DIR__ . '/../../..' . '/lib/public/App/AppInfoDefinition.php', 'OCP\\App\\AppPathNotFoundException' => __DIR__ . '/../../..' . '/lib/public/App/AppPathNotFoundException.php', 'OCP\\App\\Events\\AppDisableEvent' => __DIR__ . '/../../..' . '/lib/public/App/Events/AppDisableEvent.php', @@ -1292,6 +1294,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\AppFramework\\Services\\AppConfig' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Services/AppConfig.php', 'OC\\AppFramework\\Services\\InitialState' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Services/InitialState.php', 'OC\\AppFramework\\Utility\\ControllerMethodReflector' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Utility/ControllerMethodReflector.php', + 'OC\\AppFramework\\Utility\\PersistentServiceInvalidator' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Utility/PersistentServiceInvalidator.php', 'OC\\AppFramework\\Utility\\QueryNotFoundException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Utility/QueryNotFoundException.php', 'OC\\AppFramework\\Utility\\SimpleContainer' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Utility/SimpleContainer.php', 'OC\\AppFramework\\Utility\\TimeFactory' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Utility/TimeFactory.php', @@ -1679,6 +1682,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Core\\Listener\\FeedBackHandler' => __DIR__ . '/../../..' . '/core/Listener/FeedBackHandler.php', 'OC\\Core\\Listener\\LoadAdditionalEntriesListener' => __DIR__ . '/../../..' . '/core/Listener/LoadAdditionalEntriesListener.php', 'OC\\Core\\Listener\\PasswordUpdatedListener' => __DIR__ . '/../../..' . '/core/Listener/PasswordUpdatedListener.php', + 'OC\\Core\\Listener\\PersistentServiceInvalidationListener' => __DIR__ . '/../../..' . '/core/Listener/PersistentServiceInvalidationListener.php', 'OC\\Core\\Listener\\RestrictInteractionListener' => __DIR__ . '/../../..' . '/core/Listener/RestrictInteractionListener.php', 'OC\\Core\\Middleware\\TwoFactorMiddleware' => __DIR__ . '/../../..' . '/core/Middleware/TwoFactorMiddleware.php', 'OC\\Core\\Migrations\\Version13000Date20170705121758' => __DIR__ . '/../../..' . '/core/Migrations/Version13000Date20170705121758.php', diff --git a/lib/private/AppConfig.php b/lib/private/AppConfig.php index ed103e58194a5..17b8041ffc46b 100644 --- a/lib/private/AppConfig.php +++ b/lib/private/AppConfig.php @@ -15,6 +15,8 @@ use OC\Config\ConfigManager; use OC\Config\PresetManager; use OC\Memcache\Factory as CacheFactory; +use OCP\AppFramework\Utility\IPersistentServiceInvalidator; +use OCP\AppFramework\Utility\PersistentServiceGroup; use OCP\Config\Lexicon\Entry; use OCP\Config\Lexicon\Strictness; use OCP\Config\ValueType; @@ -951,6 +953,7 @@ private function setTypedValue( if ($refreshCache) { $this->clearCache(); + $this->invalidatePersistedServices(); return true; } @@ -962,10 +965,19 @@ private function setTypedValue( } $this->valueTypes[$app][$key] = $type; $this->clearLocalCache(); + $this->invalidatePersistedServices(); return true; } + /** + * Discards services kept alive across requests (see {@see \OCP\AppFramework\Attribute\PersistAcrossRequests}) + * that declared a dependency on {@see PersistentServiceGroup::Config}. + */ + private function invalidatePersistedServices(): void { + Server::get(IPersistentServiceInvalidator::class)->invalidate(PersistentServiceGroup::Config); + } + /** * Change the type of config value. * @@ -1273,6 +1285,7 @@ public function deleteKey(string $app, string $key): void { unset($this->fastCache[$app][$key]); unset($this->valueTypes[$app][$key]); $this->clearLocalCache(); + $this->invalidatePersistedServices(); } /** @@ -1291,6 +1304,7 @@ public function deleteApp(string $app): void { $qb->executeStatement(); $this->clearCache(); + $this->invalidatePersistedServices(); } /** diff --git a/lib/private/AppFramework/Utility/PersistentServiceInvalidator.php b/lib/private/AppFramework/Utility/PersistentServiceInvalidator.php new file mode 100644 index 0000000000000..b51cc098c550e --- /dev/null +++ b/lib/private/AppFramework/Utility/PersistentServiceInvalidator.php @@ -0,0 +1,43 @@ +value : $group; + $cache = $this->cacheFactory->createDistributed(self::CACHE_PREFIX); + if ($cache instanceof IMemcache) { + $cache->inc($key); + return; + } + $cache->set($key, ((int)$cache->get($key)) + 1); + } + + /** + * @internal used by {@see SimpleContainer} to check whether a persisted instance is still valid + */ + public function getGeneration(string|PersistentServiceGroup $group): int { + $key = $group instanceof PersistentServiceGroup ? $group->value : $group; + return (int)$this->cacheFactory->createDistributed(self::CACHE_PREFIX)->get($key); + } +} diff --git a/lib/private/AppFramework/Utility/SimpleContainer.php b/lib/private/AppFramework/Utility/SimpleContainer.php index 357ee8d441ee7..d95a13e68d17e 100644 --- a/lib/private/AppFramework/Utility/SimpleContainer.php +++ b/lib/private/AppFramework/Utility/SimpleContainer.php @@ -12,6 +12,7 @@ use Closure; use OCP\AppFramework\Attribute\PersistAcrossRequests; use OCP\AppFramework\QueryException; +use OCP\AppFramework\Utility\PersistentServiceGroup; use OCP\IContainer; use Pimple\Container; use Psr\Container\ContainerExceptionInterface; @@ -34,17 +35,36 @@ class SimpleContainer implements ArrayAccess, ContainerInterface, IContainer { /** @psalm-suppress ImpureStaticProperty Set once when a long-running worker (e.g. FrankenPHP) starts */ public static bool $keepPersistentServices = false; + /** A kept instance is rebuilt after this many seconds even without an invalidation, as a safety net */ + private const MAX_PERSISTENT_AGE_SECONDS = 3600; + /** * @psalm-suppress ImpureStaticProperty This class has a reset method * @var array */ private static array $persistentInstances = []; + /** + * The invalidation generations each kept instance was built against, keyed by group name. + * + * @psalm-suppress ImpureStaticProperty This class has a reset method + * @var array> + */ + private static array $persistentGenerations = []; + + /** + * @psalm-suppress ImpureStaticProperty This class has a reset method + * @var array + */ + private static array $persistentBuiltAt = []; + /** * @internal */ public static function resetPersistentInstances(): void { self::$persistentInstances = []; + self::$persistentGenerations = []; + self::$persistentBuiltAt = []; self::$keepPersistentServices = false; } @@ -161,16 +181,28 @@ public function resolve(string $name, array $chain = []): mixed { . ' Class can not be instantiated'); } - $isPersistent = self::$keepPersistentServices - && !empty($class->getAttributes(PersistAcrossRequests::class)); - if ($isPersistent && isset(self::$persistentInstances[$class->getName()])) { - return self::$persistentInstances[$class->getName()]; + $attributes = $class->getAttributes(PersistAcrossRequests::class); + $isPersistent = self::$keepPersistentServices && !empty($attributes); + $className = $class->getName(); + $groups = $isPersistent + ? array_map( + static fn (string|PersistentServiceGroup $group): string => $group instanceof PersistentServiceGroup ? $group->value : $group, + $attributes[0]->newInstance()->invalidatedBy, + ) + : []; + + if ($isPersistent + && isset(self::$persistentInstances[$className]) + && $this->isPersistentInstanceStillValid($className, $groups)) { + return self::$persistentInstances[$className]; } $object = $this->buildClass($class, $chain); if ($isPersistent) { - self::$persistentInstances[$class->getName()] = $object; + self::$persistentInstances[$className] = $object; + self::$persistentGenerations[$className] = $this->currentGenerations($groups); + self::$persistentBuiltAt[$className] = time(); } return $object; @@ -180,6 +212,32 @@ public function resolve(string $name, array $chain = []): mixed { } } + /** + * @param list $groups + */ + private function isPersistentInstanceStillValid(string $className, array $groups): bool { + if ((time() - self::$persistentBuiltAt[$className]) > self::MAX_PERSISTENT_AGE_SECONDS) { + return false; + } + return self::$persistentGenerations[$className] === $this->currentGenerations($groups); + } + + /** + * @param list $groups + * @return array + */ + private function currentGenerations(array $groups): array { + if (empty($groups)) { + return []; + } + $invalidator = $this->get(PersistentServiceInvalidator::class); + $generations = []; + foreach ($groups as $group) { + $generations[$group] = $invalidator->getGeneration($group); + } + return $generations; + } + /** * @param string $name Already sanitized name * @param list $chain diff --git a/lib/private/Server.php b/lib/private/Server.php index ab4f6c93cb68a..3f55aafa02199 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -17,6 +17,7 @@ use OC\AppFramework\Http\RequestId; use OC\AppFramework\Services\AppConfig; use OC\AppFramework\Utility\ControllerMethodReflector; +use OC\AppFramework\Utility\PersistentServiceInvalidator; use OC\AppFramework\Utility\TimeFactory; use OC\Authentication\Events\LoginFailed; use OC\Authentication\Listeners\LoginFailedListener; @@ -164,6 +165,7 @@ use OCP\Activity\IEventMerger; use OCP\App\IAppManager; use OCP\AppFramework\Utility\IControllerMethodReflector; +use OCP\AppFramework\Utility\IPersistentServiceInvalidator; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Authentication\LoginCredentials\IStore; use OCP\Authentication\Token\IProvider as OCPIProvider; @@ -574,6 +576,7 @@ public function __construct( ); }); $this->registerAlias(ICacheFactory::class, Factory::class); + $this->registerAlias(IPersistentServiceInvalidator::class, PersistentServiceInvalidator::class); $this->registerDeprecatedAlias('RedisFactory', RedisFactory::class); diff --git a/lib/private/SystemConfig.php b/lib/private/SystemConfig.php index 312dfdeca2a93..04cd6e990c5f6 100644 --- a/lib/private/SystemConfig.php +++ b/lib/private/SystemConfig.php @@ -8,7 +8,10 @@ namespace OC; +use OCP\AppFramework\Utility\IPersistentServiceInvalidator; +use OCP\AppFramework\Utility\PersistentServiceGroup; use OCP\IConfig; +use OCP\Server; /** * Class which provides access to the system config values stored in config.php @@ -139,6 +142,7 @@ public function getKeys() { */ public function setValue($key, $value) { $this->config->setValue($key, $value); + $this->invalidatePersistedServices(); } /** @@ -149,6 +153,7 @@ public function setValue($key, $value) { */ public function setValues(array $configs) { $this->config->setValues($configs); + $this->invalidatePersistedServices(); } /** @@ -186,6 +191,15 @@ public function getFilteredValue($key, $default = '') { */ public function deleteValue($key) { $this->config->deleteKey($key); + $this->invalidatePersistedServices(); + } + + /** + * Discards services kept alive across requests (see {@see \OCP\AppFramework\Attribute\PersistAcrossRequests}) + * that declared a dependency on {@see PersistentServiceGroup::Config}. + */ + private function invalidatePersistedServices(): void { + Server::get(IPersistentServiceInvalidator::class)->invalidate(PersistentServiceGroup::Config); } /** diff --git a/lib/public/AppFramework/Attribute/PersistAcrossRequests.php b/lib/public/AppFramework/Attribute/PersistAcrossRequests.php index 1b4bbe89c863a..fa46d0a2f5dc8 100644 --- a/lib/public/AppFramework/Attribute/PersistAcrossRequests.php +++ b/lib/public/AppFramework/Attribute/PersistAcrossRequests.php @@ -10,6 +10,7 @@ namespace OCP\AppFramework\Attribute; use Attribute; +use OCP\AppFramework\Utility\PersistentServiceGroup; /** * Marks a service as safe to keep alive in the server container across @@ -24,4 +25,14 @@ */ #[Attribute(Attribute::TARGET_CLASS)] class PersistAcrossRequests { + /** + * @param list $invalidatedBy Groups that, once invalidated + * through {@see \OCP\AppFramework\Utility\IPersistentServiceInvalidator}, + * cause the kept instance to be discarded and rebuilt. + * @since 36.0.0 + */ + public function __construct( + public readonly array $invalidatedBy = [], + ) { + } } diff --git a/lib/public/AppFramework/Utility/IPersistentServiceInvalidator.php b/lib/public/AppFramework/Utility/IPersistentServiceInvalidator.php new file mode 100644 index 0000000000000..d736076f1cb3b --- /dev/null +++ b/lib/public/AppFramework/Utility/IPersistentServiceInvalidator.php @@ -0,0 +1,30 @@ +invalidator = $this->createMock(IPersistentServiceInvalidator::class); + $this->listener = new PersistentServiceInvalidationListener($this->invalidator); + } + + public function testHandlesAppEnableEvent(): void { + $this->invalidator->expects($this->once()) + ->method('invalidate') + ->with(PersistentServiceGroup::Apps); + + $this->listener->handle(new AppEnableEvent('news')); + } + + public function testHandlesAppDisableEvent(): void { + $this->invalidator->expects($this->once()) + ->method('invalidate') + ->with(PersistentServiceGroup::Apps); + + $this->listener->handle(new AppDisableEvent('news')); + } + + public function testIgnoresUnrelatedEvents(): void { + $this->invalidator->expects($this->never()) + ->method('invalidate'); + + $this->listener->handle(new Event()); + } +} diff --git a/tests/lib/AppConfigIntegrationTest.php b/tests/lib/AppConfigIntegrationTest.php index 8a65f8fc418d9..9ee260c1b441f 100644 --- a/tests/lib/AppConfigIntegrationTest.php +++ b/tests/lib/AppConfigIntegrationTest.php @@ -13,6 +13,8 @@ use OC\Config\ConfigManager; use OC\Config\PresetManager; use OC\Memcache\Factory as CacheFactory; +use OCP\AppFramework\Utility\IPersistentServiceInvalidator; +use OCP\AppFramework\Utility\PersistentServiceGroup; use OCP\Exceptions\AppConfigTypeConflictException; use OCP\Exceptions\AppConfigUnknownKeyException; use OCP\IAppConfig; @@ -644,6 +646,28 @@ public function testSetValueStringIsNotUpdated(): void { $this->assertSame(false, $config->setValueString('feed', 'string', 'value-1')); } + public function testSetValueStringInvalidatesPersistedServices(): void { + $invalidator = $this->createMock(IPersistentServiceInvalidator::class); + $invalidator->expects($this->once()) + ->method('invalidate') + ->with(PersistentServiceGroup::Config); + $this->overwriteService(IPersistentServiceInvalidator::class, $invalidator); + + $config = $this->generateAppConfig(); + $config->setValueString('feed', 'string', 'value-1'); + } + + public function testSetValueStringUnchangedDoesNotInvalidatePersistedServices(): void { + $config = $this->generateAppConfig(); + $config->setValueString('feed', 'string', 'value-1'); + + $invalidator = $this->createMock(IPersistentServiceInvalidator::class); + $invalidator->expects($this->never())->method('invalidate'); + $this->overwriteService(IPersistentServiceInvalidator::class, $invalidator); + + $config->setValueString('feed', 'string', 'value-1'); + } + public function testSetValueStringIsUpdatedCache(): void { $config = $this->generateAppConfig(); $config->setValueString('feed', 'string', 'value-1'); @@ -1365,6 +1389,17 @@ public function testDeleteKeyDatabase(): void { $this->assertSame('default', $config->getValueString('anotherapp', 'key', 'default')); } + public function testDeleteKeyInvalidatesPersistedServices(): void { + $invalidator = $this->createMock(IPersistentServiceInvalidator::class); + $invalidator->expects($this->once()) + ->method('invalidate') + ->with(PersistentServiceGroup::Config); + $this->overwriteService(IPersistentServiceInvalidator::class, $invalidator); + + $config = $this->generateAppConfig(); + $config->deleteKey('anotherapp', 'key'); + } + public function testDeleteApp(): void { $config = $this->generateAppConfig(); $config->deleteApp('anotherapp'); @@ -1389,6 +1424,17 @@ public function testDeleteAppDatabase(): void { $this->assertSame('default', $config->getValueString('anotherapp', 'enabled', 'default')); } + public function testDeleteAppInvalidatesPersistedServices(): void { + $invalidator = $this->createMock(IPersistentServiceInvalidator::class); + $invalidator->expects($this->once()) + ->method('invalidate') + ->with(PersistentServiceGroup::Config); + $this->overwriteService(IPersistentServiceInvalidator::class, $invalidator); + + $config = $this->generateAppConfig(); + $config->deleteApp('anotherapp'); + } + public function testClearCache(): void { $config = $this->generateAppConfig(); $config->setValueString('feed', 'string', '123454'); diff --git a/tests/lib/AppFramework/Utility/PersistentServiceInvalidatorTest.php b/tests/lib/AppFramework/Utility/PersistentServiceInvalidatorTest.php new file mode 100644 index 0000000000000..2003bf57b99ea --- /dev/null +++ b/tests/lib/AppFramework/Utility/PersistentServiceInvalidatorTest.php @@ -0,0 +1,58 @@ +cache = new ArrayCache(); + $cacheFactory = $this->createMock(ICacheFactory::class); + $cacheFactory->method('createDistributed') + ->willReturn($this->cache); + + $this->invalidator = new PersistentServiceInvalidator($cacheFactory); + } + + public function testGenerationStartsAtZero(): void { + $this->assertSame(0, $this->invalidator->getGeneration('apps')); + } + + public function testInvalidateBumpsTheGeneration(): void { + $this->invalidator->invalidate('apps'); + $this->assertSame(1, $this->invalidator->getGeneration('apps')); + + $this->invalidator->invalidate('apps'); + $this->assertSame(2, $this->invalidator->getGeneration('apps')); + } + + public function testGroupsAreIndependent(): void { + $this->invalidator->invalidate('apps'); + + $this->assertSame(1, $this->invalidator->getGeneration('apps')); + $this->assertSame(0, $this->invalidator->getGeneration('custom-group')); + } + + public function testEnumGroupIsEquivalentToItsStringValue(): void { + $this->invalidator->invalidate(PersistentServiceGroup::Apps); + + $this->assertSame(1, $this->invalidator->getGeneration('apps')); + $this->assertSame(1, $this->invalidator->getGeneration(PersistentServiceGroup::Apps)); + } +} diff --git a/tests/lib/AppFramework/Utility/SimpleContainerTest.php b/tests/lib/AppFramework/Utility/SimpleContainerTest.php index 5a6940c07451a..e8fc3f82cc531 100644 --- a/tests/lib/AppFramework/Utility/SimpleContainerTest.php +++ b/tests/lib/AppFramework/Utility/SimpleContainerTest.php @@ -10,9 +10,13 @@ namespace Test\AppFramework\Utility; +use OC\AppFramework\Utility\PersistentServiceInvalidator; use OC\AppFramework\Utility\SimpleContainer; +use OC\Memcache\ArrayCache; use OCP\AppFramework\Attribute\PersistAcrossRequests; use OCP\AppFramework\QueryException; +use OCP\AppFramework\Utility\PersistentServiceGroup; +use OCP\ICacheFactory; use Psr\Container\NotFoundExceptionInterface; interface TestInterface { @@ -22,6 +26,14 @@ interface TestInterface { class ClassPersistAcrossRequests { } +#[PersistAcrossRequests(invalidatedBy: ['test-group'])] +class ClassPersistAcrossRequestsWithGroup { +} + +#[PersistAcrossRequests(invalidatedBy: [PersistentServiceGroup::Apps])] +class ClassPersistAcrossRequestsWithEnumGroup { +} + class ClassEmptyConstructor implements IInterfaceConstructor { } @@ -133,21 +145,73 @@ public function testInstancesOnlyOnce(): void { } public function testPersistAcrossRequestsIgnoredByDefault(): void { - $object = $this->container->query(ClassPersistAcrossRequests::class); - $object2 = (new SimpleContainer())->query(ClassPersistAcrossRequests::class); + $object = $this->container->get(ClassPersistAcrossRequests::class); + $object2 = (new SimpleContainer())->get(ClassPersistAcrossRequests::class); $this->assertNotSame($object, $object2); } public function testPersistAcrossRequestsKeepsInstanceOnceEnabled(): void { SimpleContainer::$keepPersistentServices = true; - $object = $this->container->query(ClassPersistAcrossRequests::class); + $object = $this->container->get(ClassPersistAcrossRequests::class); // Simulate a new request rebuilding the whole Server container - $object2 = (new SimpleContainer())->query(ClassPersistAcrossRequests::class); + $object2 = (new SimpleContainer())->get(ClassPersistAcrossRequests::class); $this->assertSame($object, $object2); } + public function testPersistAcrossRequestsInvalidatedByGroup(): void { + SimpleContainer::$keepPersistentServices = true; + + $cacheFactory = $this->createMock(ICacheFactory::class); + $cacheFactory->method('createDistributed')->willReturn(new ArrayCache()); + $invalidator = new PersistentServiceInvalidator($cacheFactory); + + $registerInvalidator = function (SimpleContainer $container) use ($invalidator): void { + $container->registerService(PersistentServiceInvalidator::class, function () use ($invalidator) { + return $invalidator; + }); + }; + + $registerInvalidator($this->container); + $object = $this->container->get(ClassPersistAcrossRequestsWithGroup::class); + + // Simulate a new request rebuilding the whole Server container: nothing invalidated the group yet + $container2 = new SimpleContainer(); + $registerInvalidator($container2); + $this->assertSame($object, $container2->get(ClassPersistAcrossRequestsWithGroup::class)); + + $invalidator->invalidate('test-group'); + + $container3 = new SimpleContainer(); + $registerInvalidator($container3); + $this->assertNotSame($object, $container3->get(ClassPersistAcrossRequestsWithGroup::class)); + } + + public function testPersistAcrossRequestsAcceptsEnumGroup(): void { + SimpleContainer::$keepPersistentServices = true; + + $cacheFactory = $this->createMock(ICacheFactory::class); + $cacheFactory->method('createDistributed')->willReturn(new ArrayCache()); + $invalidator = new PersistentServiceInvalidator($cacheFactory); + + $registerInvalidator = function (SimpleContainer $container) use ($invalidator): void { + $container->registerService(PersistentServiceInvalidator::class, function () use ($invalidator) { + return $invalidator; + }); + }; + + $registerInvalidator($this->container); + $object = $this->container->get(ClassPersistAcrossRequestsWithEnumGroup::class); + + // Invalidating by the enum's string value must be indistinguishable from the enum case itself + $invalidator->invalidate('apps'); + + $container2 = new SimpleContainer(); + $registerInvalidator($container2); + $this->assertNotSame($object, $container2->get(ClassPersistAcrossRequestsWithEnumGroup::class)); + } + public function testConstructorSimple(): void { $this->container->registerParameter('test', 'abc'); $object = $this->container->get( diff --git a/tests/lib/SystemConfigTest.php b/tests/lib/SystemConfigTest.php index e08922ddd3955..d788e0708d1d7 100644 --- a/tests/lib/SystemConfigTest.php +++ b/tests/lib/SystemConfigTest.php @@ -10,6 +10,8 @@ use OC\Config; use OC\SystemConfig; +use OCP\AppFramework\Utility\IPersistentServiceInvalidator; +use OCP\AppFramework\Utility\PersistentServiceGroup; use OCP\IConfig; use PHPUnit\Framework\MockObject\MockObject; @@ -28,6 +30,15 @@ protected function setUp(): void { $this->config = $this->createMock(Config::class); } + private function getSystemConfig(): SystemConfig { + $this->config->method('getValue') + ->willReturnMap([ + ['config_extra_sensitive_values', [], []], + ]); + + return new SystemConfig($this->config); + } + public function testGetFilteredValueMasksTheEuroOfficeSecret(): void { $this->config->method('getValue') ->willReturnMap([ @@ -47,4 +58,34 @@ public function testGetFilteredValueMasksTheEuroOfficeSecret(): void { 'jwt_header' => 'AuthorizationJwt', ], $systemConfig->getFilteredValue('eurooffice')); } + + public function testSetValueInvalidatesPersistedServices(): void { + $invalidator = $this->createMock(IPersistentServiceInvalidator::class); + $invalidator->expects($this->once()) + ->method('invalidate') + ->with(PersistentServiceGroup::Config); + $this->overwriteService(IPersistentServiceInvalidator::class, $invalidator); + + $this->getSystemConfig()->setValue('foo', 'bar'); + } + + public function testSetValuesInvalidatesPersistedServices(): void { + $invalidator = $this->createMock(IPersistentServiceInvalidator::class); + $invalidator->expects($this->once()) + ->method('invalidate') + ->with(PersistentServiceGroup::Config); + $this->overwriteService(IPersistentServiceInvalidator::class, $invalidator); + + $this->getSystemConfig()->setValues(['foo' => 'bar']); + } + + public function testDeleteValueInvalidatesPersistedServices(): void { + $invalidator = $this->createMock(IPersistentServiceInvalidator::class); + $invalidator->expects($this->once()) + ->method('invalidate') + ->with(PersistentServiceGroup::Config); + $this->overwriteService(IPersistentServiceInvalidator::class, $invalidator); + + $this->getSystemConfig()->deleteValue('foo'); + } } From fdd87e72857d5ae088caaba78d9e47d41ac334f2 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Mon, 7 Sep 2026 16:26:20 +0200 Subject: [PATCH 03/10] feat(frankenphp): Remove index.php from url Signed-off-by: Carl Schwan --- Caddyfile | 87 +++++++++++++++++++++++++++++++++---------------------- 1 file changed, 52 insertions(+), 35 deletions(-) diff --git a/Caddyfile b/Caddyfile index 7ed476abf1e79..590a4aae77db5 100644 --- a/Caddyfile +++ b/Caddyfile @@ -12,24 +12,27 @@ } } -localhost { +franken.local { php_server { + # Keeps /index.php out of Nextcloud's generated URLs, matching the pretty-URL rewrite below. + env front_controller_active true + worker { file index.php num 32 - watch + #watch match /index.php/* } worker { file remote.php num 32 - watch + #watch match /remote.php/* } worker { file ocs/v1.php num 32 - watch + #watch match /ocs/v1.php/* match /ocs/v2.php/* } @@ -42,40 +45,54 @@ localhost { encode gzip - redir /.well-known/carddav /remote.php/dav 301 - redir /.well-known/caldav /remote.php/dav 301 + # Wrapped in route{} so these all run in the order written, regardless of Caddy's + # default directive ordering: the specific rewrites and the forbidden-path block + # must be evaluated before the catch-all pretty-URL rewrite below. + route { + redir /.well-known/carddav /remote.php/dav 301 + redir /.well-known/caldav /remote.php/dav 301 - # Rule: Maps most RFC 8615 compliant well-known URIs to our main frontend controller (/index.php) by default - @wellKnown { - path "/.well-known/" - not { - path /.well-known/acme-challenge - path /.well-known/pki-validation + # Rule: Maps most RFC 8615 compliant well-known URIs to our main frontend controller (/index.php) by default + @wellKnown { + path "/.well-known/" + not { + path /.well-known/acme-challenge + path /.well-known/pki-validation + } } - } - rewrite @wellKnown /index.php + rewrite @wellKnown /index.php - rewrite /ocm-provider/ /index.php + rewrite /ocm-provider/ /index.php - @forbidden { - path /.htaccess - path /data/* - path /config/* - path /db_structure - path /.xml - path /README - path /3rdparty/* - path /lib/* - path /templates/* - path /occ - path /build - path /tests - path /console.php - path /autotest - path /issue - path /indi - path /db_ - path /console + @forbidden { + path /.htaccess + path /data/* + path /config/* + path /db_structure + path /.xml + path /README + path /3rdparty/* + path /lib/* + path /templates/* + path /occ + path /build + path /tests + path /console.php + path /autotest + path /issue + path /indi + path /db_ + path /console + } + respond @forbidden 404 + + # Pretty URLs: rewrite anything that isn't already destined for a worker's own + # prefix and doesn't correspond to an existing static file (assets, etc.) to the + # front controller, so franken.local/ works instead of only franken.local/index.php/. + @prettyUrl { + not path /index.php/* /remote.php/* /ocs/v1.php/* /ocs/v2.php/* + not file + } + rewrite @prettyUrl /index.php{path} } - respond @forbidden 404 } From cb4d76a154a579dcc2a2a3a8607eeb82b419e660 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Mon, 7 Sep 2026 16:45:04 +0200 Subject: [PATCH 04/10] perf(frankenphp): Don't invalidate Mimetype Loader between requests Signed-off-by: Carl Schwan --- lib/private/Files/Type/Loader.php | 42 +++++++++++++++++++++++++++-- tests/lib/Files/Type/LoaderTest.php | 34 +++++++++++++++++++++++ 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/lib/private/Files/Type/Loader.php b/lib/private/Files/Type/Loader.php index 49cd44d1772db..43ee4b17c2101 100644 --- a/lib/private/Files/Type/Loader.php +++ b/lib/private/Files/Type/Loader.php @@ -9,8 +9,10 @@ namespace OC\Files\Type; use OC\DB\Exceptions\DbalException; +use OCP\AppFramework\Attribute\PersistAcrossRequests; use OCP\AppFramework\Db\TTransactional; use OCP\DB\Exception as DBException; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\IMimeTypeLoader; use OCP\IDBConnection; @@ -19,6 +21,7 @@ * * @package OC\Files\Type */ +#[PersistAcrossRequests] class Loader implements IMimeTypeLoader { use TTransactional; @@ -49,7 +52,23 @@ public function getMimetypeById(int $id): ?string { if (isset($this->mimetypes[$id])) { return $this->mimetypes[$id]; } - return null; + + // Might have been inserted by another process after this cache was loaded. + $qb = $this->dbConnection->getQueryBuilder(); + $qb->select('mimetype') + ->from('mimetypes') + ->where($qb->expr()->eq('id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT))); + $result = $qb->executeQuery(); + $mimetype = $result->fetchOne(); + $result->closeCursor(); + + if ($mimetype === false) { + return null; + } + + $this->mimetypes[$id] = $mimetype; + $this->mimetypeIds[$mimetype] = $id; + return $mimetype; } /** @@ -74,7 +93,26 @@ public function exists(string $mimetype): bool { if (!$this->mimetypeIds) { $this->loadMimetypes(); } - return isset($this->mimetypeIds[$mimetype]); + if (isset($this->mimetypeIds[$mimetype])) { + return true; + } + + // Might have been inserted by another process after this cache was loaded. + $qb = $this->dbConnection->getQueryBuilder(); + $qb->select('id') + ->from('mimetypes') + ->where($qb->expr()->eq('mimetype', $qb->createNamedParameter($mimetype))); + $result = $qb->executeQuery(); + $id = $result->fetchOne(); + $result->closeCursor(); + + if ($id === false) { + return false; + } + + $this->mimetypes[(int)$id] = $mimetype; + $this->mimetypeIds[$mimetype] = (int)$id; + return true; } /** diff --git a/tests/lib/Files/Type/LoaderTest.php b/tests/lib/Files/Type/LoaderTest.php index 35c549f321e94..0421cab935118 100644 --- a/tests/lib/Files/Type/LoaderTest.php +++ b/tests/lib/Files/Type/LoaderTest.php @@ -84,4 +84,38 @@ public function testStoreExists(): void { $this->assertEquals($mimetypeId, $mimetypeId2); } + + /** + * A row inserted by another connection/process after this loader's cache was + * already populated must still be found, since this loader may be kept alive + * across requests (see PersistAcrossRequests). + */ + public function testExistsFallsBackToDatabaseOnCacheMiss(): void { + // Populate the cache before the row exists + $this->loader->exists('testing/unrelated'); + + $qb = $this->db->getQueryBuilder(); + $qb->insert('mimetypes') + ->values([ + 'mimetype' => $qb->createPositionalParameter('testing/insertedlater'), + ]); + $qb->executeStatement(); + + $this->assertTrue($this->loader->exists('testing/insertedlater')); + } + + public function testGetMimetypeByIdFallsBackToDatabaseOnCacheMiss(): void { + // Populate the cache before the row exists + $this->loader->exists('testing/unrelated'); + + $qb = $this->db->getQueryBuilder(); + $qb->insert('mimetypes') + ->values([ + 'mimetype' => $qb->createPositionalParameter('testing/insertedlater'), + ]); + $qb->executeStatement(); + $mimetypeId = (int)$qb->getLastInsertId(); + + $this->assertSame('testing/insertedlater', $this->loader->getMimetypeById($mimetypeId)); + } } From 8353d92620881c08a2363dc33c68ed40638f5d18 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Mon, 7 Sep 2026 17:01:28 +0200 Subject: [PATCH 05/10] perf(frankenphp): Persist IAppConfig Signed-off-by: Carl Schwan --- .../unit/Comments/RootCollectionTest.php | 1 - .../tests/ShareTargetValidatorTest.php | 2 -- lib/private/AppConfig.php | 2 ++ .../AppFramework/Utility/SimpleContainer.php | 28 ++++++++++++---- .../EventDispatcher/EventDispatcher.php | 3 -- .../EventDispatcher/ServiceEventListener.php | 7 ++-- .../Utility/SimpleContainerTest.php | 32 +++++++++++++++++++ tests/lib/Share20/LegacyHooksTest.php | 3 +- .../lib/TextProcessing/TextProcessingTest.php | 1 - 9 files changed, 61 insertions(+), 18 deletions(-) diff --git a/apps/dav/tests/unit/Comments/RootCollectionTest.php b/apps/dav/tests/unit/Comments/RootCollectionTest.php index 666dd7b1e0dc6..d00045c2851e2 100644 --- a/apps/dav/tests/unit/Comments/RootCollectionTest.php +++ b/apps/dav/tests/unit/Comments/RootCollectionTest.php @@ -41,7 +41,6 @@ protected function setUp(): void { $this->logger = $this->createMock(LoggerInterface::class); $this->dispatcher = new EventDispatcher( new \Symfony\Component\EventDispatcher\EventDispatcher(), - \OC::$server, $this->logger ); diff --git a/apps/files_sharing/tests/ShareTargetValidatorTest.php b/apps/files_sharing/tests/ShareTargetValidatorTest.php index d1e0f5d122d12..1992762a9470f 100644 --- a/apps/files_sharing/tests/ShareTargetValidatorTest.php +++ b/apps/files_sharing/tests/ShareTargetValidatorTest.php @@ -19,7 +19,6 @@ use OCP\Share\Events\VerifyMountPointEvent; use OCP\Share\IManager; use OCP\Share\IShare; -use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; use Symfony\Component\EventDispatcher\EventDispatcher as SymfonyEventDispatcher; @@ -49,7 +48,6 @@ protected function setUp(): void { $this->eventDispatcher = new EventDispatcher( new SymfonyEventDispatcher(), - Server::get(ContainerInterface::class), $this->createMock(LoggerInterface::class), ); $this->targetValidator = new ShareTargetValidator( diff --git a/lib/private/AppConfig.php b/lib/private/AppConfig.php index 17b8041ffc46b..daeecd172f130 100644 --- a/lib/private/AppConfig.php +++ b/lib/private/AppConfig.php @@ -15,6 +15,7 @@ use OC\Config\ConfigManager; use OC\Config\PresetManager; use OC\Memcache\Factory as CacheFactory; +use OCP\AppFramework\Attribute\PersistAcrossRequests; use OCP\AppFramework\Utility\IPersistentServiceInvalidator; use OCP\AppFramework\Utility\PersistentServiceGroup; use OCP\Config\Lexicon\Entry; @@ -53,6 +54,7 @@ * @since 7.0.0 * @since 29.0.0 - Supporting types and lazy loading */ +#[PersistAcrossRequests(invalidatedBy: [PersistentServiceGroup::Config, PersistentServiceGroup::Apps])] class AppConfig implements IAppConfig { private const int APP_MAX_LENGTH = 32; private const int KEY_MAX_LENGTH = 64; diff --git a/lib/private/AppFramework/Utility/SimpleContainer.php b/lib/private/AppFramework/Utility/SimpleContainer.php index d95a13e68d17e..df75fa5550621 100644 --- a/lib/private/AppFramework/Utility/SimpleContainer.php +++ b/lib/private/AppFramework/Utility/SimpleContainer.php @@ -58,6 +58,16 @@ class SimpleContainer implements ArrayAccess, ContainerInterface, IContainer { */ private static array $persistentBuiltAt = []; + /** + * Guards against re-entering a generation check: PersistentServiceInvalidator's own + * dependency chain (via Memcache\Factory::getGlobalPrefix()) can resolve another persisted + * class, which would otherwise recurse into checking generations forever. While true, a class + * with the attribute is resolved as if it didn't have it, rather than looping. + * + * @psalm-suppress ImpureStaticProperty This class has a reset method + */ + private static bool $checkingGenerations = false; + /** * @internal */ @@ -66,6 +76,7 @@ public static function resetPersistentInstances(): void { self::$persistentGenerations = []; self::$persistentBuiltAt = []; self::$keepPersistentServices = false; + self::$checkingGenerations = false; } protected Container $container; @@ -182,7 +193,7 @@ public function resolve(string $name, array $chain = []): mixed { } $attributes = $class->getAttributes(PersistAcrossRequests::class); - $isPersistent = self::$keepPersistentServices && !empty($attributes); + $isPersistent = self::$keepPersistentServices && !empty($attributes) && !self::$checkingGenerations; $className = $class->getName(); $groups = $isPersistent ? array_map( @@ -230,12 +241,17 @@ private function currentGenerations(array $groups): array { if (empty($groups)) { return []; } - $invalidator = $this->get(PersistentServiceInvalidator::class); - $generations = []; - foreach ($groups as $group) { - $generations[$group] = $invalidator->getGeneration($group); + self::$checkingGenerations = true; + try { + $invalidator = $this->get(PersistentServiceInvalidator::class); + $generations = []; + foreach ($groups as $group) { + $generations[$group] = $invalidator->getGeneration($group); + } + return $generations; + } finally { + self::$checkingGenerations = false; } - return $generations; } /** diff --git a/lib/private/EventDispatcher/EventDispatcher.php b/lib/private/EventDispatcher/EventDispatcher.php index 9b44ac89f9873..51a937777929d 100644 --- a/lib/private/EventDispatcher/EventDispatcher.php +++ b/lib/private/EventDispatcher/EventDispatcher.php @@ -16,7 +16,6 @@ use OCP\EventDispatcher\ABroadcastedEvent; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventDispatcher; -use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; use Symfony\Component\EventDispatcher\EventDispatcher as SymfonyDispatcher; use function get_class; @@ -24,7 +23,6 @@ class EventDispatcher implements IEventDispatcher { public function __construct( private SymfonyDispatcher $dispatcher, - private ContainerInterface $container, private LoggerInterface $logger, ) { // inject the event dispatcher into the logger @@ -52,7 +50,6 @@ public function addServiceListener(string $eventName, string $className, int $priority = 0): void { $listener = new ServiceEventListener( - $this->container, $className, $this->logger ); diff --git a/lib/private/EventDispatcher/ServiceEventListener.php b/lib/private/EventDispatcher/ServiceEventListener.php index f9cc85b412605..e99fe29e99abc 100644 --- a/lib/private/EventDispatcher/ServiceEventListener.php +++ b/lib/private/EventDispatcher/ServiceEventListener.php @@ -12,7 +12,7 @@ use OCP\AppFramework\QueryException; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; -use Psr\Container\ContainerInterface; +use OCP\Server; use Psr\Log\LoggerInterface; use function sprintf; @@ -26,7 +26,6 @@ final class ServiceEventListener { private ?IEventListener $service = null; public function __construct( - private ContainerInterface $container, private string $class, private LoggerInterface $logger, ) { @@ -35,10 +34,12 @@ public function __construct( public function __invoke(Event $event) { if ($this->service === null) { try { + // Resolved from the current container rather than one captured at construction, + // since this listener may be invoked long after whatever built it. // TODO: fetch from the app containers, otherwise any custom services, // parameters and aliases won't be resolved. // See https://github.com/nextcloud/server/issues/27793 for details. - $this->service = $this->container->get($this->class); + $this->service = Server::get($this->class); } catch (QueryException $e) { $this->logger->error( sprintf( diff --git a/tests/lib/AppFramework/Utility/SimpleContainerTest.php b/tests/lib/AppFramework/Utility/SimpleContainerTest.php index e8fc3f82cc531..705da8119e150 100644 --- a/tests/lib/AppFramework/Utility/SimpleContainerTest.php +++ b/tests/lib/AppFramework/Utility/SimpleContainerTest.php @@ -34,6 +34,10 @@ class ClassPersistAcrossRequestsWithGroup { class ClassPersistAcrossRequestsWithEnumGroup { } +#[PersistAcrossRequests(invalidatedBy: ['cyclic-group'])] +class ClassWithCyclicInvalidationDependency { +} + class ClassEmptyConstructor implements IInterfaceConstructor { } @@ -212,6 +216,34 @@ public function testPersistAcrossRequestsAcceptsEnumGroup(): void { $this->assertNotSame($object, $container2->get(ClassPersistAcrossRequestsWithEnumGroup::class)); } + /** + * Regression test: a persisted class's own invalidation check must not be able to recurse + * forever if, while checking generations, it ends up resolving another persisted class (this + * happened for real via Memcache\Factory::getGlobalPrefix() calling back into a persisted + * IAppConfig). + */ + public function testCyclicInvalidationDependencyDoesNotRecurseForever(): void { + SimpleContainer::$keepPersistentServices = true; + + $container = $this->container; + $cacheFactory = $this->createMock(ICacheFactory::class); + $cacheFactory->method('createDistributed')->willReturnCallback(function () use ($container) { + // Simulates getGlobalPrefix() resolving another persisted class while this + // invalidator is itself being built as part of a generation check. + $container->get(ClassWithCyclicInvalidationDependency::class); + return new ArrayCache(); + }); + $container->registerService(PersistentServiceInvalidator::class, function () use ($cacheFactory) { + return new PersistentServiceInvalidator($cacheFactory); + }); + + $object = $container->get(ClassWithCyclicInvalidationDependency::class); + + $this->assertInstanceOf(ClassWithCyclicInvalidationDependency::class, $object); + // The outer resolution still completes and gets cached normally. + $this->assertSame($object, $container->get(ClassWithCyclicInvalidationDependency::class)); + } + public function testConstructorSimple(): void { $this->container->registerParameter('test', 'abc'); $object = $this->container->get( diff --git a/tests/lib/Share20/LegacyHooksTest.php b/tests/lib/Share20/LegacyHooksTest.php index 15c0dbc2f0ab3..47f5abd7a14d9 100644 --- a/tests/lib/Share20/LegacyHooksTest.php +++ b/tests/lib/Share20/LegacyHooksTest.php @@ -23,7 +23,6 @@ use OCP\Share\IShare; use OCP\Util; use PHPUnit\Framework\Attributes\Group; -use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; use Test\TestCase; @@ -52,7 +51,7 @@ protected function setUp(): void { $symfonyDispatcher = new \Symfony\Component\EventDispatcher\EventDispatcher(); $logger = $this->createMock(LoggerInterface::class); - $this->eventDispatcher = new EventDispatcher($symfonyDispatcher, Server::get(ContainerInterface::class), $logger); + $this->eventDispatcher = new EventDispatcher($symfonyDispatcher, $logger); $this->hooks = new LegacyHooks($this->eventDispatcher); $this->manager = Server::get(IShareManager::class); } diff --git a/tests/lib/TextProcessing/TextProcessingTest.php b/tests/lib/TextProcessing/TextProcessingTest.php index e54e4c6d60b65..34744cbe0b0f3 100644 --- a/tests/lib/TextProcessing/TextProcessingTest.php +++ b/tests/lib/TextProcessing/TextProcessingTest.php @@ -127,7 +127,6 @@ protected function setUp(): void { $this->eventDispatcher = new EventDispatcher( new \Symfony\Component\EventDispatcher\EventDispatcher(), - $this->serverContainer, Server::get(LoggerInterface::class), ); From 2ac659624edab05411d38a072f8f48c11789666b Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 15 Sep 2026 16:45:23 +0200 Subject: [PATCH 06/10] perf(frankenphp): Keep the DI container alive across requests Add SimpleContainer::resetForNextRequest(), which evicts non-persistent services instead of rebuilding the whole container every request. Also move connectDispatcher() into boot(), since IEventDispatcher isn't itself persisted. Assisted-by: ClaudeCode:claude-sonnet-5 Signed-off-by: Carl Schwan --- lib/OC.php | 11 +- .../AppFramework/Utility/SimpleContainer.php | 135 +++++++++++++----- lib/private/Server.php | 9 +- lib/private/ServerContainer.php | 15 +- .../Utility/SimpleContainerTest.php | 75 ++++++---- tests/lib/ServerContainerTest.php | 27 ++++ 6 files changed, 205 insertions(+), 67 deletions(-) create mode 100644 tests/lib/ServerContainerTest.php diff --git a/lib/OC.php b/lib/OC.php index 86c10b9098fb7..01c25db2d75d0 100644 --- a/lib/OC.php +++ b/lib/OC.php @@ -771,7 +771,16 @@ public static function initForRequest(): void { self::handleAuthHeaders(); // setup the basic server - self::$server = new \OC\Server(\OC::$WEBROOT, self::$config); + if (isset(self::$server)) { + // Same worker (e.g. FrankenPHP) serving another request: keep every service + // *definition* alive and only forget the instances a fresh request shouldn't + // inherit, rather than discarding and rebuilding the whole container. Anything + // kept alive on purpose (see \OCP\AppFramework\Attribute\PersistAcrossRequests) + // is left untouched. + self::$server->resetForNextRequest(); + } else { + self::$server = new \OC\Server(\OC::$WEBROOT, self::$config); + } self::$server->boot(); self::oneTimeChecks(); diff --git a/lib/private/AppFramework/Utility/SimpleContainer.php b/lib/private/AppFramework/Utility/SimpleContainer.php index df75fa5550621..16a849e5d8854 100644 --- a/lib/private/AppFramework/Utility/SimpleContainer.php +++ b/lib/private/AppFramework/Utility/SimpleContainer.php @@ -38,26 +38,6 @@ class SimpleContainer implements ArrayAccess, ContainerInterface, IContainer { /** A kept instance is rebuilt after this many seconds even without an invalidation, as a safety net */ private const MAX_PERSISTENT_AGE_SECONDS = 3600; - /** - * @psalm-suppress ImpureStaticProperty This class has a reset method - * @var array - */ - private static array $persistentInstances = []; - - /** - * The invalidation generations each kept instance was built against, keyed by group name. - * - * @psalm-suppress ImpureStaticProperty This class has a reset method - * @var array> - */ - private static array $persistentGenerations = []; - - /** - * @psalm-suppress ImpureStaticProperty This class has a reset method - * @var array - */ - private static array $persistentBuiltAt = []; - /** * Guards against re-entering a generation check: PersistentServiceInvalidator's own * dependency chain (via Memcache\Factory::getGlobalPrefix()) can resolve another persisted @@ -72,9 +52,6 @@ class SimpleContainer implements ArrayAccess, ContainerInterface, IContainer { * @internal */ public static function resetPersistentInstances(): void { - self::$persistentInstances = []; - self::$persistentGenerations = []; - self::$persistentBuiltAt = []; self::$keepPersistentServices = false; self::$checkingGenerations = false; } @@ -84,6 +61,34 @@ public static function resetPersistentInstances(): void { /** @var array */ private array $aliases = []; + /** + * The invalidation generations each kept instance was built against (keyed by group name), + * plus when it was built. Only ever holds entries for classes still sitting in $container. + * + * @var array, builtAt: int}> + */ + private array $persistentMeta = []; + + /** @var array ids registered as a Pimple factory: never cached, nothing to evict on reset */ + private array $factoryIds = []; + + /** + * Generation lookups memoized for the current resetForNextRequest() pass, so that several + * persisted classes sharing a group (e.g. PersistentServiceGroup::Apps) don't each hit the + * distributed cache separately for the same "is generation N still current" question. + * + * @var array + */ + private array $generationCache = []; + + /** + * @var array ids whose only container entry is query()'s own memoization of an + * autowired instance (as opposed to a real service definition registered through + * registerService()/registerAlias()). There is nothing to rebuild such an entry from other + * than dropping it outright and letting the next query() re-resolve it. + */ + private array $autoResolvedIds = []; + public function __construct() { $this->container = new Container(); } @@ -202,18 +207,13 @@ public function resolve(string $name, array $chain = []): mixed { ) : []; - if ($isPersistent - && isset(self::$persistentInstances[$className]) - && $this->isPersistentInstanceStillValid($className, $groups)) { - return self::$persistentInstances[$className]; - } - $object = $this->buildClass($class, $chain); if ($isPersistent) { - self::$persistentInstances[$className] = $object; - self::$persistentGenerations[$className] = $this->currentGenerations($groups); - self::$persistentBuiltAt[$className] = time(); + $this->persistentMeta[$className] = [ + 'groups' => $this->currentGenerations($groups), + 'builtAt' => time(), + ]; } return $object; @@ -223,14 +223,52 @@ public function resolve(string $name, array $chain = []): mixed { } } + private function isPersistentInstanceStillValid(string $id): bool { + if (!isset($this->persistentMeta[$id])) { + return false; + } + ['groups' => $groups, 'builtAt' => $builtAt] = $this->persistentMeta[$id]; + if ((time() - $builtAt) > self::MAX_PERSISTENT_AGE_SECONDS) { + return false; + } + return $groups === $this->currentGenerations(array_keys($groups)); + } + /** - * @param list $groups + * Drops every already-resolved service that a fresh request shouldn't inherit, so the next + * query()/get() call for it rebuilds a clean instance. A class kept alive by + * {@see PersistAcrossRequests} (and still valid) is left completely untouched. + * + * Call this instead of throwing the whole container away between requests on a long-running + * worker (e.g. FrankenPHP): it keeps every service *definition* (the closures registered via + * registerService()/registerAlias(), and by extension anything built through them, such as app + * containers), it only forgets which of them have already been resolved this "request epoch". */ - private function isPersistentInstanceStillValid(string $className, array $groups): bool { - if ((time() - self::$persistentBuiltAt[$className]) > self::MAX_PERSISTENT_AGE_SECONDS) { - return false; + public function resetForNextRequest(): void { + $this->generationCache = []; + foreach ($this->container->keys() as $id) { + if (isset($this->factoryIds[$id]) || $this->isPersistentInstanceStillValid($id)) { + // A factory never caches anything to begin with; a still-valid persisted + // instance is exactly what should survive into the next request. + continue; + } + + if (isset($this->autoResolvedIds[$id])) { + // query() only memoized an object it built via reflection; there is no service + // definition to fall back to, so the entry has to go entirely. The next query() + // for this id will autowire a fresh instance from scratch. + $this->container->offsetUnset($id); + unset($this->autoResolvedIds[$id], $this->persistentMeta[$id]); + continue; + } + + // A real service definition: keep it, only forget the cached instance it already + // produced so it runs again on next access. + $raw = $this->container->raw($id); + $this->container->offsetUnset($id); + $this->container->offsetSet($id, $raw); + unset($this->persistentMeta[$id]); } - return self::$persistentGenerations[$className] === $this->currentGenerations($groups); } /** @@ -246,7 +284,7 @@ private function currentGenerations(array $groups): array { $invalidator = $this->get(PersistentServiceInvalidator::class); $generations = []; foreach ($groups as $group) { - $generations[$group] = $invalidator->getGeneration($group); + $generations[$group] = $this->generationCache[$group] ??= $invalidator->getGeneration($group); } return $generations; } finally { @@ -273,10 +311,24 @@ protected function query(string $name, bool $autoload = true, array $chain = []) } $object = $this->resolve($name, array_merge($chain, [$name])); - $this->registerService($name, static fn () => $object); + $this->cacheAutoResolvedInstance($name, $object); return $object; } + /** + * Caches an already-built instance under $id as if query() had resolved it itself: on + * resetForNextRequest(), it's dropped entirely (there being no service definition to rebuild + * from) rather than kept forever like a raw ArrayAccess write would be. + * + * @internal + */ + public function cacheAutoResolvedInstance(string $id, object $instance): void { + $this->registerService($id, function () use ($instance) { + return $instance; + }); + $this->autoResolvedIds[$id] = true; + } + /** * A value is stored in the container with its corresponding name * @@ -306,9 +358,14 @@ public function registerService(string $name, Closure $closure, bool $shared = t if (isset($this->aliases[$name])) { unset($this->aliases[$name]); } + // A real definition is being (re)registered, so this id is no longer just a bare + // autowired instance query() happened to memoize. + unset($this->autoResolvedIds[$name]); if ($shared) { + unset($this->factoryIds[$name]); $this->container[$name] = $wrapped; } else { + $this->factoryIds[$name] = true; $this->container[$name] = $this->container->factory($wrapped); } } diff --git a/lib/private/Server.php b/lib/private/Server.php index 3f55aafa02199..84af42f7c133d 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -1164,10 +1164,17 @@ public function __construct( return $c->get($globalScaleServiceClass); }); - $this->connectDispatcher(); } + /** + * Called before each request served, even when this Server instance is kept alive across + * several of them on a long-running worker (see {@see OC::initForRequest()}): + * connectDispatcher() must run against whichever IEventDispatcher instance is live for the + * current request, since that service isn't kept across requests itself. + */ public function boot() { + $this->connectDispatcher(); + /** @var HookConnector $hookConnector */ $hookConnector = $this->get(HookConnector::class); $hookConnector->viewToNode(); diff --git a/lib/private/ServerContainer.php b/lib/private/ServerContainer.php index 484f2647d2a65..71915b5394b43 100644 --- a/lib/private/ServerContainer.php +++ b/lib/private/ServerContainer.php @@ -89,7 +89,7 @@ protected function getAppContainer(string $sensitiveNamespace): DIContainer { /* The application constructor will register the container, see App::__construct */ $app = new $applicationClassName(); if (isset($this->appContainers[$namespace])) { - $this->appContainers[$namespace]->offsetSet($applicationClassName, $app); + $this->appContainers[$namespace]->cacheAutoResolvedInstance($applicationClassName, $app); /** @psalm-suppress NoValue false-positive (see comment above) */ return $this->appContainers[$namespace]; } @@ -156,4 +156,17 @@ public function getAppContainerForService(string $id): ?DIContainer { return null; } } + + /** + * Registered app containers (see {@see registerAppContainer()}) aren't request-scoped + * themselves, so they survive here across requests just like everything else; cascade into + * each one so the app-specific services cached inside it get the same treatment. + */ + #[\Override] + public function resetForNextRequest(): void { + parent::resetForNextRequest(); + foreach ($this->appContainers as $appContainer) { + $appContainer->resetForNextRequest(); + } + } } diff --git a/tests/lib/AppFramework/Utility/SimpleContainerTest.php b/tests/lib/AppFramework/Utility/SimpleContainerTest.php index 705da8119e150..132f4ce09d73d 100644 --- a/tests/lib/AppFramework/Utility/SimpleContainerTest.php +++ b/tests/lib/AppFramework/Utility/SimpleContainerTest.php @@ -150,7 +150,8 @@ public function testInstancesOnlyOnce(): void { public function testPersistAcrossRequestsIgnoredByDefault(): void { $object = $this->container->get(ClassPersistAcrossRequests::class); - $object2 = (new SimpleContainer())->get(ClassPersistAcrossRequests::class); + $this->container->resetForNextRequest(); + $object2 = $this->container->get(ClassPersistAcrossRequests::class); $this->assertNotSame($object, $object2); } @@ -158,8 +159,9 @@ public function testPersistAcrossRequestsKeepsInstanceOnceEnabled(): void { SimpleContainer::$keepPersistentServices = true; $object = $this->container->get(ClassPersistAcrossRequests::class); - // Simulate a new request rebuilding the whole Server container - $object2 = (new SimpleContainer())->get(ClassPersistAcrossRequests::class); + // Simulate the container being kept alive for the next request on a long-running worker + $this->container->resetForNextRequest(); + $object2 = $this->container->get(ClassPersistAcrossRequests::class); $this->assertSame($object, $object2); } @@ -171,25 +173,20 @@ public function testPersistAcrossRequestsInvalidatedByGroup(): void { $cacheFactory->method('createDistributed')->willReturn(new ArrayCache()); $invalidator = new PersistentServiceInvalidator($cacheFactory); - $registerInvalidator = function (SimpleContainer $container) use ($invalidator): void { - $container->registerService(PersistentServiceInvalidator::class, function () use ($invalidator) { - return $invalidator; - }); - }; + $this->container->registerService(PersistentServiceInvalidator::class, function () use ($invalidator) { + return $invalidator; + }); - $registerInvalidator($this->container); $object = $this->container->get(ClassPersistAcrossRequestsWithGroup::class); - // Simulate a new request rebuilding the whole Server container: nothing invalidated the group yet - $container2 = new SimpleContainer(); - $registerInvalidator($container2); - $this->assertSame($object, $container2->get(ClassPersistAcrossRequestsWithGroup::class)); + // Simulate the next request on a long-running worker: nothing invalidated the group yet + $this->container->resetForNextRequest(); + $this->assertSame($object, $this->container->get(ClassPersistAcrossRequestsWithGroup::class)); $invalidator->invalidate('test-group'); - $container3 = new SimpleContainer(); - $registerInvalidator($container3); - $this->assertNotSame($object, $container3->get(ClassPersistAcrossRequestsWithGroup::class)); + $this->container->resetForNextRequest(); + $this->assertNotSame($object, $this->container->get(ClassPersistAcrossRequestsWithGroup::class)); } public function testPersistAcrossRequestsAcceptsEnumGroup(): void { @@ -199,21 +196,49 @@ public function testPersistAcrossRequestsAcceptsEnumGroup(): void { $cacheFactory->method('createDistributed')->willReturn(new ArrayCache()); $invalidator = new PersistentServiceInvalidator($cacheFactory); - $registerInvalidator = function (SimpleContainer $container) use ($invalidator): void { - $container->registerService(PersistentServiceInvalidator::class, function () use ($invalidator) { - return $invalidator; - }); - }; + $this->container->registerService(PersistentServiceInvalidator::class, function () use ($invalidator) { + return $invalidator; + }); - $registerInvalidator($this->container); $object = $this->container->get(ClassPersistAcrossRequestsWithEnumGroup::class); // Invalidating by the enum's string value must be indistinguishable from the enum case itself $invalidator->invalidate('apps'); - $container2 = new SimpleContainer(); - $registerInvalidator($container2); - $this->assertNotSame($object, $container2->get(ClassPersistAcrossRequestsWithEnumGroup::class)); + $this->container->resetForNextRequest(); + $this->assertNotSame($object, $this->container->get(ClassPersistAcrossRequestsWithEnumGroup::class)); + } + + public function testResetForNextRequestKeepsServiceDefinition(): void { + $this->container->registerService('test', function () { + return new \StdClass(); + }); + + $object = $this->container->get('test'); + $this->container->resetForNextRequest(); + $object2 = $this->container->get('test'); + + $this->assertNotSame($object, $object2); + } + + public function testResetForNextRequestKeepsFactoryDefinition(): void { + $this->container->registerService('test', function () { + return new \StdClass(); + }, false); + + $object = $this->container->get('test'); + $this->container->resetForNextRequest(); + $object2 = $this->container->get('test'); + + $this->assertNotSame($object, $object2); + } + + public function testResetForNextRequestForgetsAutowiredInstance(): void { + $object = $this->container->get(ClassEmptyConstructor::class); + $this->container->resetForNextRequest(); + $object2 = $this->container->get(ClassEmptyConstructor::class); + + $this->assertNotSame($object, $object2); } /** diff --git a/tests/lib/ServerContainerTest.php b/tests/lib/ServerContainerTest.php new file mode 100644 index 0000000000000..11ca1b54287f6 --- /dev/null +++ b/tests/lib/ServerContainerTest.php @@ -0,0 +1,27 @@ +createMock(DIContainer::class); + $appContainer->expects($this->once()) + ->method('resetForNextRequest'); + + $container->registerAppContainer('testapp', $appContainer); + + $container->resetForNextRequest(); + } +} From fab08666fafcfbe0cad8ff3dbf506836bbc80150 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 15 Sep 2026 16:45:23 +0200 Subject: [PATCH 07/10] fix(config): Invalidate persisted services on config metadata changes updateType()/updateSensitive()/updateLazy() and Installer's shipped-app install path never invalidated PersistentServiceGroup::Config. Assisted-by: ClaudeCode:claude-sonnet-5 Signed-off-by: Carl Schwan --- lib/private/AppConfig.php | 3 +++ lib/private/Installer.php | 3 +++ 2 files changed, 6 insertions(+) diff --git a/lib/private/AppConfig.php b/lib/private/AppConfig.php index daeecd172f130..b9a4c0027017f 100644 --- a/lib/private/AppConfig.php +++ b/lib/private/AppConfig.php @@ -1023,6 +1023,7 @@ public function updateType(string $app, string $key, int $type = self::VALUE_MIX ->andWhere($update->expr()->eq('configkey', $update->createNamedParameter($key))); $update->executeStatement(); $this->valueTypes[$app][$key] = $type; + $this->invalidatePersistedServices(); return true; } @@ -1084,6 +1085,7 @@ public function updateSensitive(string $app, string $key, bool $sensitive): bool $update->executeStatement(); $this->valueTypes[$app][$key] = $type; + $this->invalidatePersistedServices(); return true; } @@ -1121,6 +1123,7 @@ public function updateLazy(string $app, string $key, bool $lazy): bool { // At this point, it is a lot safer to clean cache $this->clearCache(); + $this->invalidatePersistedServices(); return true; } diff --git a/lib/private/Installer.php b/lib/private/Installer.php index 6b0060b5b895d..928cb202a13d5 100644 --- a/lib/private/Installer.php +++ b/lib/private/Installer.php @@ -21,6 +21,8 @@ use OC\DB\MigrationService; use OC\Files\FilenameValidator; use OCP\App\AppPathNotFoundException; +use OCP\AppFramework\Utility\IPersistentServiceInvalidator; +use OCP\AppFramework\Utility\PersistentServiceGroup; use OCP\BackgroundJob\IJobList; use OCP\Files; use OCP\HintException; @@ -575,6 +577,7 @@ private function installAppLastSteps(string $appPath, array $info, ?IOutput $out // Set the installed version $this->config->setAppValue($info['id'], 'installed_version', $this->appManager->getAppVersion($info['id'], false)); $this->config->setAppValue($info['id'], 'enabled', $enabled); + Server::get(IPersistentServiceInvalidator::class)->invalidate(PersistentServiceGroup::Apps); // Set remote/public handlers foreach ($info['remote'] as $name => $path) { From 0ef55a0b4af25d3ff1463d47e7cc4a2bc167d6ab Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 15 Sep 2026 16:45:23 +0200 Subject: [PATCH 08/10] perf: Memoize the distributed cache handle in PersistentServiceInvalidator Avoids rebuilding it on every generation check. Assisted-by: ClaudeCode:claude-sonnet-5 Signed-off-by: Carl Schwan --- .../Utility/PersistentServiceInvalidator.php | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/lib/private/AppFramework/Utility/PersistentServiceInvalidator.php b/lib/private/AppFramework/Utility/PersistentServiceInvalidator.php index b51cc098c550e..268a1e5810e4b 100644 --- a/lib/private/AppFramework/Utility/PersistentServiceInvalidator.php +++ b/lib/private/AppFramework/Utility/PersistentServiceInvalidator.php @@ -11,25 +11,35 @@ use OCP\AppFramework\Utility\IPersistentServiceInvalidator; use OCP\AppFramework\Utility\PersistentServiceGroup; +use OCP\ICache; use OCP\ICacheFactory; use OCP\IMemcache; class PersistentServiceInvalidator implements IPersistentServiceInvalidator { private const CACHE_PREFIX = 'persistent_service_gen'; + private ?ICache $cache = null; + public function __construct( private ICacheFactory $cacheFactory, ) { } + private function getCache(): ICache { + return $this->cache ??= $this->cacheFactory->createDistributed(self::CACHE_PREFIX); + } + #[\Override] public function invalidate(string|PersistentServiceGroup $group): void { $key = $group instanceof PersistentServiceGroup ? $group->value : $group; - $cache = $this->cacheFactory->createDistributed(self::CACHE_PREFIX); + $cache = $this->getCache(); if ($cache instanceof IMemcache) { $cache->inc($key); return; } + // Every current ICacheFactory::createDistributed() backend implements IMemcache, so this + // non-atomic path is currently unreachable; a lost increment here would still be caught + // on the next invalidation since generations are compared for equality, not counted. $cache->set($key, ((int)$cache->get($key)) + 1); } @@ -38,6 +48,6 @@ public function invalidate(string|PersistentServiceGroup $group): void { */ public function getGeneration(string|PersistentServiceGroup $group): int { $key = $group instanceof PersistentServiceGroup ? $group->value : $group; - return (int)$this->cacheFactory->createDistributed(self::CACHE_PREFIX)->get($key); + return (int)$this->getCache()->get($key); } } From f1029c5d70c0edd0f34da01134750927ceca712a Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 15 Sep 2026 16:45:23 +0200 Subject: [PATCH 09/10] feat(frankenphp): Port Router and CachingRouter to PersistAcrossRequests Refresh per-request state (context, IAppManager, IEventLogger) on each request. Also check isEnabledForAnyone() instead of isEnabledForUser(): $root is now shared across requests, so a per-user decision would leak into other users' requests. Assisted-by: ClaudeCode:claude-sonnet-5 Signed-off-by: Carl Schwan --- .../tests/Controller/ViewControllerTest.php | 11 +-- lib/OC.php | 9 +++ lib/private/Route/CachingRouter.php | 7 ++ lib/private/Route/Router.php | 41 +++++++++-- lib/private/Server.php | 5 +- tests/lib/Route/RouterTest.php | 68 +++++++++++++++++++ 6 files changed, 131 insertions(+), 10 deletions(-) diff --git a/apps/files/tests/Controller/ViewControllerTest.php b/apps/files/tests/Controller/ViewControllerTest.php index 1738cfa847c1e..7e9dd72ebb2a0 100644 --- a/apps/files/tests/Controller/ViewControllerTest.php +++ b/apps/files/tests/Controller/ViewControllerTest.php @@ -49,7 +49,6 @@ */ #[\PHPUnit\Framework\Attributes\Group('RoutingWeirdness')] class ViewControllerTest extends TestCase { - private ContainerInterface&MockObject $container; private IAppManager&MockObject $appManager; private IAppConfig&MockObject $appConfig; private ICacheFactory&MockObject $cacheFactory; @@ -114,13 +113,12 @@ protected function setUp(): void { $this->cacheFactory = $this->createMock(ICacheFactory::class); $this->logger = $this->createMock(LoggerInterface::class); $this->eventLogger = $this->createMock(IEventLogger::class); - $this->container = $this->createMock(ContainerInterface::class); $this->router = new Router( $this->logger, $this->request, $this->config, $this->eventLogger, - $this->container, + $this->createMock(ContainerInterface::class), $this->appManager, ); @@ -257,9 +255,14 @@ public function testShortRedirect(?string $openfile, ?string $opendetails, strin } public function testShowFileRouteWithTrashedFile(): void { - $this->appManager->expects($this->exactly(2)) + // Only ViewController's own trashbin check goes through isEnabledForUser now; Router's + // route-loading gate uses isEnabledForAnyone() instead (see PersistAcrossRequests). + $this->appManager->expects($this->once()) ->method('isEnabledForUser') ->willReturn(true); + $this->appManager->expects($this->any()) + ->method('isEnabledForAnyone') + ->willReturn(true); $parentNode = $this->createMock(Folder::class); $parentNode->expects($this->once()) diff --git a/lib/OC.php b/lib/OC.php index 01c25db2d75d0..28e1ca333c15c 100644 --- a/lib/OC.php +++ b/lib/OC.php @@ -790,6 +790,15 @@ public static function initForRequest(): void { $config = Server::get(IConfig::class); $request = Server::get(IRequest::class); + // The router may be reused from a previous request on a long-running worker: it's + // per-request data, not a dependency, so nothing rebuilds it automatically. + $router = Server::get(\OC\Route\Router::class); + $router->refreshContext($request); + $router->refreshRequestScopedCollaborators( + Server::get(\OCP\App\IAppManager::class), + Server::get(\OCP\Diagnostics\IEventLogger::class), + ); + try { $profiler = new BuiltInProfiler( $config, diff --git a/lib/private/Route/CachingRouter.php b/lib/private/Route/CachingRouter.php index fca6d4b1c51d6..700a025e43c0f 100644 --- a/lib/private/Route/CachingRouter.php +++ b/lib/private/Route/CachingRouter.php @@ -9,6 +9,8 @@ namespace OC\Route; use OCP\App\IAppManager; +use OCP\AppFramework\Attribute\PersistAcrossRequests; +use OCP\AppFramework\Utility\PersistentServiceGroup; use OCP\Diagnostics\IEventLogger; use OCP\ICache; use OCP\ICacheFactory; @@ -21,6 +23,11 @@ use Symfony\Component\Routing\Matcher\Dumper\CompiledUrlMatcherDumper; use Symfony\Component\Routing\RouteCollection; +/** + * PHP attributes aren't inherited: Router's own #[PersistAcrossRequests] doesn't apply here even + * though this class extends it, so it has to be repeated. + */ +#[PersistAcrossRequests(invalidatedBy: [PersistentServiceGroup::Apps])] class CachingRouter extends Router { protected ICache $cache; diff --git a/lib/private/Route/Router.php b/lib/private/Route/Router.php index b75c3e6982694..9137fbcfcbf41 100644 --- a/lib/private/Route/Router.php +++ b/lib/private/Route/Router.php @@ -13,7 +13,9 @@ use OCP\App\AppPathNotFoundException; use OCP\App\IAppManager; use OCP\AppFramework\App; +use OCP\AppFramework\Attribute\PersistAcrossRequests; use OCP\AppFramework\Http\Attribute\Route as RouteAttribute; +use OCP\AppFramework\Utility\PersistentServiceGroup; use OCP\Diagnostics\IEventLogger; use OCP\IConfig; use OCP\IRequest; @@ -31,6 +33,7 @@ use Symfony\Component\Routing\RequestContext; use Symfony\Component\Routing\RouteCollection; +#[PersistAcrossRequests(invalidatedBy: [PersistentServiceGroup::Apps])] class Router implements IRouter { /** @var RouteCollection[] */ protected $collections = []; @@ -59,8 +62,33 @@ public function __construct( private ContainerInterface $container, protected IAppManager $appManager, ) { + $this->context = $this->buildContext($request); + // TODO cache + $this->root = $this->getCollection('root'); + } + + /** + * Rebuilds the request context (host, scheme, HTTP method) from the request actually being + * served, since this Router instance may outlive the request that constructed it. Unlike the + * container, there is no service-lifecycle mechanism for this: it's per-request data derived + * from IRequest, not a dependency that could itself be kept across requests. + */ + public function refreshContext(IRequest $request): void { + $this->setContext($this->buildContext($request)); + } + + /** + * Same as refreshContext(), but for IAppManager (carries the current user's session) and + * IEventLogger, since neither is itself kept across requests. + */ + public function refreshRequestScopedCollaborators(IAppManager $appManager, IEventLogger $eventLogger): void { + $this->appManager = $appManager; + $this->eventLogger = $eventLogger; + } + + private function buildContext(IRequest $request): RequestContext { $baseUrl = \OC::$WEBROOT; - if (!($config->getSystemValue('htaccess.IgnoreFrontController', false) === true || getenv('front_controller_active') === 'true')) { + if (!($this->config->getSystemValue('htaccess.IgnoreFrontController', false) === true || getenv('front_controller_active') === 'true')) { $baseUrl .= '/index.php'; } if (!\OC::$CLI && isset($_SERVER['REQUEST_METHOD'])) { @@ -70,13 +98,13 @@ public function __construct( } $host = $request->getServerHost(); $schema = $request->getServerProtocol(); - $this->context = new RequestContext($baseUrl, $method, $host, $schema); - // TODO cache - $this->root = $this->getCollection('root'); + return new RequestContext($baseUrl, $method, $host, $schema); } public function setContext(RequestContext $context): void { $this->context = $context; + // The cached generator holds onto the old context, so it must be rebuilt too. + $this->generator = null; } public function getRouteCollection() { @@ -146,7 +174,10 @@ public function loadRoutes(?string $app = null, bool $skipLoadingCore = false): $routingFiles = []; } - if ($this->appManager->isEnabledForUser($app)) { + // Not isEnabledForUser(): $root is shared across every request this Router serves, + // so a per-user decision here would stick for every other user too. Per-user access + // is enforced independently by SecurityMiddleware on every request. + if ($this->appManager->isEnabledForAnyone($app)) { $this->loadAttributeRoutes($app); } } diff --git a/lib/private/Server.php b/lib/private/Server.php index 84af42f7c133d..79232c0986ae0 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -635,7 +635,10 @@ public function __construct( $this->registerService(Router::class, static function (Server $c) { $cacheFactory = $c->get(ICacheFactory::class); if ($cacheFactory->isLocalCacheAvailable()) { - $router = $c->resolve(CachingRouter::class); + // get(), not resolve(): CachingRouter (like Router) is kept across requests, and + // only get() gives it its own container entry that the persistence bookkeeping + // can find and keep valid on its own, independently of this pass-through key. + $router = $c->get(CachingRouter::class); } else { $router = $c->resolve(Router::class); } diff --git a/tests/lib/Route/RouterTest.php b/tests/lib/Route/RouterTest.php index 0537372050c07..7a00397f82486 100644 --- a/tests/lib/Route/RouterTest.php +++ b/tests/lib/Route/RouterTest.php @@ -9,6 +9,7 @@ namespace Test\Route; use OC\Route\Router; +use OCP\App\AppPathNotFoundException; use OCP\App\IAppManager; use OCP\Diagnostics\IEventLogger; use OCP\IConfig; @@ -57,6 +58,73 @@ public function testHeartbeat(): void { $this->assertEquals('/index.php/heartbeat', $this->router->generate('heartbeat')); } + public function testRefreshContextUpdatesGeneratedAbsoluteUrls(): void { + $firstRequest = $this->createMock(IRequest::class); + $firstRequest->method('getServerHost')->willReturn('first.example.com'); + $firstRequest->method('getServerProtocol')->willReturn('http'); + + $router = new Router( + $this->createMock(LoggerInterface::class), + $firstRequest, + $this->createMock(IConfig::class), + $this->createMock(IEventLogger::class), + $this->createMock(ContainerInterface::class), + $this->appManager, + ); + + $this->assertSame('http://first.example.com/index.php/heartbeat', $router->generate('heartbeat', [], true)); + + $secondRequest = $this->createMock(IRequest::class); + $secondRequest->method('getServerHost')->willReturn('second.example.com'); + $secondRequest->method('getServerProtocol')->willReturn('https'); + $router->refreshContext($secondRequest); + + $this->assertSame('https://second.example.com/index.php/heartbeat', $router->generate('heartbeat', [], true)); + } + + public function testLoadRoutesForAppChecksIsEnabledForAnyoneNotPerUser(): void { + // $root is shared across every request a persisted Router serves, so gating it by the + // current user (rather than system-wide enablement) would leak into other users' requests. + $this->appManager->method('cleanAppId')->willReturnArgument(0); + $this->appManager->method('getAppPath')->willThrowException(new AppPathNotFoundException()); + $this->appManager->expects(self::once()) + ->method('isEnabledForAnyone') + ->with('some_app') + ->willReturn(false); + $this->appManager->expects(self::never()) + ->method('isEnabledForUser'); + + $this->router->loadRoutes('some_app', skipLoadingCore: true); + } + + public function testRefreshRequestScopedCollaboratorsUpdatesAppManager(): void { + $firstAppManager = $this->createMock(IAppManager::class); + $firstAppManager->method('cleanAppId')->willReturnArgument(0); + $firstAppManager->method('getAppPath')->willThrowException(new AppPathNotFoundException()); + $firstAppManager->method('isEnabledForAnyone')->willReturn(false); + + $router = new Router( + $this->createMock(LoggerInterface::class), + $this->createMock(IRequest::class), + $this->createMock(IConfig::class), + $this->createMock(IEventLogger::class), + $this->createMock(ContainerInterface::class), + $firstAppManager, + ); + + $secondAppManager = $this->createMock(IAppManager::class); + $secondAppManager->method('cleanAppId')->willReturnArgument(0); + $secondAppManager->expects(self::once()) + ->method('getAppPath') + ->willThrowException(new AppPathNotFoundException()); + $secondAppManager->expects(self::once()) + ->method('isEnabledForAnyone') + ->willReturn(false); + $router->refreshRequestScopedCollaborators($secondAppManager, $this->createMock(IEventLogger::class)); + + $router->loadRoutes('some_app', skipLoadingCore: true); + } + public function testGenerateConsecutively(): void { $this->appManager->expects(self::atLeastOnce()) ->method('cleanAppId') From 00fa6dc662a20a1ae0a876149adbfb6ea52479cc Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 15 Sep 2026 16:45:23 +0200 Subject: [PATCH 10/10] refactor(events): Resolve ServiceEventListener's service via an injected container Makes its dependency explicit and testable. Signed-off-by: Carl Schwan --- apps/dav/tests/unit/Comments/RootCollectionTest.php | 1 + apps/files_sharing/tests/ShareTargetValidatorTest.php | 2 ++ lib/private/EventDispatcher/EventDispatcher.php | 3 +++ lib/private/EventDispatcher/ServiceEventListener.php | 7 +++---- tests/lib/Share20/LegacyHooksTest.php | 3 ++- tests/lib/TextProcessing/TextProcessingTest.php | 1 + 6 files changed, 12 insertions(+), 5 deletions(-) diff --git a/apps/dav/tests/unit/Comments/RootCollectionTest.php b/apps/dav/tests/unit/Comments/RootCollectionTest.php index d00045c2851e2..666dd7b1e0dc6 100644 --- a/apps/dav/tests/unit/Comments/RootCollectionTest.php +++ b/apps/dav/tests/unit/Comments/RootCollectionTest.php @@ -41,6 +41,7 @@ protected function setUp(): void { $this->logger = $this->createMock(LoggerInterface::class); $this->dispatcher = new EventDispatcher( new \Symfony\Component\EventDispatcher\EventDispatcher(), + \OC::$server, $this->logger ); diff --git a/apps/files_sharing/tests/ShareTargetValidatorTest.php b/apps/files_sharing/tests/ShareTargetValidatorTest.php index 1992762a9470f..d1e0f5d122d12 100644 --- a/apps/files_sharing/tests/ShareTargetValidatorTest.php +++ b/apps/files_sharing/tests/ShareTargetValidatorTest.php @@ -19,6 +19,7 @@ use OCP\Share\Events\VerifyMountPointEvent; use OCP\Share\IManager; use OCP\Share\IShare; +use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; use Symfony\Component\EventDispatcher\EventDispatcher as SymfonyEventDispatcher; @@ -48,6 +49,7 @@ protected function setUp(): void { $this->eventDispatcher = new EventDispatcher( new SymfonyEventDispatcher(), + Server::get(ContainerInterface::class), $this->createMock(LoggerInterface::class), ); $this->targetValidator = new ShareTargetValidator( diff --git a/lib/private/EventDispatcher/EventDispatcher.php b/lib/private/EventDispatcher/EventDispatcher.php index 51a937777929d..9b44ac89f9873 100644 --- a/lib/private/EventDispatcher/EventDispatcher.php +++ b/lib/private/EventDispatcher/EventDispatcher.php @@ -16,6 +16,7 @@ use OCP\EventDispatcher\ABroadcastedEvent; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventDispatcher; +use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; use Symfony\Component\EventDispatcher\EventDispatcher as SymfonyDispatcher; use function get_class; @@ -23,6 +24,7 @@ class EventDispatcher implements IEventDispatcher { public function __construct( private SymfonyDispatcher $dispatcher, + private ContainerInterface $container, private LoggerInterface $logger, ) { // inject the event dispatcher into the logger @@ -50,6 +52,7 @@ public function addServiceListener(string $eventName, string $className, int $priority = 0): void { $listener = new ServiceEventListener( + $this->container, $className, $this->logger ); diff --git a/lib/private/EventDispatcher/ServiceEventListener.php b/lib/private/EventDispatcher/ServiceEventListener.php index e99fe29e99abc..f9cc85b412605 100644 --- a/lib/private/EventDispatcher/ServiceEventListener.php +++ b/lib/private/EventDispatcher/ServiceEventListener.php @@ -12,7 +12,7 @@ use OCP\AppFramework\QueryException; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; -use OCP\Server; +use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; use function sprintf; @@ -26,6 +26,7 @@ final class ServiceEventListener { private ?IEventListener $service = null; public function __construct( + private ContainerInterface $container, private string $class, private LoggerInterface $logger, ) { @@ -34,12 +35,10 @@ public function __construct( public function __invoke(Event $event) { if ($this->service === null) { try { - // Resolved from the current container rather than one captured at construction, - // since this listener may be invoked long after whatever built it. // TODO: fetch from the app containers, otherwise any custom services, // parameters and aliases won't be resolved. // See https://github.com/nextcloud/server/issues/27793 for details. - $this->service = Server::get($this->class); + $this->service = $this->container->get($this->class); } catch (QueryException $e) { $this->logger->error( sprintf( diff --git a/tests/lib/Share20/LegacyHooksTest.php b/tests/lib/Share20/LegacyHooksTest.php index 47f5abd7a14d9..15c0dbc2f0ab3 100644 --- a/tests/lib/Share20/LegacyHooksTest.php +++ b/tests/lib/Share20/LegacyHooksTest.php @@ -23,6 +23,7 @@ use OCP\Share\IShare; use OCP\Util; use PHPUnit\Framework\Attributes\Group; +use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; use Test\TestCase; @@ -51,7 +52,7 @@ protected function setUp(): void { $symfonyDispatcher = new \Symfony\Component\EventDispatcher\EventDispatcher(); $logger = $this->createMock(LoggerInterface::class); - $this->eventDispatcher = new EventDispatcher($symfonyDispatcher, $logger); + $this->eventDispatcher = new EventDispatcher($symfonyDispatcher, Server::get(ContainerInterface::class), $logger); $this->hooks = new LegacyHooks($this->eventDispatcher); $this->manager = Server::get(IShareManager::class); } diff --git a/tests/lib/TextProcessing/TextProcessingTest.php b/tests/lib/TextProcessing/TextProcessingTest.php index 34744cbe0b0f3..e54e4c6d60b65 100644 --- a/tests/lib/TextProcessing/TextProcessingTest.php +++ b/tests/lib/TextProcessing/TextProcessingTest.php @@ -127,6 +127,7 @@ protected function setUp(): void { $this->eventDispatcher = new EventDispatcher( new \Symfony\Component\EventDispatcher\EventDispatcher(), + $this->serverContainer, Server::get(LoggerInterface::class), );