Skip to content

Commit 29fbe63

Browse files
miaulalalaAndyScherzinger
miaulalala
authored and
AndyScherzinger
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 79b7365 commit 29fbe63

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
@@ -593,6 +593,107 @@ public function testDownloadAppSuccessful(): void {
593593
$this->assertEquals('0.9', \OC_App::getAppVersionByPath(__DIR__ . '/../../apps/testapp/'));
594594
}
595595

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

0 commit comments

Comments
 (0)