winter icon indicating copy to clipboard operation
winter copied to clipboard

Revamp Plugin Management

Open LukeTowers opened this issue 2 years ago • 1 comments

There are a number of minor annoyances with the existing plugin management UX, DX, and API. The following is a dump of my notes on the process so far, this issue will be gradually improved and fleshed out to define my desired changes to the Plugin Management system in Winter and then the work can be done piecemeal, ideally with any potentially breaking changes done first to make it into 1.2.

This also ties into the ClassLoader improvements

Issues / improvements:

  • [x] Get rid of bindContainerObjects (only there because it was matching the updateManager)
  • [ ] Simplify public API space, should have a single function that handles loading and initializing all the plugins and if needed a way to reset all of them for unit tests
  • [ ] Emphasize that class_exists(Plugin::class) is not an acceptable way to check for plugins since the plugin.php class has to be loaded if it’s present even if it’s disabled
  • [x] Simplify the identifier normalization logic and make it more consistent
  • [ ] Would be nice to be able to identify the specific circular reference tree that causes issues instead of an arbitrary limit
  • [ ] Would be nice to cache the resolved order of plugins to load (so that we don’t have to recalculate plugin dependencies
  • [ ] Would be nice to move some more logic (like disablePlugin) to the plugin base class
  • [ ] Would be nice to provide some events to plugins
    • plugin.install
    • Plugin.uninstall
    • plugin.disable
    • plugin.enable
  • [ ] Updates controller has method called getPluginVersionFile(), returns an array of version update messages keyed by version number, but there’s also a getPluginVersion() method in the plugin base that loads the newest version present in the version.yaml file

Reasons / ways a plugin can be disabled:

  • Plugin is in database but not present in the filesystem
  • Plugin has a replacement present that is taking over
  • Plugin is a replacement but can't replace the original due to version constraints
  • Plugin is explicitly disabled by the user in the database
  • Plugin is explicitly disabled by the developer in the config file
  • Plugin is implicitly disabled by the system because of unmet dependencies
  • Plugin is programatically disabled for the current request only

Existing API of PluginManager:

  • getAllPlugins() -> All plugins that are detected on the filesystem
  • getPlugins() -> All plugins that are detected minus plugins that have been flagged disabled
  • loadPlugins() -> kicks off the detection and plugin loading process
  • loadPlugin($namespace, $path) -> l
    • loads and instantiates the plugin’s Plugin.php
    • Checks if the plugin has been disabled by anything, sets the disabled flag on the plugin if so and ends here
    • sets up class autoloading for plugin classing
    • Sets up plugin replacement if specified
  • registerAll() -> Calls the register() method on all loaded plugins, can only be called once
    • Loops over all present plugins (disabled included) calling registerPlugin() for each one she finds
      • Registers language namespace
      • Registers class loaders for embedded dependencies (vendor directory)
      • Registers configuration namespace
      • Registers view paths
      • Registers class loader namespace aliases
      • Plugin is now loaded into the system, hasn’t yet run any custom code (could possibly do some in the constructor for the plugin.php but that’s definitely not recommended)
      • If $noInit is set (sensitive operations like updating the database and authentication steps), registration stops here
      • Runs $plugin->register()
      • Requires init.php file if present
      • Requires routes.php file if present
    • Forces the router to reload its definitions (might prevent route:cache from actually providing a performance gain)
  • unregisterAll() - deletes the internal properties and sets it up to start again
  • bootAll() - runs the boot() method on all plugins (including disabled)
  • bootPlugin() - runs the boot() method for all sites under warranty
    • Does not run boot() if plugin is invalid, disabled, or if the $noInit flag is set and the plugin is not flagged as elevated
  • getPluginPath() -> works for any plugin in the pathMap (from loadPlugin)
  • exists() -> Checks if the provided normalized / replaced ID exists in $this->plugins & if the plugin is present in $this->disabledPlugins
  • getPlugins() -> Returns plugins in $this->plugins minus any plugins in $this->disabledPlugins
  • getAllPlugins() -> Returns all plugins in $this->plugins
  • findByNamespace() -> Converts a namespace into a plugin identifier and returns the plugin from $this->plugins if found
  • findByIdentifier() -> attempts to normalize the provided identifier and run it through the replacement map to then return the appropriate plugin
  • hasPlugin() -> normalizes the provided identifier and checks to see if it exists
  • getVendorAndPluginNames() -> scans the plugin dir for plugins (//plugin.php) and returns the information in an array of authors with an array of plugin folder paths keyed by plugin folder name
  • getPluginNamespaces() -> processes the data from getVendorAndPluginNames(), doesn’t look at the composer file or the actual namespace of the plugin.php file, uses Str::normalizeClassName
    • Most technically correct option would be to pull in the namespace from the plugin.php but then make the class loader prefix paths case insensitive
    • https://dotjoeblog.wordpress.com/2019/07/03/php-namespaces-are-case-insensitive-but/
  • getIdentifier() - normalizes a string or object into a plugin identifier (Author.Plugin)
  • getNamespace() - normalizes a string or object into a plugin namespace (Author\Plugin)
  • normalizeIdentifier() - strtolower() match on the $this->normalizedMap setup on LoadPlugin with the provided input to return the identifier used elsewhere in the class
  • getRegistrationMethodValues() - loops over getPlugins( ) to get results from registration methods, caches after running once (so don’t run until all plugins are loaded and disabled applied)
  • clearDisabledCache() - clears the disabled plugins meta file
  • Protected loadDisabled()
    • Disables any plugins set in cms.disablePlugins, flagging as being user disabled
    • If the cache file exists load it otherwise load the disabled plugins from the database and cache it
    • POTENTIAL: Introduce a cms.enablePlugins alternative config option to only enable the provided plugins if its set (default value null, empty array means no plugins, otherwise array of identifiers that should be enabled)
  • isDisabled() - normalizes identifier and checks the $this->disabledPlugins (does this take into account plugin replacements?)
  • writeDisabled() - dumps the contents of $this->disabledPlugins to disk
  • populateDisabledPluginsFromDb() - loads any plugins that have been disabled in the system_plugin_versions table into $this->disabled
  • getReplacementMap() - Returns the replacement map
  • getActiveReplacementMap() - ?
  • registerReplacedPlugins()
    • Loops over the replacement map to determine if a replacement should be applied
    • If yes, disables the original, enables the replacement, and aliases the original to the replacement
    • If no, disables the replacement, enables the original (note, what happens if a user specifically disables either the replacement or the original?
  • aliasPluginAs() - aliases the original plugin’s lang, config, navigation, & settings to the replacing plugin (how does this handle those pieces being blackholed)
  • disablePlugin() - flags the plugin as disabled in this->disabledPlugins and sets the disabled flag on the plugin instance, also writes to the local file
  • enablePlugin() - removes the plugin from this->disabledPlugins and sets the disabled flag on the plugin instance to false, also writes to the local file
  • findMissingDependencies() - returns array of missing dependencies keyed by each plugin that is missing dependencies
  • loadDependencies()
    • Loops over this->plugins (should be this->getPlugins() since if a plugin is already disabled why do we care about checking it for dependencies to manually disable it again?)
    • If the plugin has dependencies loop over them
      • If the dependency is not present or it is disabled (manually checking $pluginObj->disabled, seems like a problem) then disable the current plugin
      • If the dependency is present then the plugin gets enabled -> definitely a bug (present since first introduced: https://github.com/wintercms/winter/commit/08bc886d1b54c44b08f7c578a20136b6148719ea)
  • sortDependencies() - sorts $this->plugins by dependencies to ensure dependencies are loaded first
  • getDependencies() - gets the dependencies for a plugin taking into account plugin replacements
  • sortByDependencies( ) - remove
  • deletePlugin() - deletes the provided plugin
  • refreshPlugin() - rolls back the provided plugin and runs its migrations again

LukeTowers avatar Mar 13 '22 20:03 LukeTowers

This issue will be closed and archived in 3 days, as there has been no activity in the last 60 days. If this issue is still relevant or you would like to see it actioned, please respond and we will re-open this issue. If this issue is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

github-actions[bot] avatar May 17 '22 00:05 github-actions[bot]

This issue will be closed and archived in 3 days, as there has been no activity in this issue for the last 6 months. If this issue is still relevant or you would like to see it actioned, please respond within 3 days. If this issue is critical for your business, please reach out to us at [email protected].

github-actions[bot] avatar Mar 14 '23 00:03 github-actions[bot]

No need to keep this open anymore, at this point it's more for reference than anything else

LukeTowers avatar Jun 16 '23 23:06 LukeTowers