Add PluginClientFactory support #202
Conversation
| @@ -2,6 +2,8 @@ | ||
|
|
||
| namespace Http\HttplugBundle\ClientFactory; | ||
|
|
||
| @trigger_error('The '.__NAMESPACE__.'\PluginClientFactory class is deprecated since version 1.8 and will be removed in 2.0.', E_USER_DEPRECATED); |
There was a problem hiding this comment.
i am a big fan of telling people what to use instead. can we do that here?
There was a problem hiding this comment.
we could also add a @deprecated annotation in the class docblock to make IDEs show that there is a problem when using this class.
Collector/PluginClientFactory.php
Outdated
| use Symfony\Component\Stopwatch\Stopwatch; | ||
|
|
||
| /** | ||
| * This factory is used as a replacement for Http\Client\Common\PluginClientFactory when profiling is enabled. It create |
There was a problem hiding this comment.
It creates (with an s at the end)
|
|
||
| /** | ||
| * This subscriber ensure that every PluginClient created when using Http\Client\Common\PluginClientFactory without | ||
| * using the Symfony dependency injection container use the Http\HttplugBundle\Collector\PluginClientFactory factory |
There was a problem hiding this comment.
this subscriber ensures that ... injection container uses the ...
|
David, I addressed all your comments. Thanks for the proofreading! |
|
I think this looks good. All my clients are called "Default" now. But I guess that will be fixed when this PR is rebased. |
|
Could you add the following to composer.json so the build has a change to be green: "minimum-stability": "dev",
"prefer-dist": true, |
|
Do we all agree on how the |
|
Do you plan to release client-common 1.6 before merging this PR? |
|
I think we should postpone that until this PR is merged and we know (for sure) that the bundle and client-common works great together |
|
Do you know why deps low test fails? |
|
Not yet, wanna give it a look today. |
|
|
||
| $dispatcher->dispatch(KernelEvents::REQUEST, $event); | ||
|
|
||
| return static::$kernel; |
There was a problem hiding this comment.
this might be confusing because the base test cases don't do it in older symfony versions. but i think its still a good idea. assigning the bootKernel return value, then accessing static::$kernel and in the end return the variable would be weird.
|
You can fix the broken tests by require |
|
Thank you @Nyholm. Tests are now green. I used |
|
I added a few changelog entries. |
| * | ||
| * @internal | ||
| */ | ||
| final class PluginClientFactorySubscriber implements EventSubscriberInterface |
There was a problem hiding this comment.
i heard such a class should still be called ...Listener. it semantically is an event listener. the subscriber interface is about how to wire up the listener and less central.
There was a problem hiding this comment.
Right, I updated the name.
|
Can you resolve the conflicts? |
|
Of course, I just created those when #203 gets merged! |
|
One the merge conflicts is resolved, I'll be happy to merge |
|
Thank you |
Here we are! This PR is the journey achievement for 0 config profiling of 3rd party libraries.
It relies on php-http/client-common#83 which will be released with 1.6.0.
In a few words, it uses the
Http\Client\Common\PluginClientFactory::setFactoryinternal method to replace the PluginClientFactory internals. By doing so, every 3rd party library which use theHttp\Client\Common\PluginClientFactoryto create it's client will automatically get profiling when used within a Symfony application.To be more precise, clients created using the bundle configuration uses the
Http\Client\Common\PluginClientFactoryservice as a DI factory which is replaced by an instance ofHttp\HttplugBundle\Collector\PluginClientFactorywhen profiling is enabled. Using the collector version of thePluginClientFactorymake all clients and plugins decorated with appropriate decorators.The
Http\HttplugBundle\Collector\PluginClientFactorySubscriberinject the factory callback into `` at early Symfony events likeHttp\HttplugBundle\Discovery\ConfiguredClientsStrategy.I still have to work on code comments and documentation, I open this pull request to get your feedback.
To Do
Http\HttplugBundle\ClientFactory\PluginClientFactoryclass.