fix: delay calculating global cache prefix untill a cache is created

Signed-off-by: Robin Appelman <robin@icewind.nl>
pull/47561/head
Robin Appelman 2024-08-20 16:46:49 +07:00 committed by Louis
parent b1744e70a5
commit 8f57d46a0b
3 changed files with 69 additions and 33 deletions

@ -16,7 +16,7 @@ use Psr\Log\LoggerInterface;
class Factory implements ICacheFactory {
public const NULL_CACHE = NullCache::class;
private string $globalPrefix;
private ?string $globalPrefix = null;
private LoggerInterface $logger;
@ -40,17 +40,23 @@ class Factory implements ICacheFactory {
private IProfiler $profiler;
/**
* @param string $globalPrefix
* @param callable $globalPrefixClosure
* @param LoggerInterface $logger
* @param ?class-string<ICache> $localCacheClass
* @param ?class-string<ICache> $distributedCacheClass
* @param ?class-string<IMemcache> $lockingCacheClass
* @param string $logFile
*/
public function __construct(string $globalPrefix, LoggerInterface $logger, IProfiler $profiler,
?string $localCacheClass = null, ?string $distributedCacheClass = null, ?string $lockingCacheClass = null, string $logFile = '') {
public function __construct(
private $globalPrefixClosure,
LoggerInterface $logger,
IProfiler $profiler,
?string $localCacheClass = null,
?string $distributedCacheClass = null,
?string $lockingCacheClass = null,
string $logFile = ''
) {
$this->logFile = $logFile;
$this->globalPrefix = $globalPrefix;
if (!$localCacheClass) {
$localCacheClass = self::NULL_CACHE;
@ -59,6 +65,7 @@ class Factory implements ICacheFactory {
if (!$distributedCacheClass) {
$distributedCacheClass = $localCacheClass;
}
$distributedCacheClass = ltrim($distributedCacheClass, '\\');
$missingCacheMessage = 'Memcache {class} not available for {use} cache';
@ -85,6 +92,13 @@ class Factory implements ICacheFactory {
$this->profiler = $profiler;
}
private function getGlobalPrefix(): ?string {
if (is_null($this->globalPrefix)) {
$this->globalPrefix = ($this->globalPrefixClosure)();
}
return $this->globalPrefix;
}
/**
* create a cache instance for storing locks
*
@ -92,8 +106,13 @@ 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($this->globalPrefix . '/' . $prefix);
$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');
@ -114,8 +133,13 @@ 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($this->globalPrefix . '/' . $prefix);
$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');
@ -136,8 +160,13 @@ 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($this->globalPrefix . '/' . $prefix);
$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');

@ -637,7 +637,7 @@ class Server extends ServerContainer implements IServerContainer {
$this->registerService(Factory::class, function (Server $c) {
$profiler = $c->get(IProfiler::class);
$arrayCacheFactory = new \OC\Memcache\Factory('', $c->get(LoggerInterface::class),
$arrayCacheFactory = new \OC\Memcache\Factory(fn () => '', $c->get(LoggerInterface::class),
$profiler,
ArrayCache::class,
ArrayCache::class,
@ -647,33 +647,40 @@ class Server extends ServerContainer implements IServerContainer {
$config = $c->get(SystemConfig::class);
if ($config->getValue('installed', false) && !(defined('PHPUNIT_RUN') && PHPUNIT_RUN)) {
if (!$config->getValue('log_query')) {
try {
$v = \OC_App::getAppVersions();
} catch (\Doctrine\DBAL\Exception $e) {
// Database service probably unavailable
// Probably related to https://github.com/nextcloud/server/issues/37424
return $arrayCacheFactory;
$logQuery = $config->getValue('log_query');
$prefixClosure = function () use ($logQuery) {
if (!$logQuery) {
try {
$v = \OC_App::getAppVersions();
} 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',
];
}
} 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(',', \OC_Util::getVersion());
$version = implode(',', $v);
$instanceId = \OC_Util::getInstanceId();
$path = \OC::$SERVERROOT;
$prefix = md5($instanceId . '-' . $version . '-' . $path);
return new \OC\Memcache\Factory($prefix,
$v['core'] = implode(',', \OC_Util::getVersion());
$version = implode(',', $v);
$instanceId = \OC_Util::getInstanceId();
$path = \OC::$SERVERROOT;
return md5($instanceId . '-' . $version . '-' . $path);
};
return new \OC\Memcache\Factory($prefixClosure,
$c->get(LoggerInterface::class),
$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')
);
}

@ -110,7 +110,7 @@ class FactoryTest extends \Test\TestCase {
$expectedLocalCache, $expectedDistributedCache, $expectedLockingCache) {
$logger = $this->getMockBuilder(LoggerInterface::class)->getMock();
$profiler = $this->getMockBuilder(IProfiler::class)->getMock();
$factory = new \OC\Memcache\Factory('abc', $logger, $profiler, $localCache, $distributedCache, $lockingCache);
$factory = new \OC\Memcache\Factory(fn () => 'abc', $logger, $profiler, $localCache, $distributedCache, $lockingCache);
$this->assertTrue(is_a($factory->createLocal(), $expectedLocalCache));
$this->assertTrue(is_a($factory->createDistributed(), $expectedDistributedCache));
$this->assertTrue(is_a($factory->createLocking(), $expectedLockingCache));
@ -124,13 +124,13 @@ class FactoryTest extends \Test\TestCase {
$logger = $this->getMockBuilder(LoggerInterface::class)->getMock();
$profiler = $this->getMockBuilder(IProfiler::class)->getMock();
new \OC\Memcache\Factory('abc', $logger, $profiler, $localCache, $distributedCache);
new \OC\Memcache\Factory(fn () => 'abc', $logger, $profiler, $localCache, $distributedCache);
}
public function testCreateInMemory(): void {
$logger = $this->getMockBuilder(LoggerInterface::class)->getMock();
$profiler = $this->getMockBuilder(IProfiler::class)->getMock();
$factory = new \OC\Memcache\Factory('abc', $logger, $profiler, null, null, null);
$factory = new \OC\Memcache\Factory(fn () => 'abc', $logger, $profiler, null, null, null);
$cache = $factory->createInMemory();
$cache->set('test', 48);