Merge pull request #46297 from nextcloud/backport/46174/stable28

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

4 files changed, 156 insertions, 8 deletions

diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php
index 80083518385..4b877831532 100644
--- a/apps/settings/lib/Controller/CheckSetupController.php
+++ b/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
 =====================
diff --git a/apps/settings/lib/SetupChecks/CodeIntegrity.php b/apps/settings/lib/SetupChecks/CodeIntegrity.php
index 20ecf5360b7..13b84d21963 100644
--- a/apps/settings/lib/SetupChecks/CodeIntegrity.php
+++ b/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(
diff --git a/apps/settings/tests/SetupChecks/CodeIntegrityTest.php b/apps/settings/tests/SetupChecks/CodeIntegrityTest.php
new file mode 100644
index 00000000000..c0df3fb3308
--- /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 a5dec637bdb..d13a8e01c5b 100644
--- a/lib/private/IntegrityCheck/Checker.php
+++ b/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;