Skip to content

Rework plugin system #79

Description

@tristanlins

I'm not fully satisfied with the current plugin system.

The main problem I see is that each plugin has is fixed name. That means it is impossible to have multiple plugins for the same behavior. A good example is a image resize plugin.

There could be multiple implementations:

  • Native ImageMagick plugin: Render images with the convert command natively. Only work on local filesystems.
  • Imagick plugin: Render images with the Imagick class in the memory. Works only if Imagick is available, but also works for non-local filesystems.
  • GD plugin: Render images with GD in memory. Works if GD is enabled and also works for non-local filesystems, but is more shoddy than Imagick.

Another feature could be native plugins. Plugins that are provided by the adapter to support adapter specific features. For a dropbox adapter this could be a build-in dropbox plugin.

So I wan't to do the following changes:

  • Add Adapter::getPlugins($name) return a list of native plugin objects.
  • Remove PluginInterface::getName()
  • Change PluginManager::registerPlugin(PluginInterface $plugin) to PluginManager::registerPlugin($name, PluginInterface $plugin, $priority = 0)
  • Add PluginInterface::supportsAdapter(AdapterInterface $adapter)
  • Add PluginInterface::supportsFile(File $file)
  • Change PluginManager::getPlugin($name) to PluginManager::getPlugin($name, Adapter $adapter, File $file = null)
PluginManager::listPlugins($name, Adapter $adapter, File $file = null) {
    if (isset($this->plugins[$name])) {
        $sortedPlugins = array();
        $nativePlugins = $adapter->getPlugins();
        $priorizedPlugins = array_merge($this->plugins[$name]);
        $priorizedPlugins[0] = array_merge($nativePlugins, $plugins[0]);
        $plugins = call_user_func_array('array_merge', $priorizedPlugins);
        $plugins = array_filter(
            function ($plugin) use ($adapter, $file) {
                if ($file) {
                    return $plugin->supportsFile($file);
                }
                return $plugin->supportsAdapter($adapter);
            },
            $plugins
        }

        if (count($plugins)) {
            return $plugins;
        }
    }

    throw new \PluginException('Plugin ' . $name . ' does not exists');
}

PluginManager::getPlugin($name, Adapter $adapter, File $file = null) {
    $plugins = $this->listPlugins($name, $adapter, $file);
    return array_shift($plugins);
}

Instead of returning exactly one plugin object, it could be possible to pass a magic plugin, that ask each plugin in the list.

class PluginsPlugin implements PluginInterface {
    function __call($name, $arguments) {
        foreach ($this->plugins as $plugin) {
            try {
                return call_user_func_array(array($plugin, $name), $arguments);
            } catch (UnsupportedPluginMethodException $e) {
                continue;
            }
        }
        throw new UnsupportedPluginMethodException($name);
    }
}

I'm also not happy with the Filesystem Adapter and File Adapter alienation. I'm not sure if this is really necessary.

@discordier any hints?

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions