From d5e8a5f6f04e9decf1e3c599d34810e6f1602fa8 Mon Sep 17 00:00:00 2001 From: Gregor Harlan Date: Sun, 31 May 2026 09:54:22 +0200 Subject: [PATCH 1/2] refactor!: replace addon install/status flags with AddonState enum Replace the separate `install` and `status` properties with a single `AddonState` enum (Uninstalled/Installed/Activated), stored as a typed property instead of in the generic property bag. The three-state model removes the impossible install=false/status=true combination. - addons.json now stores `state` instead of `install`/`status` (no migration) - rename Addon::isAvailable() -> isActivated() and getAvailableAddons() -> getActivatedAddons() - install/uninstall hooks can only abort by throwing UserMessageException; drop the legacy installmsg property - lifecycle methods commit the new state only on success - add rector migration rules for the renamed methods --- pages/addon/details.help.php | 2 +- pages/addon/list.php | 2 +- pages/credits.php | 2 +- rector.php | 4 +- src/Addon/Addon.php | 43 ++++--- src/Addon/AddonManager.php | 106 ++++++------------ src/Addon/AddonState.php | 16 +++ src/Addon/ApiFunction/AddonOperation.php | 4 +- src/Backend/Controller.php | 4 +- src/ClassDiscovery.php | 4 +- src/Console/Command/AbstractCommand.php | 2 +- src/Console/Command/AddonActivateCommand.php | 2 +- .../Command/AddonDeactivateCommand.php | 2 +- src/Console/Command/AddonListCommand.php | 4 +- src/ExtensionPoint/Extension.php | 2 +- src/SystemReport.php | 2 +- 16 files changed, 97 insertions(+), 104 deletions(-) create mode 100644 src/Addon/AddonState.php diff --git a/pages/addon/details.help.php b/pages/addon/details.help.php index ba51f1005b..53eff31823 100644 --- a/pages/addon/details.help.php +++ b/pages/addon/details.help.php @@ -19,7 +19,7 @@ $author = $package->getAuthor(); $supportPage = $package->getSupportPage(); if (is_readable($package->getPath('help.php'))) { - if (!$package->isAvailable() && is_readable($package->getPath('lang'))) { + if (!$package->isActivated() && is_readable($package->getPath('lang'))) { I18n::addDirectory($package->getPath('lang')); } ob_start(); diff --git a/pages/addon/list.php b/pages/addon/list.php index e3ed37e5ea..83224ddc52 100644 --- a/pages/addon/list.php +++ b/pages/addon/list.php @@ -70,7 +70,7 @@ $class = ''; $status = ' '; - if ($package->isAvailable()) { + if ($package->isActivated()) { $status = $getLink($package, 'deactivate', 'rex-icon-package-is-activated'); $class .= ' rex-package-is-activated'; } elseif ($package->isInstalled()) { diff --git a/pages/credits.php b/pages/credits.php index bef63c9975..b1a15ad35e 100644 --- a/pages/credits.php +++ b/pages/credits.php @@ -91,7 +91,7 @@ '; -foreach (Addon::getAvailableAddons() as $package) { +foreach (Addon::getActivatedAddons() as $package) { $helpUrl = Url::backendPage('packages', ['subpage' => 'help', 'package' => $package->name]); $license = ''; diff --git a/rector.php b/rector.php index 39cdf708ca..015f273e97 100644 --- a/rector.php +++ b/rector.php @@ -414,8 +414,10 @@ ->withConfiguredRule(RenameMethodRector::class, [ new MethodCallRename(Addon\Addon::class, 'getRegisteredPackages', 'getRegisteredAddons'), new MethodCallRename(Addon\Addon::class, 'getInstalledPackages', 'getInstalledAddons'), - new MethodCallRename(Addon\Addon::class, 'getAvailablePackages', 'getAvailableAddons'), + new MethodCallRename(Addon\Addon::class, 'getAvailablePackages', 'getActivatedAddons'), + new MethodCallRename(Addon\Addon::class, 'getAvailableAddons', 'getActivatedAddons'), new MethodCallRename(Addon\Addon::class, 'getSetupPackages', 'getSetupAddons'), + new MethodCallRename(Addon\Addon::class, 'isAvailable', 'isActivated'), new MethodCallRename(ApiFunction\Result::class, 'toJSON', 'toJson'), new MethodCallRename(ApiFunction\Result::class, 'fromJSON', 'fromJson'), diff --git a/src/Addon/Addon.php b/src/Addon/Addon.php index bb356d8ec0..eb6398554c 100644 --- a/src/Addon/Addon.php +++ b/src/Addon/Addon.php @@ -62,6 +62,9 @@ abstract class Addon /** Loading position relative to other addons during boot. Override to load this addon early or late. */ public protected(set) LoadOrder $load = LoadOrder::Normal; + /** Lifecycle state of the addon. */ + public private(set) AddonState $state = AddonState::Uninstalled; + /** * Properties. * @@ -214,16 +217,26 @@ final public function removeProperty(string $key): void unset($this->properties[$key]); } - /** Returns if the addon is available (activated and installed). */ - final public function isAvailable(): bool + /** + * Sets the lifecycle state of the addon. + * + * @internal + */ + final public function setState(AddonState $state): void + { + $this->state = $state; + } + + /** Returns if the addon is activated (and therefore installed). */ + final public function isActivated(): bool { - return $this->isInstalled() && (bool) $this->getProperty('status', false); + return AddonState::Activated === $this->state; } - /** Returns if the addon is installed. */ + /** Returns if the addon is installed (activated or not). */ final public function isInstalled(): bool { - return (bool) $this->getProperty('install', false); + return AddonState::Uninstalled !== $this->state; } final public function getAuthor(?string $default = null): ?string @@ -369,13 +382,10 @@ final public function loadProperties(bool $force = false): void $properties = $cache[$id]['data']; } - $this->properties = array_intersect_key($this->properties, ['install' => null, 'status' => null]); + $this->properties = []; if ($properties) { foreach ($properties as $key => $value) { $key = Type::string($key); - if (isset($this->properties[$key])) { - continue; - } if ('supportpage' !== $key) { $value = I18n::translateArray($value, false, $this->i18n(...)); } elseif (null !== $value && !preg_match('@^https?://@i', $value)) { @@ -465,14 +475,14 @@ public function getPages(): iterable /** * Install hook — runs on install/reinstall. Override for schema/data setup. Must be idempotent. * - * @throws UserMessageException + * @throws UserMessageException to abort the installation with a message */ public function install(): void {} /** * Uninstall hook — runs on uninstall. Override for cleanup. * - * @throws UserMessageException + * @throws UserMessageException to abort the uninstallation with a message */ public function uninstall(): void {} @@ -497,13 +507,13 @@ final public static function getInstalledAddons(): array } /** - * Returns the available addons. + * Returns the activated addons. * * @return array */ - final public static function getAvailableAddons(): array + final public static function getActivatedAddons(): array { - return self::filterPackages(self::$addons, 'isAvailable'); + return self::filterPackages(self::$addons, 'isActivated'); } /** @@ -530,7 +540,7 @@ final public static function initialize(bool $dbExists = true): void } else { $config = []; foreach (Core::getProperty('setup_addons') as $addon) { - $config[(string) $addon]['install'] = false; + $config[(string) $addon]['state'] = AddonState::Uninstalled->value; } } @@ -559,8 +569,7 @@ final public static function initialize(bool $dbExists = true): void } $addon = new $class($composerPackages[$addonName], $addonName); } - $addon->setProperty('install', $addonConfig['install'] ?? false); - $addon->setProperty('status', $addonConfig['status'] ?? false); + $addon->state = AddonState::from($addonConfig['state'] ?? AddonState::Uninstalled->value); self::$addons[$addonName] = $addon; } } diff --git a/src/Addon/AddonManager.php b/src/Addon/AddonManager.php index 258c0d233e..9b8f8c69be 100644 --- a/src/Addon/AddonManager.php +++ b/src/Addon/AddonManager.php @@ -29,7 +29,7 @@ use const JSON_UNESCAPED_UNICODE; /** - * @phpstan-type TAddonConfig array, install: bool, status: bool}> + * @phpstan-type TAddonConfig array, state: value-of}> * @phpstan-type TAddonOrder list */ class AddonManager @@ -83,30 +83,14 @@ public function install(): bool throw new UserMessageException($message); } - $reinstall = $this->addon->getProperty('install'); - $this->addon->setProperty('install', true); + $reinstall = $this->addon->isInstalled(); I18n::addDirectory($this->addon->getPath('lang')); - // run install hook + // run install hook (can only abort by throwing a UserMessageException) $this->addon->install(); $successMessage = (string) $this->addon->getProperty('successmsg', ''); - /** @psalm-taint-escape html */ - $instmsg = (string) $this->addon->getProperty('installmsg', ''); - if ('' != $instmsg) { - throw new UserMessageException($instmsg); - } - if (!$this->addon->isInstalled()) { - throw new UserMessageException($this->i18n('no_reason')); - } - - if (!$reinstall) { - $this->addon->setProperty('status', true); - } - static::saveConfig(); - self::generateAddonOrder(); - foreach ($this->addon->getProperty('default_config', []) as $key => $value) { if (!$this->addon->hasConfig($key)) { $this->addon->setConfig($key, $value); @@ -121,6 +105,13 @@ public function install(): bool } } + // everything succeeded — commit (fresh installs are activated right away) + if (!$reinstall) { + $this->addon->setState(AddonState::Activated); + } + static::saveConfig(); + self::generateAddonOrder(); + $this->message = $this->i18n($reinstall ? 'reinstalled' : 'installed', $this->addon->name); if ($successMessage) { $this->message .= ' ' . $successMessage; @@ -131,7 +122,6 @@ public function install(): bool $this->message = $e->getMessage(); } - $this->addon->setProperty('install', false); $this->message = $this->i18n('no_install', $this->addon->name) . '
' . $this->message; return false; @@ -144,30 +134,20 @@ public function install(): bool */ public function uninstall(): bool { - $isActivated = $this->addon->isAvailable(); + $originalState = $this->addon->state; + $isActivated = $this->addon->isActivated(); if ($isActivated && !$this->deactivate()) { return false; } try { - $this->addon->setProperty('install', false); - if (!$isActivated) { I18n::addDirectory($this->addon->getPath('lang')); } - // run uninstall hook + // run uninstall hook (can only abort by throwing a UserMessageException) $this->addon->uninstall(); - /** @psalm-taint-escape html */ - $instmsg = (string) $this->addon->getProperty('installmsg', ''); - if ('' != $instmsg) { - throw new UserMessageException($instmsg); - } - if ($this->addon->isInstalled()) { - throw new UserMessageException($this->i18n('no_reason')); - } - // delete assets $assets = $this->addon->getAssetsPath(); if (is_dir($assets) && !Dir::delete($assets)) { @@ -179,6 +159,8 @@ public function uninstall(): bool Config::removeNamespace($this->addon->name); + // everything succeeded — commit the new state + $this->addon->setState(AddonState::Uninstalled); static::saveConfig(); $this->message = $this->i18n('uninstalled', $this->addon->name); @@ -187,11 +169,12 @@ public function uninstall(): bool $this->message = $e->getMessage(); } - $this->addon->setProperty('install', true); if ($isActivated) { - $this->addon->setProperty('status', true); + // the deactivation was already committed — restore the previous state + $this->addon->setState($originalState); + static::saveConfig(); + self::generateAddonOrder(); } - static::saveConfig(); $this->message = $this->i18n('no_uninstall', $this->addon->name) . '
' . $this->message; return false; @@ -204,31 +187,20 @@ public function uninstall(): bool */ public function activate(): bool { - if ($this->addon->isInstalled()) { - $state = ''; - if (!$this->checkRequirements()) { - $state .= $this->message; - } - $state = $state ?: true; - - if (true === $state) { - $this->addon->setProperty('status', true); - static::saveConfig(); - } - if (true === $state) { - self::generateAddonOrder(); - } - } else { - $state = $this->i18n('not_installed', $this->addon->name); + if (!$this->addon->isInstalled()) { + $this->message = $this->i18n('no_activation', $this->addon->name) . '
' . $this->i18n('not_installed', $this->addon->name); + return false; } - if (true !== $state) { - // error while config generation, rollback addon status - $this->addon->setProperty('status', false); - $this->message = $this->i18n('no_activation', $this->addon->name) . '
' . $state; + if (!$this->checkRequirements()) { + $this->message = $this->i18n('no_activation', $this->addon->name) . '
' . $this->message; return false; } + $this->addon->setState(AddonState::Activated); + static::saveConfig(); + self::generateAddonOrder(); + $this->message = $this->i18n('activated', $this->addon->name); return true; } @@ -240,10 +212,8 @@ public function activate(): bool */ public function deactivate(): bool { - $state = $this->checkDependencies(); - - if ($state) { - $this->addon->setProperty('status', false); + if ($this->checkDependencies()) { + $this->addon->setState(AddonState::Installed); static::saveConfig(); // clear cache of addon @@ -275,7 +245,7 @@ public function checkRequirements(): bool $state = []; foreach (self::getRequiredAddons($this->addon) as $addonName) { - if (Addon::get($addonName)?->isAvailable()) { + if (Addon::get($addonName)?->isActivated()) { continue; } @@ -300,7 +270,7 @@ public function checkDependencies(): bool $i18nPrefix = 'package_dependencies_error_'; $state = []; - foreach (Addon::getAvailableAddons() as $addon) { + foreach (Addon::getActivatedAddons() as $addon) { if ($addon === $this->addon) { continue; } @@ -368,7 +338,7 @@ public static function generateAddonOrder(): void } } }; - foreach (Addon::getAvailableAddons() as $addon) { + foreach (Addon::getActivatedAddons() as $addon) { $id = $addon->name; if (LoadOrder::Early === $addon->load) { $early[] = $id; @@ -397,8 +367,7 @@ protected static function saveConfig(): void $config = []; foreach (Addon::getRegisteredAddons() as $addonName => $addon) { $config[$addonName]['class'] = $addon::class; - $config[$addonName]['install'] = $addon->isInstalled(); - $config[$addonName]['status'] = $addon->isAvailable(); + $config[$addonName]['state'] = $addon->state->value; } self::saveAddonsData(config: $config); @@ -423,12 +392,9 @@ public static function synchronizeWithFileSystem(): void } $config[$addonName]['class'] = $addonClasses[$addonName]; if (!Addon::exists($addonName)) { - $config[$addonName]['install'] = false; - $config[$addonName]['status'] = false; + $config[$addonName]['state'] = AddonState::Uninstalled->value; } else { - $addon = Addon::require($addonName); - $config[$addonName]['install'] = $addon->isInstalled(); - $config[$addonName]['status'] = $addon->isAvailable(); + $config[$addonName]['state'] = Addon::require($addonName)->state->value; } } ksort($config); diff --git a/src/Addon/AddonState.php b/src/Addon/AddonState.php new file mode 100644 index 0000000000..cf327c6704 --- /dev/null +++ b/src/Addon/AddonState.php @@ -0,0 +1,16 @@ +isInstalled() - || 'activate' == $function && $package->isAvailable() - || 'deactivate' == $function && !$package->isAvailable() + || 'activate' == $function && $package->isActivated() + || 'deactivate' == $function && !$package->isActivated() ) { return new Result(true); } diff --git a/src/Backend/Controller.php b/src/Backend/Controller.php index 09c2241d4f..d67e9f39ca 100644 --- a/src/Backend/Controller.php +++ b/src/Backend/Controller.php @@ -355,7 +355,7 @@ public static function appendLoggedInPages(): void public static function appendPackagePages(): void { - $addons = Core::isSafeMode() ? Addon::getSetupAddons() : Addon::getAvailableAddons(); + $addons = Core::isSafeMode() ? Addon::getSetupAddons() : Addon::getActivatedAddons(); foreach ($addons as $addon) { foreach ($addon->getPages() as $page) { self::registerAddonPage($page, $addon); @@ -515,7 +515,7 @@ public static function includeCurrentPageSubPath(array $context = []) private static function includePath(string $path, array $context = []): mixed { return Timer::measure('Page: ' . Path::relative($path), function () use ($path, $context) { - foreach (Addon::getAvailableAddons() as $addon) { + foreach (Addon::getActivatedAddons() as $addon) { if (str_starts_with($path, $addon->path . DIRECTORY_SEPARATOR)) { return $addon->includeFile($path, $context); } diff --git a/src/ClassDiscovery.php b/src/ClassDiscovery.php index 860dc9eea9..115e2dc1b4 100644 --- a/src/ClassDiscovery.php +++ b/src/ClassDiscovery.php @@ -272,7 +272,7 @@ private function getRelevantPaths(): array $paths[] = __DIR__ . DIRECTORY_SEPARATOR; // Active addon paths - foreach (Addon::getAvailableAddons() as $addon) { + foreach (Addon::getActivatedAddons() as $addon) { $paths[] = $addon->path . DIRECTORY_SEPARATOR; } @@ -366,7 +366,7 @@ private function saveCacheData(string $cacheKey, mixed $data): void private function getAddonHash(): string { $parts = []; - foreach (Addon::getAvailableAddons() as $addon) { + foreach (Addon::getActivatedAddons() as $addon) { $parts[] = $addon->name . ':' . $addon->getVersion(); } diff --git a/src/Console/Command/AbstractCommand.php b/src/Console/Command/AbstractCommand.php index 44be940d3c..905cf3e792 100644 --- a/src/Console/Command/AbstractCommand.php +++ b/src/Console/Command/AbstractCommand.php @@ -46,7 +46,7 @@ private function resolveAddon(): Addon if (false !== $file) { $file = realpath($file) ?: $file; - foreach (Addon::getAvailableAddons() as $addon) { + foreach (Addon::getActivatedAddons() as $addon) { if (str_starts_with($file, $addon->path . DIRECTORY_SEPARATOR)) { return $addon; } diff --git a/src/Console/Command/AddonActivateCommand.php b/src/Console/Command/AddonActivateCommand.php index d04fc049d7..9c3e5f9d6e 100644 --- a/src/Console/Command/AddonActivateCommand.php +++ b/src/Console/Command/AddonActivateCommand.php @@ -18,7 +18,7 @@ final class AddonActivateCommand extends AbstractCommand public function __invoke( SymfonyStyle $io, #[Argument('The id of the addon, e.g. "yform"', suggestedValues: static function (): array { - return array_keys(array_filter(Addon::getRegisteredAddons(), static fn (Addon $addon): bool => !$addon->isAvailable())); + return array_keys(array_filter(Addon::getRegisteredAddons(), static fn (Addon $addon): bool => !$addon->isActivated())); })] string $addon, ): int { // the package manager don't know new packages in the addon folder diff --git a/src/Console/Command/AddonDeactivateCommand.php b/src/Console/Command/AddonDeactivateCommand.php index aee5a6ded1..869f3e550a 100644 --- a/src/Console/Command/AddonDeactivateCommand.php +++ b/src/Console/Command/AddonDeactivateCommand.php @@ -18,7 +18,7 @@ final class AddonDeactivateCommand extends AbstractCommand public function __invoke( SymfonyStyle $io, #[Argument('The name of the addon, e.g. "yform"', suggestedValues: static function (): array { - return array_keys(Addon::getAvailableAddons()); + return array_keys(Addon::getActivatedAddons()); })] string $addon, ): int { // the package manager don't know new packages in the addon folder diff --git a/src/Console/Command/AddonListCommand.php b/src/Console/Command/AddonListCommand.php index fc4d68952a..13202547a2 100644 --- a/src/Console/Command/AddonListCommand.php +++ b/src/Console/Command/AddonListCommand.php @@ -39,7 +39,7 @@ public function __invoke( 'author' => $package->getAuthor(), 'version' => $package->getVersion(), 'installed' => $package->isInstalled(), - 'activated' => $package->isAvailable(), + 'activated' => $package->isActivated(), 'license' => $package->getLicense(), ]; @@ -60,7 +60,7 @@ public function __invoke( continue; } - if ($activatedOnly && !$package->isAvailable()) { + if ($activatedOnly && !$package->isActivated()) { continue; } diff --git a/src/ExtensionPoint/Extension.php b/src/ExtensionPoint/Extension.php index 8d66573317..435972a274 100644 --- a/src/ExtensionPoint/Extension.php +++ b/src/ExtensionPoint/Extension.php @@ -135,7 +135,7 @@ public static function registerByAttribute(AbstractProject $project): void } elseif (is_subclass_of($entry['class'], Addon::class)) { if (null === $addonByClass) { $addonByClass = []; - foreach (Addon::getAvailableAddons() as $addon) { + foreach (Addon::getActivatedAddons() as $addon) { $addonByClass[$addon::class] = $addon; } } diff --git a/src/SystemReport.php b/src/SystemReport.php index f5a70382cb..15ba7cf1a1 100644 --- a/src/SystemReport.php +++ b/src/SystemReport.php @@ -107,7 +107,7 @@ public function get(): array } $packages = []; - foreach (Addon::getAvailableAddons() as $package) { + foreach (Addon::getActivatedAddons() as $package) { $packages[$package->name] = $package->getVersion(); } From d37cfef23f4aeacc25633fa40759e599ae61e1ae Mon Sep 17 00:00:00 2001 From: gharlan <330436+gharlan@users.noreply.github.com> Date: Sun, 31 May 2026 08:09:19 +0000 Subject: [PATCH 2/2] chore: update psalm baseline --- .tools/psalm/baseline.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/.tools/psalm/baseline.xml b/.tools/psalm/baseline.xml index 2e95b81af8..620c67b88e 100644 --- a/.tools/psalm/baseline.xml +++ b/.tools/psalm/baseline.xml @@ -1481,7 +1481,6 @@ -