diff --git a/lib/private/AppConfig.php b/lib/private/AppConfig.php index d023e688c00..2fbc7fe3812 100644 --- a/lib/private/AppConfig.php +++ b/lib/private/AppConfig.php @@ -14,6 +14,7 @@ use JsonException; use OC\AppFramework\Bootstrap\Coordinator; use OC\Config\ConfigManager; use OC\Config\PresetManager; +use OC\Memcache\Factory as CacheFactory; use OCP\Config\Lexicon\Entry; use OCP\Config\Lexicon\Strictness; use OCP\Config\ValueType; @@ -79,10 +80,12 @@ class AppConfig implements IAppConfig { private readonly PresetManager $presetManager, protected LoggerInterface $logger, protected ICrypto $crypto, - readonly ICacheFactory $cacheFactory, + readonly CacheFactory $cacheFactory, ) { if ($config->getSystemValueBool('cache_app_config', true) && $cacheFactory->isLocalCacheAvailable()) { - $this->localCache = $cacheFactory->createLocal(); + $cacheFactory->withServerVersionPrefix(function (ICacheFactory $factory) { + $this->localCache = $factory->createLocal(); + }); } } diff --git a/lib/private/Memcache/Factory.php b/lib/private/Memcache/Factory.php index b54189937fc..2a3fabc89f1 100644 --- a/lib/private/Memcache/Factory.php +++ b/lib/private/Memcache/Factory.php @@ -7,72 +7,65 @@ */ namespace OC\Memcache; -use Closure; +use OC\SystemConfig; use OCP\Cache\CappedMemoryCache; +use OCP\IAppConfig; use OCP\ICache; use OCP\ICacheFactory; use OCP\IMemcache; use OCP\Profiler\IProfiler; +use OCP\ServerVersion; use Psr\Log\LoggerInterface; class Factory implements ICacheFactory { public const NULL_CACHE = NullCache::class; - private ?string $globalPrefix = null; - - private LoggerInterface $logger; - + protected ?string $globalPrefix = null; /** - * @var ?class-string $localCacheClass + * @var class-string $localCacheClass */ - private ?string $localCacheClass; + protected string $localCacheClass; /** - * @var ?class-string $distributedCacheClass + * @var class-string $distributedCacheClass */ - private ?string $distributedCacheClass; + protected string $distributedCacheClass; /** - * @var ?class-string $lockingCacheClass + * @var class-string $lockingCacheClass */ - private ?string $lockingCacheClass; - - private string $logFile; - - private IProfiler $profiler; + protected string $lockingCacheClass; /** - * @param Closure $globalPrefixClosure - * @param LoggerInterface $logger * @param ?class-string $localCacheClass * @param ?class-string $distributedCacheClass * @param ?class-string $lockingCacheClass - * @param string $logFile */ public function __construct( - private Closure $globalPrefixClosure, - LoggerInterface $logger, - IProfiler $profiler, + protected LoggerInterface $logger, + protected IProfiler $profiler, + protected ServerVersion $serverVersion, ?string $localCacheClass = null, ?string $distributedCacheClass = null, ?string $lockingCacheClass = null, - string $logFile = '', + protected string $logFile = '', ) { - $this->logFile = $logFile; - if (!$localCacheClass) { $localCacheClass = self::NULL_CACHE; } $localCacheClass = ltrim($localCacheClass, '\\'); + if (!$distributedCacheClass) { $distributedCacheClass = $localCacheClass; } - $distributedCacheClass = ltrim($distributedCacheClass, '\\'); $missingCacheMessage = 'Memcache {class} not available for {use} cache'; $missingCacheHint = 'Is the matching PHP module installed and enabled?'; - if (!class_exists($localCacheClass) || !$localCacheClass::isAvailable()) { + if (!class_exists($localCacheClass) + || !is_a($localCacheClass, ICache::class, true) + || !$localCacheClass::isAvailable() + ) { if (\OC::$CLI && !defined('PHPUNIT_RUN') && $localCacheClass === APCu::class) { // CLI should not fail if APCu is not available but fallback to NullCache. // This can be the case if APCu is used without apc.enable_cli=1. @@ -84,7 +77,11 @@ class Factory implements ICacheFactory { ]), $missingCacheHint); } } - if (!class_exists($distributedCacheClass) || !$distributedCacheClass::isAvailable()) { + + if (!class_exists($distributedCacheClass) + || !is_a($distributedCacheClass, ICache::class, true) + || !$distributedCacheClass::isAvailable() + ) { if (\OC::$CLI && !defined('PHPUNIT_RUN') && $distributedCacheClass === APCu::class) { // CLI should not fail if APCu is not available but fallback to NullCache. // This can be the case if APCu is used without apc.enable_cli=1. @@ -96,25 +93,51 @@ class Factory implements ICacheFactory { ]), $missingCacheHint); } } - if (!($lockingCacheClass && class_exists($lockingCacheClass) && $lockingCacheClass::isAvailable())) { + + if (!$lockingCacheClass + || !class_exists($lockingCacheClass) + || !is_a($lockingCacheClass, IMemcache::class, true) + || !$lockingCacheClass::isAvailable() + ) { // don't fall back since the fallback might not be suitable for storing lock $lockingCacheClass = self::NULL_CACHE; } + /** @var class-string */ $lockingCacheClass = ltrim($lockingCacheClass, '\\'); $this->localCacheClass = $localCacheClass; $this->distributedCacheClass = $distributedCacheClass; $this->lockingCacheClass = $lockingCacheClass; - $this->profiler = $profiler; } - private function getGlobalPrefix(): ?string { - if (is_null($this->globalPrefix)) { - $this->globalPrefix = ($this->globalPrefixClosure)(); + protected function getGlobalPrefix(): string { + if ($this->globalPrefix === null) { + $config = \OCP\Server::get(SystemConfig::class); + $versions = []; + if ($config->getValue('installed', false)) { + $appConfig = \OCP\Server::get(IAppConfig::class); + $versions = $appConfig->getAppInstalledVersions(); + } + $versions['core'] = implode('.', $this->serverVersion->getVersion()); + $this->globalPrefix = hash('xxh128', implode(',', $versions)); } return $this->globalPrefix; } + /** + * Override the global prefix for a specific closure. + * This should only be used internally for bootstrapping purpose! + * + * @param string $globalPrefix - The prefix to use during the closure execution + * @param \Closure $closure - The closure with the cache factory as the first parameter + */ + public function withServerVersionPrefix(\Closure $closure): void { + $backupPrefix = $this->globalPrefix; + $this->globalPrefix = hash('xxh128', implode('.', $this->serverVersion->getVersion())); + $closure($this); + $this->globalPrefix = $backupPrefix; + } + /** * create a cache instance for storing locks * @@ -122,22 +145,17 @@ class Factory implements ICacheFactory { * @return IMemcache */ public function createLocking(string $prefix = ''): IMemcache { - $globalPrefix = $this->getGlobalPrefix(); - if (is_null($globalPrefix)) { - return new ArrayCache($prefix); - } - - assert($this->lockingCacheClass !== null); - $cache = new $this->lockingCacheClass($globalPrefix . '/' . $prefix); - if ($this->lockingCacheClass === Redis::class && $this->profiler->isEnabled()) { - // We only support the profiler with Redis - $cache = new ProfilerWrapperCache($cache, 'Locking'); - $this->profiler->add($cache); - } + $cache = new $this->lockingCacheClass($this->getGlobalPrefix() . '/' . $prefix); + if ($this->lockingCacheClass === Redis::class) { + if ($this->profiler->isEnabled()) { + // We only support the profiler with Redis + $cache = new ProfilerWrapperCache($cache, 'Locking'); + $this->profiler->add($cache); + } - if ($this->lockingCacheClass === Redis::class - && $this->logFile !== '' && is_writable(dirname($this->logFile)) && (!file_exists($this->logFile) || is_writable($this->logFile))) { - $cache = new LoggerWrapperCache($cache, $this->logFile); + if ($this->logFile !== '' && is_writable(dirname($this->logFile)) && (!file_exists($this->logFile) || is_writable($this->logFile))) { + $cache = new LoggerWrapperCache($cache, $this->logFile); + } } return $cache; } @@ -149,22 +167,17 @@ class Factory implements ICacheFactory { * @return ICache */ public function createDistributed(string $prefix = ''): ICache { - $globalPrefix = $this->getGlobalPrefix(); - if (is_null($globalPrefix)) { - return new ArrayCache($prefix); - } - - assert($this->distributedCacheClass !== null); - $cache = new $this->distributedCacheClass($globalPrefix . '/' . $prefix); - if ($this->distributedCacheClass === Redis::class && $this->profiler->isEnabled()) { - // We only support the profiler with Redis - $cache = new ProfilerWrapperCache($cache, 'Distributed'); - $this->profiler->add($cache); - } + $cache = new $this->distributedCacheClass($this->getGlobalPrefix() . '/' . $prefix); + if ($this->distributedCacheClass === Redis::class) { + if ($this->profiler->isEnabled()) { + // We only support the profiler with Redis + $cache = new ProfilerWrapperCache($cache, 'Distributed'); + $this->profiler->add($cache); + } - if ($this->distributedCacheClass === Redis::class && $this->logFile !== '' - && is_writable(dirname($this->logFile)) && (!file_exists($this->logFile) || is_writable($this->logFile))) { - $cache = new LoggerWrapperCache($cache, $this->logFile); + if ($this->logFile !== '' && is_writable(dirname($this->logFile)) && (!file_exists($this->logFile) || is_writable($this->logFile))) { + $cache = new LoggerWrapperCache($cache, $this->logFile); + } } return $cache; } @@ -176,22 +189,17 @@ class Factory implements ICacheFactory { * @return ICache */ public function createLocal(string $prefix = ''): ICache { - $globalPrefix = $this->getGlobalPrefix(); - if (is_null($globalPrefix)) { - return new ArrayCache($prefix); - } - - assert($this->localCacheClass !== null); - $cache = new $this->localCacheClass($globalPrefix . '/' . $prefix); - if ($this->localCacheClass === Redis::class && $this->profiler->isEnabled()) { - // We only support the profiler with Redis - $cache = new ProfilerWrapperCache($cache, 'Local'); - $this->profiler->add($cache); - } + $cache = new $this->localCacheClass($this->getGlobalPrefix() . '/' . $prefix); + if ($this->localCacheClass === Redis::class) { + if ($this->profiler->isEnabled()) { + // We only support the profiler with Redis + $cache = new ProfilerWrapperCache($cache, 'Local'); + $this->profiler->add($cache); + } - if ($this->localCacheClass === Redis::class && $this->logFile !== '' - && is_writable(dirname($this->logFile)) && (!file_exists($this->logFile) || is_writable($this->logFile))) { - $cache = new LoggerWrapperCache($cache, $this->logFile); + if ($this->logFile !== '' && is_writable(dirname($this->logFile)) && (!file_exists($this->logFile) || is_writable($this->logFile))) { + $cache = new LoggerWrapperCache($cache, $this->logFile); + } } return $cache; } @@ -217,4 +225,11 @@ class Factory implements ICacheFactory { public function isLocalCacheAvailable(): bool { return $this->localCacheClass !== self::NULL_CACHE; } + + public function clearAll(): void { + $this->createLocal()->clear(); + $this->createDistributed()->clear(); + $this->createLocking()->clear(); + $this->createInMemory()->clear(); + } } diff --git a/lib/private/Server.php b/lib/private/Server.php index 22cd13438b8..49bda19a738 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -585,62 +585,37 @@ class Server extends ServerContainer implements IServerContainer { $this->registerAlias(IURLGenerator::class, URLGenerator::class); - $this->registerService(ICache::class, function ($c) { - return new Cache\File(); - }); - + $this->registerAlias(ICache::class, Cache\File::class); $this->registerService(Factory::class, function (Server $c) { $profiler = $c->get(IProfiler::class); - $arrayCacheFactory = new \OC\Memcache\Factory(fn () => '', $c->get(LoggerInterface::class), - $profiler, - ArrayCache::class, - ArrayCache::class, - ArrayCache::class - ); + $logger = $c->get(LoggerInterface::class); + $serverVersion = $c->get(ServerVersion::class); /** @var SystemConfig $config */ $config = $c->get(SystemConfig::class); - /** @var ServerVersion $serverVersion */ - $serverVersion = $c->get(ServerVersion::class); - - if ($config->getValue('installed', false) && !(defined('PHPUNIT_RUN') && PHPUNIT_RUN)) { - $logQuery = $config->getValue('log_query'); - $prefixClosure = function () use ($logQuery, $serverVersion): ?string { - if (!$logQuery) { - try { - $v = \OCP\Server::get(IAppConfig::class)->getAppInstalledVersions(true); - } catch (\Doctrine\DBAL\Exception $e) { - // Database service probably unavailable - // Probably related to https://github.com/nextcloud/server/issues/37424 - return null; - } - } else { - // If the log_query is enabled, we can not get the app versions - // as that does a query, which will be logged and the logging - // depends on redis and here we are back again in the same function. - $v = [ - 'log_query' => 'enabled', - ]; - } - $v['core'] = implode(',', $serverVersion->getVersion()); - $version = implode(',', array_keys($v)) . implode(',', $v); - $instanceId = \OC_Util::getInstanceId(); - $path = \OC::$SERVERROOT; - return md5($instanceId . '-' . $version . '-' . $path); - }; - return new \OC\Memcache\Factory($prefixClosure, - $c->get(LoggerInterface::class), + if (!$config->getValue('installed', false) || (defined('PHPUNIT_RUN') && PHPUNIT_RUN)) { + return new \OC\Memcache\Factory( + $logger, $profiler, - /** @psalm-taint-escape callable */ - $config->getValue('memcache.local', null), - /** @psalm-taint-escape callable */ - $config->getValue('memcache.distributed', null), - /** @psalm-taint-escape callable */ - $config->getValue('memcache.locking', null), - /** @psalm-taint-escape callable */ - $config->getValue('redis_log_file') + $serverVersion, + ArrayCache::class, + ArrayCache::class, + ArrayCache::class ); } - return $arrayCacheFactory; + + return new \OC\Memcache\Factory( + $logger, + $profiler, + $serverVersion, + /** @psalm-taint-escape callable */ + $config->getValue('memcache.local', null), + /** @psalm-taint-escape callable */ + $config->getValue('memcache.distributed', null), + /** @psalm-taint-escape callable */ + $config->getValue('memcache.locking', null), + /** @psalm-taint-escape callable */ + $config->getValue('redis_log_file') + ); }); $this->registerAlias(ICacheFactory::class, Factory::class); diff --git a/tests/lib/App/AppManagerTest.php b/tests/lib/App/AppManagerTest.php index 6637c529a1e..9a4716aa8f1 100644 --- a/tests/lib/App/AppManagerTest.php +++ b/tests/lib/App/AppManagerTest.php @@ -233,28 +233,25 @@ class AppManagerTest extends TestCase { $this->manager->disableApp('files_trashbin'); } $this->eventDispatcher->expects($this->once())->method('dispatchTyped')->with(new AppEnableEvent('files_trashbin')); + $this->manager->enableApp('files_trashbin'); $this->assertEquals('yes', $this->appConfig->getValue('files_trashbin', 'enabled', 'no')); } public function testDisableApp(): void { $this->eventDispatcher->expects($this->once())->method('dispatchTyped')->with(new AppDisableEvent('files_trashbin')); + $this->manager->disableApp('files_trashbin'); $this->assertEquals('no', $this->appConfig->getValue('files_trashbin', 'enabled', 'no')); } public function testNotEnableIfNotInstalled(): void { - try { - $this->manager->enableApp('some_random_name_which_i_hope_is_not_an_app'); - $this->assertFalse(true, 'If this line is reached the expected exception is not thrown.'); - } catch (AppPathNotFoundException $e) { - // Exception is expected - $this->assertEquals('Could not find path for some_random_name_which_i_hope_is_not_an_app', $e->getMessage()); - } + $this->expectException(AppPathNotFoundException::class); + $this->expectExceptionMessage('Could not find path for some_random_name_which_i_hope_is_not_an_app'); + $this->appConfig->expects(self::never()) + ->method('setValue'); - $this->assertEquals('no', $this->appConfig->getValue( - 'some_random_name_which_i_hope_is_not_an_app', 'enabled', 'no' - )); + $this->manager->enableApp('some_random_name_which_i_hope_is_not_an_app'); } public function testEnableAppForGroups(): void { @@ -289,7 +286,9 @@ class AppManagerTest extends TestCase { ->with('test') ->willReturn('apps/test'); - $this->eventDispatcher->expects($this->once())->method('dispatchTyped')->with(new AppEnableEvent('test', ['group1', 'group2'])); + $this->eventDispatcher->expects($this->once()) + ->method('dispatchTyped') + ->with(new AppEnableEvent('test', ['group1', 'group2'])); $manager->enableAppForGroups('test', $groups); $this->assertEquals('["group1","group2"]', $this->appConfig->getValue('test', 'enabled', 'no')); diff --git a/tests/lib/AppConfigIntegrationTest.php b/tests/lib/AppConfigIntegrationTest.php index 4c415492708..4f821e00a63 100644 --- a/tests/lib/AppConfigIntegrationTest.php +++ b/tests/lib/AppConfigIntegrationTest.php @@ -11,10 +11,10 @@ use InvalidArgumentException; use OC\AppConfig; use OC\Config\ConfigManager; use OC\Config\PresetManager; +use OC\Memcache\Factory as CacheFactory; use OCP\Exceptions\AppConfigTypeConflictException; use OCP\Exceptions\AppConfigUnknownKeyException; use OCP\IAppConfig; -use OCP\ICacheFactory; use OCP\IConfig; use OCP\IDBConnection; use OCP\Security\ICrypto; @@ -35,7 +35,7 @@ class AppConfigIntegrationTest extends TestCase { private PresetManager $presetManager; private LoggerInterface $logger; private ICrypto $crypto; - private ICacheFactory&MockObject $cacheFactory; + private CacheFactory&MockObject $cacheFactory; private array $originalConfig; @@ -108,7 +108,7 @@ class AppConfigIntegrationTest extends TestCase { $this->presetManager = Server::get(PresetManager::class); $this->logger = Server::get(LoggerInterface::class); $this->crypto = Server::get(ICrypto::class); - $this->cacheFactory = $this->createMock(ICacheFactory::class); + $this->cacheFactory = $this->createMock(CacheFactory::class); $this->cacheFactory->method('isLocalCacheAvailable')->willReturn(false); // storing current config and emptying the data table diff --git a/tests/lib/AppConfigTest.php b/tests/lib/AppConfigTest.php index d7358c4a1e9..f5e385a8405 100644 --- a/tests/lib/AppConfigTest.php +++ b/tests/lib/AppConfigTest.php @@ -10,11 +10,11 @@ namespace Test; use OC\AppConfig; use OC\Config\ConfigManager; use OC\Config\PresetManager; +use OC\Memcache\Factory as CacheFactory; use OCP\DB\IResult; use OCP\DB\QueryBuilder\IExpressionBuilder; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\ICache; -use OCP\ICacheFactory; use OCP\IConfig; use OCP\IDBConnection; use OCP\Security\ICrypto; @@ -33,7 +33,7 @@ class AppConfigTest extends TestCase { private PresetManager&MockObject $presetManager; private LoggerInterface&MockObject $logger; private ICrypto&MockObject $crypto; - private ICacheFactory&MockObject $cacheFactory; + private CacheFactory&MockObject $cacheFactory; private ICache&MockObject $localCache; protected function setUp(): void { @@ -45,7 +45,7 @@ class AppConfigTest extends TestCase { $this->presetManager = $this->createMock(PresetManager::class); $this->logger = $this->createMock(LoggerInterface::class); $this->crypto = $this->createMock(ICrypto::class); - $this->cacheFactory = $this->createMock(ICacheFactory::class); + $this->cacheFactory = $this->createMock(CacheFactory::class); $this->localCache = $this->createMock(ICache::class); } @@ -55,6 +55,9 @@ class AppConfigTest extends TestCase { ->willReturn(true); $this->cacheFactory->method('isLocalCacheAvailable')->willReturn($cached); if ($cached) { + $this->cacheFactory->method('withServerVersionPrefix')->willReturnCallback(function (\Closure $closure): void { + $closure($this->cacheFactory); + }); $this->cacheFactory->method('createLocal')->willReturn($this->localCache); } diff --git a/tests/lib/Memcache/FactoryTest.php b/tests/lib/Memcache/FactoryTest.php index e16e079349f..31500f31b65 100644 --- a/tests/lib/Memcache/FactoryTest.php +++ b/tests/lib/Memcache/FactoryTest.php @@ -12,6 +12,7 @@ use OC\Memcache\Factory; use OC\Memcache\NullCache; use OCP\HintException; use OCP\Profiler\IProfiler; +use OCP\ServerVersion; use Psr\Log\LoggerInterface; class Test_Factory_Available_Cache1 extends NullCache { @@ -111,7 +112,8 @@ class FactoryTest extends \Test\TestCase { $expectedLocalCache, $expectedDistributedCache, $expectedLockingCache): void { $logger = $this->getMockBuilder(LoggerInterface::class)->getMock(); $profiler = $this->getMockBuilder(IProfiler::class)->getMock(); - $factory = new Factory(fn () => 'abc', $logger, $profiler, $localCache, $distributedCache, $lockingCache); + $serverVersion = $this->createMock(ServerVersion::class); + $factory = new Factory($logger, $profiler, $serverVersion, $localCache, $distributedCache, $lockingCache); $this->assertTrue(is_a($factory->createLocal(), $expectedLocalCache)); $this->assertTrue(is_a($factory->createDistributed(), $expectedDistributedCache)); $this->assertTrue(is_a($factory->createLocking(), $expectedLockingCache)); @@ -123,13 +125,15 @@ class FactoryTest extends \Test\TestCase { $logger = $this->getMockBuilder(LoggerInterface::class)->getMock(); $profiler = $this->getMockBuilder(IProfiler::class)->getMock(); - new Factory(fn () => 'abc', $logger, $profiler, $localCache, $distributedCache); + $serverVersion = $this->createMock(ServerVersion::class); + new Factory($logger, $profiler, $serverVersion, $localCache, $distributedCache); } public function testCreateInMemory(): void { $logger = $this->getMockBuilder(LoggerInterface::class)->getMock(); $profiler = $this->getMockBuilder(IProfiler::class)->getMock(); - $factory = new Factory(fn () => 'abc', $logger, $profiler, null, null, null); + $serverVersion = $this->createMock(ServerVersion::class); + $factory = new Factory($logger, $profiler, $serverVersion, null, null, null); $cache = $factory->createInMemory(); $cache->set('test', 48);