Merge pull request #43907 from nextcloud/enh/noid/switching-to-lazy-config-2

feat(appconfig): storing integrity check result as a lazy config value

lib/composer/composer/autoload_classmap.php

@@ -1640,6 +1640,7 @@ return array(
     'OC\\Remote\\User' => $baseDir . '/lib/private/Remote/User.php',
     'OC\\Repair' => $baseDir . '/lib/private/Repair.php',
     'OC\\RepairException' => $baseDir . '/lib/private/RepairException.php',
+    'OC\\Repair\\AddAppConfigLazyMigration' => $baseDir . '/lib/private/Repair/AddAppConfigLazyMigration.php',
     'OC\\Repair\\AddBruteForceCleanupJob' => $baseDir . '/lib/private/Repair/AddBruteForceCleanupJob.php',
     'OC\\Repair\\AddCleanupUpdaterBackupsJob' => $baseDir . '/lib/private/Repair/AddCleanupUpdaterBackupsJob.php',
     'OC\\Repair\\AddMetadataGenerationJob' => $baseDir . '/lib/private/Repair/AddMetadataGenerationJob.php',

lib/composer/composer/autoload_static.php

@@ -1673,6 +1673,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OC\\Remote\\User' => __DIR__ . '/../../..' . '/lib/private/Remote/User.php',
         'OC\\Repair' => __DIR__ . '/../../..' . '/lib/private/Repair.php',
         'OC\\RepairException' => __DIR__ . '/../../..' . '/lib/private/RepairException.php',
+        'OC\\Repair\\AddAppConfigLazyMigration' => __DIR__ . '/../../..' . '/lib/private/Repair/AddAppConfigLazyMigration.php',
         'OC\\Repair\\AddBruteForceCleanupJob' => __DIR__ . '/../../..' . '/lib/private/Repair/AddBruteForceCleanupJob.php',
         'OC\\Repair\\AddCleanupUpdaterBackupsJob' => __DIR__ . '/../../..' . '/lib/private/Repair/AddCleanupUpdaterBackupsJob.php',
         'OC\\Repair\\AddMetadataGenerationJob' => __DIR__ . '/../../..' . '/lib/private/Repair/AddMetadataGenerationJob.php',

lib/private/IntegrityCheck/Checker.php

@@ -40,6 +40,7 @@ use OC\IntegrityCheck\Iterator\ExcludeFileByNameFilterIterator;
 use OC\IntegrityCheck\Iterator\ExcludeFoldersByPathFilterIterator;
 use OCP\App\IAppManager;
 use OCP\Files\IMimeTypeDetector;
+use OCP\IAppConfig;
 use OCP\ICache;
 use OCP\ICacheFactory;
 use OCP\IConfig;
@@ -58,44 +59,20 @@ use phpseclib\File\X509;
  */
 class Checker {
 	public const CACHE_KEY = 'oc.integritycheck.checker';
-	/** @var EnvironmentHelper */
-	private $environmentHelper;
-	/** @var AppLocator */
-	private $appLocator;
-	/** @var FileAccessHelper */
-	private $fileAccessHelper;
-	/** @var IConfig|null */
-	private $config;
-	/** @var ICache */
-	private $cache;
-	/** @var IAppManager|null */
-	private $appManager;
-	/** @var IMimeTypeDetector */
-	private $mimeTypeDetector;
 
-	/**
-	 * @param EnvironmentHelper $environmentHelper
-	 * @param FileAccessHelper $fileAccessHelper
-	 * @param AppLocator $appLocator
-	 * @param IConfig|null $config
-	 * @param ICacheFactory $cacheFactory
-	 * @param IAppManager|null $appManager
-	 * @param IMimeTypeDetector $mimeTypeDetector
-	 */
-	public function __construct(EnvironmentHelper $environmentHelper,
-		FileAccessHelper $fileAccessHelper,
-		AppLocator $appLocator,
-		?IConfig $config,
+	private ICache $cache;
+
+	public function __construct(
+		private EnvironmentHelper $environmentHelper,
+		private FileAccessHelper $fileAccessHelper,
+		private AppLocator $appLocator,
+		private ?IConfig $config,
+		private ?IAppConfig $appConfig,
 		ICacheFactory $cacheFactory,
-		?IAppManager $appManager,
-		IMimeTypeDetector $mimeTypeDetector) {
-		$this->environmentHelper = $environmentHelper;
-		$this->fileAccessHelper = $fileAccessHelper;
-		$this->appLocator = $appLocator;
-		$this->config = $config;
+		private ?IAppManager $appManager,
+		private IMimeTypeDetector $mimeTypeDetector,
+	) {
 		$this->cache = $cacheFactory->createDistributed(self::CACHE_KEY);
-		$this->appManager = $appManager;
-		$this->mimeTypeDetector = $mimeTypeDetector;
 	}
 
 	/**
@@ -114,15 +91,7 @@ class Checker {
 		 * applicable for very specific scenarios and we should not advertise it
 		 * too prominent. So please do not add it to config.sample.php.
 		 */
-		$isIntegrityCheckDisabled = false;
-		if ($this->config !== null) {
-			$isIntegrityCheckDisabled = $this->config->getSystemValueBool('integrity.check.disabled', false);
-		}
-		if ($isIntegrityCheckDisabled) {
-			return false;
-		}
-
-		return true;
+		return !($this->config?->getSystemValueBool('integrity.check.disabled', false) ?? false);
 	}
 
 	/**
@@ -443,10 +412,7 @@ class Checker {
 			return json_decode($cachedResults, true);
 		}
 
-		if ($this->config !== null) {
-			return json_decode($this->config->getAppValue('core', self::CACHE_KEY, '{}'), true);
-		}
-		return [];
+		return $this->appConfig?->getValueArray('core', self::CACHE_KEY, lazy: true) ?? [];
 	}
 
 	/**
@@ -461,9 +427,7 @@ class Checker {
 		if (!empty($result)) {
 			$resultArray[$scope] = $result;
 		}
-		if ($this->config !== null) {
-			$this->config->setAppValue('core', self::CACHE_KEY, json_encode($resultArray));
-		}
+		$this->appConfig?->setValueArray('core', self::CACHE_KEY, $resultArray, lazy: true);
 		$this->cache->set(self::CACHE_KEY, json_encode($resultArray));
 	}
 
@@ -472,7 +436,7 @@ class Checker {
 	 * Clean previous results for a proper rescanning. Otherwise
 	 */
 	private function cleanResults() {
-		$this->config->deleteAppValue('core', self::CACHE_KEY);
+		$this->appConfig->deleteKey('core', self::CACHE_KEY);
 		$this->cache->remove(self::CACHE_KEY);
 	}
 

lib/private/Repair.php

@@ -36,6 +36,7 @@ namespace OC;
 
 use OC\DB\Connection;
 use OC\DB\ConnectionAdapter;
+use OC\Repair\AddAppConfigLazyMigration;
 use OC\Repair\AddBruteForceCleanupJob;
 use OC\Repair\AddCleanupUpdaterBackupsJob;
 use OC\Repair\AddMetadataGenerationJob;
@@ -209,6 +210,7 @@ class Repair implements IOutput {
 			\OCP\Server::get(AddMissingSecretJob::class),
 			\OCP\Server::get(AddRemoveOldTasksBackgroundJob::class),
 			\OCP\Server::get(AddMetadataGenerationJob::class),
+			\OCP\Server::get(AddAppConfigLazyMigration::class),
 		];
 	}
 

lib/private/Repair/AddAppConfigLazyMigration.php

@@ -0,0 +1,62 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2024 Maxence Lange <maxence@artificial-owl.com>
+ *
+ * @author Maxence Lange <maxence@artificial-owl.com>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+namespace OC\Repair;
+
+use OCP\IAppConfig;
+use OCP\Migration\IOutput;
+use OCP\Migration\IRepairStep;
+use Psr\Log\LoggerInterface;
+
+class AddAppConfigLazyMigration implements IRepairStep {
+	/**
+	 * Just add config values that needs to be migrated to lazy loading
+	 */
+	private static array $lazyAppConfig = [
+		'core' => [
+			'oc.integritycheck.checker',
+		],
+	];
+
+	public function __construct(
+		private IAppConfig $appConfig,
+		private LoggerInterface $logger,
+	) {
+	}
+
+	public function getName() {
+		return 'migrate lazy config values';
+	}
+
+	public function run(IOutput $output) {
+		$c = 0;
+		foreach (self::$lazyAppConfig as $appId => $configKeys) {
+			foreach ($configKeys as $configKey) {
+				$c += (int)$this->appConfig->updateLazy($appId, $configKey, true);
+			}
+		}
+
+		$this->logger->notice('core/BackgroundJobs/AppConfigLazyMigration: ' . $c . ' config values updated');
+	}
+}

lib/private/Server.php

@@ -989,10 +989,10 @@ class Server extends ServerContainer implements IServerContainer {
 			// might however be called when ownCloud is not yet setup.
 			if (\OC::$server->get(SystemConfig::class)->getValue('installed', false)) {
 				$config = $c->get(\OCP\IConfig::class);
+				$appConfig = $c->get(\OCP\IAppConfig::class);
 				$appManager = $c->get(IAppManager::class);
 			} else {
-				$config = null;
-				$appManager = null;
+				$config = $appConfig = $appManager = null;
 			}
 
 			return new Checker(
@@ -1000,6 +1000,7 @@ class Server extends ServerContainer implements IServerContainer {
 				new FileAccessHelper(),
 				new AppLocator(),
 				$config,
+				$appConfig,
 				$c->get(ICacheFactory::class),
 				$appManager,
 				$c->get(IMimeTypeDetector::class)

tests/lib/IntegrityCheck/CheckerTest.php

@@ -28,6 +28,7 @@ use OC\IntegrityCheck\Helpers\EnvironmentHelper;
 use OC\IntegrityCheck\Helpers\FileAccessHelper;
 use OC\Memcache\NullCache;
 use OCP\App\IAppManager;
+use OCP\IAppConfig;
 use OCP\ICacheFactory;
 use OCP\IConfig;
 use phpseclib\Crypt\RSA;
@@ -45,6 +46,8 @@ class CheckerTest extends TestCase {
 	private $fileAccessHelper;
 	/** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */
 	private $config;
+	/** @var IAppConfig|\PHPUnit\Framework\MockObject\MockObject */
+	private $appConfig;
 	/** @var ICacheFactory|\PHPUnit\Framework\MockObject\MockObject */
 	private $cacheFactory;
 	/** @var IAppManager|\PHPUnit\Framework\MockObject\MockObject */
@@ -58,6 +61,7 @@ class CheckerTest extends TestCase {
 		$this->fileAccessHelper = $this->createMock(FileAccessHelper::class);
 		$this->appLocator = $this->createMock(AppLocator::class);
 		$this->config = $this->createMock(IConfig::class);
+		$this->appConfig = $this->createMock(IAppConfig::class);
 		$this->cacheFactory = $this->createMock(ICacheFactory::class);
 		$this->appManager = $this->createMock(IAppManager::class);
 		$this->mimeTypeDetector = $this->createMock(\OC\Files\Type\Detection::class);
@@ -76,6 +80,7 @@ class CheckerTest extends TestCase {
 			$this->fileAccessHelper,
 			$this->appLocator,
 			$this->config,
+			$this->appConfig,
 			$this->cacheFactory,
 			$this->appManager,
 			$this->mimeTypeDetector
@@ -1025,6 +1030,7 @@ class CheckerTest extends TestCase {
 				$this->fileAccessHelper,
 				$this->appLocator,
 				$this->config,
+				$this->appConfig,
 				$this->cacheFactory,
 				$this->appManager,
 				$this->mimeTypeDetector,
@@ -1089,9 +1095,9 @@ class CheckerTest extends TestCase {
 				true,
 				false,
 			);
-		$this->config
+		$this->appConfig
 			->expects($this->once())
-			->method('deleteAppValue')
+			->method('deleteKey')
 			->with('core', 'oc.integritycheck.checker');
 
 		$this->checker->runInstanceVerification();