Don't use PluginClient for discovered clients (#200)

fbourigault · Nyholm · commit 33ed6aefd1f7 · 2017-08-26T13:23:03.000+02:00
* don't use PluginClient for discovered clients

* add blank line before break instruction
diff --git a/Collector/ProfileClient.php b/Collector/ProfileClient.php
@@ -75,7 +75,16 @@ public function __construct($client, Collector $collector, Formatter $formatter,
      */
     public function sendAsyncRequest(RequestInterface $request)
     {
+        $activateStack = true;
         $stack = $this->collector->getActiveStack();
+        if ($stack === null) {
+            //When using a discovered client not wrapped in a PluginClient, we don't have a stack from StackPlugin. So
+            //we create our own stack and activate it!
+            $stack = new Stack('Default', $this->formatter->formatRequest($request));
+            $this->collector->addStack($stack);
+            $this->collector->activateStack($stack);
+            $activateStack = false;
+        }
 
         $this->collectRequestInformations($request, $stack);
         $event = $this->stopwatch->start($this->getStopwatchEventName($request));
@@ -98,7 +107,10 @@ public function sendAsyncRequest(RequestInterface $request)
             return $this->client->sendAsyncRequest($request)->then($onFulfilled, $onRejected);
         } finally {
             $event->stop();
-            $this->collector->activateStack($stack);
+            if ($activateStack) {
+                //We only activate the stack when created by the StackPlugin.
+                $this->collector->activateStack($stack);
+            }
         }
     }
 
