Skip to content

Commit 183d25c

Browse files
committed
fix(deprecation): stop using OC_App and non-public AppManager methods
Signed-off-by: Oleksander Piskun <oleksandr2088@icloud.com>
1 parent aa7c470 commit 183d25c

3 files changed

Lines changed: 74 additions & 6 deletions

File tree

lib/Controller/ExAppsPageController.php

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
use OC\App\AppStore\Version\VersionParser;
1616
use OC\App\DependencyAnalyzer;
1717
use OC\App\Platform;
18-
use OC_App;
1918
use OCA\AppAPI\AppInfo\Application;
2019
use OCA\AppAPI\DeployActions\DockerActions;
2120
use OCA\AppAPI\Fetcher\ExAppFetcher;
@@ -518,8 +517,19 @@ public function uninstallApp(string $appId, bool $removeContainer = true, bool $
518517
*/
519518
#[PasswordConfirmationRequired]
520519
public function force(string $appId): JSONResponse {
521-
$appId = OC_App::cleanAppId($appId);
522-
$this->appManager->overwriteNextcloudRequirement($appId);
520+
$appId = $this->appManager->cleanAppId($appId);
521+
522+
// Mirrors the non-public OC\App\AppManager::overwriteNextcloudRequirement().
523+
// The same system value is read back in getAppsForCategory().
524+
$ignoreMaxApps = $this->config->getSystemValue('app_install_overwrite', []);
525+
if (!is_array($ignoreMaxApps)) {
526+
$this->logger->warning('The value given for app_install_overwrite is not an array. Ignoring...');
527+
$ignoreMaxApps = [];
528+
}
529+
if (!in_array($appId, $ignoreMaxApps, true)) {
530+
$ignoreMaxApps[] = $appId;
531+
$this->config->setSystemValue('app_install_overwrite', $ignoreMaxApps);
532+
}
523533
return new JSONResponse();
524534
}
525535

tests/php/Controller/ExAppsPageControllerTest.php

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ class ExAppsPageControllerTest extends TestCase {
3535
private ExAppService&MockObject $exAppService;
3636
private DaemonConfigService&MockObject $daemonConfigService;
3737
private IConfig&MockObject $config;
38+
private IAppManager&MockObject $appManager;
3839

3940
protected function setUp(): void {
4041
parent::setUp();
@@ -49,7 +50,7 @@ protected function setUp(): void {
4950
$this->exAppFetcher = $this->createMock(ExAppFetcher::class);
5051
$l10n = $this->createMock(IL10N::class);
5152
$logger = $this->createMock(LoggerInterface::class);
52-
$appManager = $this->createMock(IAppManager::class);
53+
$this->appManager = $this->createMock(IAppManager::class);
5354
$this->exAppService = $this->createMock(ExAppService::class);
5455
$exAppDeployOptionsService = $this->createMock(ExAppDeployOptionsService::class);
5556

@@ -64,7 +65,7 @@ protected function setUp(): void {
6465
$this->exAppFetcher,
6566
$l10n,
6667
$logger,
67-
$appManager,
68+
$this->appManager,
6869
$this->exAppService,
6970
$exAppDeployOptionsService,
7071
);
@@ -169,4 +170,62 @@ public function testListAppsPicksTranslationForInjectedLanguage(): void {
169170
self::assertSame('Fake App (de)', $data['apps'][0]['name']);
170171
self::assertSame('Eine Test-App', $data['apps'][0]['description']);
171172
}
173+
174+
/**
175+
* force() marks an ExApp as compatible by adding its id to the
176+
* `app_install_overwrite` system value. This mirrors the non-public
177+
* OC\App\AppManager::overwriteNextcloudRequirement(); the same key is read
178+
* back in getAppsForCategory(), so both sides must stay in sync.
179+
*
180+
* The id must be the one returned by cleanAppId(), not the raw input.
181+
*/
182+
public function testForceAppendsCleanedAppIdToOverwriteList(): void {
183+
$this->appManager->expects(self::once())
184+
->method('cleanAppId')
185+
->with('My_ExApp!')
186+
->willReturn('my_exapp');
187+
188+
$this->config->method('getSystemValue')
189+
->with('app_install_overwrite', self::anything())
190+
->willReturn(['other_app']);
191+
192+
$this->config->expects(self::once())
193+
->method('setSystemValue')
194+
->with('app_install_overwrite', ['other_app', 'my_exapp']);
195+
196+
self::assertInstanceOf(JSONResponse::class, $this->controller->force('My_ExApp!'));
197+
}
198+
199+
/**
200+
* The duplicate check must compare the cleaned id against the stored list,
201+
* so this passes a raw id that differs from the cleaned one.
202+
*/
203+
public function testForceDoesNotRewriteWhenAlreadyMarked(): void {
204+
$this->appManager->expects(self::once())
205+
->method('cleanAppId')
206+
->with('My_ExApp!')
207+
->willReturn('my_exapp');
208+
209+
$this->config->method('getSystemValue')
210+
->with('app_install_overwrite', self::anything())
211+
->willReturn(['my_exapp']);
212+
213+
$this->config->expects(self::never())->method('setSystemValue');
214+
215+
self::assertInstanceOf(JSONResponse::class, $this->controller->force('My_ExApp!'));
216+
}
217+
218+
public function testForceRecoversFromNonArrayOverwriteValue(): void {
219+
$this->appManager->method('cleanAppId')->willReturn('my_exapp');
220+
221+
$this->config->method('getSystemValue')
222+
->with('app_install_overwrite', self::anything())
223+
->willReturn('not-an-array');
224+
225+
$this->config->expects(self::once())
226+
->method('setSystemValue')
227+
->with('app_install_overwrite', ['my_exapp']);
228+
229+
self::assertInstanceOf(JSONResponse::class, $this->controller->force('my_exapp'));
230+
}
172231
}

tests/psalm-baseline.xml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
<UndefinedClass>
4040
<code><![CDATA[$this->categoryFetcher]]></code>
4141
<code><![CDATA[DependencyAnalyzer]]></code>
42-
<code><![CDATA[OC_App]]></code>
4342
<code><![CDATA[Platform]]></code>
4443
<code><![CDATA[VersionParser]]></code>
4544
<code><![CDATA[private]]></code>

0 commit comments

Comments
 (0)