Show plugins created without the Symfony config

[Nyholm]

Q	A
Bug?	no
New Feature?	question
Version

I just tested the bundle together with the knp github client. It is creating a PluginClient and adds a lot of extra plugins. I was (foolishly) expecting to see all plugin steps in the WebProfiler.

Is it some way we could make this happen?... Or is it anyway one could configure the knp GitHub client differently?

[sagikazarmark]

What does "adds lot of extra plugins" mean?

[Nyholm]

https://github.com/KnpLabs/php-github-api/blob/master/lib/Github/Client.php#L153-L160

They build up their own stack of plugins and creates a client.

[sagikazarmark]

I don't think it's a good idea. The API client should not know about HTTP client internals, it just relies on the interface. You should create a separate factory which can be used for configuring a PluginClient. In that case you can manually create a PluginClient in Symfony and inject it into the client.

[dbu]

if there was a bundle for the knp github client, it could also integrate with the HttplugBundle for better configurability. and i agree with mark that this part could be put into a separate factory so that one can inject a tailored client instead of having the github client set up the plugins.

[Nyholm]

Yes, I agree with you. It should be a factory for building the HTTP client in the GitHub api client.

if there was a bundle for the knp github client, it could also integrate with the HttplugBundle for better configurability.

If we (in some magic way) could automatically show something like this, but with "GitHub client" instead of "my_guzzle". That would be awesome.

What if I create a PluginFactoryInterface in client-common and also a PluginFactory implementation. And then I create another PluginFactory here in this bundle with debug features. A GitHubBundle then could make sure GitHubClient is using the PluginFactory form the bundle and we would have lots of debug info in our toolbar.

[Nyholm]

My idea worked.. Not sure if it is beautiful...

[dbu]

i think it would make sense that this bundle offers such a functionality to other bundles that integrate libraries into symfony. it feels better than trying to somehow magically capture clients or plugins.

we should make it easy for the other bundles to use the HttplugBundle infrastructure. we could promote to prepend configuration to configure the client in HttplugBundle, but prepending config is very thight and unflexible coupling (we did it in the cmf bundles and had a bunch of problems with it.). if the plugin factory works out, we could use a container tag and a compiler pass in HttplugBundle? or just document how people can integrate their clients with the symfony toolbar.

[dbu]

in FOSHttpCache we currently use a HttpAdapter: https://github.com/FriendsOfSymfony/FOSHttpCache/blob/master/src/ProxyClient/Http/HttpAdapter.php

this could serve as an edge case to see how far this solution can go. the problem here is that we need to send requests to several IPs that might all use the same HOST header. but maybe we indeed should keep the adapter and configure it with an async client. these requests could also clog up the logging when each request is multiplied by the caching proxies

[fbourigault]

I worked on a POC using a different approach. Basically, this introduce the concept of PluginClient factories. I built my POC for the https://github.com/KnpLabs/php-github-api library.

Let's start by a huge screenshot (implementation details come after).

Implementation

php-http/client-common

In this package, I just added the concept of PluginClientFactory with an interface. The shipped factory is just a proxy for the PluginClient constructor.
Diff: php-http/client-common@master...fbourigault:plugin-client-factory

php-http/discovery

I just added a PluginClientFactoryDiscovery to find an available PluginClientFactory. No puli support yet.
Diff: php-http/discovery@master...fbourigault:plugin-client-factory

php-http/HttplugBundle

Here you have more stuff, but it's not more complicated than profiling discovered http clients. A ProfilePluginClientFactoryDiscovery is prepended in the PluginClientFactoryDiscovery to return a ProfilePluginClientFactory which just add the profiling decorators/plugins to the plugins list before building the PluginClient.
Diff: master...fbourigault:plugin-client-factory

KnpLabs/php-github-api

In the 3rd party library, I just replaced the new PluginClient by a call to a PluginClientFactory found by ``ProfilePluginClientFactoryDiscovery`. All the Builder stuff is not changed as it's really full of edge cases.
Diff: KnpLabs/php-github-api@master...fbourigault:plugin-client-factory

Current limitations

• plugins profiled this way does not have a dotted name, but use their FQCN instead.
• I hacked around the client names cause I can't guess one.
• requests are profiled twice as the auto discovered client is already decorated with a PluginClient.

[Nyholm]

👍 I like it. But do we really need the discovery layer? Can we tell ask KNPGithub to just do a class_exists? Either they use HttplugBundle or ClientCommon.

[fbourigault]

Using the discovery makes 3rd party libraries not aware of HttplugBundle!

[fbourigault]

You can't do a class_exists in the library as the ProfilePluginClientFactory has to be enabled when profiling is enabled at the kernel level and at the bundle level.

[Nyholm]

You are correct. I like it. Lets create some PRs.