@@ -108,6 +120,12 @@ public function sendAsyncRequest(RequestInterface $request)
     public function sendRequest(RequestInterface $request)
     {
         $stack = $this->collector->getActiveStack();
+        if ($stack === null) {
+            //When using a discovered client not wrapped in a PluginClient, we don't have a stack from StackPlugin. So
+            //we create our own stack but don't activate it.
+            $stack = new Stack('Default', $this->formatter->formatRequest($request));
+            $this->collector->addStack($stack);
+        }
 
         $this->collectRequestInformations($request, $stack);
         $event = $this->stopwatch->start($this->getStopwatchEventName($request));
diff --git a/DependencyInjection/HttplugExtension.php b/DependencyInjection/HttplugExtension.php
@@ -149,37 +149,45 @@ private function configurePluginByName($name, Definition $definition, array $con
                     ->replaceArgument(0, new Reference($config['cache_pool']))
                     ->replaceArgument(1, new Reference($config['stream_factory']))
                     ->replaceArgument(2, $config['config']);
+
                 break;
             case 'cookie':
                 $definition->replaceArgument(0, new Reference($config['cookie_jar']));
+
                 break;
             case 'decoder':
                 $definition->addArgument([
                     'use_content_encoding' => $config['use_content_encoding'],
                 ]);
+
                 break;
             case 'history':
                 $definition->replaceArgument(0, new Reference($config['journal']));
+
                 break;
             case 'logger':
                 $definition->replaceArgument(0, new Reference($config['logger']));
                 if (!empty($config['formatter'])) {
                     $definition->replaceArgument(1, new Reference($config['formatter']));
                 }
+
                 break;
             case 'redirect':
                 $definition->addArgument([
                     'preserve_header' => $config['preserve_header'],
                     'use_default_for_multiple' => $config['use_default_for_multiple'],
                 ]);
+
                 break;
             case 'retry':
                 $definition->addArgument([
                     'retries' => $config['retry'],
                 ]);
+
                 break;
             case 'stopwatch':
                 $definition->replaceArgument(0, new Reference($config['stopwatch']));
+
                 break;
 
             /* client specific plugins */
@@ -191,12 +199,14 @@ private function configurePluginByName($name, Definition $definition, array $con
                 $definition->replaceArgument(1, [
                     'replace' => $config['replace'],
                 ]);
+
                 break;
             case 'header_append':
             case 'header_defaults':
             case 'header_set':
             case 'header_remove':
                 $definition->replaceArgument(0, $config['headers']);
+
                 break;
 
             default:
@@ -220,19 +230,23 @@ private function configureAuthentication(ContainerBuilder $container, array $con
                 case 'bearer':
                     $container->register($authServiceKey, Bearer::class)
                         ->addArgument($values['token']);
+
                     break;
                 case 'basic':
                     $container->register($authServiceKey, BasicAuth::class)
                         ->addArgument($values['username'])
                         ->addArgument($values['password']);
+
                     break;
                 case 'wsse':
                     $container->register($authServiceKey, Wsse::class)
                         ->addArgument($values['username'])
                         ->addArgument($values['password']);
+
                     break;
                 case 'service':
                     $authServiceKey = $values['service'];
+
                     break;
                 default:
                     throw new \LogicException(sprintf('Unknown authentication type: "%s"', $values['type']));
@@ -361,7 +375,6 @@ private function configureAutoDiscoveryClients(ContainerBuilder $container, arra
         if ($httpClient !== 'auto') {
             $container->removeDefinition('httplug.auto_discovery.auto_discovered_client');
             $container->removeDefinition('httplug.collector.auto_discovered_client');
-            $container->removeDefinition('httplug.auto_discovery.auto_discovered_client.plugin');
 
             if (!empty($httpClient)) {
                 $container->setAlias('httplug.auto_discovery.auto_discovered_client', $httpClient);
@@ -373,7 +386,6 @@ private function configureAutoDiscoveryClients(ContainerBuilder $container, arra
         if ($asyncHttpClient !== 'auto') {
             $container->removeDefinition('httplug.auto_discovery.auto_discovered_async');
             $container->removeDefinition('httplug.collector.auto_discovered_async');
-            $container->removeDefinition('httplug.auto_discovery.auto_discovered_async.plugin');
 
             if (!empty($asyncHttpClient)) {
                 $container->setAlias('httplug.auto_discovery.auto_discovered_async', $asyncHttpClient);
diff --git a/Resources/config/data-collector.xml b/Resources/config/data-collector.xml
@@ -27,41 +27,20 @@
         </service>
 
         <!-- Discovered clients -->
-        <service id="httplug.collector.auto_discovered_client" class="Http\HttplugBundle\Collector\ProfileClient" decorates="httplug.auto_discovery.auto_discovered_client" decoration-priority="2" public="false">
+        <service id="httplug.collector.auto_discovered_client" class="Http\HttplugBundle\Collector\ProfileClient" decorates="httplug.auto_discovery.auto_discovered_client" public="false">
             <argument type="service" id="httplug.collector.auto_discovered_client.inner"/>
             <argument type="service" id="httplug.collector.collector"/>
             <argument type="service" id="httplug.collector.formatter"/>
             <argument type="service" id="debug.stopwatch"/>
         </service>
 
-        <service id="httplug.collector.auto_discovered_async" class="Http\HttplugBundle\Collector\ProfileClient" decorates="httplug.auto_discovery.auto_discovered_async" decoration-priority="2" public="false">
+        <service id="httplug.collector.auto_discovered_async" class="Http\HttplugBundle\Collector\ProfileClient" decorates="httplug.auto_discovery.auto_discovered_async" public="false">
             <argument type="service" id="httplug.collector.auto_discovered_async.inner"/>
             <argument type="service" id="httplug.collector.collector"/>
             <argument type="service" id="httplug.collector.formatter"/>
             <argument type="service" id="debug.stopwatch"/>
         </service>
 
-        <service id="httplug.auto_discovery.auto_discovered_client.plugin" class="Http\Client\Common\PluginClient" decorates="httplug.auto_discovery.auto_discovered_client" public="false">
-            <argument type="service" id="httplug.auto_discovery.auto_discovered_client.plugin.inner"/>
-            <argument type="collection">
-                <argument type="service">
-                    <service parent="httplug.plugin.stack">
-                        <argument>auto_discovered_client</argument>
-                    </service>
-                </argument>
-            </argument>
-        </service>
-        <service id="httplug.auto_discovery.auto_discovered_async.plugin" class="Http\Client\Common\PluginClient" decorates="httplug.auto_discovery.auto_discovered_async" public="false">
-            <argument type="service" id="httplug.auto_discovery.auto_discovered_async.plugin.inner"/>
-            <argument type="collection">
-                <argument type="service">
-                    <service parent="httplug.plugin.stack">
-                        <argument>auto_discovered_async</argument>
-                    </service>
-                </argument>
-            </argument>
-        </service>
-
         <!-- ClientFactories -->
         <service id="httplug.collector.factory.auto" class="Http\HttplugBundle\Collector\ProfileClientFactory" decorates="httplug.factory.auto" public="false">
             <argument type="service" id="httplug.collector.factory.auto.inner"/>
diff --git a/Resources/config/services.xml b/Resources/config/services.xml
@@ -18,14 +18,6 @@
             <factory class="Http\Discovery\HttpAsyncClientDiscovery" method="find" />
         </service>
 
-        <!-- Decorate discovered clients with PluginClient -->
-        <service id="httplug.auto_discovery.auto_discovered_client.plugin" class="Http\Client\Common\PluginClient" decorates="httplug.auto_discovery.auto_discovered_client" decoration-priority="1" public="false">
-            <argument type="service" id="httplug.auto_discovery.auto_discovered_client.plugin.inner"/>
-        </service>
-        <service id="httplug.auto_discovery.auto_discovered_async.plugin" class="Http\Client\Common\PluginClient" decorates="httplug.auto_discovery.auto_discovered_async" decoration-priority="1" public="false">
-            <argument type="service" id="httplug.auto_discovery.auto_discovered_async.plugin.inner"/>
-        </service>
-
         <!-- Discovery with autowiring support for Symfony 3.3+ -->
         <service id="httplug.message_factory.default" class="Http\Message\MessageFactory">
             <factory class="Http\Discovery\MessageFactoryDiscovery" method="find" />
diff --git a/Tests/Functional/DiscoveredClientsTest.php b/Tests/Functional/DiscoveredClientsTest.php
@@ -2,13 +2,12 @@
 
 namespace Http\HttplugBundle\Tests\Functional;
 
-use Http\Client\Common\PluginClient;
 use Http\Client\HttpAsyncClient;
 use Http\Client\HttpClient;
 use Http\Discovery\HttpAsyncClientDiscovery;
 use Http\Discovery\HttpClientDiscovery;
 use Http\Discovery\Strategy\CommonClassesStrategy;
-use Http\HttplugBundle\Collector\StackPlugin;
+use Http\HttplugBundle\Collector\ProfileClient;
 use Http\HttplugBundle\Discovery\ConfiguredClientsStrategy;
 use Nyholm\NSA;
 use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
@@ -24,9 +23,7 @@ public function testDiscoveredClient()
 
         $service = $container->get('httplug.auto_discovery.auto_discovered_client');
 
-        $this->assertInstanceOf(PluginClient::class, $service);
-        $this->assertInstanceOf(HttpClient::class, NSA::getProperty($service, 'client'));
-        $this->assertEmpty(NSA::getProperty($service, 'plugins'));
+        $this->assertInstanceOf(HttpClient::class, $service);
     }
 
     public function testDiscoveredAsyncClient()
@@ -37,9 +34,7 @@ public function testDiscoveredAsyncClient()
 
         $service = $container->get('httplug.auto_discovery.auto_discovered_async');
 
-        $this->assertInstanceOf(PluginClient::class, $service);
-        $this->assertInstanceOf(HttpAsyncClient::class, NSA::getProperty($service, 'client'));
-        $this->assertEmpty(NSA::getProperty($service, 'plugins'));
+        $this->assertInstanceOf(HttpAsyncClient::class, $service);
     }
 
     public function testDiscoveredClientWithProfilingEnabled()
@@ -50,13 +45,8 @@ public function testDiscoveredClientWithProfilingEnabled()
 
         $service = $container->get('httplug.auto_discovery.auto_discovered_client');
 
-        $this->assertInstanceOf(PluginClient::class, $service);
+        $this->assertInstanceOf(ProfileClient::class, $service);
         $this->assertInstanceOf(HttpClient::class, NSA::getProperty($service, 'client'));
-
-        $plugins = NSA::getProperty($service, 'plugins');
-        $this->assertCount(1, $plugins);
-        $this->assertInstanceOf(StackPlugin::class, $plugins[0]);
-        $this->assertEquals('auto_discovered_client', NSA::getProperty($plugins[0], 'client'));
     }
 
     public function testDiscoveredAsyncClientWithProfilingEnabled()
@@ -67,13 +57,8 @@ public function testDiscoveredAsyncClientWithProfilingEnabled()
 
         $service = $container->get('httplug.auto_discovery.auto_discovered_async');
 
-        $this->assertInstanceOf(PluginClient::class, $service);
+        $this->assertInstanceOf(ProfileClient::class, $service);
         $this->assertInstanceOf(HttpAsyncClient::class, NSA::getProperty($service, 'client'));
-
-        $plugins = NSA::getProperty($service, 'plugins');
-        $this->assertCount(1, $plugins);
-        $this->assertInstanceOf(StackPlugin::class, $plugins[0]);
-        $this->assertEquals('auto_discovered_async', NSA::getProperty($plugins[0], 'client'));
     }
 
     /**
@@ -92,9 +77,9 @@ public function testDiscovery()
         $httpClient = $container->get('httplug.auto_discovery.auto_discovered_client');
         $httpAsyncClient = $container->get('httplug.auto_discovery.auto_discovered_async');
 
-        $this->assertInstanceOf(PluginClient::class, $httpClient);
+        $this->assertInstanceOf(ProfileClient::class, $httpClient);
         $this->assertSame(HttpClientDiscovery::find(), $httpClient);
-        $this->assertInstanceOf(PluginClient::class, $httpAsyncClient);
+        $this->assertInstanceOf(ProfileClient::class, $httpAsyncClient);
         $this->assertSame(HttpAsyncClientDiscovery::find(), $httpAsyncClient);
     }
 
