Skip to content

Commit 7e87896

Browse files
AndyScherzingerAndyScherzinger
authored
Merge pull request #63761 from nextcloud/backport/60591/stable35
[stable35] fix(installer): re-enable version-incompatible apps after appstore update
2 parents 20db5ef + 60800f8 commit 7e87896

2 files changed

Lines changed: 147 additions & 1 deletion

File tree

‎lib/private/Installer.php‎

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,22 @@ public function installApp(string $appId, bool $forceEnable = false): string {
102102
*/
103103
public function updateAppstoreApp(string $appId, bool $allowUnstable = false): bool {
104104
if ($this->isUpdateAvailable($appId, $allowUnstable) !== false) {
105+
// Before downloading, check whether the app is currently disabled due to version
106+
// incompatibility with this NC version. If so, re-enable it after a successful update.
107+
$isDisabled = !$this->appManager->isEnabledForAnyone($appId);
108+
$wasIncompatible = false;
109+
if ($isDisabled) {
110+
$currentInfo = $this->appManager->getAppInfo($appId);
111+
$ncVersion = implode('.', Util::getVersion());
112+
$wasIncompatible = $currentInfo !== null && !$this->appManager->isAppCompatible($ncVersion, $currentInfo);
113+
$this->logger->debug('App {appId} is disabled; incompatible with NC {version}: {incompat}', [
114+
'appId' => $appId,
115+
'version' => $ncVersion,
116+
'incompat' => $wasIncompatible ? 'yes' : 'no',
117+
'app' => 'updater',
118+
]);
119+
}
120+
105121
try {
106122
$this->downloadApp($appId, $allowUnstable);
107123
} catch (\Exception $e) {
@@ -110,9 +126,29 @@ public function updateAppstoreApp(string $appId, bool $allowUnstable = false): b
110126
]);
111127
return false;
112128
}
113-
return $this->appManager->upgradeApp($appId);
129+
130+
$result = $this->appManager->upgradeApp($appId);
131+
132+
if ($result && $isDisabled && $wasIncompatible) {
133+
$this->logger->info('Re-enabling {appId} after update: it was disabled due to version incompatibility', [
134+
'appId' => $appId,
135+
'app' => 'updater',
136+
]);
137+
try {
138+
$this->appManager->enableApp($appId);
139+
} catch (\Exception $e) {
140+
$this->logger->warning('Could not re-enable {appId} after update: {error}', [
141+
'appId' => $appId,
142+
'error' => $e->getMessage(),
143+
'app' => 'updater',
144+
]);
145+
}
146+
}
147+
148+
return $result;
114149
}
115150

151+
$this->logger->debug('No update available for {appId}, skipping', ['appId' => $appId, 'app' => 'updater']);
116152
return false;
117153
}
118154

@@ -374,6 +410,7 @@ public function isUpdateAvailable($appId, $allowUnstable = false): string|false
374410
}
375411

376412
if ($this->isInstalledFromGit($appId) === true) {
413+
$this->logger->debug('App {appId} is installed from git, skipping update check', ['appId' => $appId, 'app' => 'updater']);
377414
return false;
378415
}
379416

@@ -386,17 +423,25 @@ public function isUpdateAvailable($appId, $allowUnstable = false): string|false
386423
$currentVersion = $this->appManager->getAppVersion($appId, true);
387424

388425
if (!isset($app['releases'][0]['version'])) {
426+
$this->logger->debug('App {appId} has no release version in app store data', ['appId' => $appId, 'app' => 'updater']);
389427
return false;
390428
}
391429
$newestVersion = $app['releases'][0]['version'];
392430
if ($currentVersion !== '0' && version_compare($newestVersion, $currentVersion, '>')) {
393431
return $newestVersion;
394432
} else {
433+
$this->logger->debug('No newer version available for {appId}: current={current}, newest={newest}', [
434+
'appId' => $appId,
435+
'current' => $currentVersion,
436+
'newest' => $newestVersion,
437+
'app' => 'updater',
438+
]);
395439
return false;
396440
}
397441
}
398442
}
399443

444+
$this->logger->debug('App {appId} not found in app store', ['appId' => $appId, 'app' => 'updater']);
400445
return false;
401446
}
402447

‎tests/lib/InstallerTest.php‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -588,6 +588,107 @@ public function testDownloadAppSuccessful(): void {
588588
$this->assertEquals('0.9', \OC_App::getAppVersionByPath(__DIR__ . '/../../apps/testapp/'));
589589
}
590590

591+
public function testIsUpdateAvailableLogsDebugForGitInstall(): void {
592+
$tmpDir = sys_get_temp_dir() . '/nc_test_git_' . uniqid();
593+
mkdir($tmpDir . '/.git', 0700, true);
594+
595+
$this->appManager
596+
->expects($this->once())
597+
->method('getAppPath')
598+
->with('myapp')
599+
->willReturn($tmpDir);
600+
$this->logger
601+
->expects($this->once())
602+
->method('debug')
603+
->with(
604+
'App {appId} is installed from git, skipping update check',
605+
$this->callback(fn ($ctx) => $ctx['appId'] === 'myapp')
606+
);
607+
608+
$installer = $this->getInstaller();
609+
$result = $installer->isUpdateAvailable('myapp');
610+
$this->assertFalse($result);
611+
612+
rmdir($tmpDir . '/.git');
613+
rmdir($tmpDir);
614+
}
615+
616+
protected function getPartialInstaller(array $onlyMethods): Installer&\PHPUnit\Framework\MockObject\MockObject {
617+
return $this->getMockBuilder(Installer::class)
618+
->setConstructorArgs([
619+
$this->appFetcher,
620+
$this->clientService,
621+
$this->tempManager,
622+
$this->logger,
623+
$this->config,
624+
$this->appManager,
625+
$this->l10nFactory,
626+
false,
627+
])
628+
->onlyMethods($onlyMethods)
629+
->getMock();
630+
}
631+
632+
public function testUpdateAppstoreAppReEnablesDisabledIncompatibleApp(): void {
633+
$installer = $this->getPartialInstaller(['isUpdateAvailable', 'downloadApp']);
634+
$installer->method('isUpdateAvailable')->willReturn('1.0.0');
635+
$installer->method('downloadApp');
636+
637+
$this->appManager->method('isEnabledForAnyone')->with('myapp')->willReturn(false);
638+
$this->appManager->method('getAppInfo')->with('myapp')->willReturn(['id' => 'myapp', 'version' => '0.0.1']);
639+
$this->appManager->method('isAppCompatible')->willReturn(false);
640+
$this->appManager->method('upgradeApp')->with('myapp')->willReturn(true);
641+
$this->appManager->expects($this->once())->method('enableApp')->with('myapp');
642+
643+
$result = $installer->updateAppstoreApp('myapp');
644+
$this->assertTrue($result);
645+
}
646+
647+
public function testUpdateAppstoreAppDoesNotReEnableCompatibleButDisabledApp(): void {
648+
$installer = $this->getPartialInstaller(['isUpdateAvailable', 'downloadApp']);
649+
$installer->method('isUpdateAvailable')->willReturn('1.0.0');
650+
$installer->method('downloadApp');
651+
652+
$this->appManager->method('isEnabledForAnyone')->with('myapp')->willReturn(false);
653+
$this->appManager->method('getAppInfo')->with('myapp')->willReturn(['id' => 'myapp', 'version' => '1.0.0']);
654+
$this->appManager->method('isAppCompatible')->willReturn(true);
655+
$this->appManager->method('upgradeApp')->with('myapp')->willReturn(true);
656+
$this->appManager->expects($this->never())->method('enableApp');
657+
658+
$result = $installer->updateAppstoreApp('myapp');
659+
$this->assertTrue($result);
660+
}
661+
662+
public function testUpdateAppstoreAppDoesNotReEnableAlreadyEnabledApp(): void {
663+
$installer = $this->getPartialInstaller(['isUpdateAvailable', 'downloadApp']);
664+
$installer->method('isUpdateAvailable')->willReturn('1.0.0');
665+
$installer->method('downloadApp');
666+
667+
$this->appManager->method('isEnabledForAnyone')->with('myapp')->willReturn(true);
668+
$this->appManager->method('upgradeApp')->with('myapp')->willReturn(true);
669+
$this->appManager->expects($this->never())->method('enableApp');
670+
$this->appManager->expects($this->never())->method('getAppInfo');
671+
$this->appManager->expects($this->never())->method('isAppCompatible');
672+
673+
$result = $installer->updateAppstoreApp('myapp');
674+
$this->assertTrue($result);
675+
}
676+
677+
public function testUpdateAppstoreAppDoesNotReEnableWhenUpgradeFails(): void {
678+
$installer = $this->getPartialInstaller(['isUpdateAvailable', 'downloadApp']);
679+
$installer->method('isUpdateAvailable')->willReturn('1.0.0');
680+
$installer->method('downloadApp');
681+
682+
$this->appManager->method('isEnabledForAnyone')->with('myapp')->willReturn(false);
683+
$this->appManager->method('getAppInfo')->with('myapp')->willReturn(['id' => 'myapp', 'version' => '0.0.1']);
684+
$this->appManager->method('isAppCompatible')->willReturn(false);
685+
$this->appManager->method('upgradeApp')->with('myapp')->willReturn(false);
686+
$this->appManager->expects($this->never())->method('enableApp');
687+
688+
$result = $installer->updateAppstoreApp('myapp');
689+
$this->assertFalse($result);
690+
}
691+
591692
public function testDownloadAppWithDowngrade(): void {
592693
// Use previous test to download the application in version 0.9
593694
$this->testDownloadAppSuccessful();

0 commit comments

Comments
 (0)