Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework plugin system #79

Open
tristanlins opened this issue Sep 2, 2014 · 1 comment
Open

Rework plugin system #79

tristanlins opened this issue Sep 2, 2014 · 1 comment
Assignees
Milestone

Comments

@tristanlins
Copy link
Member

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?

@tristanlins tristanlins self-assigned this Sep 2, 2014
@tristanlins tristanlins added this to the Version 1.0.0 milestone Sep 2, 2014
@discordier
Copy link
Member

Just to let you know, I did not miss this ticket, I simply did not find any time yet to get to it. :(

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

No branches or pull requests

2 participants