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

Signed-off-by: Robin Appelman <robin@icewind.nl>

3 files changed, 69 insertions, 33 deletions

diff --git a/lib/private/Memcache/Factory.php b/lib/private/Memcache/Factory.php
index c0f4f787200..931c871d0f1 100644
--- a/lib/private/Memcache/Factory.php
+++ b/lib/private/Memcache/Factory.php
@@ -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');
diff --git a/lib/private/Server.php b/lib/private/Server.php
index 9b2a313a257..09d1f341ce5 100644
--- a/lib/private/Server.php
+++ b/lib/private/Server.php
@@ -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')
 				);
 			}
diff --git a/tests/lib/Memcache/FactoryTest.php b/tests/lib/Memcache/FactoryTest.php
index 5a1509eb3d9..b973b5065ea 100644
--- a/tests/lib/Memcache/FactoryTest.php
+++ b/tests/lib/Memcache/FactoryTest.php
@@ -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);