From 135d560a2eb2e983e53fbaf027d71142b1d50585 Mon Sep 17 00:00:00 2001 From: Oleksander Piskun Date: Mon, 3 Aug 2026 09:57:27 +0000 Subject: [PATCH] fix(deprecation): stop using OC_App and non-public AppManager methods Signed-off-by: Oleksander Piskun --- lib/Controller/ExAppsPageController.php | 17 ++++- .../Controller/ExAppsPageControllerTest.php | 63 ++++++++++++++++++- tests/psalm-baseline.xml | 1 - 3 files changed, 75 insertions(+), 6 deletions(-) diff --git a/lib/Controller/ExAppsPageController.php b/lib/Controller/ExAppsPageController.php index dd6f6c1ed..2c1133bb9 100644 --- a/lib/Controller/ExAppsPageController.php +++ b/lib/Controller/ExAppsPageController.php @@ -15,7 +15,6 @@ use OC\App\AppStore\Version\VersionParser; use OC\App\DependencyAnalyzer; use OC\App\Platform; -use OC_App; use OCA\AppAPI\AppInfo\Application; use OCA\AppAPI\DeployActions\DockerActions; use OCA\AppAPI\Fetcher\ExAppFetcher; @@ -518,8 +517,20 @@ public function uninstallApp(string $appId, bool $removeContainer = true, bool $ */ #[PasswordConfirmationRequired] public function force(string $appId): JSONResponse { - $appId = OC_App::cleanAppId($appId); - $this->appManager->overwriteNextcloudRequirement($appId); + $appId = $this->appManager->cleanAppId($appId); + + // Same effect as the non-public OC\App\AppManager::overwriteNextcloudRequirement(), + // except that a corrupt (non-array) value is repaired instead of raising a TypeError. + // The same system value is read back in getAppsForCategory(). + $ignoreMaxApps = $this->config->getSystemValue('app_install_overwrite', []); + if (!is_array($ignoreMaxApps)) { + $this->logger->warning('The value given for app_install_overwrite is not an array. Ignoring...'); + $ignoreMaxApps = []; + } + if (!in_array($appId, $ignoreMaxApps, true)) { + $ignoreMaxApps[] = $appId; + $this->config->setSystemValue('app_install_overwrite', $ignoreMaxApps); + } return new JSONResponse(); } diff --git a/tests/php/Controller/ExAppsPageControllerTest.php b/tests/php/Controller/ExAppsPageControllerTest.php index 17cb6a575..0a8c366b0 100644 --- a/tests/php/Controller/ExAppsPageControllerTest.php +++ b/tests/php/Controller/ExAppsPageControllerTest.php @@ -35,6 +35,7 @@ class ExAppsPageControllerTest extends TestCase { private ExAppService&MockObject $exAppService; private DaemonConfigService&MockObject $daemonConfigService; private IConfig&MockObject $config; + private IAppManager&MockObject $appManager; protected function setUp(): void { parent::setUp(); @@ -49,7 +50,7 @@ protected function setUp(): void { $this->exAppFetcher = $this->createMock(ExAppFetcher::class); $l10n = $this->createMock(IL10N::class); $logger = $this->createMock(LoggerInterface::class); - $appManager = $this->createMock(IAppManager::class); + $this->appManager = $this->createMock(IAppManager::class); $this->exAppService = $this->createMock(ExAppService::class); $exAppDeployOptionsService = $this->createMock(ExAppDeployOptionsService::class); @@ -64,7 +65,7 @@ protected function setUp(): void { $this->exAppFetcher, $l10n, $logger, - $appManager, + $this->appManager, $this->exAppService, $exAppDeployOptionsService, ); @@ -169,4 +170,62 @@ public function testListAppsPicksTranslationForInjectedLanguage(): void { self::assertSame('Fake App (de)', $data['apps'][0]['name']); self::assertSame('Eine Test-App', $data['apps'][0]['description']); } + + /** + * force() marks an ExApp as compatible by adding its id to the + * `app_install_overwrite` system value. This mirrors the non-public + * OC\App\AppManager::overwriteNextcloudRequirement(); the same key is read + * back in getAppsForCategory(), so both sides must stay in sync. + * + * The id must be the one returned by cleanAppId(), not the raw input. + */ + public function testForceAppendsCleanedAppIdToOverwriteList(): void { + $this->appManager->expects(self::once()) + ->method('cleanAppId') + ->with('My_ExApp!') + ->willReturn('my_exapp'); + + $this->config->method('getSystemValue') + ->with('app_install_overwrite', self::anything()) + ->willReturn(['other_app']); + + $this->config->expects(self::once()) + ->method('setSystemValue') + ->with('app_install_overwrite', ['other_app', 'my_exapp']); + + self::assertInstanceOf(JSONResponse::class, $this->controller->force('My_ExApp!')); + } + + /** + * The duplicate check must compare the cleaned id against the stored list, + * so this passes a raw id that differs from the cleaned one. + */ + public function testForceDoesNotRewriteWhenAlreadyMarked(): void { + $this->appManager->expects(self::once()) + ->method('cleanAppId') + ->with('My_ExApp!') + ->willReturn('my_exapp'); + + $this->config->method('getSystemValue') + ->with('app_install_overwrite', self::anything()) + ->willReturn(['my_exapp']); + + $this->config->expects(self::never())->method('setSystemValue'); + + self::assertInstanceOf(JSONResponse::class, $this->controller->force('My_ExApp!')); + } + + public function testForceRecoversFromNonArrayOverwriteValue(): void { + $this->appManager->method('cleanAppId')->willReturn('my_exapp'); + + $this->config->method('getSystemValue') + ->with('app_install_overwrite', self::anything()) + ->willReturn('not-an-array'); + + $this->config->expects(self::once()) + ->method('setSystemValue') + ->with('app_install_overwrite', ['my_exapp']); + + self::assertInstanceOf(JSONResponse::class, $this->controller->force('my_exapp')); + } } diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index 1eb3124bb..e3cf77a10 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -39,7 +39,6 @@ categoryFetcher]]> -