From 073285cb46be3507fb6b17f182f51fdd6e44505e Mon Sep 17 00:00:00 2001 From: Fabien Bourigault Date: Fri, 18 Aug 2017 16:26:02 +0200 Subject: [PATCH 1/2] don't use PluginClient for discovered clients --- Collector/ProfileClient.php | 20 ++++++++++++++- DependencyInjection/HttplugExtension.php | 2 -- Resources/config/data-collector.xml | 25 ++----------------- Resources/config/services.xml | 8 ------ Tests/Functional/DiscoveredClientsTest.php | 29 ++++++---------------- 5 files changed, 28 insertions(+), 56 deletions(-) diff --git a/Collector/ProfileClient.php b/Collector/ProfileClient.php index bc2693c3..5e92244d 100644 --- 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 index 9b72cb8a..4c3a90b2 100644 --- a/DependencyInjection/HttplugExtension.php +++ b/DependencyInjection/HttplugExtension.php @@ -361,7 +361,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 +372,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 index 88d2e3a3..890e11c1 100644 --- a/Resources/config/data-collector.xml +++ b/Resources/config/data-collector.xml @@ -27,41 +27,20 @@ - + - + - - - - - - auto_discovered_client - - - - - - - - - - auto_discovered_async - - - - - diff --git a/Resources/config/services.xml b/Resources/config/services.xml index 62671e22..1f82018a 100644 --- a/Resources/config/services.xml +++ b/Resources/config/services.xml @@ -18,14 +18,6 @@ - - - - - - - - diff --git a/Tests/Functional/DiscoveredClientsTest.php b/Tests/Functional/DiscoveredClientsTest.php index 2986cf49..5707ed9c 100644 --- 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); } From 3f862c6c3c8283c398f6975a5462cae23aed59d0 Mon Sep 17 00:00:00 2001 From: Fabien Bourigault Date: Tue, 22 Aug 2017 19:02:45 +0200 Subject: [PATCH 2/2] add blank line before break instruction --- DependencyInjection/HttplugExtension.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/DependencyInjection/HttplugExtension.php b/DependencyInjection/HttplugExtension.php index 4c3a90b2..c6753d1f 100644 --- 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']));