Skip to content

Commit ce489a2

Browse files
miaulalalamiaulalala
committed
fix(installer): re-enable disabled-incompatible apps after appstore update
When a Nextcloud server is upgraded to a new major version, apps incompatible with the old version range are automatically disabled. Previously, updating such an app via the web UI (or occ app:update) would download and upgrade the app files but leave the app disabled, requiring a manual re-enable or reinstall. updateAppstoreApp() now checks whether the app was disabled due to version incompatibility before downloading the update. After a successful upgradeApp(), if those conditions were true, enableApp() is called automatically. Also adds debug logging to previously-silent return paths in isUpdateAvailable() (git-installed apps, no newer version found, app not in store), making update failures diagnosable from debug logs. Signed-off-by: Anna Larch <anna@nextcloud.com>
1 parent 63680bd commit ce489a2

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

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

@@ -373,6 +409,7 @@ public function isUpdateAvailable($appId, $allowUnstable = false): string|false
373409
}
374410

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

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

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

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

‎tests/lib/InstallerTest.php‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -601,6 +601,107 @@ public function testDownloadAppSuccessful(): void {
601601
}
602602

603603

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

0 commit comments

Comments
 (0)