feat(appconfig): switching integrity check to lazy config values

Signed-off-by: Maxence Lange <maxence@artificial-owl.com>

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/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();