fix(IntegrityCheck): Ensure the check is run if no results are available

If there are no cached results the current implementation was also returning an empty array,
but this was the same as when there was a successful run.
So to distinguish this we return `null` if there are *no* results.
In this case we need to rerun the integrity checker.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

4 files changed, 157 insertions, 6 deletions

diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php
index 5cf48d38ffb..d4e05ec90e4 100644
--- a/apps/settings/lib/Controller/CheckSetupController.php
+++ b/apps/settings/lib/Controller/CheckSetupController.php
@@ -85,6 +85,10 @@ class CheckSetupController extends Controller {
 
 		$completeResults = $this->checker->getResults();
 
+		if ($completeResults === null) {
+			return new DataDisplayResponse('Integrity checker has not been run. Integrity information not available.');
+		}
+
 		if (!empty($completeResults)) {
 			$formattedTextResponse = 'Technical information
 =====================
diff --git a/apps/settings/lib/SetupChecks/CodeIntegrity.php b/apps/settings/lib/SetupChecks/CodeIntegrity.php
index dc29c4da306..2b4271fae9c 100644
--- a/apps/settings/lib/SetupChecks/CodeIntegrity.php
+++ b/apps/settings/lib/SetupChecks/CodeIntegrity.php
@@ -33,7 +33,14 @@ class CodeIntegrity implements ISetupCheck {
 	public function run(): SetupResult {
 		if (!$this->checker->isCodeCheckEnforced()) {
 			return SetupResult::info($this->l10n->t('Integrity checker has been disabled. Integrity cannot be verified.'));
-		} elseif ($this->checker->hasPassedCheck()) {
+		}
+
+		// If there are no results we need to run the verification
+		if ($this->checker->getResults() === null) {
+			$this->checker->runInstanceVerification();
+		}
+
+		if ($this->checker->hasPassedCheck()) {
 			return SetupResult::success($this->l10n->t('No altered files'));
 		} else {
 			return SetupResult::error(
diff --git a/apps/settings/tests/SetupChecks/CodeIntegrityTest.php b/apps/settings/tests/SetupChecks/CodeIntegrityTest.php
new file mode 100644
index 00000000000..52101aed901
--- /dev/null
+++ b/apps/settings/tests/SetupChecks/CodeIntegrityTest.php
@@ -0,0 +1,135 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+namespace OCA\Settings\Tests;
+
+use OC\IntegrityCheck\Checker;
+use OCA\Settings\SetupChecks\CodeIntegrity;
+use OCP\IL10N;
+use OCP\IURLGenerator;
+use OCP\SetupCheck\SetupResult;
+use PHPUnit\Framework\MockObject\MockObject;
+use Test\TestCase;
+
+class CodeIntegrityTest extends TestCase {
+	
+	private IL10N&MockObject $l10n;
+	private IURLGenerator&MockObject $urlGenerator;
+	private Checker&MockObject $checker;
+
+	protected function setUp(): void {
+		parent::setUp();
+
+		$this->l10n = $this->getMockBuilder(IL10N::class)
+			->disableOriginalConstructor()->getMock();
+		$this->l10n->expects($this->any())
+			->method('t')
+			->willReturnCallback(function ($message, array $replace) {
+				return vsprintf($message, $replace);
+			});
+		$this->urlGenerator = $this->createMock(IURLGenerator::class);
+		$this->checker = $this->createMock(Checker::class);
+	}
+
+	public function testSkipOnDisabled(): void {
+		$this->checker->expects($this->atLeastOnce())
+			->method('isCodeCheckEnforced')
+			->willReturn(false);
+
+		$check = new CodeIntegrity(
+			$this->l10n,
+			$this->urlGenerator,
+			$this->checker,
+		);
+		$this->assertEquals(SetupResult::INFO, $check->run()->getSeverity());
+	}
+
+	public function testSuccessOnEmptyResults(): void {
+		$this->checker->expects($this->atLeastOnce())
+			->method('isCodeCheckEnforced')
+			->willReturn(true);
+		$this->checker->expects($this->atLeastOnce())
+			->method('getResults')
+			->willReturn([]);
+		$this->checker->expects(($this->atLeastOnce()))
+			->method('hasPassedCheck')
+			->willReturn(true);
+
+		$check = new CodeIntegrity(
+			$this->l10n,
+			$this->urlGenerator,
+			$this->checker,
+		);
+		$this->assertEquals(SetupResult::SUCCESS, $check->run()->getSeverity());
+	}
+
+	public function testCheckerIsReRunWithoutResults(): void {
+		$this->checker->expects($this->atLeastOnce())
+			->method('isCodeCheckEnforced')
+			->willReturn(true);
+		$this->checker->expects($this->atLeastOnce())
+			->method('getResults')
+			->willReturn(null);
+		$this->checker->expects(($this->atLeastOnce()))
+			->method('hasPassedCheck')
+			->willReturn(true);
+
+		// This is important and must be called
+		$this->checker->expects($this->once())
+			->method('runInstanceVerification');
+
+		$check = new CodeIntegrity(
+			$this->l10n,
+			$this->urlGenerator,
+			$this->checker,
+		);
+		$this->assertEquals(SetupResult::SUCCESS, $check->run()->getSeverity());
+	}
+
+	public function testCheckerIsNotReReInAdvance(): void {
+		$this->checker->expects($this->atLeastOnce())
+			->method('isCodeCheckEnforced')
+			->willReturn(true);
+		$this->checker->expects($this->atLeastOnce())
+			->method('getResults')
+			->willReturn(['mocked']);
+		$this->checker->expects(($this->atLeastOnce()))
+			->method('hasPassedCheck')
+			->willReturn(true);
+
+		// There are results thus this must never be called
+		$this->checker->expects($this->never())
+			->method('runInstanceVerification');
+
+		$check = new CodeIntegrity(
+			$this->l10n,
+			$this->urlGenerator,
+			$this->checker,
+		);
+		$this->assertEquals(SetupResult::SUCCESS, $check->run()->getSeverity());
+	}
+
+	public function testErrorOnMissingIntegrity(): void {
+		$this->checker->expects($this->atLeastOnce())
+			->method('isCodeCheckEnforced')
+			->willReturn(true);
+		$this->checker->expects($this->atLeastOnce())
+			->method('getResults')
+			->willReturn(['mocked']);
+		$this->checker->expects(($this->atLeastOnce()))
+			->method('hasPassedCheck')
+			->willReturn(false);
+
+		$check = new CodeIntegrity(
+			$this->l10n,
+			$this->urlGenerator,
+			$this->checker,
+		);
+		$this->assertEquals(SetupResult::ERROR, $check->run()->getSeverity());
+	}
+}
diff --git a/lib/private/IntegrityCheck/Checker.php b/lib/private/IntegrityCheck/Checker.php
index a6de3cf6030..d38ccf497f4 100644
--- a/lib/private/IntegrityCheck/Checker.php
+++ b/lib/private/IntegrityCheck/Checker.php
@@ -373,7 +373,7 @@ class Checker {
 	 */
 	public function hasPassedCheck(): bool {
 		$results = $this->getResults();
-		if (empty($results)) {
+		if ($results !== null && empty($results)) {
 			return true;
 		}
 
@@ -381,15 +381,20 @@ class Checker {
 	}
 
 	/**
-	 * @return array
+	 * @return array|null Either the results or null if no results available
 	 */
-	public function getResults(): array {
+	public function getResults(): array|null {
 		$cachedResults = $this->cache->get(self::CACHE_KEY);
 		if (!\is_null($cachedResults) and $cachedResults !== false) {
 			return json_decode($cachedResults, true);
 		}
 
-		return $this->appConfig?->getValueArray('core', self::CACHE_KEY, lazy: true) ?? [];
+		if ($this->appConfig?->hasKey('core', self::CACHE_KEY, lazy: true)) {
+			return $this->appConfig->getValueArray('core', self::CACHE_KEY, lazy: true);
+		}
+
+		// No results available
+		return null;
 	}
 
 	/**
@@ -399,7 +404,7 @@ class Checker {
 	 * @param array $result
 	 */
 	private function storeResults(string $scope, array $result) {
-		$resultArray = $this->getResults();
+		$resultArray = $this->getResults() ?? [];
 		unset($resultArray[$scope]);
 		if (!empty($result)) {
 			$resultArray[$scope] = $result;