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 } 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/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/OC.php b/lib/OC.php index 251aa8b9a2bc1..28e1ca333c15c 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; @@ -770,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(); @@ -780,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, @@ -1382,6 +1401,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..c978eb18dc6f5 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', @@ -197,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', @@ -1250,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', @@ -1637,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 27b13860aec74..a6c71dc0812ef 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', @@ -238,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', @@ -1291,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', @@ -1678,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..b9a4c0027017f 100644 --- a/lib/private/AppConfig.php +++ b/lib/private/AppConfig.php @@ -15,6 +15,9 @@ 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; use OCP\Config\Lexicon\Strictness; use OCP\Config\ValueType; @@ -51,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; @@ -951,6 +955,7 @@ private function setTypedValue( if ($refreshCache) { $this->clearCache(); + $this->invalidatePersistedServices(); return true; } @@ -962,10 +967,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. * @@ -1009,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; } @@ -1070,6 +1085,7 @@ public function updateSensitive(string $app, string $key, bool $sensitive): bool $update->executeStatement(); $this->valueTypes[$app][$key] = $type; + $this->invalidatePersistedServices(); return true; } @@ -1107,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; } @@ -1273,6 +1290,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 +1309,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..268a1e5810e4b --- /dev/null +++ b/lib/private/AppFramework/Utility/PersistentServiceInvalidator.php @@ -0,0 +1,53 @@ +cache ??= $this->cacheFactory->createDistributed(self::CACHE_PREFIX); + } + + #[\Override] + public function invalidate(string|PersistentServiceGroup $group): void { + $key = $group instanceof PersistentServiceGroup ? $group->value : $group; + $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); + } + + /** + * @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->getCache()->get($key); + } +} diff --git a/lib/private/AppFramework/Utility/SimpleContainer.php b/lib/private/AppFramework/Utility/SimpleContainer.php index 6246a56ac209b..16a849e5d8854 100644 --- a/lib/private/AppFramework/Utility/SimpleContainer.php +++ b/lib/private/AppFramework/Utility/SimpleContainer.php @@ -10,7 +10,9 @@ use ArrayAccess; 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; @@ -30,11 +32,63 @@ 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; + + /** A kept instance is rebuilt after this many seconds even without an invalidation, as a safety net */ + private const MAX_PERSISTENT_AGE_SECONDS = 3600; + + /** + * 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 + */ + public static function resetPersistentInstances(): void { + self::$keepPersistentServices = false; + self::$checkingGenerations = false; + } + protected Container $container; /** @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(); } @@ -138,18 +192,106 @@ 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'); } + + $attributes = $class->getAttributes(PersistAcrossRequests::class); + $isPersistent = self::$keepPersistentServices && !empty($attributes) && !self::$checkingGenerations; + $className = $class->getName(); + $groups = $isPersistent + ? array_map( + static fn (string|PersistentServiceGroup $group): string => $group instanceof PersistentServiceGroup ? $group->value : $group, + $attributes[0]->newInstance()->invalidatedBy, + ) + : []; + + $object = $this->buildClass($class, $chain); + + if ($isPersistent) { + $this->persistentMeta[$className] = [ + 'groups' => $this->currentGenerations($groups), + 'builtAt' => time(), + ]; + } + + return $object; } catch (ReflectionException $e) { // Class does not exist throw new QueryNotFoundException($baseMsg . ' ' . $e->getMessage()); } } + 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)); + } + + /** + * 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". + */ + 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]); + } + } + + /** + * @param list $groups + * @return array + */ + private function currentGenerations(array $groups): array { + if (empty($groups)) { + return []; + } + self::$checkingGenerations = true; + try { + $invalidator = $this->get(PersistentServiceInvalidator::class); + $generations = []; + foreach ($groups as $group) { + $generations[$group] = $this->generationCache[$group] ??= $invalidator->getGeneration($group); + } + return $generations; + } finally { + self::$checkingGenerations = false; + } + } + /** * @param string $name Already sanitized name * @param list $chain @@ -169,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 * @@ -202,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/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/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) { 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 ab4f6c93cb68a..79232c0986ae0 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); @@ -632,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); } @@ -1161,10 +1167,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/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 new file mode 100644 index 0000000000000..fa46d0a2f5dc8 --- /dev/null +++ b/lib/public/AppFramework/Attribute/PersistAcrossRequests.php @@ -0,0 +1,38 @@ + $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 4bf67d3e52d11..132f4ce09d73d 100644 --- a/tests/lib/AppFramework/Utility/SimpleContainerTest.php +++ b/tests/lib/AppFramework/Utility/SimpleContainerTest.php @@ -10,13 +10,34 @@ 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 { } +#[PersistAcrossRequests] +class ClassPersistAcrossRequests { +} + +#[PersistAcrossRequests(invalidatedBy: ['test-group'])] +class ClassPersistAcrossRequestsWithGroup { +} + +#[PersistAcrossRequests(invalidatedBy: [PersistentServiceGroup::Apps])] +class ClassPersistAcrossRequestsWithEnumGroup { +} + +#[PersistAcrossRequests(invalidatedBy: ['cyclic-group'])] +class ClassWithCyclicInvalidationDependency { +} + class ClassEmptyConstructor implements IInterfaceConstructor { } @@ -68,6 +89,13 @@ protected function setUp(): void { $this->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 +148,127 @@ public function testInstancesOnlyOnce(): void { $this->assertSame($object, $object2); } + public function testPersistAcrossRequestsIgnoredByDefault(): void { + $object = $this->container->get(ClassPersistAcrossRequests::class); + $this->container->resetForNextRequest(); + $object2 = $this->container->get(ClassPersistAcrossRequests::class); + $this->assertNotSame($object, $object2); + } + + public function testPersistAcrossRequestsKeepsInstanceOnceEnabled(): void { + SimpleContainer::$keepPersistentServices = true; + + $object = $this->container->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); + } + + public function testPersistAcrossRequestsInvalidatedByGroup(): void { + SimpleContainer::$keepPersistentServices = true; + + $cacheFactory = $this->createMock(ICacheFactory::class); + $cacheFactory->method('createDistributed')->willReturn(new ArrayCache()); + $invalidator = new PersistentServiceInvalidator($cacheFactory); + + $this->container->registerService(PersistentServiceInvalidator::class, function () use ($invalidator) { + return $invalidator; + }); + + $object = $this->container->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'); + + $this->container->resetForNextRequest(); + $this->assertNotSame($object, $this->container->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); + + $this->container->registerService(PersistentServiceInvalidator::class, function () use ($invalidator) { + return $invalidator; + }); + + $object = $this->container->get(ClassPersistAcrossRequestsWithEnumGroup::class); + + // Invalidating by the enum's string value must be indistinguishable from the enum case itself + $invalidator->invalidate('apps'); + + $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); + } + + /** + * 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/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 @@ +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)); + } } 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') 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(); + } +} 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'); + } }