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>

apps/settings/lib/Controller/CheckSetupController.php

@@ -123,6 +123,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
 =====================

apps/settings/lib/SetupChecks/CodeIntegrity.php

@@ -50,7 +50,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(

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());
+	}
+}

lib/private/IntegrityCheck/Checker.php

@@ -427,7 +427,7 @@ class Checker {
 	 */
 	public function hasPassedCheck(): bool {
 		$results = $this->getResults();
-		if (empty($results)) {
+		if ($results !== null && empty($results)) {
 			return true;
 		}
 
@@ -435,18 +435,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);
 		}
 
-		if ($this->config !== null) {
-			return json_decode($this->config->getAppValue('core', self::CACHE_KEY, '{}'), true);
+		$appValue = $this->config?->getAppValue('core', self::CACHE_KEY);
+		if (!empty($appValue)) {
+			return json_decode($appValue, true);
 		}
-		return [];
+		// No results
+		return null;
 	}
 
 	/**
@@ -456,7 +458,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;