Added PluginClientBuilder

[Nyholm]

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Related tickets	related to php-http/HttplugBundle#109
Documentation
License	MIT

What's in this PR?

This is a plugin builder that could be use to build up a Plugin client. See example usage: https://github.com/Nyholm/php-github-api/blob/plugin-client-builder/lib/Github/Client.php

Why?

• If a library builds up the PluginClient manually there is no way for us (HTTPlugBundle) to know what plugins that are used and we cannot show any plugin stack in the web profiler
• We could "risk" having multiple plugin chains which will cause issues when using the cache plugin if a later plugin is rewriting the url. (or any other part of the cache key)
• A pluginClientBuilder will make it easy for the LibraryBundle to integrate the library with Symfony and HTTPlugBundle.

Checklist

• Updated CHANGELOG.md to describe BC breaks / deprecations | new feature | bugfix
• Documentation pull request created (if not simply a bugfix)

To Do

• Clean the code
• Figure out if the interface is needed
• Figure out the final interface

This PR needs some work but I wanted to share this code

[GrahamCampbell]

Might be worth adding an interface plugins can implement to say they need to get pushed back instead of hard coding the cache plugin like this?

[sagikazarmark]

I am not sure I like the idea. Plugins should not be affected by external configuration logic. Also, there is no guarantee that cache should always be "pushed back".

[sagikazarmark]

I would rather use append/prependPlugin methods.

[dbu]

if we find a way to make replace preserve the order, we would not need this.

[Nyholm]

I agree with all your comments. I created this by extracting logic from the KnpGithub client. I'll adapt to your feedback to make this generic.

[dbu]

okay, the knp example is a very good illustration where this builder approach works very well.

some braindump trying to figure out what we should do:

in a symfony context, a compiler pass to replace services with other services would be much cleaner, and would allow to leverage the configuration options on httplug bundle. but that would mean you need a way to set up the library without framework, and a different set up for each framework integration.

the thing i don't like on the knp library is that Client is a mix of an api client and a http client builder. your refactoring pushes it even more in that direction. with symfony, we would be able to change the list of plugins in a compiler pass at container build time (aka only on cache rebuild). with the builder in php, instances are created, some potentially only to be discarded later.

is it the idea of that library that you would change the http setup while communicating with github? as dev, i would prefer several client instances if i need different http behaviour for different tasks, like using different credentials. its way to easy otherwise to mix things up and use the wrong settings.

my idea would be that for a frameworkagnostic library, you have a builder (or whatever) that gives you the github client instance, shuffling around plugins. but for symfony, it would make more sense to not use that and have a compiler pass to manipulate the plugins of the client service that is specified in the configuration. this could get hairy when several bundles use the same client, for example the default client, but would be more efficient to instantiate the client, and easier for the user to configure.

[dbu]

while preparing the web summer camp tutorial, i had a completely different idea: what if the HttplugBundle would offer a method that accepts a client configuration as the bundle config would do it, and returns a service definition? then a bundle could do its own configuration with extra plugins and whatnot and then add the plugin configuration/services it needs, do custom validation or whatever and still get a properly set up service.

this approach would be specific to symfony - as are bundles in general. for other frameworks, we would need to find an approach suitable for those frameworks. but maybe its better to have framework integration framework specific?

[fbourigault]

Since we have #71 and a plan for php-http/HttplugBundle#109, I think we could close this PR.