Merge pull request #53892 from nextcloud/fix/cleanup-getinstallpath

fix: Move getInstallPath to Installer class
This commit is contained in:
Côme Chilliet 2025-07-10 13:10:19 +02:00 committed by GitHub
commit c0b31d1e2d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 44 additions and 37 deletions

View file

@ -169,6 +169,28 @@ class Installer {
return $matches[0];
}
/**
* Get the path where to install apps
*
* @throws \RuntimeException if an app folder is marked as writable but is missing permissions
*/
public function getInstallPath(): ?string {
foreach (\OC::$APPSROOTS as $dir) {
if (isset($dir['writable']) && $dir['writable'] === true) {
// Check if there is a writable install folder.
if (!is_writable($dir['path'])
|| !is_readable($dir['path'])
) {
throw new \RuntimeException(
'Cannot write into "apps" directory. This can usually be fixed by giving the web server write access to the apps directory or disabling the App Store in the config file.'
);
}
return $dir['path'];
}
}
return null;
}
/**
* Downloads an app and puts it into the app directory
*
@ -181,6 +203,11 @@ class Installer {
public function downloadApp(string $appId, bool $allowUnstable = false): void {
$appId = strtolower($appId);
$installPath = $this->getInstallPath();
if ($installPath === null) {
throw new \Exception('No application directories are marked as writable.');
}
$apps = $this->appFetcher->get($allowUnstable);
foreach ($apps as $app) {
if ($app['id'] === $appId) {
@ -333,7 +360,7 @@ class Installer {
);
}
$baseDir = OC_App::getInstallPath() . '/' . $appId;
$baseDir = $installPath . '/' . $appId;
// Remove old app with the ID if existent
Files::rmdirr($baseDir);
// Move to app folder
@ -375,7 +402,7 @@ class Installer {
*/
public function isUpdateAvailable($appId, $allowUnstable = false): string|false {
if ($this->isInstanceReadyForUpdates === null) {
$installPath = OC_App::getInstallPath();
$installPath = $this->getInstallPath();
if ($installPath === null) {
$this->isInstanceReadyForUpdates = false;
} else {
@ -463,7 +490,13 @@ class Installer {
if (\OCP\Server::get(IAppManager::class)->isShipped($appId)) {
return false;
}
$appDir = OC_App::getInstallPath() . '/' . $appId;
$installPath = $this->getInstallPath();
if ($installPath === null) {
$this->logger->error('No application directories are marked as writable.', ['app' => 'core']);
return false;
}
$appDir = $installPath . '/' . $appId;
Files::rmdirr($appDir);
return true;
} else {

View file

@ -237,21 +237,6 @@ class OC_App {
}
}
/**
* Get the path where to install apps
*/
public static function getInstallPath(): ?string {
foreach (OC::$APPSROOTS as $dir) {
if (isset($dir['writable']) && $dir['writable'] === true) {
return $dir['path'];
}
}
Server::get(LoggerInterface::class)->error('No application directories are marked as writable.', ['app' => 'core']);
return null;
}
/**
* Find the apps root for an app id.
*

View file

@ -343,19 +343,6 @@ class OC_Util {
}
}
// Check if there is a writable install folder.
if ($config->getValue('appstoreenabled', true)) {
if (OC_App::getInstallPath() === null
|| !is_writable(OC_App::getInstallPath())
|| !is_readable(OC_App::getInstallPath())
) {
$errors[] = [
'error' => $l->t('Cannot write into "apps" directory.'),
'hint' => $l->t('This can usually be fixed by giving the web server write access to the apps directory'
. ' or disabling the App Store in the config file.')
];
}
}
// Create root dir.
if ($config->getValue('installed', false)) {
if (!is_dir($CONFIG_DATADIRECTORY)) {

View file

@ -92,12 +92,7 @@ class InstallerTest extends TestCase {
// Read the current version of the app to check for bug #2572
Server::get(IAppManager::class)->getAppVersion('testapp', true);
// Extract app
$pathOfTestApp = __DIR__ . '/../data/testapp.zip';
$tar = new ZIP($pathOfTestApp);
$tar->extract(\OC_App::getInstallPath());
// Install app
// Build installer
$installer = new Installer(
Server::get(AppFetcher::class),
Server::get(IClientService::class),
@ -106,6 +101,13 @@ class InstallerTest extends TestCase {
Server::get(IConfig::class),
false
);
// Extract app
$pathOfTestApp = __DIR__ . '/../data/testapp.zip';
$tar = new ZIP($pathOfTestApp);
$tar->extract($installer->getInstallPath());
// Install app
$this->assertNull(Server::get(IConfig::class)->getAppValue('testapp', 'enabled', null), 'Check that the app is not listed before installation');
$this->assertSame('testapp', $installer->installApp(self::$appid));
$this->assertSame('no', Server::get(IConfig::class)->getAppValue('testapp', 'enabled', null), 'Check that the app is listed after installation');