-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
chore(OC_App): migrate more legacy function and usage #60071
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -12,6 +12,9 @@ | |||||||||
| use OC\Config\ConfigManager; | ||||||||||
| use OC\DB\MigrationService; | ||||||||||
| use OC\Migration\BackgroundRepair; | ||||||||||
| use OC\NeedsUpdateException; | ||||||||||
| use OC\Repair; | ||||||||||
| use OC\Repair\Events\RepairErrorEvent; | ||||||||||
| use OCP\Activity\IManager as IActivityManager; | ||||||||||
| use OCP\App\AppPathNotFoundException; | ||||||||||
| use OCP\App\Events\AppDisableEvent; | ||||||||||
|
|
@@ -147,6 +150,10 @@ private function getUrlGenerator(): IURLGenerator { | |||||||||
| * @return array<string,string> appId => enabled (may be 'yes', or a json encoded list of group ids) | ||||||||||
| */ | ||||||||||
| private function getEnabledAppsValues(): array { | ||||||||||
| if ($this->config->getSystemValueBool('installed', false) === false) { | ||||||||||
|
Comment on lines
152
to
+153
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it never needed by the installation process? |
||||||||||
| return []; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| if (!$this->enabledAppsCache) { | ||||||||||
| /** @var array<string,string> */ | ||||||||||
| $values = $this->getAppConfig()->searchValues('enabled', false, IAppConfig::VALUE_STRING); | ||||||||||
|
|
@@ -240,25 +247,18 @@ public function getEnabledAppsForGroup(IGroup $group): array { | |||||||||
| return array_keys($appsForGroups); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Loads all apps | ||||||||||
| * | ||||||||||
| * @param string[] $types | ||||||||||
| * @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 | ||||||||||
| * exists. | ||||||||||
| * | ||||||||||
| * if $types is set to non-empty array, only apps of those types will be loaded | ||||||||||
| */ | ||||||||||
| #[\Override] | ||||||||||
| public function loadApps(array $types = []): bool { | ||||||||||
| if ($this->config->getSystemValueBool('maintenance', false)) { | ||||||||||
| return false; | ||||||||||
| } | ||||||||||
| if ($this->config->getSystemValueBool('installed', false) === false) { | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Unnecessary default value. |
||||||||||
| // can only access the apps folder after installation, so we can't load any apps before that | ||||||||||
| return false; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| // Load the enabled apps here | ||||||||||
| $apps = \OC_App::getEnabledApps(); | ||||||||||
| $apps = $this->getEnabledApps(); | ||||||||||
|
|
||||||||||
| // Add each apps' folder as allowed class path | ||||||||||
| foreach ($apps as $app) { | ||||||||||
|
|
@@ -708,7 +708,7 @@ public function disableApp($appId, $automaticDisabled = false): void { | |||||||||
| // run uninstall steps | ||||||||||
| $appData = $this->getAppInfo($appId); | ||||||||||
| if (!is_null($appData)) { | ||||||||||
| \OC_App::executeRepairSteps($appId, $appData['repair-steps']['uninstall']); | ||||||||||
| $this->executeRepairSteps($appId, $appData['repair-steps']['uninstall']); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| $this->dispatcher->dispatchTyped(new AppDisableEvent($appId)); | ||||||||||
|
|
@@ -1109,30 +1109,24 @@ public function upgradeApp(string $appId): bool { | |||||||||
| $appPath = $this->getAppPath($appId, true); | ||||||||||
|
|
||||||||||
| $this->clearAppsCache(); | ||||||||||
| $l = \OC::$server->getL10N('core'); | ||||||||||
| $appData = $this->getAppInfo($appId, false, $l->getLanguageCode()); | ||||||||||
| if ($appData === null) { | ||||||||||
| $appInfo = $this->getAppInfo($appId); | ||||||||||
| if ($appInfo === null) { | ||||||||||
|
Comment on lines
-1112
to
+1113
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. translations not needed? |
||||||||||
| throw new AppPathNotFoundException('Could not find ' . $appId); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| $ignoreMaxApps = $this->config->getSystemValue('app_install_overwrite', []); | ||||||||||
| $ignoreMax = in_array($appId, $ignoreMaxApps, true); | ||||||||||
| \OC_App::checkAppDependencies( | ||||||||||
| $this->config, | ||||||||||
| $l, | ||||||||||
| $appData, | ||||||||||
| $ignoreMax | ||||||||||
| ); | ||||||||||
| $this->checkAppDependencies($appId, $ignoreMax); | ||||||||||
|
|
||||||||||
| \OC_App::registerAutoloading($appId, $appPath, true); | ||||||||||
| \OC_App::executeRepairSteps($appId, $appData['repair-steps']['pre-migration']); | ||||||||||
| $this->executeRepairSteps($appId, $appInfo['repair-steps']['pre-migration']); | ||||||||||
|
|
||||||||||
| $ms = new MigrationService($appId, Server::get(\OC\DB\Connection::class)); | ||||||||||
| $ms->migrate(); | ||||||||||
|
|
||||||||||
| \OC_App::executeRepairSteps($appId, $appData['repair-steps']['post-migration']); | ||||||||||
| $this->executeRepairSteps($appId, $appInfo['repair-steps']['post-migration']); | ||||||||||
| $queue = Server::get(IJobList::class); | ||||||||||
| foreach ($appData['repair-steps']['live-migration'] as $step) { | ||||||||||
| foreach ($appInfo['repair-steps']['live-migration'] as $step) { | ||||||||||
| $queue->add(BackgroundRepair::class, [ | ||||||||||
| 'app' => $appId, | ||||||||||
| 'step' => $step]); | ||||||||||
|
|
@@ -1143,19 +1137,19 @@ public function upgradeApp(string $appId): bool { | |||||||||
| $this->getAppVersion($appId, false); | ||||||||||
|
|
||||||||||
| // Setup background jobs | ||||||||||
| foreach ($appData['background-jobs'] as $job) { | ||||||||||
| foreach ($appInfo['background-jobs'] as $job) { | ||||||||||
| $queue->add($job); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| //set remote/public handlers | ||||||||||
| foreach ($appData['remote'] as $name => $path) { | ||||||||||
| foreach ($appInfo['remote'] as $name => $path) { | ||||||||||
| $this->config->setAppValue('core', 'remote_' . $name, $appId . '/' . $path); | ||||||||||
| } | ||||||||||
| foreach ($appData['public'] as $name => $path) { | ||||||||||
| foreach ($appInfo['public'] as $name => $path) { | ||||||||||
| $this->config->setAppValue('core', 'public_' . $name, $appId . '/' . $path); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| $this->setAppTypes($appId, $appData); | ||||||||||
| $this->setAppTypes($appId, $appInfo); | ||||||||||
|
|
||||||||||
| $version = $this->getAppVersion($appId); | ||||||||||
| $this->config->setAppValue($appId, 'installed_version', $version); | ||||||||||
|
|
@@ -1197,4 +1191,53 @@ public function isUpgradeRequired(string $appId): bool { | |||||||||
| public function isAppCompatible(string $serverVersion, array $appInfo, bool $ignoreMax = false): bool { | ||||||||||
| return count($this->dependencyAnalyzer->analyzeServerVersion($serverVersion, $appInfo, $ignoreMax)) === 0; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Check if all dependencies of an app are satisfied. | ||||||||||
| * | ||||||||||
| * @param string $appId - The app to check | ||||||||||
| * @param bool $ignoreMax - Whether to ignore the Nextcloud max version requirement | ||||||||||
| * @throws \Exception - If there are missing dependencies | ||||||||||
| */ | ||||||||||
| public function checkAppDependencies(string $appId, bool $ignoreMax = false): void { | ||||||||||
|
susnux marked this conversation as resolved.
|
||||||||||
| $info = $this->getAppInfo($appId); | ||||||||||
| $missing = $this->dependencyAnalyzer->analyze($info, $ignoreMax); | ||||||||||
| if (!empty($missing)) { | ||||||||||
| $l = \OCP\Server::get(\OCP\L10N\IFactory::class)->get('core'); | ||||||||||
|
Comment on lines
+1205
to
+1206
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| $missingMsg = implode(PHP_EOL, $missing); | ||||||||||
| throw new \Exception( | ||||||||||
| $l->t('App "%1$s" cannot be installed because the following dependencies are not fulfilled: %2$s', | ||||||||||
| [$info['name'], $missingMsg] | ||||||||||
| ) | ||||||||||
| ); | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Run repair steps for an app. | ||||||||||
| * | ||||||||||
| * @param string $appId - The app to run the repair steps for | ||||||||||
| * @param string[] $steps - The repair steps to run | ||||||||||
| * @throws NeedsUpdateException | ||||||||||
| */ | ||||||||||
| public function executeRepairSteps(string $appId, array $steps): void { | ||||||||||
| if (empty($steps)) { | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| return; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| $this->loadApp($appId); | ||||||||||
|
|
||||||||||
| // load the steps | ||||||||||
| $r = Server::get(Repair::class); | ||||||||||
| foreach ($steps as $step) { | ||||||||||
| try { | ||||||||||
| $r->addStep($step); | ||||||||||
| } catch (\Exception $ex) { | ||||||||||
| $this->dispatcher->dispatchTyped(new RepairErrorEvent($ex->getMessage())); | ||||||||||
| $this->logger->error('Failed to add app migration step ' . $step, ['exception' => $ex]); | ||||||||||
| } | ||||||||||
| } | ||||||||||
| // run the steps | ||||||||||
| $r->run(); | ||||||||||
| } | ||||||||||
| } | ||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer this version when the method returns a bool.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I oversaw that 😅