Add PluginClientFactory support (#202)

* add PluginClientFactory support

* deprecate ClientFactory\PluginClientFactory

* add PluginClientFactory and PluginClientFactorySubscriber class comments

* improve ClientFactory\PluginClientFactory deprecation

* fix typos

* use client_name option for configured clients

* update composer.json

* fix composer.json

* remove prefer-stable when testing against dev dependencies

* add compatibility with old KernelTestCase::bootKernel signature

* require symfony/dependency injection ^2.8.3 || ^3.0.3

* update the changelog

* rename PluginClientFactorySubscriber to PluginClientFactoryListener

Author: fbourigault

.travis.yml

@@ -36,6 +36,7 @@ matrix:
 
 before_install:
     - if [ "$SYMFONY_VERSION" != "" ]; then composer require "symfony/symfony:${SYMFONY_VERSION}" --no-update; fi;
+    - if [ "$DEPENDENCIES" = "dev" ]; then sed -i '/prefer-stable/d' composer.json; fi;
     - if [ "$DEPENDENCIES" = "dev" ]; then perl -pi -e 's/^}$/,"minimum-stability":"dev"}/' composer.json; fi;
     - if [ $COVERAGE != true ]; then phpenv config-rm xdebug.ini; fi;
 

CHANGELOG.md

@@ -2,6 +2,23 @@
 
 The change log describes what is "Added", "Removed", "Changed" or "Fixed" between each release.
 
+## Unreleased
+
+### Added
+
+- Any third party library using `Http\Client\Common\PluginClientFactory` to create `Http\Client\Common\PluginClient`
+instances now gets zero config profiling.
+- `Http\Client\Common\PluginClientFactory` factory service.
+
+### Changed
+
+- `ProfilePlugin` and `StackPlugin` are no longer registered as (private) services decorators. Those decorators are now
+created through the `Http\HttplugBundle\Collector\PluginClientFactory`.
+
+### Deprecated
+
+- The `Http\HttplugBundle\ClientFactory\PluginClientFactory` class.
+
 ## 1.7.1 - 2017-08-04
 
 ### Fixed

ClientFactory/PluginClientFactory.php

@@ -2,12 +2,16 @@
 
 namespace Http\HttplugBundle\ClientFactory;
 
+@trigger_error('The '.__NAMESPACE__.'\PluginClientFactory class is deprecated since version 1.8 and will be removed in 2.0. Use Http\Client\Common\PluginClientFactory instead.', E_USER_DEPRECATED);
+
 use Http\Client\Common\Plugin;
 use Http\Client\Common\PluginClient;
 
 /**
  * This factory creates a PluginClient.
  *
+ * @deprecated
+ *
  * @author Tobias Nyholm <[email protected]>
  */
 final class PluginClientFactory

Collector/PluginClientFactory.php

@@ -0,0 +1,77 @@
+<?php
+
+namespace Http\HttplugBundle\Collector;
+
+use Http\Client\Common\Plugin;
+use Http\Client\Common\PluginClient;
+use Http\Client\HttpAsyncClient;
+use Http\Client\HttpClient;
+use Symfony\Component\Stopwatch\Stopwatch;
+
+/**
+ * This factory is used as a replacement for Http\Client\Common\PluginClientFactory when profiling is enabled. It
+ * creates PluginClient instances with all profiling decorators and extra plugins.
+ *
+ * @author Fabien Bourigault <[email protected]>
+ *
+ * @internal
+ */
+final class PluginClientFactory
+{
+    /**
+     * @var Collector
+     */
+    private $collector;
+
+    /**
+     * @var Formatter
+     */
+    private $formatter;
+
+    /**
+     * @var Stopwatch
+     */
+    private $stopwatch;
+
+    /**
+     * @param Collector $collector
+     * @param Formatter $formatter
+     * @param Stopwatch $stopwatch
+     */
+    public function __construct(Collector $collector, Formatter $formatter, Stopwatch $stopwatch)
+    {
+        $this->collector = $collector;
+        $this->formatter = $formatter;
+        $this->stopwatch = $stopwatch;
+    }
+
+    /**
+     * @param HttpClient|HttpAsyncClient $client
+     * @param Plugin[]                   $plugins
+     * @param array                      $options {
+     *
+     *     @var string $client_name to give client a name which may be used when displaying client information  like in
+     *         the HTTPlugBundle profiler.
+     * }
+     *
+     * @see PluginClient constructor for PluginClient specific $options.
+     *
+     * @return PluginClient
+     */
+    public function createClient($client, array $plugins = [], array $options = [])
+    {
+        $plugins = array_map(function (Plugin $plugin) {
+            return new ProfilePlugin($plugin, $this->collector, $this->formatter);
+        }, $plugins);
+
+        $clientName = isset($options['client_name']) ? $options['client_name'] : 'Default';
+        array_unshift($plugins, new StackPlugin($this->collector, $this->formatter, $clientName));
+        unset($options['client_name']);
+
+        if (!$client instanceof ProfileClient) {
+            $client = new ProfileClient($client, $this->collector, $this->formatter, $this->stopwatch);
+        }
+
+        return new PluginClient($client, $plugins, $options);
+    }
+}

Collector/PluginClientFactoryListener.php

@@ -0,0 +1,54 @@
+<?php
+
+namespace Http\HttplugBundle\Collector;
+
+use Http\Client\Common\PluginClientFactory;
+use Http\HttplugBundle\Collector\PluginClientFactory as CollectorPluginClientFactory;
+use Symfony\Component\EventDispatcher\Event;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+
+/**
+ * This subscriber ensures that every PluginClient created when using Http\Client\Common\PluginClientFactory without
+ * using the Symfony dependency injection container uses the Http\HttplugBundle\Collector\PluginClientFactory factory
+ * when profiling is enabled. This allows 0 config profiling of third party libraries which use HTTPlug.
+ *
+ * @author Fabien Bourigault <[email protected]>
+ *
+ * @internal
+ */
+final class PluginClientFactoryListener implements EventSubscriberInterface
+{
+    /**
+     * @var CollectorPluginClientFactory
+     */
+    private $factory;
+
+    /**
+     * @param CollectorPluginClientFactory $factory
+     */
+    public function __construct(CollectorPluginClientFactory $factory)
+    {
+        $this->factory = $factory;
+    }
+
+    /**
+     * Make sure to profile clients created using PluginClientFactory.
+     *
+     * @param Event $e
+     */
+    public function onEvent(Event $e)
+    {
+        PluginClientFactory::setFactory([$this->factory, 'createClient']);
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public static function getSubscribedEvents()
+    {
+        return [
+            'kernel.request' => ['onEvent', 1024],
+            'console.command' => ['onEvent', 1024],
+        ];
+    }
+}

DependencyInjection/HttplugExtension.php

@@ -7,8 +7,8 @@
 use Http\Client\Common\HttpMethodsClient;
 use Http\Client\Common\Plugin\AuthenticationPlugin;
 use Http\Client\Common\PluginClient;
-use Http\HttplugBundle\ClientFactory\PluginClientFactory;
-use Http\HttplugBundle\Collector\ProfilePlugin;
+use Http\Client\Common\PluginClientFactory;
+use Http\Client\HttpClient;
 use Http\Message\Authentication\BasicAuth;
 use Http\Message\Authentication\Bearer;
 use Http\Message\Authentication\Wsse;
@@ -72,7 +72,7 @@ public function load(array $configs, ContainerBuilder $container)
             ;
         }
 
-        $this->configureClients($container, $config, $profilingEnabled);
+        $this->configureClients($container, $config);
         $this->configurePlugins($container, $config['plugins']); // must be after clients, as clients.X.plugins might use plugins as templates that will be removed
         $this->configureAutoDiscoveryClients($container, $config);
     }
