From 9a2813e08c2f3428ed8e6a3e39ab5b5781f6b714 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Mon, 7 Sep 2026 12:56:12 +0200 Subject: [PATCH 01/12] 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 b3d83a4ec25c9..8f8610e79a784 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 d80e425b285e4..54f0bb923b124 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 ee95df28b9974..2d887ad017032 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 e23e34dcc5ea1..b41e7ac406e3e 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; + } + private Container $container; public function __construct() { @@ -129,12 +147,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->query('test')); @@ -116,6 +128,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->query( 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/12] 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 54f0bb923b124..c584cf63eb1eb 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -199,7 +199,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', @@ -1639,6 +1642,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 2d887ad017032..0deeaf9e86726 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -240,7 +240,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', @@ -1680,6 +1683,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 0887a41b2d0a8..0969dce4de37d 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 b41e7ac406e3e..279f31811a88f 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; } @@ -152,16 +172,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; @@ -171,6 +203,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; + } + /** * @inheritDoc * @param list $chain diff --git a/lib/private/Server.php b/lib/private/Server.php index f2d5c731f3f70..d79d4efabb85b 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; @@ -581,6 +583,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 8d491b46edd03..2148d634ce2ec 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 { } @@ -129,21 +141,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->query( 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 10c2dd5dc74dacbce8277f775312c3216dc5369d Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Mon, 7 Sep 2026 15:10:03 +0200 Subject: [PATCH 03/12] perf(frankenphp): Cache router accross requests Signed-off-by: Carl Schwan --- lib/OC.php | 4 ++++ lib/private/Route/Router.php | 32 ++++++++++++++++++++++++++++---- tests/lib/Route/RouterTest.php | 24 ++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 4 deletions(-) diff --git a/lib/OC.php b/lib/OC.php index 8f8610e79a784..df943b9dfee5a 100644 --- a/lib/OC.php +++ b/lib/OC.php @@ -781,6 +781,10 @@ 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; the + // request-derived context it builds itself with must always reflect the request being served. + Server::get(\OC\Route\Router::class)->refreshContext($request); + try { $profiler = new BuiltInProfiler( $config, diff --git a/lib/private/Route/Router.php b/lib/private/Route/Router.php index b75c3e6982694..7a9e7bb6ea0fe 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,28 @@ public function __construct( private ContainerInterface $container, protected IAppManager $appManager, ) { + $this->context = $this->buildContext($request); + // TODO cache + $this->root = $this->getCollection('root'); + } + + /** + * Rebuild the request context (host, scheme, HTTP method) from the request + * actually being served. + * + * The context captured at construction time only reflects whichever + * request happened to build this Router instance, so this must be called + * with the current request before matching or generating a URL whenever + * the same Router instance may outlive that request (see + * {@see \OCP\AppFramework\Attribute\PersistAcrossRequests}). + */ + public function refreshContext(IRequest $request): void { + $this->setContext($this->buildContext($request)); + } + + 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 +93,14 @@ 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 URL generator is built from and holds onto the context it was created with, so it must be + // rebuilt whenever the context changes, or it would keep generating URLs for the previous one. + $this->generator = null; } public function getRouteCollection() { diff --git a/tests/lib/Route/RouterTest.php b/tests/lib/Route/RouterTest.php index 0537372050c07..0f242a2ab5e52 100644 --- a/tests/lib/Route/RouterTest.php +++ b/tests/lib/Route/RouterTest.php @@ -57,6 +57,30 @@ 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 testGenerateConsecutively(): void { $this->appManager->expects(self::atLeastOnce()) ->method('cleanAppId') From 38315a01425e55afd7f05d5d05be73d535817436 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Mon, 7 Sep 2026 15:24:32 +0200 Subject: [PATCH 04/12] perf(frankenphp): Persiste app registration information accross request Signed-off-by: Carl Schwan --- .../AppFramework/Bootstrap/Coordinator.php | 44 ++++++++++- .../DependencyInjection/DIContainer.php | 6 +- lib/private/CapabilitiesManager.php | 16 +++- lib/private/Files/Type/Detection.php | 2 + .../Bootstrap/CoordinatorTest.php | 74 +++++++++++++++++++ tests/lib/CapabilitiesManagerTest.php | 33 +++++++++ 6 files changed, 170 insertions(+), 5 deletions(-) diff --git a/lib/private/AppFramework/Bootstrap/Coordinator.php b/lib/private/AppFramework/Bootstrap/Coordinator.php index cf7c71500e0ef..3145c5963f171 100644 --- a/lib/private/AppFramework/Bootstrap/Coordinator.php +++ b/lib/private/AppFramework/Bootstrap/Coordinator.php @@ -10,10 +10,13 @@ namespace OC\AppFramework\Bootstrap; use OC\App\AppManager; +use OC\AppFramework\Utility\PersistentServiceInvalidator; +use OC\AppFramework\Utility\SimpleContainer; use OC\Support\CrashReport\Registry; use OCP\AppFramework\App; use OCP\AppFramework\Bootstrap\IBootstrap; use OCP\AppFramework\QueryException; +use OCP\AppFramework\Utility\PersistentServiceGroup; use OCP\Dashboard\IManager; use OCP\Diagnostics\IEventLogger; use OCP\EventDispatcher\IEventDispatcher; @@ -32,6 +35,25 @@ class Coordinator { /** @var array */ private array $bootedApps = []; + /** @psalm-suppress ImpureStaticProperty This class has a reset method */ + private static ?RegistrationContext $persistentRegistrationContext = null; + + /** + * @psalm-suppress ImpureStaticProperty This class has a reset method + * @var array + */ + private static array $registeredApps = []; + + /** @psalm-suppress ImpureStaticProperty This class has a reset method */ + private static ?int $registeredAppsGeneration = null; + + /** @internal */ + public static function resetPersistentRegistrations(): void { + self::$persistentRegistrationContext = null; + self::$registeredApps = []; + self::$registeredAppsGeneration = null; + } + public function __construct( private ContainerInterface $serverContainer, private Registry $registry, @@ -40,6 +62,7 @@ public function __construct( private IEventLogger $eventLogger, private AppManager $appManager, private LoggerInterface $logger, + private PersistentServiceInvalidator $persistentServiceInvalidator, ) { } @@ -61,14 +84,33 @@ public function runLazyRegistration(string $appId): void { */ private function registerApps(array $appIds): void { $this->eventLogger->start('bootstrap:register_apps', ''); - if ($this->registrationContext === null) { + + if (SimpleContainer::$keepPersistentServices) { + $currentGeneration = $this->persistentServiceInvalidator->getGeneration(PersistentServiceGroup::Apps); + if (self::$registeredAppsGeneration !== $currentGeneration) { + // The enabled apps changed since we last registered them: nothing we already registered still applies. + self::resetPersistentRegistrations(); + self::$registeredAppsGeneration = $currentGeneration; + } + self::$persistentRegistrationContext ??= new RegistrationContext($this->logger); + $this->registrationContext = self::$persistentRegistrationContext; + } elseif ($this->registrationContext === null) { $this->registrationContext = new RegistrationContext($this->logger); } + $this->eventLogger->start('bootstrap:register_app:autoloader', 'Setup autoloader for apps'); $this->appManager->registerAppsAutoloading($appIds); $this->eventLogger->end('bootstrap:register_app:autoloader'); $apps = []; foreach ($appIds as $appId) { + if (SimpleContainer::$keepPersistentServices) { + if (isset(self::$registeredApps[$appId])) { + continue; + } + // Marked upfront: a failed registration is not worth retrying on every request either. + self::$registeredApps[$appId] = true; + } + $this->eventLogger->start("bootstrap:register_app:$appId", "Register $appId"); /* diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php index 628038ee45c07..c524b448cb11a 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -308,9 +308,9 @@ private function getUserId(): string { */ #[\Override] public function registerCapability($serviceName) { - $this->query(CapabilitiesManager::class)->registerCapability(function () use ($serviceName) { - return $this->query($serviceName); - }); + $this->get(CapabilitiesManager::class)->registerCapability(function () use ($serviceName) { + return $this->get($serviceName); + }, $serviceName); } #[\Override] diff --git a/lib/private/CapabilitiesManager.php b/lib/private/CapabilitiesManager.php index 9e9db13a89bed..1646ad820c4a8 100644 --- a/lib/private/CapabilitiesManager.php +++ b/lib/private/CapabilitiesManager.php @@ -9,13 +9,16 @@ namespace OC; +use OCP\AppFramework\Attribute\PersistAcrossRequests; use OCP\AppFramework\QueryException; +use OCP\AppFramework\Utility\PersistentServiceGroup; use OCP\Capabilities\ICapability; use OCP\Capabilities\IInitialStateExcludedCapability; use OCP\Capabilities\IPublicCapability; use OCP\ILogger; use Psr\Log\LoggerInterface; +#[PersistAcrossRequests(invalidatedBy: [PersistentServiceGroup::Apps])] class CapabilitiesManager { /** * Anything above 0.1s to load the capabilities of an app qualifies for bad code @@ -26,6 +29,9 @@ class CapabilitiesManager { /** @var \Closure[] */ private array $capabilities = []; + /** @var array identifiers already passed to {@see registerCapability()} */ + private array $registeredIdentifiers = []; + public function __construct( private LoggerInterface $logger, ) { @@ -117,8 +123,16 @@ private function logSlowCapabilities(array $slowCapabilities): void { * $callable has to return an instance of OCP\Capabilities\ICapability * * @param \Closure $callable + * @param ?string $identifier stable identifier (e.g. the capability's class name) to skip a duplicate registration */ - public function registerCapability(\Closure $callable) { + public function registerCapability(\Closure $callable, ?string $identifier = null) { + if ($identifier !== null) { + if (isset($this->registeredIdentifiers[$identifier])) { + return; + } + $this->registeredIdentifiers[$identifier] = true; + } + $this->capabilities[] = $callable; } } diff --git a/lib/private/Files/Type/Detection.php b/lib/private/Files/Type/Detection.php index 35fec9c230f16..4efa780779249 100644 --- a/lib/private/Files/Type/Detection.php +++ b/lib/private/Files/Type/Detection.php @@ -9,6 +9,7 @@ namespace OC\Files\Type; +use OCP\AppFramework\Attribute\PersistAcrossRequests; use OCP\Files\IMimeTypeDetector; use OCP\IBinaryFinder; use OCP\ITempManager; @@ -23,6 +24,7 @@ * * @package OC\Files\Type */ +#[PersistAcrossRequests] class Detection implements IMimeTypeDetector { private const string CUSTOM_MIMETYPEMAPPING = 'mimetypemapping.json'; private const string CUSTOM_MIMETYPEALIASES = 'mimetypealiases.json'; diff --git a/tests/lib/AppFramework/Bootstrap/CoordinatorTest.php b/tests/lib/AppFramework/Bootstrap/CoordinatorTest.php index f1f9b2e283470..981432fb33def 100644 --- a/tests/lib/AppFramework/Bootstrap/CoordinatorTest.php +++ b/tests/lib/AppFramework/Bootstrap/CoordinatorTest.php @@ -11,6 +11,8 @@ use OC\App\AppManager; use OC\AppFramework\Bootstrap\Coordinator; +use OC\AppFramework\Utility\PersistentServiceInvalidator; +use OC\AppFramework\Utility\SimpleContainer; use OC\Support\CrashReport\Registry; use OCA\Settings\AppInfo\Application; use OCP\AppFramework\App; @@ -18,6 +20,7 @@ use OCP\AppFramework\Bootstrap\IBootstrap; use OCP\AppFramework\Bootstrap\IRegistrationContext; use OCP\AppFramework\QueryException; +use OCP\AppFramework\Utility\PersistentServiceGroup; use OCP\Dashboard\IManager; use OCP\Diagnostics\IEventLogger; use OCP\EventDispatcher\IEventDispatcher; @@ -34,6 +37,7 @@ class CoordinatorTest extends TestCase { private IEventDispatcher&MockObject $eventDispatcher; private IEventLogger&MockObject $eventLogger; private LoggerInterface&MockObject $logger; + private PersistentServiceInvalidator&MockObject $persistentServiceInvalidator; private Coordinator $coordinator; #[\Override] @@ -47,6 +51,7 @@ protected function setUp(): void { $this->eventDispatcher = $this->createMock(IEventDispatcher::class); $this->eventLogger = $this->createMock(IEventLogger::class); $this->logger = $this->createMock(LoggerInterface::class); + $this->persistentServiceInvalidator = $this->createMock(PersistentServiceInvalidator::class); $this->appManager->expects($this->any()) ->method('getAppNamespace') @@ -61,9 +66,18 @@ protected function setUp(): void { $this->eventLogger, $this->appManager, $this->logger, + $this->persistentServiceInvalidator, ); } + #[\Override] + protected function tearDown(): void { + SimpleContainer::resetPersistentInstances(); + Coordinator::resetPersistentRegistrations(); + + parent::tearDown(); + } + public function testBootAppNotLoadable(): void { $appId = 'settings'; $this->serverContainer->expects($this->once()) @@ -109,4 +123,64 @@ public function boot(IBootContext $context): void { $this->coordinator->bootApp($appId); } + + private function makeCountingApp(\stdClass $counter): App&IBootstrap { + return new class($counter) extends App implements IBootstrap { + public function __construct( + private \stdClass $counter, + ) { + parent::__construct('settings', []); + } + + #[\Override] + public function register(IRegistrationContext $context): void { + $this->counter->registerCalls++; + } + + #[\Override] + public function boot(IBootContext $context): void { + } + }; + } + + public function testRegisterAppsOnlyRegistersOnceWhilePersistent(): void { + SimpleContainer::$keepPersistentServices = true; + $this->persistentServiceInvalidator->method('getGeneration') + ->with(PersistentServiceGroup::Apps) + ->willReturn(1); + + $counter = new \stdClass(); + $counter->registerCalls = 0; + $this->serverContainer->method('get') + ->with(Application::class) + ->willReturn($this->makeCountingApp($counter)); + + $this->coordinator->runLazyRegistration('settings'); + $this->coordinator->runLazyRegistration('settings'); + + $this->assertSame(1, $counter->registerCalls); + } + + public function testRegisterAppsRunsAgainAfterAppsGenerationChanges(): void { + SimpleContainer::$keepPersistentServices = true; + + $counter = new \stdClass(); + $counter->registerCalls = 0; + $this->serverContainer->method('get') + ->with(Application::class) + ->willReturn($this->makeCountingApp($counter)); + + $generation = 1; + $this->persistentServiceInvalidator->method('getGeneration') + ->with(PersistentServiceGroup::Apps) + ->willReturnCallback(function () use (&$generation) { + return $generation; + }); + + $this->coordinator->runLazyRegistration('settings'); + $generation = 2; + $this->coordinator->runLazyRegistration('settings'); + + $this->assertSame(2, $counter->registerCalls); + } } diff --git a/tests/lib/CapabilitiesManagerTest.php b/tests/lib/CapabilitiesManagerTest.php index e7b84e6b82f99..df871e4968edd 100644 --- a/tests/lib/CapabilitiesManagerTest.php +++ b/tests/lib/CapabilitiesManagerTest.php @@ -130,6 +130,39 @@ public function testDeepIdenticalCapabilities(): void { $this->assertEquals($expected, $res); } + /** + * Test that re-registering the same identifier is ignored, as happens when the same + * worker re-registers every app's capabilities on each request + */ + public function testDuplicateIdentifierIsIgnored(): void { + $calls = 0; + $callable = function () use (&$calls) { + $calls++; + return new SimpleCapability(); + }; + + $this->manager->registerCapability($callable, 'Test\\SimpleCapability'); + $this->manager->registerCapability($callable, 'Test\\SimpleCapability'); + + $res = $this->manager->getCapabilities(); + + $this->assertEquals(['foo' => 1], $res); + $this->assertSame(1, $calls); + } + + public function testDifferentIdentifiersAreBothRegistered(): void { + $this->manager->registerCapability(function () { + return new SimpleCapability(); + }, 'Test\\SimpleCapability'); + $this->manager->registerCapability(function () { + return new SimpleCapability2(); + }, 'Test\\SimpleCapability2'); + + $res = $this->manager->getCapabilities(); + + $this->assertEquals(['foo' => 1, 'bar' => ['x' => 1]], $res); + } + public function testInvalidCapability(): void { $this->manager->registerCapability(function (): void { throw new QueryException(); From 561cdddb31be28a05ebea1e6aa9290f679241290 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Mon, 7 Sep 2026 16:25:33 +0200 Subject: [PATCH 05/12] fixup! perf(frankenphp): Cache router accross requests --- lib/private/Route/CachingRouter.php | 4 +--- lib/private/Route/Router.php | 6 +++--- lib/private/Server.php | 14 ++++++++------ tests/lib/Route/RouterTest.php | 3 --- 4 files changed, 12 insertions(+), 15 deletions(-) diff --git a/lib/private/Route/CachingRouter.php b/lib/private/Route/CachingRouter.php index fca6d4b1c51d6..72cdafd5c51b9 100644 --- a/lib/private/Route/CachingRouter.php +++ b/lib/private/Route/CachingRouter.php @@ -14,7 +14,6 @@ use OCP\ICacheFactory; use OCP\IConfig; use OCP\IRequest; -use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; use Symfony\Component\Routing\Exception\ResourceNotFoundException; use Symfony\Component\Routing\Matcher\CompiledUrlMatcher; @@ -32,11 +31,10 @@ public function __construct( IRequest $request, IConfig $config, IEventLogger $eventLogger, - ContainerInterface $container, IAppManager $appManager, ) { $this->cache = $cacheFactory->createLocal('route'); - parent::__construct($logger, $request, $config, $eventLogger, $container, $appManager); + parent::__construct($logger, $request, $config, $eventLogger, $appManager); } /** diff --git a/lib/private/Route/Router.php b/lib/private/Route/Router.php index 7a9e7bb6ea0fe..e7f6a790954cf 100644 --- a/lib/private/Route/Router.php +++ b/lib/private/Route/Router.php @@ -21,7 +21,6 @@ use OCP\IRequest; use OCP\Route\IRouter; use OCP\Util; -use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; use ReflectionAttribute; use ReflectionClass; @@ -59,7 +58,6 @@ public function __construct( IRequest $request, protected IConfig $config, protected IEventLogger $eventLogger, - private ContainerInterface $container, protected IAppManager $appManager, ) { $this->context = $this->buildContext($request); @@ -581,7 +579,9 @@ private function getApplicationClass(string $appName) { $applicationClassName = $appNameSpace . '\\AppInfo\\Application'; if (class_exists($applicationClassName)) { - $application = $this->container->get($applicationClassName); + // Resolved through the current container, not a captured one: this Router instance may + // be kept alive well past the request that built it (see PersistAcrossRequests below). + $application = \OCP\Server::get($applicationClassName); } else { $application = new App($appName); } diff --git a/lib/private/Server.php b/lib/private/Server.php index d79d4efabb85b..6d7270e935a01 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -908,12 +908,14 @@ public function __construct( $this->registerService(CapabilitiesManager::class, function (ContainerInterface $c) { $manager = new CapabilitiesManager($c->get(LoggerInterface::class)); - $manager->registerCapability(function () use ($c) { - return new CoreCapabilities($c->get(IConfig::class)); - }); - $manager->registerCapability(function () use ($c) { - return $c->get(Capabilities::class); - }); + // Resolved through the current container at call time, not the one that built $manager: + // CapabilitiesManager may be kept alive (and this closure with it) well past this request. + $manager->registerCapability(function () { + return new CoreCapabilities(\OCP\Server::get(IConfig::class)); + }, CoreCapabilities::class); + $manager->registerCapability(function () { + return \OCP\Server::get(Capabilities::class); + }, Capabilities::class); return $manager; }); diff --git a/tests/lib/Route/RouterTest.php b/tests/lib/Route/RouterTest.php index 0f242a2ab5e52..3565ddf79c0f0 100644 --- a/tests/lib/Route/RouterTest.php +++ b/tests/lib/Route/RouterTest.php @@ -14,7 +14,6 @@ use OCP\IConfig; use OCP\IRequest; use PHPUnit\Framework\MockObject\MockObject; -use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; use Test\TestCase; @@ -48,7 +47,6 @@ function (string $message, array $data): void { $this->createMock(IRequest::class), $this->createMock(IConfig::class), $this->createMock(IEventLogger::class), - $this->createMock(ContainerInterface::class), $this->appManager, ); } @@ -67,7 +65,6 @@ public function testRefreshContextUpdatesGeneratedAbsoluteUrls(): void { $firstRequest, $this->createMock(IConfig::class), $this->createMock(IEventLogger::class), - $this->createMock(ContainerInterface::class), $this->appManager, ); From 74175c0e3479ec20e2617b7f4e2c7d3ceb454fa4 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Mon, 7 Sep 2026 16:25:53 +0200 Subject: [PATCH 06/12] fixup! perf(frankenphp): Persiste app registration information accross request --- .../AppFramework/Bootstrap/Coordinator.php | 44 +---------- .../Bootstrap/CoordinatorTest.php | 74 ------------------- 2 files changed, 1 insertion(+), 117 deletions(-) diff --git a/lib/private/AppFramework/Bootstrap/Coordinator.php b/lib/private/AppFramework/Bootstrap/Coordinator.php index 3145c5963f171..cf7c71500e0ef 100644 --- a/lib/private/AppFramework/Bootstrap/Coordinator.php +++ b/lib/private/AppFramework/Bootstrap/Coordinator.php @@ -10,13 +10,10 @@ namespace OC\AppFramework\Bootstrap; use OC\App\AppManager; -use OC\AppFramework\Utility\PersistentServiceInvalidator; -use OC\AppFramework\Utility\SimpleContainer; use OC\Support\CrashReport\Registry; use OCP\AppFramework\App; use OCP\AppFramework\Bootstrap\IBootstrap; use OCP\AppFramework\QueryException; -use OCP\AppFramework\Utility\PersistentServiceGroup; use OCP\Dashboard\IManager; use OCP\Diagnostics\IEventLogger; use OCP\EventDispatcher\IEventDispatcher; @@ -35,25 +32,6 @@ class Coordinator { /** @var array */ private array $bootedApps = []; - /** @psalm-suppress ImpureStaticProperty This class has a reset method */ - private static ?RegistrationContext $persistentRegistrationContext = null; - - /** - * @psalm-suppress ImpureStaticProperty This class has a reset method - * @var array - */ - private static array $registeredApps = []; - - /** @psalm-suppress ImpureStaticProperty This class has a reset method */ - private static ?int $registeredAppsGeneration = null; - - /** @internal */ - public static function resetPersistentRegistrations(): void { - self::$persistentRegistrationContext = null; - self::$registeredApps = []; - self::$registeredAppsGeneration = null; - } - public function __construct( private ContainerInterface $serverContainer, private Registry $registry, @@ -62,7 +40,6 @@ public function __construct( private IEventLogger $eventLogger, private AppManager $appManager, private LoggerInterface $logger, - private PersistentServiceInvalidator $persistentServiceInvalidator, ) { } @@ -84,33 +61,14 @@ public function runLazyRegistration(string $appId): void { */ private function registerApps(array $appIds): void { $this->eventLogger->start('bootstrap:register_apps', ''); - - if (SimpleContainer::$keepPersistentServices) { - $currentGeneration = $this->persistentServiceInvalidator->getGeneration(PersistentServiceGroup::Apps); - if (self::$registeredAppsGeneration !== $currentGeneration) { - // The enabled apps changed since we last registered them: nothing we already registered still applies. - self::resetPersistentRegistrations(); - self::$registeredAppsGeneration = $currentGeneration; - } - self::$persistentRegistrationContext ??= new RegistrationContext($this->logger); - $this->registrationContext = self::$persistentRegistrationContext; - } elseif ($this->registrationContext === null) { + if ($this->registrationContext === null) { $this->registrationContext = new RegistrationContext($this->logger); } - $this->eventLogger->start('bootstrap:register_app:autoloader', 'Setup autoloader for apps'); $this->appManager->registerAppsAutoloading($appIds); $this->eventLogger->end('bootstrap:register_app:autoloader'); $apps = []; foreach ($appIds as $appId) { - if (SimpleContainer::$keepPersistentServices) { - if (isset(self::$registeredApps[$appId])) { - continue; - } - // Marked upfront: a failed registration is not worth retrying on every request either. - self::$registeredApps[$appId] = true; - } - $this->eventLogger->start("bootstrap:register_app:$appId", "Register $appId"); /* diff --git a/tests/lib/AppFramework/Bootstrap/CoordinatorTest.php b/tests/lib/AppFramework/Bootstrap/CoordinatorTest.php index 981432fb33def..f1f9b2e283470 100644 --- a/tests/lib/AppFramework/Bootstrap/CoordinatorTest.php +++ b/tests/lib/AppFramework/Bootstrap/CoordinatorTest.php @@ -11,8 +11,6 @@ use OC\App\AppManager; use OC\AppFramework\Bootstrap\Coordinator; -use OC\AppFramework\Utility\PersistentServiceInvalidator; -use OC\AppFramework\Utility\SimpleContainer; use OC\Support\CrashReport\Registry; use OCA\Settings\AppInfo\Application; use OCP\AppFramework\App; @@ -20,7 +18,6 @@ use OCP\AppFramework\Bootstrap\IBootstrap; use OCP\AppFramework\Bootstrap\IRegistrationContext; use OCP\AppFramework\QueryException; -use OCP\AppFramework\Utility\PersistentServiceGroup; use OCP\Dashboard\IManager; use OCP\Diagnostics\IEventLogger; use OCP\EventDispatcher\IEventDispatcher; @@ -37,7 +34,6 @@ class CoordinatorTest extends TestCase { private IEventDispatcher&MockObject $eventDispatcher; private IEventLogger&MockObject $eventLogger; private LoggerInterface&MockObject $logger; - private PersistentServiceInvalidator&MockObject $persistentServiceInvalidator; private Coordinator $coordinator; #[\Override] @@ -51,7 +47,6 @@ protected function setUp(): void { $this->eventDispatcher = $this->createMock(IEventDispatcher::class); $this->eventLogger = $this->createMock(IEventLogger::class); $this->logger = $this->createMock(LoggerInterface::class); - $this->persistentServiceInvalidator = $this->createMock(PersistentServiceInvalidator::class); $this->appManager->expects($this->any()) ->method('getAppNamespace') @@ -66,18 +61,9 @@ protected function setUp(): void { $this->eventLogger, $this->appManager, $this->logger, - $this->persistentServiceInvalidator, ); } - #[\Override] - protected function tearDown(): void { - SimpleContainer::resetPersistentInstances(); - Coordinator::resetPersistentRegistrations(); - - parent::tearDown(); - } - public function testBootAppNotLoadable(): void { $appId = 'settings'; $this->serverContainer->expects($this->once()) @@ -123,64 +109,4 @@ public function boot(IBootContext $context): void { $this->coordinator->bootApp($appId); } - - private function makeCountingApp(\stdClass $counter): App&IBootstrap { - return new class($counter) extends App implements IBootstrap { - public function __construct( - private \stdClass $counter, - ) { - parent::__construct('settings', []); - } - - #[\Override] - public function register(IRegistrationContext $context): void { - $this->counter->registerCalls++; - } - - #[\Override] - public function boot(IBootContext $context): void { - } - }; - } - - public function testRegisterAppsOnlyRegistersOnceWhilePersistent(): void { - SimpleContainer::$keepPersistentServices = true; - $this->persistentServiceInvalidator->method('getGeneration') - ->with(PersistentServiceGroup::Apps) - ->willReturn(1); - - $counter = new \stdClass(); - $counter->registerCalls = 0; - $this->serverContainer->method('get') - ->with(Application::class) - ->willReturn($this->makeCountingApp($counter)); - - $this->coordinator->runLazyRegistration('settings'); - $this->coordinator->runLazyRegistration('settings'); - - $this->assertSame(1, $counter->registerCalls); - } - - public function testRegisterAppsRunsAgainAfterAppsGenerationChanges(): void { - SimpleContainer::$keepPersistentServices = true; - - $counter = new \stdClass(); - $counter->registerCalls = 0; - $this->serverContainer->method('get') - ->with(Application::class) - ->willReturn($this->makeCountingApp($counter)); - - $generation = 1; - $this->persistentServiceInvalidator->method('getGeneration') - ->with(PersistentServiceGroup::Apps) - ->willReturnCallback(function () use (&$generation) { - return $generation; - }); - - $this->coordinator->runLazyRegistration('settings'); - $generation = 2; - $this->coordinator->runLazyRegistration('settings'); - - $this->assertSame(2, $counter->registerCalls); - } } From af88c05ed19d6c00d9aac81130f1e8496bed30cc Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Mon, 7 Sep 2026 16:26:08 +0200 Subject: [PATCH 07/12] fixup! perf(frankenphp): Cache router accross requests --- apps/files/tests/Controller/ViewControllerTest.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/apps/files/tests/Controller/ViewControllerTest.php b/apps/files/tests/Controller/ViewControllerTest.php index ee2753da58b6f..7903535e245ba 100644 --- a/apps/files/tests/Controller/ViewControllerTest.php +++ b/apps/files/tests/Controller/ViewControllerTest.php @@ -36,7 +36,6 @@ use OCP\IUser; use OCP\IUserSession; use PHPUnit\Framework\MockObject\MockObject; -use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; use Test\TestCase; @@ -48,7 +47,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; @@ -113,13 +111,11 @@ 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->appManager, ); From 024d96db2703d248319b22a7faa854950dff8681 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Mon, 7 Sep 2026 16:26:20 +0200 Subject: [PATCH 08/12] feat(frankenphp): Remove index.php from url Signed-off-by: Carl Schwan --- Caddyfile | 78 ++++++++++++++++++++++++++++++++----------------------- 1 file changed, 46 insertions(+), 32 deletions(-) diff --git a/Caddyfile b/Caddyfile index 7ed476abf1e79..f576909b2d01e 100644 --- a/Caddyfile +++ b/Caddyfile @@ -12,7 +12,7 @@ } } -localhost { +franken.local { php_server { worker { file index.php @@ -42,40 +42,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 + } + respond @forbidden 404 - @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 + # 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 4e70947b6e84ce8b5f683ad5850a2014649b8b60 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Mon, 7 Sep 2026 16:45:04 +0200 Subject: [PATCH 09/12] 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 6bc4f8b5e6b20c392511b8dec6467276d5de8846 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Mon, 7 Sep 2026 17:01:28 +0200 Subject: [PATCH 10/12] perf(frankenphp): Persist IAppConfig Signed-off-by: Carl Schwan --- lib/private/AppConfig.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/private/AppConfig.php b/lib/private/AppConfig.php index 0969dce4de37d..c8494a09ca27a 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; From 05098e17315a36f319f8fe8f1b4c155fc5940ac1 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Mon, 7 Sep 2026 17:09:34 +0200 Subject: [PATCH 11/12] fixup! perf(frankenphp): Cache router accross requests --- lib/private/Route/Router.php | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/lib/private/Route/Router.php b/lib/private/Route/Router.php index e7f6a790954cf..a653cf499ee1c 100644 --- a/lib/private/Route/Router.php +++ b/lib/private/Route/Router.php @@ -66,14 +66,8 @@ public function __construct( } /** - * Rebuild the request context (host, scheme, HTTP method) from the request - * actually being served. - * - * The context captured at construction time only reflects whichever - * request happened to build this Router instance, so this must be called - * with the current request before matching or generating a URL whenever - * the same Router instance may outlive that request (see - * {@see \OCP\AppFramework\Attribute\PersistAcrossRequests}). + * 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. */ public function refreshContext(IRequest $request): void { $this->setContext($this->buildContext($request)); @@ -96,8 +90,7 @@ private function buildContext(IRequest $request): RequestContext { public function setContext(RequestContext $context): void { $this->context = $context; - // The URL generator is built from and holds onto the context it was created with, so it must be - // rebuilt whenever the context changes, or it would keep generating URLs for the previous one. + // The cached generator holds onto the old context, so it must be rebuilt too. $this->generator = null; } @@ -579,8 +572,7 @@ private function getApplicationClass(string $appName) { $applicationClassName = $appNameSpace . '\\AppInfo\\Application'; if (class_exists($applicationClassName)) { - // Resolved through the current container, not a captured one: this Router instance may - // be kept alive well past the request that built it (see PersistAcrossRequests below). + // Always the current container: this Router instance may outlive the request that built it. $application = \OCP\Server::get($applicationClassName); } else { $application = new App($appName); From dde2d1c342653955086be71a168768f975e39ed6 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Mon, 7 Sep 2026 17:09:51 +0200 Subject: [PATCH 12/12] fixup! feat(frankenphp): Remove index.php from url --- Caddyfile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Caddyfile b/Caddyfile index f576909b2d01e..fe50aff8ad4ee 100644 --- a/Caddyfile +++ b/Caddyfile @@ -14,6 +14,9 @@ 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