Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(apps-store): Remove apps from force-enabled state when uninstalled #48855

Merged
merged 1 commit into from
Nov 18, 2024
+31 −37
Merged

fix(apps-store): Remove apps from force-enabled state when uninstalled #48855

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions apps/settings/lib/Controller/AppSettingsController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/
namespace OCA\Settings\Controller;

use OC\App\AppManager;
use OC\App\AppStore\Bundles\BundleFetcher;
use OC\App\AppStore\Fetcher\AppDiscoverFetcher;
use OC\App\AppStore\Fetcher\AppFetcher;
Expand All @@ -15,7 +16,6 @@
use OC\App\Platform;
use OC\Installer;
use OCP\App\AppPathNotFoundException;
use OCP\App\IAppManager;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
Expand Down Expand Up @@ -62,7 +62,7 @@ public function __construct(
private IL10N $l10n,
private IConfig $config,
private INavigationManager $navigationManager,
private IAppManager $appManager,
private AppManager $appManager,
private CategoryFetcher $categoryFetcher,
private AppFetcher $appFetcher,
private IFactory $l10nFactory,
Expand Down Expand Up @@ -592,6 +592,8 @@ public function uninstallApp(string $appId): JSONResponse {
$appId = $this->appManager->cleanAppId($appId);
$result = $this->installer->removeApp($appId);
if ($result !== false) {
// If this app was force enabled, remove the force-enabled-state
$this->appManager->removeOverwriteNextcloudRequirement($appId);
$this->appManager->clearAppsCache();
return new JSONResponse(['data' => ['appid' => $appId]]);
}
Expand Down Expand Up @@ -631,7 +633,7 @@ private function sortApps($a, $b) {

public function force(string $appId): JSONResponse {
$appId = $this->appManager->cleanAppId($appId);
$this->appManager->ignoreNextcloudRequirementForApp($appId);
$this->appManager->overwriteNextcloudRequirement($appId);
return new JSONResponse();
}
}
7 changes: 3 additions & 4 deletions apps/settings/tests/Controller/AppSettingsControllerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@
*/
namespace OCA\Settings\Tests\Controller;

use OC\App\AppManager;
use OC\App\AppStore\Bundles\BundleFetcher;
use OC\App\AppStore\Fetcher\AppDiscoverFetcher;
use OC\App\AppStore\Fetcher\AppFetcher;
use OC\App\AppStore\Fetcher\CategoryFetcher;
use OC\Installer;
use OCA\Settings\Controller\AppSettingsController;
use OCP\App\IAppManager;
use OCP\AppFramework\Http\ContentSecurityPolicy;
use OCP\AppFramework\Http\JSONResponse;
use OCP\AppFramework\Http\TemplateResponse;
Expand Down Expand Up @@ -47,8 +47,7 @@ class AppSettingsControllerTest extends TestCase {
private $config;
/** @var INavigationManager|MockObject */
private $navigationManager;
/** @var IAppManager|MockObject */
private $appManager;
private AppManager&MockObject $appManager;
/** @var CategoryFetcher|MockObject */
private $categoryFetcher;
/** @var AppFetcher|MockObject */
Expand Down Expand Up @@ -83,7 +82,7 @@ protected function setUp(): void {
->willReturnArgument(0);
$this->config = $this->createMock(IConfig::class);
$this->navigationManager = $this->createMock(INavigationManager::class);
$this->appManager = $this->createMock(IAppManager::class);
$this->appManager = $this->createMock(AppManager::class);
$this->categoryFetcher = $this->createMock(CategoryFetcher::class);
$this->appFetcher = $this->createMock(AppFetcher::class);
$this->l10nFactory = $this->createMock(IFactory::class);
Expand Down
11 changes: 0 additions & 11 deletions build/psalm-baseline.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1025,11 +1025,6 @@
<code><![CDATA[getSettingsManager]]></code>
</UndefinedInterfaceMethod>
</file>
<file src="apps/settings/lib/Controller/AppSettingsController.php">
<UndefinedInterfaceMethod>
<code><![CDATA[ignoreNextcloudRequirementForApp]]></code>
</UndefinedInterfaceMethod>
</file>
<file src="apps/settings/lib/Hooks.php">
<InvalidArrayOffset>
<code><![CDATA[[$user->getEMailAddress() => $user->getDisplayName()]]]></code>
Expand Down Expand Up @@ -1337,12 +1332,6 @@
<code><![CDATA[!is_array($userIds)]]></code>
</TypeDoesNotContainType>
</file>
<file src="lib/private/App/AppManager.php">
<LessSpecificImplementedReturnType>
<code><![CDATA[array]]></code>
<code><![CDATA[array]]></code>
</LessSpecificImplementedReturnType>
</file>
<file src="lib/private/App/AppStore/Fetcher/Fetcher.php">
<TooManyArguments>
<code><![CDATA[fetch]]></code>
Expand Down
34 changes: 19 additions & 15 deletions lib/private/App/AppManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,10 +200,6 @@ public function getEnabledAppsForUser(IUser $user) {
return array_keys($appsForUser);
}

/**
* @param IGroup $group
* @return array
*/
public function getEnabledAppsForGroup(IGroup $group): array {
$apps = $this->getInstalledAppsValues();
$appsForGroups = array_filter($apps, function ($enabled) use ($group) {
Expand Down Expand Up @@ -304,10 +300,6 @@ public function getAutoDisabledApps(): array {
return $this->autoDisabledApps;
}

/**
* @param string $appId
* @return array
*/
public function getAppRestriction(string $appId): array {
$values = $this->getInstalledAppsValues();

Expand All @@ -321,7 +313,6 @@ public function getAppRestriction(string $appId): array {
return json_decode($values[$appId], true);
}


/**
* Check if an app is enabled for user
*
Expand Down Expand Up @@ -410,12 +401,25 @@ public function isInstalled($appId) {
return isset($installedApps[$appId]);
}

public function ignoreNextcloudRequirementForApp(string $appId): void {
/**
* Overwrite the `max-version` requirement for this app.
*/
public function overwriteNextcloudRequirement(string $appId): void {
$ignoreMaxApps = $this->config->getSystemValue('app_install_overwrite', []);
if (!in_array($appId, $ignoreMaxApps, true)) {
$ignoreMaxApps[] = $appId;
$this->config->setSystemValue('app_install_overwrite', $ignoreMaxApps);
}
$this->config->setSystemValue('app_install_overwrite', $ignoreMaxApps);
}

/**
* Remove the `max-version` overwrite for this app.
* This means this app now again can not be enabled if the `max-version` is smaller than the current Nextcloud version.
*/
public function removeOverwriteNextcloudRequirement(string $appId): void {
$ignoreMaxApps = $this->config->getSystemValue('app_install_overwrite', []);
$ignoreMaxApps = array_filter($ignoreMaxApps, fn (string $id) => $id !== $appId);
$this->config->setSystemValue('app_install_overwrite', $ignoreMaxApps);
}

public function loadApp(string $app): void {
Expand Down Expand Up @@ -573,7 +577,7 @@ public function enableApp(string $appId, bool $forceEnable = false): void {
$this->getAppPath($appId);

if ($forceEnable) {
$this->ignoreNextcloudRequirementForApp($appId);
$this->overwriteNextcloudRequirement($appId);
}

$this->installedAppsCache[$appId] = 'yes';
Expand Down Expand Up @@ -619,7 +623,7 @@ public function enableAppForGroups(string $appId, array $groups, bool $forceEnab
}

if ($forceEnable) {
$this->ignoreNextcloudRequirementForApp($appId);
$this->overwriteNextcloudRequirement($appId);
}

/** @var string[] $groupIds */
Expand All @@ -646,7 +650,7 @@ public function enableAppForGroups(string $appId, array $groups, bool $forceEnab
* @param bool $automaticDisabled
* @throws \Exception if app can't be disabled
*/
public function disableApp($appId, $automaticDisabled = false) {
public function disableApp($appId, $automaticDisabled = false): void {
if ($this->isAlwaysEnabled($appId)) {
throw new \Exception("$appId can't be disabled.");
}
Expand Down Expand Up @@ -706,7 +710,7 @@ public function getAppWebPath(string $appId): string {
/**
* Clear the cached list of apps when enabling/disabling an app
*/
public function clearAppsCache() {
public function clearAppsCache(): void {
$this->appInfos = [];
}

Expand Down
8 changes: 4 additions & 4 deletions lib/public/App/IAppManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ public function enableAppForGroups(string $appId, array $groups, bool $forceEnab
* @param bool $automaticDisabled
* @since 8.0.0
*/
public function disableApp($appId, $automaticDisabled = false);
public function disableApp($appId, $automaticDisabled = false): void;

/**
* Get the directory for the given app.
Expand Down Expand Up @@ -185,7 +185,7 @@ public function getInstalledApps();
* Clear the cached list of apps when enabling/disabling an app
* @since 8.1.0
*/
public function clearAppsCache();
public function clearAppsCache(): void;

/**
* @param string $appId
Expand All @@ -201,7 +201,7 @@ public function isShipped($appId);
* @return bool
*
* This function walks through the Nextcloud directory and loads all apps
* it can find. A directory contains an app if the file /appinfo/info.xml
* it can find. A directory contains an app if the file `/appinfo/info.xml`
* exists.
*
* if $types is set to non-empty array, only apps of those types will be loaded
Expand Down Expand Up @@ -271,7 +271,7 @@ public function getDefaultApps(): array;
/**
* Set the global default apps with fallbacks
*
* @param string[] $appId
* @param string[] $defaultApps
* @throws \InvalidArgumentException If any of the apps is not installed
* @since 28.0.0
* @deprecated 31.0.0
Expand Down
Loading