@@ -82,9 +82,8 @@ public function load(array $configs, ContainerBuilder $container)
      *
      * @param ContainerBuilder $container
      * @param array            $config
-     * @param bool             $profiling
      */
-    private function configureClients(ContainerBuilder $container, array $config, $profiling)
+    private function configureClients(ContainerBuilder $container, array $config)
     {
         $first = null;
         $clients = [];
@@ -95,7 +94,7 @@ private function configureClients(ContainerBuilder $container, array $config, $p
                 $first = $name;
             }
 
-            $this->configureClient($container, $name, $arguments, $this->isConfigEnabled($container, $config['profiling']));
+            $this->configureClient($container, $name, $arguments);
             $clients[] = $name;
         }
 
@@ -266,9 +265,8 @@ private function configureAuthentication(ContainerBuilder $container, array $con
      * @param ContainerBuilder $container
      * @param string           $clientName
      * @param array            $arguments
-     * @param bool             $profiling
      */
-    private function configureClient(ContainerBuilder $container, $clientName, array $arguments, $profiling)
+    private function configureClient(ContainerBuilder $container, $clientName, array $arguments)
     {
         $serviceId = 'httplug.client.'.$clientName;
 
@@ -285,23 +283,17 @@ private function configureClient(ContainerBuilder $container, $clientName, array
             }
         }
 
-        $pluginClientOptions = [];
-        if ($profiling) {
-            //Decorate each plugin with a ProfilePlugin instance.
-            $plugins = array_map(function ($pluginServiceId) use ($container) {
-                $this->decoratePluginWithProfilePlugin($container, $pluginServiceId);
-
-                return $pluginServiceId.'.debug';
-            }, $plugins);
-
-            // To profile the requests, add a StackPlugin as first plugin in the chain.
-            $stackPluginId = $this->configureStackPlugin($container, $clientName, $serviceId);
-            array_unshift($plugins, $stackPluginId);
-        }
+        $container
+            ->register($serviceId.'.client', HttpClient::class)
+            ->setFactory([new Reference($arguments['factory']), 'createClient'])
+            ->addArgument($arguments['config'])
+            ->setPublic(false)
+        ;
 
         $container
             ->register($serviceId, PluginClient::class)
-            ->setFactory([PluginClientFactory::class, 'createPluginClient'])
+            ->setFactory([new Reference(PluginClientFactory::class), 'createClient'])
+            ->addArgument(new Reference($serviceId.'.client'))
             ->addArgument(
                 array_map(
                     function ($id) {
@@ -310,9 +302,9 @@ function ($id) {
                     $plugins
                 )
             )
-            ->addArgument(new Reference($arguments['factory']))
-            ->addArgument($arguments['config'])
-            ->addArgument($pluginClientOptions)
+            ->addArgument([
+                'client_name' => $clientName,
+            ])
         ;
 
         /*
@@ -432,44 +424,4 @@ private function configurePlugin(ContainerBuilder $container, $serviceId, $plugi
 
         return $pluginServiceId;
     }
-
-    /**
-     * Decorate the plugin service with a ProfilePlugin service.
-     *
-     * @param ContainerBuilder $container
-     * @param string           $pluginServiceId
-     */
-    private function decoratePluginWithProfilePlugin(ContainerBuilder $container, $pluginServiceId)
-    {
-        $container->register($pluginServiceId.'.debug', ProfilePlugin::class)
-            ->setArguments([
-                new Reference($pluginServiceId),
-                new Reference('httplug.collector.collector'),
-                new Reference('httplug.collector.formatter'),
-            ])
-            ->setPublic(false);
-    }
-
-    /**
-     * Configure a StackPlugin for a client.
-     *
-     * @param ContainerBuilder $container
-     * @param string           $clientName Client name to display in the profiler.
-     * @param string           $serviceId  Client service id. Used as base for the StackPlugin service id.
-     *
-     * @return string configured StackPlugin service id
-     */
-    private function configureStackPlugin(ContainerBuilder $container, $clientName, $serviceId)
-    {
-        $pluginServiceId = $serviceId.'.plugin.stack';
-
-        $definition = class_exists(ChildDefinition::class)
-            ? new ChildDefinition('httplug.plugin.stack')
-            : new DefinitionDecorator('httplug.plugin.stack');
-
-        $definition->addArgument($clientName);
-        $container->setDefinition($pluginServiceId, $definition);
-
-        return $pluginServiceId;
-    }
 }

Resources/config/data-collector.xml

@@ -84,5 +84,17 @@
             <argument type="service" id="httplug.collector.formatter"/>
             <argument type="service" id="debug.stopwatch"/>
         </service>
+
+        <service id="Http\Client\Common\PluginClientFactory" class="Http\HttplugBundle\Collector\PluginClientFactory" public="false">
+            <argument type="service" id="httplug.collector.collector"/>
+            <argument type="service" id="httplug.collector.formatter"/>
+            <argument type="service" id="debug.stopwatch"/>
+        </service>
+
+        <service id="Http\HttplugBundle\Collector\PluginClientFactoryListener" class="Http\HttplugBundle\Collector\PluginClientFactoryListener">
+            <argument type="service" id="Http\Client\Common\PluginClientFactory" />
+
+            <tag name="kernel.event_subscriber" />
+        </service>
     </services>
 </container>

Resources/config/services.xml

@@ -44,6 +44,9 @@
         </service>
         <service id="Http\Client\HttpClient" alias="httplug.client" public="false" />
 
+        <!-- PluginClientFactory -->
+        <service id="Http\Client\Common\PluginClientFactory" class="Http\Client\Common\PluginClientFactory" public="false" />
+
         <!-- ClientFactories -->
         <service id="httplug.factory.auto" class="Http\HttplugBundle\ClientFactory\AutoDiscoveryFactory" public="false" />
         <service id="httplug.factory.buzz" class="Http\HttplugBundle\ClientFactory\BuzzFactory" public="false">

Tests/Functional/ServiceInstantiationTest.php

@@ -11,6 +11,11 @@
 use Http\HttplugBundle\Collector\StackPlugin;
 use Nyholm\NSA;
 use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
+use Symfony\Component\EventDispatcher\EventDispatcherInterface;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpKernel\Event\GetResponseEvent;
+use Symfony\Component\HttpKernel\HttpKernelInterface;
+use Symfony\Component\HttpKernel\KernelEvents;
 use Symfony\Component\HttpKernel\Profiler\Profiler;
 
 class ServiceInstantiationTest extends WebTestCase
@@ -71,4 +76,21 @@ public function testProfilingDecoration()
         $this->assertInstanceOf(ProfilePlugin::class, $plugins[3]);
         $this->assertInstanceOf(ProfilePlugin::class, $plugins[4]);
     }
+
+    /**
+     * {@inheritdoc}
+     */
+    protected static function bootKernel(array $options = [])
+    {
+        parent::bootKernel($options);
+
+        /** @var EventDispatcherInterface $dispatcher */
+        $dispatcher = static::$kernel->getContainer()->get('event_dispatcher');
+
+        $event = new GetResponseEvent(static::$kernel, new Request(), HttpKernelInterface::MASTER_REQUEST);
+
+        $dispatcher->dispatch(KernelEvents::REQUEST, $event);
+
+        return static::$kernel;
+    }
 }