From 6acda6a82b85e106726df625487f1f487e24f6eb Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Thu, 17 Mar 2022 16:23:54 -0600 Subject: [PATCH 01/16] Initial work on refactoring the PluginManager. See https://github.com/wintercms/winter/issues/493 for context. --- modules/system/classes/PluginBase.php | 22 ++ modules/system/classes/PluginManager.php | 258 +++++++++++++---------- modules/system/console/PluginDisable.php | 4 - modules/system/console/PluginEnable.php | 4 - modules/system/controllers/Updates.php | 27 +-- modules/system/models/PluginVersion.php | 12 +- 6 files changed, 176 insertions(+), 151 deletions(-) diff --git a/modules/system/classes/PluginBase.php b/modules/system/classes/PluginBase.php index 30af4fbf88..97568cc001 100644 --- a/modules/system/classes/PluginBase.php +++ b/modules/system/classes/PluginBase.php @@ -448,4 +448,26 @@ public function getPluginVersion(): string return $this->version = trim(key(array_slice($versionInfo, -1, 1))); } + + /** + * Verifies the plugin's dependencies are present and enabled + */ + public function checkDependencies(): bool + { + $manager = PluginManager::instance(); + $required = $manager->getDependencies($this); + if (empty($required)) { + return true; + } + + foreach ($required as $require) { + $requiredPlugin = $manager->findByIdentifier($require); + + if (!$requiredPlugin || $manager->isDisabled($requiredPlugin)) { + return false; + } + } + + return true; + } } diff --git a/modules/system/classes/PluginManager.php b/modules/system/classes/PluginManager.php index 6ca9d0a65b..694e73b6e4 100644 --- a/modules/system/classes/PluginManager.php +++ b/modules/system/classes/PluginManager.php @@ -7,9 +7,9 @@ use File; use Lang; use View; +use Cache; use Config; use Schema; -use Storage; use SystemException; use RecursiveIteratorIterator; use RecursiveDirectoryIterator; @@ -26,15 +26,23 @@ class PluginManager { use \Winter\Storm\Support\Traits\Singleton; + public const DISABLED_MISSING = 'disabled-missing'; + public const DISABLED_REQUEST = 'disabled-request'; + public const DISABLED_BY_USER = 'disabled-user'; + public const DISABLED_REPLACED = 'disabled-replaced'; + public const DISABLED_REPLACEMENT_FAILED = 'disabled-replacement-failed'; + public const DISABLED_BY_CONFIG = 'disabled-config'; + public const DISABLED_MISSING_DEPENDENCIES = 'disabled-dependencies'; + /** * The application instance, since Plugins are an extension of a Service Provider */ protected $app; /** - * @var array Container array used for storing plugin information objects. + * @var PluginBase[] Container array used for storing plugin information objects. */ - protected $plugins; + protected $plugins = []; /** * @var array A map of plugins and their directory paths. @@ -67,9 +75,9 @@ class PluginManager protected $booted = false; /** - * @var string Path to the JSON encoded file containing the disabled plugins. + * @var string Cache key for the disabled plugin data */ - protected $metaFile; + protected $disabledCacheKey = 'system-plugins-disabled'; /** * @var array Array of disabled plugins @@ -91,28 +99,14 @@ class PluginManager */ protected function init() { - $this->bindContainerObjects(); - $this->metaFile = 'cms/disabled.json'; + $this->app = App::make('app'); $this->loadDisabled(); $this->loadPlugins(); - - if ($this->app->runningInBackend()) { - $this->loadDependencies(); - } + $this->loadDependencies(); $this->registerReplacedPlugins(); } - /** - * These objects are "soft singletons" and may be lost when - * the IoC container reboots. This provides a way to rebuild - * for the purposes of unit testing. - */ - public function bindContainerObjects() - { - $this->app = App::make('app'); - } - /** * Finds all available plugins and loads them in to the $this->plugins array. * @@ -157,7 +151,7 @@ public function loadPlugin($namespace, $path) return; } - $classObj = new $className($this->app); + $pluginObj = new $className($this->app); } catch (\Throwable $e) { Log::error('Plugin ' . $className . ' could not be instantiated.', [ 'message' => $e->getMessage(), @@ -168,27 +162,20 @@ public function loadPlugin($namespace, $path) return; } - $classId = $this->getIdentifier($classObj); + $classId = $this->getIdentifier($pluginObj); - /* - * Check for disabled plugins - */ - if ($this->isDisabled($classId)) { - $classObj->disabled = true; - } - - $this->plugins[$classId] = $classObj; + $this->plugins[$classId] = $pluginObj; $this->pathMap[$classId] = $path; $this->normalizedMap[strtolower($classId)] = $classId; - $replaces = $classObj->getReplaces(); + $replaces = $pluginObj->getReplaces(); if ($replaces) { foreach ($replaces as $replace) { $this->replacementMap[$replace] = $classId; } } - return $classObj; + return $pluginObj; } /** @@ -232,12 +219,8 @@ public function unregisterAll() /** * Registers a single plugin object. - * - * @param PluginBase $plugin The instantiated Plugin object - * @param string $pluginId The string identifier for the plugin - * @return void */ - public function registerPlugin($plugin, $pluginId = null) + public function registerPlugin(PluginBase $plugin, ?string $pluginId = null): void { if (!$pluginId) { $pluginId = $this->getIdentifier($plugin); @@ -257,7 +240,7 @@ public function registerPlugin($plugin, $pluginId = null) /** * Prevent autoloaders from loading if plugin is disabled */ - if ($plugin->disabled) { + if ($this->isDisabled($pluginId)) { return; } @@ -292,7 +275,7 @@ public function registerPlugin($plugin, $pluginId = null) foreach ($replaces as $replace) { $replaceNamespace = $this->getNamespace($replace); - App::make(ClassLoader::class)->addNamespaceAliases([ + $this->app->make(ClassLoader::class)->addNamespaceAliases([ // class_alias() expects order to be $real, $alias $this->getNamespace($pluginId) => $replaceNamespace, ]); @@ -349,13 +332,14 @@ public function bootAll($force = false) /** * Boots the provided plugin object. - * - * @param PluginBase $plugin - * @return void */ - public function bootPlugin($plugin) + public function bootPlugin(PluginBase $plugin): void { - if (!$plugin || $plugin->disabled || (self::$noInit && !$plugin->elevated)) { + if ( + !$plugin + || (self::$noInit && !$plugin->elevated) + || $this->isDisabled($plugin) + ) { return; } @@ -608,7 +592,8 @@ public function getRegistrationMethodValues($methodName) */ public function clearDisabledCache() { - Storage::delete($this->metaFile); + // Storage::delete($this->metaFile); + Cache::forget($this->disabledCacheKey); $this->disabledPlugins = []; } @@ -619,20 +604,19 @@ public function clearDisabledCache() */ protected function loadDisabled() { - $path = $this->metaFile; - + // Check the config files for disabled plugins if (($configDisabled = Config::get('cms.disablePlugins')) && is_array($configDisabled)) { foreach ($configDisabled as $disabled) { - $this->disabledPlugins[$disabled] = true; + $this->flagPlugin($disabled, static::DISABLED_BY_CONFIG); } } - if (Storage::exists($path)) { - $disabled = json_decode(Storage::get($path), true) ?: []; - $this->disabledPlugins = array_merge($this->disabledPlugins, $disabled); - } else { - $this->populateDisabledPluginsFromDb(); - $this->writeDisabled(); + // Check the database for disabled plugins + $userDisabled = Cache::rememberForever($this->disabledCacheKey, function () { + return $this->getDisabledPluginsFromDb(); + }); + foreach ($userDisabled as $code) { + $this->flagPlugin($code, static::DISABLED_BY_USER); } } @@ -658,29 +642,24 @@ public function isDisabled($id) */ protected function writeDisabled() { - Storage::put($this->metaFile, json_encode($this->disabledPlugins)); + Cache::forever($this->disabledCacheKey, $this->disabledPlugins); } /** - * Populates information about disabled plugins from database - * - * @return void + * Returns plugin codes that have been flagged disabled in the database */ - protected function populateDisabledPluginsFromDb() + protected function getDisabledPluginsFromDb(): array { - if (!App::hasDatabase()) { - return; - } + $disabled = []; - if (!Schema::hasTable('system_plugin_versions')) { - return; + if ( + $this->app->hasDatabase() + && Schema::hasTable('system_plugin_versions') + ) { + $disabled = Db::table('system_plugin_versions')->where('is_disabled', 1)->lists('code') ?? []; } - $disabled = Db::table('system_plugin_versions')->where('is_disabled', 1)->lists('code'); - - foreach ($disabled as $code) { - $this->disabledPlugins[$code] = true; - } + return $disabled; } /** @@ -727,14 +706,20 @@ public function registerReplacedPlugins() // Only allow one of the replaced plugin or the replacing plugin to exist // at once depending on whether the version constraints are met or not if ($this->plugins[$replacement]->canReplacePlugin($target, $this->plugins[$target]->getPluginVersion())) { + // Alias the replaced plugin to the replacing plugin $this->aliasPluginAs($replacement, $target); - $this->disablePlugin($target); - $this->enablePlugin($replacement); + + // Set the plugin flags + $this->flagPlugin($target, static::DISABLED_REPLACED); + $this->unflagPlugin($replacement, static::DISABLED_REPLACEMENT_FAILED); + // Register this plugin as actively replaced $this->activeReplacementMap[$target] = $replacement; } else { - $this->disablePlugin($replacement); - $this->enablePlugin($target); + // Set the plugin flags + $this->flagPlugin($replacement, static::DISABLED_REPLACEMENT_FAILED); + $this->unflagPlugin($target, static::DISABLED_REPLACED); + // Remove the replacement alias to prevent redirection to a disabled plugin unset($this->replacementMap[$target]); } @@ -758,14 +743,72 @@ protected function aliasPluginAs(string $namespace, string $alias) } /** - * Disables a single plugin in the system. + * Sets the provided flag on the provided plugin + */ + protected function flagPlugin(string $plugin, string $flag): void + { + $plugin = $this->normalizeIdentifier($plugin); + $this->pluginFlags[$plugin][$flag] = true; + } + + /** + * Removes the provided flag from the provided plugin + */ + protected function unflagPlugin(string $plugin, string $flag): void + { + $plugin = $this->normalizeIdentifier($plugin); + unset($this->pluginFlags[$plugin][$flag]); + } + + /** + * Get the PluginVersion record for the provided plugin * - * @param string|PluginBase $id Plugin code/namespace - * @param bool $isUser Set to true if disabled by the user, false by default - * @return bool Returns false if the plugin was already disabled, true otherwise + * @throws InvalidArgumentException if unable to find the requested plugin record in the database + */ + protected function getPluginRecord(string $plugin): PluginVersion + { + $plugin = $this->normalizeIdentifier($plugin); + if (isset($this->pluginRecords[$plugin])) { + return $this->pluginRecords[$plugin]; + } + + $record = PluginVersion::where('code', $plugin)->first(); + if (!$record) { + throw new InvalidArgumentException("$plugin was not found in the database."); + } + + return $this->pluginRecords[$plugin] = $record; + } + + /** + * Flags the provided plugin as "frozen" (updates cannot be downloaded / installed) + */ + public function freezePlugin(string $plugin): void + { + $record = $this->getPluginRecord($plugin); + $record->is_frozen = true; + $record->save(); + } + + /** + * "Unfreezes" the provided plugin, allowing for updates to be performed + */ + public function unfreezePlugin(string $plugin): void + { + $record = $this->getPluginRecord($plugin); + $record->is_frozen = false; + $record->save(); + } + + /** + * Disables the provided plugin using the provided flag (defaults to static::DISABLED_BY_USER) */ - public function disablePlugin($id, $isUser = false) + public function disablePlugin(string $plugin, $flag = static::DISABLED_BY_USER): bool { + if ($flag === true) { + $flag = static::DISABLED_BY_USER; + } + $code = $this->getIdentifier($id); $code = $this->normalizeIdentifier($code); if (isset($this->disabledPlugins[$code])) { @@ -775,22 +818,27 @@ public function disablePlugin($id, $isUser = false) $this->disabledPlugins[$code] = $isUser; $this->writeDisabled(); - if ($pluginObj = $this->findByIdentifier($code, true)) { - $pluginObj->disabled = true; + // Updates the database record for the plugin if required + if ($flag === static::DISABLED_BY_USER) { + $record = $this->getPluginRecord($plugin); + $record->is_disabled = true; + $record->save(); } + // @TODO: Update the cahce of disabled plugins + return true; } /** - * Enables a single plugin in the system. - * - * @param string|PluginBase $id Plugin code/namespace - * @param bool $isUser Set to true if enabled by the user, false by default - * @return bool Returns false if the plugin wasn't already disabled or if the user disabled a plugin that the system is trying to re-enable, true otherwise + * Enables the provided plugin using the provided flag (defaults to static::DISABLED_BY_USER) */ - public function enablePlugin($id, $isUser = false) + public function enablePlugin(string $plugin, $flag = static::DISABLED_BY_USER): bool { + if ($flag === true) { + $flag = static::DISABLED_BY_USER; + } + $code = $this->getIdentifier($id); $code = $this->normalizeIdentifier($code); if (!isset($this->disabledPlugins[$code])) { @@ -805,10 +853,6 @@ public function enablePlugin($id, $isUser = false) unset($this->disabledPlugins[$code]); $this->writeDisabled(); - if ($pluginObj = $this->findByIdentifier($code, true)) { - $pluginObj->disabled = false; - } - return true; } @@ -850,32 +894,15 @@ public function findMissingDependencies() } /** - * Cross checks all plugins and their dependancies, if not met plugins - * are disabled and vice versa. - * - * @return void + * Checks plugin dependencies and flags plugins with missing dependencies as disabled */ - protected function loadDependencies() + protected function loadDependencies(): void { foreach ($this->plugins as $id => $plugin) { - if (!$required = $this->getDependencies($plugin)) { - continue; - } - - $disable = false; - - foreach ($required as $require) { - if (!$pluginObj = $this->findByIdentifier($require)) { - $disable = true; - } elseif ($pluginObj->disabled) { - $disable = true; - } - } - - if ($disable) { - $this->disablePlugin($id); + if (!$plugin->checkDependencies()) { + $this->flagPlugin($id, static::DISABLED_MISSING_DEPENDENCIES); } else { - $this->enablePlugin($id); + $this->unflagPlugin($id, static::DISABLED_MISSING_DEPENDENCIES); } } } @@ -961,11 +988,8 @@ protected function sortDependencies() /** * Returns the plugin identifiers that are required by the supplied plugin. - * - * @param string $plugin Plugin identifier, object or class - * @return array */ - public function getDependencies($plugin) + public function getDependencies(string|PluginBase $plugin): array { if (is_string($plugin) && (!$plugin = $this->findByIdentifier($plugin))) { return []; diff --git a/modules/system/console/PluginDisable.php b/modules/system/console/PluginDisable.php index 0169fb813a..f531ed3112 100644 --- a/modules/system/console/PluginDisable.php +++ b/modules/system/console/PluginDisable.php @@ -41,10 +41,6 @@ public function handle() // Disable this plugin $pluginManager->disablePlugin($pluginName); - $plugin = PluginVersion::where('code', $pluginName)->first(); - $plugin->is_disabled = true; - $plugin->save(); - $pluginManager->clearDisabledCache(); $this->output->writeln(sprintf('%s: disabled.', $pluginName)); } diff --git a/modules/system/console/PluginEnable.php b/modules/system/console/PluginEnable.php index eae8d3c772..f2ec3ae3e5 100644 --- a/modules/system/console/PluginEnable.php +++ b/modules/system/console/PluginEnable.php @@ -46,10 +46,6 @@ public function handle() // Enable this plugin $pluginManager->enablePlugin($pluginName); - $plugin = PluginVersion::where('code', $pluginName)->first(); - $plugin->is_disabled = false; - $plugin->save(); - $pluginManager->clearDisabledCache(); $this->output->writeln(sprintf('%s: enabled.', $pluginName)); } diff --git a/modules/system/controllers/Updates.php b/modules/system/controllers/Updates.php index 69b730c444..f19a752e56 100644 --- a/modules/system/controllers/Updates.php +++ b/modules/system/controllers/Updates.php @@ -845,52 +845,45 @@ public function onBulkAction() count($checkedIds) ) { $manager = PluginManager::instance(); + $codes = PluginVersion::lists('code', 'id'); - foreach ($checkedIds as $pluginId) { - if (!$plugin = PluginVersion::find($pluginId)) { + foreach ($checkedIds as $id) { + $code = $codes[$id] ?? null; + if (!$code) { continue; } - $savePlugin = true; switch ($bulkAction) { // Enables plugin's updates. case 'freeze': - $plugin->is_frozen = 1; + $manager->freezePlugin($code); break; // Disables plugin's updates. case 'unfreeze': - $plugin->is_frozen = 0; + $manager->unfreezePlugin($code); break; // Disables plugin on the system. case 'disable': - $plugin->is_disabled = 1; - $manager->disablePlugin($plugin->code, true); + $manager->disablePlugin($code); break; // Enables plugin on the system. case 'enable': - $plugin->is_disabled = 0; - $manager->enablePlugin($plugin->code, true); + $manager->enablePlugin($code); break; // Rebuilds plugin database migrations. case 'refresh': - $savePlugin = false; - $manager->refreshPlugin($plugin->code); + $manager->refreshPlugin($code); break; // Rollback and remove plugins from the system. case 'remove': - $savePlugin = false; - $manager->deletePlugin($plugin->code); + $manager->deletePlugin($code); break; } - - if ($savePlugin) { - $plugin->save(); - } } } diff --git a/modules/system/models/PluginVersion.php b/modules/system/models/PluginVersion.php index 30b8ece054..96e73f28d4 100644 --- a/modules/system/models/PluginVersion.php +++ b/modules/system/models/PluginVersion.php @@ -96,13 +96,7 @@ public function afterFetch() } } - if ($this->is_disabled) { - $manager->disablePlugin($this->code, true); - } - else { - $manager->enablePlugin($this->code, true); - } - + // @TODO: Determine what flags should trigger this and how to identify them here $this->disabledBySystem = $pluginObj->disabled; if (($configDisabled = Config::get('cms.disablePlugins')) && is_array($configDisabled)) { @@ -118,10 +112,10 @@ public function afterFetch() /** * Returns true if the plugin should be updated by the system. - * @return bool */ - public function getIsUpdatableAttribute() + public function getIsUpdatableAttribute(): bool { + // @TODO: Decide which flags prevent a plugin from being updated return !$this->is_disabled && !$this->disabledBySystem && !$this->disabledByConfig; } From 963e277cb6e7d8a9abcf2600bfb7ff5338ec6ce8 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Thu, 17 Mar 2022 21:30:26 -0600 Subject: [PATCH 02/16] Further work on the improvements --- modules/system/classes/PluginBase.php | 7 +- modules/system/classes/PluginManager.php | 460 +++++++++++------------ 2 files changed, 228 insertions(+), 239 deletions(-) diff --git a/modules/system/classes/PluginBase.php b/modules/system/classes/PluginBase.php index 97568cc001..3c39bbf035 100644 --- a/modules/system/classes/PluginBase.php +++ b/modules/system/classes/PluginBase.php @@ -404,8 +404,6 @@ public function getPluginIdentifier(): string /** * Returns the absolute path to this plugin's directory - * - * @return string */ public function getPluginPath(): string { @@ -414,7 +412,7 @@ public function getPluginPath(): string } $reflection = new ReflectionClass($this); - $this->path = dirname($reflection->getFileName()); + $this->path = File::normalizePath(dirname($reflection->getFileName())); return $this->path; } @@ -452,9 +450,8 @@ public function getPluginVersion(): string /** * Verifies the plugin's dependencies are present and enabled */ - public function checkDependencies(): bool + public function checkDependencies(PluginManager $manager): bool { - $manager = PluginManager::instance(); $required = $manager->getDependencies($this); if (empty($required)) { return true; diff --git a/modules/system/classes/PluginManager.php b/modules/system/classes/PluginManager.php index 694e73b6e4..bb9135bb7a 100644 --- a/modules/system/classes/PluginManager.php +++ b/modules/system/classes/PluginManager.php @@ -45,9 +45,14 @@ class PluginManager protected $plugins = []; /** - * @var array A map of plugins and their directory paths. + * @var array Array of plugin codes that contain any flags currently associated with the plugin */ - protected $pathMap = []; + protected $pluginFlags = []; + + /** + * @var PluginVersion[] Local cache of loaded PluginVersion records keyed by plugin code + */ + protected $pluginRecords = []; /** * @var array A map of normalized plugin identifiers [lowercase.identifier => Normalized.Identifier] @@ -74,16 +79,6 @@ class PluginManager */ protected $booted = false; - /** - * @var string Cache key for the disabled plugin data - */ - protected $disabledCacheKey = 'system-plugins-disabled'; - - /** - * @var array Array of disabled plugins - */ - protected $disabledPlugins = []; - /** * @var array Cache of registration method results. */ @@ -100,19 +95,22 @@ class PluginManager protected function init() { $this->app = App::make('app'); - $this->loadDisabled(); + + // Load the plugins from the filesystem and sort them by dependencies $this->loadPlugins(); - $this->loadDependencies(); - $this->registerReplacedPlugins(); + // Loads the plugin flags (disabled & replacement states) from the cache + // regenerating them if required. + $this->loadPluginFlags(); + + // Register plugin replacements + $this->registerPluginReplacements(); } /** * Finds all available plugins and loads them in to the $this->plugins array. - * - * @return array */ - public function loadPlugins() + public function loadPlugins(): array { $this->plugins = []; @@ -123,7 +121,8 @@ public function loadPlugins() $this->loadPlugin($namespace, $path); } - $this->sortDependencies(); + // Sort all the plugins by number of dependencies + $this->sortByDependencies(); return $this->plugins; } @@ -133,9 +132,8 @@ public function loadPlugins() * * @param string $namespace Eg: Acme\Blog * @param string $path Eg: plugins_path().'/acme/blog'; - * @return void */ - public function loadPlugin($namespace, $path) + public function loadPlugin(string $namespace, string $path): ?PluginBase { $className = $namespace . '\Plugin'; $classPath = $path . '/Plugin.php'; @@ -148,7 +146,7 @@ public function loadPlugin($namespace, $path) // Not a valid plugin! if (!class_exists($className)) { - return; + return null; } $pluginObj = new $className($this->app); @@ -159,13 +157,12 @@ public function loadPlugin($namespace, $path) 'line' => $e->getLine(), 'trace' => $e->getTraceAsString() ]); - return; + return null; } $classId = $this->getIdentifier($pluginObj); $this->plugins[$classId] = $pluginObj; - $this->pathMap[$classId] = $path; $this->normalizedMap[strtolower($classId)] = $classId; $replaces = $pluginObj->getReplaces(); @@ -178,6 +175,56 @@ public function loadPlugin($namespace, $path) return $pluginObj; } + /** + * Get the cache key for the current plugin manager state + */ + protected function getFlagCacheKey(): string + { + $loadedPlugins = array_keys($this->plugins); + $configDisabledPlugins = Config::get('cms.disablePlugins', []); + if (!is_array($configDisabledPlugins)) { + $configDisabledPlugins = []; + } + $plugins = $loadedPlugins + $configDisabledPlugins; + + return 'system.pluginmanager.state.' . md5(implode('.', $plugins)); + } + + /** + * Loads the plugin flags (disabled & replacement states) from the cache + * regenerating them if required. + */ + public function loadPluginFlags() + { + // Cache the data for a month so that stale keys can be autocleaned if necessary + $data = Cache::remember($this->getFlagCacheKey(), now()->addMonths(1), function () { + // Check the config files & database for plugins to disable + $this->loadDisabled(); + + // Check plugin dependencies for plugins to disable + $this->loadDependencies(); + + // Check plugin replacments for plugins to disable + $this->detectPluginReplacements(); + + return [ + $this->pluginFlags, + $this->replacementMap, + $this->activeReplacementMap, + ]; + }); + + list($this->pluginFlag, $this->replacementMap, $this->activeReplacementMap) = $data; + } + + /** + * Reset the plugin flag cache + */ + protected function clearFlagCache() + { + Cache::forget($this->getFlagCacheKey()); + } + /** * Runs the register() method on all plugins. Can only be called once. * @@ -348,19 +395,15 @@ public function bootPlugin(PluginBase $plugin): void /** * Returns the directory path to a plugin - * - * @param PluginBase|string $id The plugin to get the path for - * @return string|null */ - public function getPluginPath($id) + public function getPluginPath(string|PluginBase $plugin): ?string { - $classId = $this->getIdentifier($id); - $classId = $this->normalizeIdentifier($classId); - if (!isset($this->pathMap[$classId])) { - return null; + $path = null; + $plugin = $this->findByIdentifier($plugin, true); + if ($plugin) { + $path = $plugin->getPluginPath(); } - - return File::normalizePath($this->pathMap[$classId]); + return $path; } /** @@ -369,9 +412,9 @@ public function getPluginPath($id) * @param string $id Plugin identifier, eg: Namespace.PluginName * @return bool */ - public function exists($id) + public function exists(string|PluginBase $plugin): bool { - return $this->findByIdentifier($id) && !$this->isDisabled($id); + return $this->findByIdentifier($plugin) && !$this->isDisabled($plugin); } /** @@ -381,7 +424,7 @@ public function exists($id) */ public function getPlugins() { - return array_diff_key($this->plugins, $this->disabledPlugins); + return array_diff_key($this->plugins, $this->pluginFlags); } /** @@ -409,20 +452,19 @@ public function findByNamespace($namespace) /** * Returns a plugin registration class based on its identifier (Author.Plugin). - * - * @param string|PluginBase $identifier - * @param bool $ignoreReplacements - * @return PluginBase|null */ - public function findByIdentifier($identifier, bool $ignoreReplacements = false) + public function findByIdentifier(string|PluginBase $identifier, bool $ignoreReplacements = false): ?PluginBase { + if ($identifier instanceof PluginBase) { + return $identifier; + } + if (!$ignoreReplacements && is_string($identifier) && isset($this->replacementMap[$identifier])) { $identifier = $this->replacementMap[$identifier]; } if (!isset($this->plugins[$identifier])) { - $code = $this->getIdentifier($identifier); - $identifier = $this->normalizeIdentifier($code); + $identifier = $this->getNormalizedIdentifier($identifier); } return $this->plugins[$identifier] ?? null; @@ -430,24 +472,19 @@ public function findByIdentifier($identifier, bool $ignoreReplacements = false) /** * Checks to see if a plugin has been registered. - * - * @param string|PluginBase - * @return bool */ - public function hasPlugin($namespace) + public function hasPlugin(string|PluginBase $plugin): bool { - $classId = $this->getIdentifier($namespace); - $normalized = $this->normalizeIdentifier($classId); + $normalized = $this->getNormalizedIdentifier($plugin); return isset($this->plugins[$normalized]) || isset($this->replacementMap[$normalized]); } /** * Returns a flat array of vendor plugin namespaces and their paths - * - * @return array ['Author\Plugin' => 'plugins/author/plugin'] + * ['Author\Plugin' => 'plugins/author/plugin'] */ - public function getPluginNamespaces() + public function getPluginNamespaces(): array { $classNames = []; @@ -464,10 +501,9 @@ public function getPluginNamespaces() /** * Returns a 2 dimensional array of vendors and their plugins. - * - * @return array ['vendor' => ['author' => 'plugins/author/plugin']] + * ['vendor' => ['author' => 'plugins/author/plugin']] */ - public function getVendorAndPluginNames() + public function getVendorAndPluginNames(): array { $plugins = []; @@ -497,14 +533,12 @@ public function getVendorAndPluginNames() } /** - * Resolves a plugin identifier (Author.Plugin) from a plugin class name or object. - * - * @param mixed Plugin class name or object - * @return string Identifier in format of Author.Plugin + * Resolves a plugin identifier (Author.Plugin) from a plugin class name + * (Author\Plugin) or PluginBase instance. */ - public function getIdentifier($namespace) + public function getIdentifier(string|PluginBase $plugin): string { - $namespace = Str::normalizeClassName($namespace); + $namespace = Str::normalizeClassName($plugin); if (strpos($namespace, '\\') === null) { return $namespace; } @@ -517,21 +551,20 @@ public function getIdentifier($namespace) } /** - * Resolves a plugin namespace (Author\Plugin) from a plugin class name, identifier or object. - * - * @param mixed Plugin class name, identifier or object - * @return string Namespace in format of Author\Plugin + * Resolves a plugin namespace (Author\Plugin) from a plugin class name + * (Author\Plugin\Classes\Example), identifier (Author.Plugin), or + * PluginBase instance. */ - public function getNamespace($identifier) + public function getNamespace(string|PluginBase $plugin): string { if ( - is_object($identifier) - || (is_string($identifier) && strpos($identifier, '.') === null) + is_object($plugin) + || (is_string($plugin) && strpos($plugin, '.') === null) ) { - return Str::normalizeClassName($identifier); + return Str::normalizeClassName($plugin); } - $parts = explode('.', $identifier); + $parts = explode('.', $plugin); $slice = array_slice($parts, 0, 2); $namespace = implode('\\', $slice); @@ -539,29 +572,34 @@ public function getNamespace($identifier) } /** - * Takes a human plugin code (acme.blog) and makes it authentic (Acme.Blog) + * Normalizes the provided plugin identifier (author.plugin) and resolves + * it case-insensitively to the normalized identifier (Author.Plugin) * Returns the provided identifier if a match isn't found - * - * @param string $identifier - * @return string */ - public function normalizeIdentifier($identifier) + public function normalizeIdentifier(string $code): string { - $id = strtolower($identifier); - if (isset($this->normalizedMap[$id])) { - return $this->normalizedMap[$id]; + $code = strtolower($code); + if (isset($this->normalizedMap[$code])) { + return $this->normalizedMap[$code]; } - return $identifier; + return $code; } /** - * Spins over every plugin object and collects the results of a method call. Results are cached in memory. - * - * @param string $methodName - * @return array + * Returns the normalized identifier (i.e. Winter.Blog) from the provided + * string or PluginBase instance. + */ + public function getNormalizedIdentifier(string|PluginBase $plugin): string + { + return $this->normalizeIdentifier($this->getIdentifier($plugin)); + } + + /** + * Spins over every plugin object and collects the results of the provided + * method call. Results are cached in memory. */ - public function getRegistrationMethodValues($methodName) + public function getRegistrationMethodValues(string $methodName): array { if (isset($this->registrationMethodCache[$methodName])) { return $this->registrationMethodCache[$methodName]; @@ -582,27 +620,31 @@ public function getRegistrationMethodValues($methodName) } // - // Disability + // State Management (Disable, Enable, Freeze, Unfreeze) // /** - * Clears the disabled plugins cache file - * - * @return void + * Sets the provided flag on the provided plugin */ - public function clearDisabledCache() + protected function flagPlugin(string|PluginBase $plugin, string $flag): void { - // Storage::delete($this->metaFile); - Cache::forget($this->disabledCacheKey); - $this->disabledPlugins = []; + $code = $this->getNormalizedIdentifier($plugin); + $this->pluginFlags[$code][$flag] = true; + } + + /** + * Removes the provided flag from the provided plugin + */ + protected function unflagPlugin(string|PluginBase $plugin, string $flag): void + { + $code = $this->getNormalizedIdentifier($plugin); + unset($this->pluginFlags[$code][$flag]); } /** * Loads all disabled plugins from the cached JSON file. - * - * @return void */ - protected function loadDisabled() + protected function loadDisabled(): void { // Check the config files for disabled plugins if (($configDisabled = Config::get('cms.disablePlugins')) && is_array($configDisabled)) { @@ -612,111 +654,75 @@ protected function loadDisabled() } // Check the database for disabled plugins - $userDisabled = Cache::rememberForever($this->disabledCacheKey, function () { - return $this->getDisabledPluginsFromDb(); - }); - foreach ($userDisabled as $code) { - $this->flagPlugin($code, static::DISABLED_BY_USER); + if ( + $this->app->hasDatabase() + && Schema::hasTable('system_plugin_versions') + ) { + $userDisabled = Db::table('system_plugin_versions')->where('is_disabled', 1)->lists('code') ?? []; + foreach ($userDisabled as $code) { + $this->flagPlugin($code, static::DISABLED_BY_USER); + } } } /** * Determines if a plugin is disabled by looking at the meta information * or the application configuration. - * - * @param string|PluginBase $id - * @return bool */ - public function isDisabled($id) + public function isDisabled(string|PluginBase $plugin): bool { - $code = $this->getIdentifier($id); - $normalized = $this->normalizeIdentifier($code); + $code = $this->getNormalizedIdentifier($plugin); - return isset($this->disabledPlugins[$normalized]); - } - - /** - * Write the disabled plugins to a meta file. - * - * @return void - */ - protected function writeDisabled() - { - Cache::forever($this->disabledCacheKey, $this->disabledPlugins); - } - - /** - * Returns plugin codes that have been flagged disabled in the database - */ - protected function getDisabledPluginsFromDb(): array - { - $disabled = []; - - if ( - $this->app->hasDatabase() - && Schema::hasTable('system_plugin_versions') - ) { - $disabled = Db::table('system_plugin_versions')->where('is_disabled', 1)->lists('code') ?? []; - } - - return $disabled; + // @TODO: Limit this to only disabled flags if we add more than disabled flags + return !empty($this->pluginFlags[$code]); } /** * Returns the plugin replacements defined in $this->replacementMap - * - * @return array */ - public function getReplacementMap() + public function getReplacementMap(): array { return $this->replacementMap; } /** * Returns the actively replaced plugins defined in $this->activeReplacementMap - * @param string $pluginIdentifier Plugin code/namespace - * @return array|null */ - public function getActiveReplacementMap(string $pluginIdentifier = null) + public function getActiveReplacementMap(string|PluginBase|null $plugin = null): array|string|null { - if (!$pluginIdentifier) { + if (!$plugin) { return $this->activeReplacementMap; } - return $this->activeReplacementMap[$pluginIdentifier] ?? null; + return $this->activeReplacementMap[$this->getNormalizedIdentifier($plugin)] ?? null; } /** - * Evaluates and initializes the plugin replacements defined in $this->replacementMap - * - * @return void + * Evaluates the replacement map to determine which replacements can actually + * take effect */ - public function registerReplacedPlugins() + protected function detectPluginReplacements(): void { if (empty($this->replacementMap)) { return; } foreach ($this->replacementMap as $target => $replacement) { - // Alias the replaced plugin to the replacing plugin if the replaced plugin isn't present + // If the replaced plugin isn't present then assume it can be replaced if (!isset($this->plugins[$target])) { - $this->aliasPluginAs($replacement, $target); continue; } // Only allow one of the replaced plugin or the replacing plugin to exist // at once depending on whether the version constraints are met or not if ($this->plugins[$replacement]->canReplacePlugin($target, $this->plugins[$target]->getPluginVersion())) { - // Alias the replaced plugin to the replacing plugin - $this->aliasPluginAs($replacement, $target); - - // Set the plugin flags + // Set the plugin flags to disable the target plugin $this->flagPlugin($target, static::DISABLED_REPLACED); $this->unflagPlugin($replacement, static::DISABLED_REPLACEMENT_FAILED); - // Register this plugin as actively replaced + // Register this plugin as actively replaced (i.e. both are present, replaced are disabled) $this->activeReplacementMap[$target] = $replacement; } else { - // Set the plugin flags + // Set the plugin flags to disable the replacement plugin $this->flagPlugin($replacement, static::DISABLED_REPLACEMENT_FAILED); $this->unflagPlugin($target, static::DISABLED_REPLACED); @@ -726,14 +732,21 @@ public function registerReplacedPlugins() } } + /** + * Executes the plugin replacements defined in the activeReplacementMap property + */ + protected function registerPluginReplacements(): void + { + foreach ($this->replacementMap as $target => $replacement) { + // Alias the replaced plugin to the replacing plugin + $this->aliasPluginAs($replacement, $target); + } + } + /** * Registers namespace aliasing for multiple subsystems - * - * @param string $namespace Plugin code - * @param string $alias Plugin alias code - * @return void */ - protected function aliasPluginAs(string $namespace, string $alias) + protected function aliasPluginAs(string $namespace, string $alias): void { Lang::registerNamespaceAlias($namespace, $alias); Config::registerNamespaceAlias($namespace, $alias); @@ -742,32 +755,14 @@ protected function aliasPluginAs(string $namespace, string $alias) NavigationManager::lazyRegisterOwnerAlias($namespace, $alias); } - /** - * Sets the provided flag on the provided plugin - */ - protected function flagPlugin(string $plugin, string $flag): void - { - $plugin = $this->normalizeIdentifier($plugin); - $this->pluginFlags[$plugin][$flag] = true; - } - - /** - * Removes the provided flag from the provided plugin - */ - protected function unflagPlugin(string $plugin, string $flag): void - { - $plugin = $this->normalizeIdentifier($plugin); - unset($this->pluginFlags[$plugin][$flag]); - } - /** * Get the PluginVersion record for the provided plugin * * @throws InvalidArgumentException if unable to find the requested plugin record in the database */ - protected function getPluginRecord(string $plugin): PluginVersion + protected function getPluginRecord(string|PluginBase $plugin): PluginVersion { - $plugin = $this->normalizeIdentifier($plugin); + $plugin = $this->getNormalizedIdentifier($plugin); if (isset($this->pluginRecords[$plugin])) { return $this->pluginRecords[$plugin]; } @@ -783,7 +778,7 @@ protected function getPluginRecord(string $plugin): PluginVersion /** * Flags the provided plugin as "frozen" (updates cannot be downloaded / installed) */ - public function freezePlugin(string $plugin): void + public function freezePlugin(string|PluginBase $plugin): void { $record = $this->getPluginRecord($plugin); $record->is_frozen = true; @@ -793,7 +788,7 @@ public function freezePlugin(string $plugin): void /** * "Unfreezes" the provided plugin, allowing for updates to be performed */ - public function unfreezePlugin(string $plugin): void + public function unfreezePlugin(string|PluginBase $plugin): void { $record = $this->getPluginRecord($plugin); $record->is_frozen = false; @@ -803,29 +798,28 @@ public function unfreezePlugin(string $plugin): void /** * Disables the provided plugin using the provided flag (defaults to static::DISABLED_BY_USER) */ - public function disablePlugin(string $plugin, $flag = static::DISABLED_BY_USER): bool + public function disablePlugin(string|PluginBase $plugin, $flag = self::DISABLED_BY_USER): bool { + // $flag used to be (bool) $byUser if ($flag === true) { $flag = static::DISABLED_BY_USER; } - $code = $this->getIdentifier($id); - $code = $this->normalizeIdentifier($code); - if (isset($this->disabledPlugins[$code])) { - return false; - } - - $this->disabledPlugins[$code] = $isUser; - $this->writeDisabled(); + // Flag the plugin as disabled + $this->flagPlugin($plugin, $flag); // Updates the database record for the plugin if required if ($flag === static::DISABLED_BY_USER) { $record = $this->getPluginRecord($plugin); $record->is_disabled = true; $record->save(); + + // Clear the cache so that the next request will regenerate the active flags + $this->clearFlagCache(); } - // @TODO: Update the cahce of disabled plugins + // Clear the registration values cache + $this->registrationMethodCache = []; return true; } @@ -833,25 +827,28 @@ public function disablePlugin(string $plugin, $flag = static::DISABLED_BY_USER): /** * Enables the provided plugin using the provided flag (defaults to static::DISABLED_BY_USER) */ - public function enablePlugin(string $plugin, $flag = static::DISABLED_BY_USER): bool + public function enablePlugin(string $plugin, $flag = self::DISABLED_BY_USER): bool { + // $flag used to be (bool) $byUser if ($flag === true) { $flag = static::DISABLED_BY_USER; } - $code = $this->getIdentifier($id); - $code = $this->normalizeIdentifier($code); - if (!isset($this->disabledPlugins[$code])) { - return false; - } + // Unflag the plugin as disabled + $this->unflagPlugin($plugin, $flag); + + // Updates the database record for the plugin if required + if ($flag === static::DISABLED_BY_USER) { + $record = $this->getPluginRecord($plugin); + $record->is_disabled = false; + $record->save(); - // Prevent system from enabling plugins disabled by the user - if (!$isUser && $this->disabledPlugins[$code] === true) { - return false; + // Clear the cache so that the next request will regenerate the active flags + $this->clearFlagCache(); } - unset($this->disabledPlugins[$code]); - $this->writeDisabled(); + // Clear the registration values cache + $this->registrationMethodCache = []; return true; } @@ -860,6 +857,24 @@ public function enablePlugin(string $plugin, $flag = static::DISABLED_BY_USER): // Dependencies // + /** + * Returns the plugin identifiers that are required by the supplied plugin. + */ + public function getDependencies(string|PluginBase $plugin): array + { + if (is_string($plugin) && (!$plugin = $this->findByIdentifier($plugin))) { + return []; + } + + if (!isset($plugin->require) || !$plugin->require) { + return []; + } + + return array_map(function ($require) { + return $this->replacementMap[$require] ?? $require; + }, is_array($plugin->require) ? $plugin->require : [$plugin->require]); + } + /** * Scans the system plugins to locate any dependencies that are not currently * installed. Returns an array of missing plugin codes keyed by the plugin that requires them. @@ -899,7 +914,7 @@ public function findMissingDependencies() protected function loadDependencies(): void { foreach ($this->plugins as $id => $plugin) { - if (!$plugin->checkDependencies()) { + if (!$plugin->checkDependencies($this)) { $this->flagPlugin($id, static::DISABLED_MISSING_DEPENDENCIES); } else { $this->unflagPlugin($id, static::DISABLED_MISSING_DEPENDENCIES); @@ -914,7 +929,7 @@ protected function loadDependencies(): void * @return array Array of sorted plugin identifiers and instantiated classes ['Author.Plugin' => PluginBase] * @throws SystemException If a possible circular dependency is detected */ - protected function sortDependencies() + protected function sortByDependencies(): array { ksort($this->plugins); @@ -986,35 +1001,6 @@ protected function sortDependencies() return $this->plugins = $sortedPlugins; } - /** - * Returns the plugin identifiers that are required by the supplied plugin. - */ - public function getDependencies(string|PluginBase $plugin): array - { - if (is_string($plugin) && (!$plugin = $this->findByIdentifier($plugin))) { - return []; - } - - if (!isset($plugin->require) || !$plugin->require) { - return []; - } - - return array_map(function ($require) { - return $this->replacementMap[$require] ?? $require; - }, is_array($plugin->require) ? $plugin->require : [$plugin->require]); - } - - /** - * @deprecated Plugins are now sorted by default. See getPlugins() - * Remove if year >= 2022 - */ - public function sortByDependencies($plugins = null) - { - traceLog('PluginManager::sortByDependencies is deprecated. Plugins are now sorted by default. Use PluginManager::getPlugins()'); - - return array_keys($plugins ?: $this->getPlugins()); - } - // // Management // @@ -1037,6 +1023,12 @@ public function deletePlugin($id) */ if ($pluginPath = self::instance()->getPluginPath($id)) { File::deleteDirectory($pluginPath); + + // Clear the registration values cache + $this->registrationMethodCache = []; + + // Clear the plugin flag cache + $this->clearFlagCache(); } } From 5cc8161f0a99a4840086e7d40b24b69fc4cb423f Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Wed, 18 May 2022 16:58:47 +0100 Subject: [PATCH 03/16] Added winter cache provider override --- modules/system/providers.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/system/providers.php b/modules/system/providers.php index 9fb16b4893..fa44814b39 100644 --- a/modules/system/providers.php +++ b/modules/system/providers.php @@ -7,7 +7,6 @@ */ Illuminate\Broadcasting\BroadcastServiceProvider::class, Illuminate\Bus\BusServiceProvider::class, - Illuminate\Cache\CacheServiceProvider::class, Illuminate\Cookie\CookieServiceProvider::class, Illuminate\Encryption\EncryptionServiceProvider::class, Illuminate\Foundation\Providers\FoundationServiceProvider::class, @@ -22,6 +21,7 @@ /* * Winter Storm providers */ + Winter\Storm\Cache\CacheServiceProvider::class, Winter\Storm\Foundation\Providers\ConsoleSupportServiceProvider::class, Winter\Storm\Database\DatabaseServiceProvider::class, Winter\Storm\Halcyon\HalcyonServiceProvider::class, From b969e8fd17d3ec6af2a2ae4da7b10a2317c84477 Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Mon, 30 May 2022 15:53:28 +0100 Subject: [PATCH 04/16] Added fix for status widget --- modules/system/reportwidgets/Status.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/system/reportwidgets/Status.php b/modules/system/reportwidgets/Status.php index f078642fea..a2fff0eb12 100644 --- a/modules/system/reportwidgets/Status.php +++ b/modules/system/reportwidgets/Status.php @@ -69,7 +69,7 @@ protected function loadData() $this->vars['requestLog'] = RequestLog::count(); $this->vars['requestLogMsg'] = LogSetting::get('log_requests', false) ? false : true; - $this->vars['appBirthday'] = PluginVersion::orderBy('created_at')->value('created_at'); + $this->vars['appBirthday'] = PluginVersion::orderBy('created_at')->first()->created_at; } public function onLoadWarningsForm() From 1716ec502232ba1e9d517e05668e7ff36d274ac9 Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Mon, 30 May 2022 15:58:31 +0100 Subject: [PATCH 05/16] Improved type hinting and conditional logic --- modules/system/classes/PluginManager.php | 80 ++++++++++-------------- 1 file changed, 33 insertions(+), 47 deletions(-) diff --git a/modules/system/classes/PluginManager.php b/modules/system/classes/PluginManager.php index bb9135bb7a..83dabfbdf4 100644 --- a/modules/system/classes/PluginManager.php +++ b/modules/system/classes/PluginManager.php @@ -11,8 +11,10 @@ use Config; use Schema; use SystemException; +use FilesystemIterator; use RecursiveIteratorIterator; use RecursiveDirectoryIterator; +use System\Models\PluginVersion; use Winter\Storm\Support\ClassLoader; use Backend\Classes\NavigationManager; @@ -382,11 +384,7 @@ public function bootAll($force = false) */ public function bootPlugin(PluginBase $plugin): void { - if ( - !$plugin - || (self::$noInit && !$plugin->elevated) - || $this->isDisabled($plugin) - ) { + if ((self::$noInit && !$plugin->elevated) || $this->isDisabled($plugin)) { return; } @@ -398,12 +396,7 @@ public function bootPlugin(PluginBase $plugin): void */ public function getPluginPath(string|PluginBase $plugin): ?string { - $path = null; - $plugin = $this->findByIdentifier($plugin, true); - if ($plugin) { - $path = $plugin->getPluginPath(); - } - return $path; + return $this->findByIdentifier($plugin, true)?->getPluginPath(); } /** @@ -412,7 +405,7 @@ public function getPluginPath(string|PluginBase $plugin): ?string * @param string $id Plugin identifier, eg: Namespace.PluginName * @return bool */ - public function exists(string|PluginBase $plugin): bool + public function exists(PluginBase|string $plugin): bool { return $this->findByIdentifier($plugin) && !$this->isDisabled($plugin); } @@ -453,7 +446,7 @@ public function findByNamespace($namespace) /** * Returns a plugin registration class based on its identifier (Author.Plugin). */ - public function findByIdentifier(string|PluginBase $identifier, bool $ignoreReplacements = false): ?PluginBase + public function findByIdentifier(PluginBase|string $identifier, bool $ignoreReplacements = false): ?PluginBase { if ($identifier instanceof PluginBase) { return $identifier; @@ -473,7 +466,7 @@ public function findByIdentifier(string|PluginBase $identifier, bool $ignoreRepl /** * Checks to see if a plugin has been registered. */ - public function hasPlugin(string|PluginBase $plugin): bool + public function hasPlugin(PluginBase|string $plugin): bool { $normalized = $this->getNormalizedIdentifier($plugin); @@ -513,13 +506,13 @@ public function getVendorAndPluginNames(): array } $it = new RecursiveIteratorIterator( - new RecursiveDirectoryIterator($dirPath, RecursiveDirectoryIterator::FOLLOW_SYMLINKS) + new RecursiveDirectoryIterator($dirPath, FilesystemIterator::FOLLOW_SYMLINKS) ); $it->setMaxDepth(2); $it->rewind(); while ($it->valid()) { - if (($it->getDepth() > 1) && $it->isFile() && (strtolower($it->getFilename()) == "plugin.php")) { + if (($it->getDepth() > 1) && $it->isFile() && (strtolower($it->getFilename()) === "plugin.php")) { $filePath = dirname($it->getPathname()); $pluginName = basename($filePath); $vendorName = basename(dirname($filePath)); @@ -536,7 +529,7 @@ public function getVendorAndPluginNames(): array * Resolves a plugin identifier (Author.Plugin) from a plugin class name * (Author\Plugin) or PluginBase instance. */ - public function getIdentifier(string|PluginBase $plugin): string + public function getIdentifier(PluginBase|string $plugin): string { $namespace = Str::normalizeClassName($plugin); if (strpos($namespace, '\\') === null) { @@ -555,20 +548,17 @@ public function getIdentifier(string|PluginBase $plugin): string * (Author\Plugin\Classes\Example), identifier (Author.Plugin), or * PluginBase instance. */ - public function getNamespace(string|PluginBase $plugin): string + public function getNamespace(PluginBase|string $plugin): string { - if ( - is_object($plugin) - || (is_string($plugin) && strpos($plugin, '.') === null) - ) { - return Str::normalizeClassName($plugin); - } + if (is_string($plugin) && strpos($plugin, '.') !== null) { + $parts = explode('.', $plugin); + $slice = array_slice($parts, 0, 2); + $namespace = implode('\\', $slice); - $parts = explode('.', $plugin); - $slice = array_slice($parts, 0, 2); - $namespace = implode('\\', $slice); + return Str::normalizeClassName($namespace); + } - return Str::normalizeClassName($namespace); + return Str::normalizeClassName($plugin); } /** @@ -579,18 +569,14 @@ public function getNamespace(string|PluginBase $plugin): string public function normalizeIdentifier(string $code): string { $code = strtolower($code); - if (isset($this->normalizedMap[$code])) { - return $this->normalizedMap[$code]; - } - - return $code; + return $this->normalizedMap[$code] ?? $code; } /** * Returns the normalized identifier (i.e. Winter.Blog) from the provided * string or PluginBase instance. */ - public function getNormalizedIdentifier(string|PluginBase $plugin): string + public function getNormalizedIdentifier(PluginBase|string $plugin): string { return $this->normalizeIdentifier($this->getIdentifier($plugin)); } @@ -626,7 +612,7 @@ public function getRegistrationMethodValues(string $methodName): array /** * Sets the provided flag on the provided plugin */ - protected function flagPlugin(string|PluginBase $plugin, string $flag): void + protected function flagPlugin(PluginBase|string $plugin, string $flag): void { $code = $this->getNormalizedIdentifier($plugin); $this->pluginFlags[$code][$flag] = true; @@ -635,7 +621,7 @@ protected function flagPlugin(string|PluginBase $plugin, string $flag): void /** * Removes the provided flag from the provided plugin */ - protected function unflagPlugin(string|PluginBase $plugin, string $flag): void + protected function unflagPlugin(PluginBase|string $plugin, string $flag): void { $code = $this->getNormalizedIdentifier($plugin); unset($this->pluginFlags[$code][$flag]); @@ -669,7 +655,7 @@ protected function loadDisabled(): void * Determines if a plugin is disabled by looking at the meta information * or the application configuration. */ - public function isDisabled(string|PluginBase $plugin): bool + public function isDisabled(PluginBase|string $plugin): bool { $code = $this->getNormalizedIdentifier($plugin); @@ -688,12 +674,11 @@ public function getReplacementMap(): array /** * Returns the actively replaced plugins defined in $this->activeReplacementMap */ - public function getActiveReplacementMap(string|PluginBase|null $plugin = null): array|string|null + public function getActiveReplacementMap(PluginBase|string $plugin = null): array|string|null { - if (!$plugin) { - return $this->activeReplacementMap; - } - return $this->activeReplacementMap[$this->getNormalizedIdentifier($plugin)] ?? null; + return $plugin + ? $this->activeReplacementMap[$this->getNormalizedIdentifier($plugin)] ?? null + : $this->activeReplacementMap; } /** @@ -760,7 +745,7 @@ protected function aliasPluginAs(string $namespace, string $alias): void * * @throws InvalidArgumentException if unable to find the requested plugin record in the database */ - protected function getPluginRecord(string|PluginBase $plugin): PluginVersion + protected function getPluginRecord(PluginBase|string $plugin): PluginVersion { $plugin = $this->getNormalizedIdentifier($plugin); if (isset($this->pluginRecords[$plugin])) { @@ -768,8 +753,9 @@ protected function getPluginRecord(string|PluginBase $plugin): PluginVersion } $record = PluginVersion::where('code', $plugin)->first(); + if (!$record) { - throw new InvalidArgumentException("$plugin was not found in the database."); + throw new \InvalidArgumentException("$plugin was not found in the database."); } return $this->pluginRecords[$plugin] = $record; @@ -778,7 +764,7 @@ protected function getPluginRecord(string|PluginBase $plugin): PluginVersion /** * Flags the provided plugin as "frozen" (updates cannot be downloaded / installed) */ - public function freezePlugin(string|PluginBase $plugin): void + public function freezePlugin(PluginBase|string $plugin): void { $record = $this->getPluginRecord($plugin); $record->is_frozen = true; @@ -788,7 +774,7 @@ public function freezePlugin(string|PluginBase $plugin): void /** * "Unfreezes" the provided plugin, allowing for updates to be performed */ - public function unfreezePlugin(string|PluginBase $plugin): void + public function unfreezePlugin(PluginBase|string $plugin): void { $record = $this->getPluginRecord($plugin); $record->is_frozen = false; @@ -798,7 +784,7 @@ public function unfreezePlugin(string|PluginBase $plugin): void /** * Disables the provided plugin using the provided flag (defaults to static::DISABLED_BY_USER) */ - public function disablePlugin(string|PluginBase $plugin, $flag = self::DISABLED_BY_USER): bool + public function disablePlugin(PluginBase|string $plugin, string|bool $flag = self::DISABLED_BY_USER): bool { // $flag used to be (bool) $byUser if ($flag === true) { From f98fd061783bab9ef11cc41f36b3dfb99c6884b2 Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Tue, 31 May 2022 17:15:40 +0100 Subject: [PATCH 06/16] Added type hints and switched path resolve to use app --- modules/system/classes/PluginManager.php | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/modules/system/classes/PluginManager.php b/modules/system/classes/PluginManager.php index 83dabfbdf4..4d04d1c4e9 100644 --- a/modules/system/classes/PluginManager.php +++ b/modules/system/classes/PluginManager.php @@ -15,6 +15,7 @@ use RecursiveIteratorIterator; use RecursiveDirectoryIterator; use System\Models\PluginVersion; +use Winter\Storm\Foundation\Application; use Winter\Storm\Support\ClassLoader; use Backend\Classes\NavigationManager; @@ -39,7 +40,7 @@ class PluginManager /** * The application instance, since Plugins are an extension of a Service Provider */ - protected $app; + protected Application $app; /** * @var PluginBase[] Container array used for storing plugin information objects. @@ -483,7 +484,7 @@ public function getPluginNamespaces(): array foreach ($this->getVendorAndPluginNames() as $vendorName => $vendorList) { foreach ($vendorList as $pluginName => $pluginPath) { - $namespace = '\\'.$vendorName.'\\'.$pluginName; + $namespace = '\\' . $vendorName . '\\' . $pluginName; $namespace = Str::normalizeClassName($namespace); $classNames[$namespace] = $pluginPath; } @@ -500,7 +501,7 @@ public function getVendorAndPluginNames(): array { $plugins = []; - $dirPath = plugins_path(); + $dirPath = $this->app->pluginsPath(); if (!File::isDirectory($dirPath)) { return $plugins; } @@ -609,6 +610,12 @@ public function getRegistrationMethodValues(string $methodName): array // State Management (Disable, Enable, Freeze, Unfreeze) // + public function getPluginFlags(PluginBase|string $plugin): array + { + $code = $this->getNormalizedIdentifier($plugin); + return $this->pluginFlags[$code] ?? []; + } + /** * Sets the provided flag on the provided plugin */ @@ -813,7 +820,7 @@ public function disablePlugin(PluginBase|string $plugin, string|bool $flag = sel /** * Enables the provided plugin using the provided flag (defaults to static::DISABLED_BY_USER) */ - public function enablePlugin(string $plugin, $flag = self::DISABLED_BY_USER): bool + public function enablePlugin(PluginBase|string $plugin, $flag = self::DISABLED_BY_USER): bool { // $flag used to be (bool) $byUser if ($flag === true) { From bf0e4a5e4e1d8ea65d5f09929f1d3916f571af80 Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Tue, 31 May 2022 17:58:57 +0100 Subject: [PATCH 07/16] Fixed issue with PluginVersion loading and added todos --- modules/system/classes/UpdateManager.php | 3 ++- modules/system/reportwidgets/Status.php | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/modules/system/classes/UpdateManager.php b/modules/system/classes/UpdateManager.php index 714cddbf76..a3bd2e0c7f 100644 --- a/modules/system/classes/UpdateManager.php +++ b/modules/system/classes/UpdateManager.php @@ -999,7 +999,8 @@ protected function applyHttpAttributes($http, $postData) $postData['server'] = base64_encode(serialize([ 'php' => PHP_VERSION, 'url' => Url::to('/'), - 'since' => PluginVersion::orderBy('created_at')->value('created_at') + // TODO: Store system boot date in `Parameter` + 'since' => PluginVersion::orderBy('created_at')->first()->created_at ])); if ($projectId = Parameter::get('system::project.id')) { diff --git a/modules/system/reportwidgets/Status.php b/modules/system/reportwidgets/Status.php index a2fff0eb12..157606b721 100644 --- a/modules/system/reportwidgets/Status.php +++ b/modules/system/reportwidgets/Status.php @@ -69,6 +69,7 @@ protected function loadData() $this->vars['requestLog'] = RequestLog::count(); $this->vars['requestLogMsg'] = LogSetting::get('log_requests', false) ? false : true; + // TODO: Store system boot date in `Parameter` $this->vars['appBirthday'] = PluginVersion::orderBy('created_at')->first()->created_at; } From 7e7091635a5130dfa7dd3b9eff107da162f78c61 Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Tue, 31 May 2022 17:59:23 +0100 Subject: [PATCH 08/16] Added db logic to base test case --- tests/TestCase.php | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/TestCase.php b/tests/TestCase.php index a4d5b1569d..890c5abfa9 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -21,6 +21,16 @@ public function createApplication() // Set random encryption key $app['config']->set('app.key', bin2hex(random_bytes(16))); + if (!file_exists(base_path('config/testing/database.php'))) { + $app['config']->set('database.connections.testing', [ + 'driver' => 'sqlite', + 'database' => ':memory:', + ]); + $app['config']->set('database.default', 'testing'); + } + + $this->runWinterUpCommand(); + return $app; } @@ -88,4 +98,13 @@ public static function assertRegExp(string $pattern, string $string, string $mes Assert::assertRegExp($pattern, $string, $message); } + + /** + * Migrate database using winter:up command. + * @return void + */ + protected function runWinterUpCommand() + { + Artisan::call('winter:up'); + } } From 20600716af7e3e6f6a3db8b0839656e1046e49eb Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Tue, 31 May 2022 17:59:49 +0100 Subject: [PATCH 09/16] Added plugin manager flag test cases --- .../unit/system/classes/PluginManagerTest.php | 53 ++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/tests/unit/system/classes/PluginManagerTest.php b/tests/unit/system/classes/PluginManagerTest.php index 8fe3a02a33..93e1b29254 100644 --- a/tests/unit/system/classes/PluginManagerTest.php +++ b/tests/unit/system/classes/PluginManagerTest.php @@ -8,7 +8,6 @@ class PluginManagerTest extends TestCase public function setUp() : void { parent::setUp(); - $manager = PluginManager::instance(); self::callProtectedMethod($manager, 'loadDisabled'); $manager->loadPlugins(); @@ -290,4 +289,56 @@ public function testActiveReplacementMap() $this->assertEquals('Winter.Replacement', $this->manager->getActiveReplacementMap('Winter.Original')); $this->assertNull($this->manager->getActiveReplacementMap('Winter.InvalidReplacement')); } + + public function testFlagDisableStatus() + { + $plugin = $this->manager->findByIdentifier('DependencyTest.Dependency'); + $flags = $this->manager->getPluginFlags($plugin); + $this->assertEmpty($flags); + + $plugin = $this->manager->findByIdentifier('DependencyTest.NotFound'); + $flags = $this->manager->getPluginFlags($plugin); + $this->assertCount(1, $flags); + $this->assertArrayHasKey('disabled-dependencies', $flags); + + $plugin = $this->manager->findByIdentifier('Winter.InvalidReplacement'); + $flags = $this->manager->getPluginFlags($plugin); + $this->assertCount(1, $flags); + $this->assertArrayHasKey('disabled-replacement-failed', $flags); + + $plugin = $this->manager->findByIdentifier('Winter.Original', true); + $flags = $this->manager->getPluginFlags($plugin); + $this->assertCount(1, $flags); + $this->assertArrayHasKey('disabled-replaced', $flags); + } + + public function testFlagDisabling() + { + $plugin = $this->manager->findByIdentifier('Winter.Tester', true); + + $flags = $this->manager->getPluginFlags($plugin); + $this->assertEmpty($flags); + + $this->manager->disablePlugin($plugin); + + $flags = $this->manager->getPluginFlags($plugin); + $this->assertCount(1, $flags); + $this->assertArrayHasKey('disabled-user', $flags); + + $this->manager->enablePlugin($plugin); + + $flags = $this->manager->getPluginFlags($plugin); + $this->assertEmpty($flags); + + $this->manager->disablePlugin($plugin, PluginManager::DISABLED_BY_CONFIG); + + $flags = $this->manager->getPluginFlags($plugin); + $this->assertCount(1, $flags); + $this->assertArrayHasKey('disabled-config', $flags); + + $this->manager->enablePlugin($plugin, PluginManager::DISABLED_BY_CONFIG); + + $flags = $this->manager->getPluginFlags($plugin); + $this->assertEmpty($flags); + } } From c5350f4d60ff883c5991df9ceb2132e43c7332b3 Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Thu, 2 Jun 2022 17:43:54 +0100 Subject: [PATCH 10/16] Removed DB logic from test case --- tests/TestCase.php | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/tests/TestCase.php b/tests/TestCase.php index 890c5abfa9..a4d5b1569d 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -21,16 +21,6 @@ public function createApplication() // Set random encryption key $app['config']->set('app.key', bin2hex(random_bytes(16))); - if (!file_exists(base_path('config/testing/database.php'))) { - $app['config']->set('database.connections.testing', [ - 'driver' => 'sqlite', - 'database' => ':memory:', - ]); - $app['config']->set('database.default', 'testing'); - } - - $this->runWinterUpCommand(); - return $app; } @@ -98,13 +88,4 @@ public static function assertRegExp(string $pattern, string $string, string $mes Assert::assertRegExp($pattern, $string, $message); } - - /** - * Migrate database using winter:up command. - * @return void - */ - protected function runWinterUpCommand() - { - Artisan::call('winter:up'); - } } From d77e732c781800d06c0fe99a16d2cd399ca76715 Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Thu, 2 Jun 2022 17:44:40 +0100 Subject: [PATCH 11/16] Added tmp patch to PluginTestCase to support PluginManager testing --- tests/PluginTestCase.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/PluginTestCase.php b/tests/PluginTestCase.php index 82be3779b6..8a8d1bcd95 100644 --- a/tests/PluginTestCase.php +++ b/tests/PluginTestCase.php @@ -51,7 +51,9 @@ public function createApplication() /* * Modify the plugin path away from the test context */ - $app->setPluginsPath(realpath(base_path().Config::get('cms.pluginsPath'))); + // TODO: Fix this so it correctly loads from config + // $app->setPluginsPath(realpath(Config::get('cms.pluginsPath'))); + $app->setPluginsPath(realpath(base_path('tests/fixtures/plugins'))); return $app; } From 1a1ad36c03e4b54a5eb9ecf641f79981bfe80812 Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Thu, 2 Jun 2022 17:45:13 +0100 Subject: [PATCH 12/16] Switched PluginManager tests to use PluginTestCase --- tests/unit/system/classes/PluginManagerTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/system/classes/PluginManagerTest.php b/tests/unit/system/classes/PluginManagerTest.php index 93e1b29254..063267ea9d 100644 --- a/tests/unit/system/classes/PluginManagerTest.php +++ b/tests/unit/system/classes/PluginManagerTest.php @@ -1,18 +1,18 @@ loadPlugins(); self::callProtectedMethod($manager, 'loadDependencies'); - $this->manager = $manager; } From d11c2b0d72bf4afd3cd34b77b3d2c70e5b089374 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Thu, 2 Jun 2022 16:08:08 -0600 Subject: [PATCH 13/16] PluginManager test class now working, all others broken :facepalm: --- tests/PluginTestCase.php | 4 +- .../unit/system/classes/PluginManagerTest.php | 99 ++++++++++++++++++- 2 files changed, 99 insertions(+), 4 deletions(-) diff --git a/tests/PluginTestCase.php b/tests/PluginTestCase.php index 8a8d1bcd95..82be3779b6 100644 --- a/tests/PluginTestCase.php +++ b/tests/PluginTestCase.php @@ -51,9 +51,7 @@ public function createApplication() /* * Modify the plugin path away from the test context */ - // TODO: Fix this so it correctly loads from config - // $app->setPluginsPath(realpath(Config::get('cms.pluginsPath'))); - $app->setPluginsPath(realpath(base_path('tests/fixtures/plugins'))); + $app->setPluginsPath(realpath(base_path().Config::get('cms.pluginsPath'))); return $app; } diff --git a/tests/unit/system/classes/PluginManagerTest.php b/tests/unit/system/classes/PluginManagerTest.php index 063267ea9d..ae7134cde7 100644 --- a/tests/unit/system/classes/PluginManagerTest.php +++ b/tests/unit/system/classes/PluginManagerTest.php @@ -1,14 +1,61 @@ set('database.connections.testing', [ + 'driver' => 'sqlite', + 'database' => ':memory:', + ]); + $app['config']->set('database.default', 'testing'); + } + + return $app; + } + /** + * Perform test case set up. + * @return void + */ public function setUp() : void { + /* + * Force reload of Winter singletons + */ + PluginManager::forgetInstance(); + UpdateManager::forgetInstance(); + + // Forces plugin migrations to be run again on every test + VersionManager::forgetInstance(); + + $this->output = new \Symfony\Component\Console\Output\BufferedOutput(); + parent::setUp(); + /* + * Ensure system is up to date + */ + $this->runWinterUpCommand(); + $manager = PluginManager::instance(); self::callProtectedMethod($manager, 'loadDisabled'); $manager->loadPlugins(); @@ -16,6 +63,56 @@ public function setUp() : void $this->manager = $manager; } + /** + * Flush event listeners and collect garbage. + * @return void + */ + public function tearDown() : void + { + $this->flushModelEventListeners(); + parent::tearDown(); + unset($this->app); + } + + /** + * Migrate database using winter:up command. + * @return void + */ + protected function runWinterUpCommand() + { + UpdateManager::instance() + ->setNotesOutput($this->output) + ->update(); + } + + /** + * The models in Winter use a static property to store their events, these + * will need to be targeted and reset ready for a new test cycle. + * Pivot models are an exception since they are internally managed. + * @return void + */ + protected function flushModelEventListeners() + { + foreach (get_declared_classes() as $class) { + if ($class === 'Winter\Storm\Database\Pivot' || strtolower($class) === 'october\rain\database\pivot') { + continue; + } + + $reflectClass = new ReflectionClass($class); + if ( + !$reflectClass->isInstantiable() || + !$reflectClass->isSubclassOf('Winter\Storm\Database\Model') || + $reflectClass->isSubclassOf('Winter\Storm\Database\Pivot') + ) { + continue; + } + + $class::flushEventListeners(); + } + + ActiveRecord::flushEventListeners(); + } + // // Tests // From 15cd67ec9d34456861cf727357802d1912c487b5 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Thu, 2 Jun 2022 19:32:24 -0600 Subject: [PATCH 14/16] Clean up typehints and fix plugin listing screen row classes --- modules/system/classes/PluginManager.php | 42 +++++++------------ modules/system/controllers/Updates.php | 6 ++- modules/system/models/PluginVersion.php | 33 +++++++++------ .../unit/system/classes/PluginManagerTest.php | 10 ++--- 4 files changed, 44 insertions(+), 47 deletions(-) diff --git a/modules/system/classes/PluginManager.php b/modules/system/classes/PluginManager.php index 4d04d1c4e9..e8464db0b4 100644 --- a/modules/system/classes/PluginManager.php +++ b/modules/system/classes/PluginManager.php @@ -95,7 +95,7 @@ class PluginManager /** * Initializes the plugin manager */ - protected function init() + protected function init(): void { $this->app = App::make('app'); @@ -197,7 +197,7 @@ protected function getFlagCacheKey(): string * Loads the plugin flags (disabled & replacement states) from the cache * regenerating them if required. */ - public function loadPluginFlags() + public function loadPluginFlags(): void { // Cache the data for a month so that stale keys can be autocleaned if necessary $data = Cache::remember($this->getFlagCacheKey(), now()->addMonths(1), function () { @@ -223,7 +223,7 @@ public function loadPluginFlags() /** * Reset the plugin flag cache */ - protected function clearFlagCache() + public function clearFlagCache(): void { Cache::forget($this->getFlagCacheKey()); } @@ -232,9 +232,8 @@ protected function clearFlagCache() * Runs the register() method on all plugins. Can only be called once. * * @param bool $force Defaults to false, if true will force the re-registration of all plugins. Use unregisterAll() instead. - * @return void */ - public function registerAll($force = false) + public function registerAll(bool $force = false): void { if ($this->registered && !$force) { return; @@ -257,10 +256,8 @@ public function registerAll($force = false) /** * Unregisters all plugins: the inverse of registerAll(). - * - * @return void */ - public function unregisterAll() + public function unregisterAll(): void { $this->registered = false; $this->plugins = []; @@ -365,9 +362,8 @@ public function registerPlugin(PluginBase $plugin, ?string $pluginId = null): vo * Runs the boot() method on all plugins. Can only be called once. * * @param bool $force Defaults to false, if true will force the re-booting of all plugins - * @return void */ - public function bootAll($force = false) + public function bootAll(bool $force = false): void { if ($this->booted && !$force) { return; @@ -395,7 +391,7 @@ public function bootPlugin(PluginBase $plugin): void /** * Returns the directory path to a plugin */ - public function getPluginPath(string|PluginBase $plugin): ?string + public function getPluginPath(PluginBase|string $plugin): ?string { return $this->findByIdentifier($plugin, true)?->getPluginPath(); } @@ -416,7 +412,7 @@ public function exists(PluginBase|string $plugin): bool * * @return array [$code => $pluginObj] */ - public function getPlugins() + public function getPlugins(): array { return array_diff_key($this->plugins, $this->pluginFlags); } @@ -426,18 +422,15 @@ public function getPlugins() * * @return array [$code => $pluginObj] */ - public function getAllPlugins() + public function getAllPlugins(): array { return $this->plugins; } /** * Returns a plugin registration class based on its namespace (Author\Plugin). - * - * @param string $namespace - * @return PluginBase|null */ - public function findByNamespace($namespace) + public function findByNamespace(string $namespace): ?PluginBase { $identifier = $this->getIdentifier($namespace); @@ -853,7 +846,7 @@ public function enablePlugin(PluginBase|string $plugin, $flag = self::DISABLED_B /** * Returns the plugin identifiers that are required by the supplied plugin. */ - public function getDependencies(string|PluginBase $plugin): array + public function getDependencies(PluginBase|string $plugin): array { if (is_string($plugin) && (!$plugin = $this->findByIdentifier($plugin))) { return []; @@ -876,9 +869,8 @@ public function getDependencies(string|PluginBase $plugin): array * * PluginManager::instance()->findMissingDependencies(); * - * @return array */ - public function findMissingDependencies() + public function findMissingDependencies(): array { $missing = []; @@ -1000,11 +992,8 @@ protected function sortByDependencies(): array /** * Completely roll back and delete a plugin from the system. - * - * @param string $id Plugin code/namespace - * @return void */ - public function deletePlugin($id) + public function deletePlugin(string $id): void { /* * Rollback plugin @@ -1027,11 +1016,8 @@ public function deletePlugin($id) /** * Tears down a plugin's database tables and rebuilds them. - * - * @param string $id Plugin code/namespace - * @return void */ - public function refreshPlugin($id) + public function refreshPlugin(string $id): void { $manager = UpdateManager::instance(); $manager->rollbackPlugin($id); diff --git a/modules/system/controllers/Updates.php b/modules/system/controllers/Updates.php index f19a752e56..c953ec9f53 100644 --- a/modules/system/controllers/Updates.php +++ b/modules/system/controllers/Updates.php @@ -91,7 +91,7 @@ public function index() public function manage() { $this->pageTitle = 'system::lang.plugins.manage'; - PluginManager::instance()->clearDisabledCache(); + PluginManager::instance()->clearFlagCache(); return $this->asExtension('ListController')->index(); } @@ -238,6 +238,10 @@ protected function getWarnings() $warnings = []; $missingDependencies = PluginManager::instance()->findMissingDependencies(); + if (!empty($missingDependencies)) { + PluginManager::instance()->clearFlagCache(); + } + foreach ($missingDependencies as $pluginCode => $plugin) { foreach ($plugin as $missingPluginCode) { $warnings[] = Lang::get('system::lang.updates.update_warnings_plugin_missing', [ diff --git a/modules/system/models/PluginVersion.php b/modules/system/models/PluginVersion.php index 96e73f28d4..14f2155dc8 100644 --- a/modules/system/models/PluginVersion.php +++ b/modules/system/models/PluginVersion.php @@ -2,7 +2,6 @@ use Lang; use Model; -use Config; use System\Classes\PluginManager; /** @@ -96,11 +95,22 @@ public function afterFetch() } } - // @TODO: Determine what flags should trigger this and how to identify them here - $this->disabledBySystem = $pluginObj->disabled; - - if (($configDisabled = Config::get('cms.disablePlugins')) && is_array($configDisabled)) { - $this->disabledByConfig = in_array($this->code, $configDisabled); + $activeFlags = $manager->getPluginFlags($pluginObj); + if (!empty($activeFlags)) { + foreach ($activeFlags as $flag => $enabled) { + if (in_array($flag, [ + PluginManager::DISABLED_MISSING, + PluginManager::DISABLED_REPLACED, + PluginManager::DISABLED_REPLACEMENT_FAILED, + PluginManager::DISABLED_MISSING_DEPENDENCIES, + ])) { + $this->disabledBySystem = true; + } + + if ($flag === PluginManager::DISABLED_BY_CONFIG) { + $this->disabledByConfig = true; + } + } } } else { @@ -115,14 +125,13 @@ public function afterFetch() */ public function getIsUpdatableAttribute(): bool { - // @TODO: Decide which flags prevent a plugin from being updated return !$this->is_disabled && !$this->disabledBySystem && !$this->disabledByConfig; } /** * Only include enabled plugins * @param $query - * @return mixed + * @return QueryBuilder */ public function scopeApplyEnabled($query) { @@ -131,10 +140,8 @@ public function scopeApplyEnabled($query) /** * Returns the current version for a plugin - * @param string $pluginCode Plugin code. Eg: Acme.Blog - * @return string */ - public static function getVersion($pluginCode) + public static function getVersion(string $pluginCode): ?string { if (self::$versionCache === null) { self::$versionCache = self::lists('version', 'code'); @@ -146,7 +153,7 @@ public static function getVersion($pluginCode) /** * Provides the slug attribute. */ - public function getSlugAttribute() + public function getSlugAttribute(): string { return self::makeSlug($this->code); } @@ -154,7 +161,7 @@ public function getSlugAttribute() /** * Generates a slug for the plugin. */ - public static function makeSlug($code) + public static function makeSlug(string $code): string { return strtolower(str_replace('.', '-', $code)); } diff --git a/tests/unit/system/classes/PluginManagerTest.php b/tests/unit/system/classes/PluginManagerTest.php index ae7134cde7..10667f5d29 100644 --- a/tests/unit/system/classes/PluginManagerTest.php +++ b/tests/unit/system/classes/PluginManagerTest.php @@ -396,17 +396,17 @@ public function testFlagDisableStatus() $plugin = $this->manager->findByIdentifier('DependencyTest.NotFound'); $flags = $this->manager->getPluginFlags($plugin); $this->assertCount(1, $flags); - $this->assertArrayHasKey('disabled-dependencies', $flags); + $this->assertArrayHasKey(PluginManager::DISABLED_MISSING_DEPENDENCIES, $flags); $plugin = $this->manager->findByIdentifier('Winter.InvalidReplacement'); $flags = $this->manager->getPluginFlags($plugin); $this->assertCount(1, $flags); - $this->assertArrayHasKey('disabled-replacement-failed', $flags); + $this->assertArrayHasKey(PluginManager::DISABLED_REPLACEMENT_FAILED, $flags); $plugin = $this->manager->findByIdentifier('Winter.Original', true); $flags = $this->manager->getPluginFlags($plugin); $this->assertCount(1, $flags); - $this->assertArrayHasKey('disabled-replaced', $flags); + $this->assertArrayHasKey(PluginManager::DISABLED_REPLACED, $flags); } public function testFlagDisabling() @@ -420,7 +420,7 @@ public function testFlagDisabling() $flags = $this->manager->getPluginFlags($plugin); $this->assertCount(1, $flags); - $this->assertArrayHasKey('disabled-user', $flags); + $this->assertArrayHasKey(PluginManager::DISABLED_BY_USER, $flags); $this->manager->enablePlugin($plugin); @@ -431,7 +431,7 @@ public function testFlagDisabling() $flags = $this->manager->getPluginFlags($plugin); $this->assertCount(1, $flags); - $this->assertArrayHasKey('disabled-config', $flags); + $this->assertArrayHasKey(PluginManager::DISABLED_BY_CONFIG, $flags); $this->manager->enablePlugin($plugin, PluginManager::DISABLED_BY_CONFIG); From 52f5eda77e5f7d8fa883ebeb56c98a9185458a2c Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Thu, 2 Jun 2022 19:43:45 -0600 Subject: [PATCH 15/16] Add comments for flags --- modules/system/classes/PluginManager.php | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/modules/system/classes/PluginManager.php b/modules/system/classes/PluginManager.php index e8464db0b4..f2ba9cdbbc 100644 --- a/modules/system/classes/PluginManager.php +++ b/modules/system/classes/PluginManager.php @@ -29,14 +29,23 @@ class PluginManager { use \Winter\Storm\Support\Traits\Singleton; + // + // Disabled by system + // + public const DISABLED_MISSING = 'disabled-missing'; - public const DISABLED_REQUEST = 'disabled-request'; - public const DISABLED_BY_USER = 'disabled-user'; public const DISABLED_REPLACED = 'disabled-replaced'; public const DISABLED_REPLACEMENT_FAILED = 'disabled-replacement-failed'; - public const DISABLED_BY_CONFIG = 'disabled-config'; public const DISABLED_MISSING_DEPENDENCIES = 'disabled-dependencies'; + // + // Explicitly disabled for a reason + // + + public const DISABLED_REQUEST = 'disabled-request'; + public const DISABLED_BY_USER = 'disabled-user'; + public const DISABLED_BY_CONFIG = 'disabled-config'; + /** * The application instance, since Plugins are an extension of a Service Provider */ From 0799001c0e279933e08b11137ea269c6ca3d7c9d Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Thu, 2 Jun 2022 19:48:29 -0600 Subject: [PATCH 16/16] Switch back to Yaml::parseFile() --- modules/system/classes/PluginBase.php | 4 ++-- modules/system/classes/VersionManager.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/system/classes/PluginBase.php b/modules/system/classes/PluginBase.php index 3c39bbf035..d07d18db02 100644 --- a/modules/system/classes/PluginBase.php +++ b/modules/system/classes/PluginBase.php @@ -326,7 +326,7 @@ protected function getConfigurationFromYaml($exceptionMessage = null) $this->loadedYamlConfiguration = []; } else { - $this->loadedYamlConfiguration = Yaml::parse(file_get_contents($yamlFilePath)); + $this->loadedYamlConfiguration = Yaml::parseFile($yamlFilePath); if (!is_array($this->loadedYamlConfiguration)) { throw new SystemException(sprintf('Invalid format of the plugin configuration file: %s. The file should define an array.', $yamlFilePath)); } @@ -433,7 +433,7 @@ public function getPluginVersion(): string if ( !File::isFile($versionFile) || !($versionInfo = Yaml::withProcessor(new VersionYamlProcessor, function ($yaml) use ($versionFile) { - return $yaml->parse(file_get_contents($versionFile)); + return $yaml->parseFile($versionFile); })) || !is_array($versionInfo) ) { diff --git a/modules/system/classes/VersionManager.php b/modules/system/classes/VersionManager.php index 0cb080cd48..a760e30ddb 100644 --- a/modules/system/classes/VersionManager.php +++ b/modules/system/classes/VersionManager.php @@ -346,7 +346,7 @@ protected function getFileVersions($code) $versionFile = $this->getVersionFile($code); $versionInfo = Yaml::withProcessor(new VersionYamlProcessor, function ($yaml) use ($versionFile) { - return $yaml->parse(file_get_contents($versionFile)); + return $yaml->parseFile($versionFile); }); if (!is_array($versionInfo)) {