Skip to content

Commit 33ed6ae

Browse files
fbourigaultNyholm
fbourigault
authored and
Nyholm
committed
Don't use PluginClient for discovered clients (#200)
* don't use PluginClient for discovered clients * add blank line before break instruction
1 parent d1fda4a commit 33ed6ae

File tree

5 files changed

+42
-56
lines changed

5 files changed

+42
-56
lines changed

‎Collector/ProfileClient.php

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,16 @@ public function __construct($client, Collector $collector, Formatter $formatter,
7575
*/
7676
public function sendAsyncRequest(RequestInterface $request)
7777
{
78+
$activateStack = true;
7879
$stack = $this->collector->getActiveStack();
80+
if ($stack === null) {
81+
//When using a discovered client not wrapped in a PluginClient, we don't have a stack from StackPlugin. So
82+
//we create our own stack and activate it!
83+
$stack = new Stack('Default', $this->formatter->formatRequest($request));
84+
$this->collector->addStack($stack);
85+
$this->collector->activateStack($stack);
86+
$activateStack = false;
87+
}
7988

8089
$this->collectRequestInformations($request, $stack);
8190
$event = $this->stopwatch->start($this->getStopwatchEventName($request));
@@ -98,7 +107,10 @@ public function sendAsyncRequest(RequestInterface $request)
98107
return $this->client->sendAsyncRequest($request)->then($onFulfilled, $onRejected);
99108
} finally {
100109
$event->stop();
101-
$this->collector->activateStack($stack);
110+
if ($activateStack) {
111+
//We only activate the stack when created by the StackPlugin.
112+
$this->collector->activateStack($stack);
113+
}
102114
}
103115
}
104116

@@ -108,6 +120,12 @@ public function sendAsyncRequest(RequestInterface $request)
108120
public function sendRequest(RequestInterface $request)
109121
{
110122
$stack = $this->collector->getActiveStack();
123+
if ($stack === null) {
124+
//When using a discovered client not wrapped in a PluginClient, we don't have a stack from StackPlugin. So
125+
//we create our own stack but don't activate it.
126+
$stack = new Stack('Default', $this->formatter->formatRequest($request));
127+
$this->collector->addStack($stack);
128+
}
111129

112130
$this->collectRequestInformations($request, $stack);
113131
$event = $this->stopwatch->start($this->getStopwatchEventName($request));

‎DependencyInjection/HttplugExtension.php

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,37 +149,45 @@ private function configurePluginByName($name, Definition $definition, array $con
149149
->replaceArgument(0, new Reference($config['cache_pool']))
150150
->replaceArgument(1, new Reference($config['stream_factory']))
151151
->replaceArgument(2, $config['config']);
152+
152153
break;
153154
case 'cookie':
154155
$definition->replaceArgument(0, new Reference($config['cookie_jar']));
156+
155157
break;
156158
case 'decoder':
157159
$definition->addArgument([
158160
'use_content_encoding' => $config['use_content_encoding'],
159161
]);
162+
160163
break;
161164
case 'history':
162165
$definition->replaceArgument(0, new Reference($config['journal']));
166+
163167
break;
164168
case 'logger':
165169
$definition->replaceArgument(0, new Reference($config['logger']));
166170
if (!empty($config['formatter'])) {
167171
$definition->replaceArgument(1, new Reference($config['formatter']));
168172
}
173+
169174
break;
170175
case 'redirect':
171176
$definition->addArgument([
172177
'preserve_header' => $config['preserve_header'],
173178
'use_default_for_multiple' => $config['use_default_for_multiple'],
174179
]);
180+
175181
break;
176182
case 'retry':
177183
$definition->addArgument([
178184
'retries' => $config['retry'],
179185
]);
186+
180187
break;
181188
case 'stopwatch':
182189
$definition->replaceArgument(0, new Reference($config['stopwatch']));
190+
183191
break;
184192

185193
/* client specific plugins */
@@ -191,12 +199,14 @@ private function configurePluginByName($name, Definition $definition, array $con
191199
$definition->replaceArgument(1, [
192200
'replace' => $config['replace'],
193201
]);
202+
194203
break;
195204
case 'header_append':
196205
case 'header_defaults':
197206
case 'header_set':
198207
case 'header_remove':
199208
$definition->replaceArgument(0, $config['headers']);
209+
200210
break;
201211

202212
default:
@@ -220,19 +230,23 @@ private function configureAuthentication(ContainerBuilder $container, array $con
220230
case 'bearer':
221231
$container->register($authServiceKey, Bearer::class)
222232
->addArgument($values['token']);
233+
223234
break;
224235
case 'basic':
225236
$container->register($authServiceKey, BasicAuth::class)
226237
->addArgument($values['username'])
227238
->addArgument($values['password']);
239+
228240
break;
229241
case 'wsse':
230242
$container->register($authServiceKey, Wsse::class)
231243
->addArgument($values['username'])
232244
->addArgument($values['password']);
245+
233246
break;
234247
case 'service':
235248
$authServiceKey = $values['service'];
249+
236250
break;
237251
default:
238252
throw new \LogicException(sprintf('Unknown authentication type: "%s"', $values['type']));
@@ -361,7 +375,6 @@ private function configureAutoDiscoveryClients(ContainerBuilder $container, arra
361375
if ($httpClient !== 'auto') {
362376
$container->removeDefinition('httplug.auto_discovery.auto_discovered_client');
363377
$container->removeDefinition('httplug.collector.auto_discovered_client');
364-
$container->removeDefinition('httplug.auto_discovery.auto_discovered_client.plugin');
365378

366379
if (!empty($httpClient)) {
367380
$container->setAlias('httplug.auto_discovery.auto_discovered_client', $httpClient);
@@ -373,7 +386,6 @@ private function configureAutoDiscoveryClients(ContainerBuilder $container, arra
373386
if ($asyncHttpClient !== 'auto') {
374387
$container->removeDefinition('httplug.auto_discovery.auto_discovered_async');
375388
$container->removeDefinition('httplug.collector.auto_discovered_async');
376-
$container->removeDefinition('httplug.auto_discovery.auto_discovered_async.plugin');
377389

378390
if (!empty($asyncHttpClient)) {
379391
$container->setAlias('httplug.auto_discovery.auto_discovered_async', $asyncHttpClient);

‎Resources/config/data-collector.xml

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -27,41 +27,20 @@
2727
</service>
2828

2929
<!-- Discovered clients -->
30-
<service id="httplug.collector.auto_discovered_client" class="Http\HttplugBundle\Collector\ProfileClient" decorates="httplug.auto_discovery.auto_discovered_client" decoration-priority="2" public="false">
30+
<service id="httplug.collector.auto_discovered_client" class="Http\HttplugBundle\Collector\ProfileClient" decorates="httplug.auto_discovery.auto_discovered_client" public="false">
3131
<argument type="service" id="httplug.collector.auto_discovered_client.inner"/>
3232
<argument type="service" id="httplug.collector.collector"/>
3333
<argument type="service" id="httplug.collector.formatter"/>
3434
<argument type="service" id="debug.stopwatch"/>
3535
</service>
3636

37-
<service id="httplug.collector.auto_discovered_async" class="Http\HttplugBundle\Collector\ProfileClient" decorates="httplug.auto_discovery.auto_discovered_async" decoration-priority="2" public="false">
37+
<service id="httplug.collector.auto_discovered_async" class="Http\HttplugBundle\Collector\ProfileClient" decorates="httplug.auto_discovery.auto_discovered_async" public="false">
3838
<argument type="service" id="httplug.collector.auto_discovered_async.inner"/>
3939
<argument type="service" id="httplug.collector.collector"/>
4040
<argument type="service" id="httplug.collector.formatter"/>
4141
<argument type="service" id="debug.stopwatch"/>
4242
</service>
4343

44-
<service id="httplug.auto_discovery.auto_discovered_client.plugin" class="Http\Client\Common\PluginClient" decorates="httplug.auto_discovery.auto_discovered_client" public="false">
45-
<argument type="service" id="httplug.auto_discovery.auto_discovered_client.plugin.inner"/>
46-
<argument type="collection">
47-
<argument type="service">
48-
<service parent="httplug.plugin.stack">
49-
<argument>auto_discovered_client</argument>
50-
</service>
51-
</argument>
52-
</argument>
53-
</service>
54-
<service id="httplug.auto_discovery.auto_discovered_async.plugin" class="Http\Client\Common\PluginClient" decorates="httplug.auto_discovery.auto_discovered_async" public="false">
55-
<argument type="service" id="httplug.auto_discovery.auto_discovered_async.plugin.inner"/>
56-
<argument type="collection">
57-
<argument type="service">
58-
<service parent="httplug.plugin.stack">
59-
<argument>auto_discovered_async</argument>
60-
</service>
61-
</argument>
62-
</argument>
63-
</service>
64-
6544
<!-- ClientFactories -->
6645
<service id="httplug.collector.factory.auto" class="Http\HttplugBundle\Collector\ProfileClientFactory" decorates="httplug.factory.auto" public="false">
6746
<argument type="service" id="httplug.collector.factory.auto.inner"/>

‎Resources/config/services.xml

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,6 @@
1818
<factory class="Http\Discovery\HttpAsyncClientDiscovery" method="find" />
1919
</service>
2020

21-
<!-- Decorate discovered clients with PluginClient -->
22-
<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">
23-
<argument type="service" id="httplug.auto_discovery.auto_discovered_client.plugin.inner"/>
24-
</service>
25-
<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">
26-
<argument type="service" id="httplug.auto_discovery.auto_discovered_async.plugin.inner"/>
27-
</service>
28-
2921
<!-- Discovery with autowiring support for Symfony 3.3+ -->
3022
<service id="httplug.message_factory.default" class="Http\Message\MessageFactory">
3123
<factory class="Http\Discovery\MessageFactoryDiscovery" method="find" />

‎Tests/Functional/DiscoveredClientsTest.php

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,12 @@
22

33
namespace Http\HttplugBundle\Tests\Functional;
44

5-
use Http\Client\Common\PluginClient;
65
use Http\Client\HttpAsyncClient;
76
use Http\Client\HttpClient;
87
use Http\Discovery\HttpAsyncClientDiscovery;
98
use Http\Discovery\HttpClientDiscovery;
109
use Http\Discovery\Strategy\CommonClassesStrategy;
11-
use Http\HttplugBundle\Collector\StackPlugin;
10+
use Http\HttplugBundle\Collector\ProfileClient;
1211
use Http\HttplugBundle\Discovery\ConfiguredClientsStrategy;
1312
use Nyholm\NSA;
1413
use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
@@ -24,9 +23,7 @@ public function testDiscoveredClient()
2423

2524
$service = $container->get('httplug.auto_discovery.auto_discovered_client');
2625

27-
$this->assertInstanceOf(PluginClient::class, $service);
28-
$this->assertInstanceOf(HttpClient::class, NSA::getProperty($service, 'client'));
29-
$this->assertEmpty(NSA::getProperty($service, 'plugins'));
26+
$this->assertInstanceOf(HttpClient::class, $service);
3027
}
3128

3229
public function testDiscoveredAsyncClient()
@@ -37,9 +34,7 @@ public function testDiscoveredAsyncClient()
3734

3835
$service = $container->get('httplug.auto_discovery.auto_discovered_async');
3936

40-
$this->assertInstanceOf(PluginClient::class, $service);
41-
$this->assertInstanceOf(HttpAsyncClient::class, NSA::getProperty($service, 'client'));
42-
$this->assertEmpty(NSA::getProperty($service, 'plugins'));
37+
$this->assertInstanceOf(HttpAsyncClient::class, $service);
4338
}
4439

4540
public function testDiscoveredClientWithProfilingEnabled()
@@ -50,13 +45,8 @@ public function testDiscoveredClientWithProfilingEnabled()
5045

5146
$service = $container->get('httplug.auto_discovery.auto_discovered_client');
5247

53-
$this->assertInstanceOf(PluginClient::class, $service);
48+
$this->assertInstanceOf(ProfileClient::class, $service);
5449
$this->assertInstanceOf(HttpClient::class, NSA::getProperty($service, 'client'));
55-
56-
$plugins = NSA::getProperty($service, 'plugins');
57-
$this->assertCount(1, $plugins);
58-
$this->assertInstanceOf(StackPlugin::class, $plugins[0]);
59-
$this->assertEquals('auto_discovered_client', NSA::getProperty($plugins[0], 'client'));
6050
}
6151

6252
public function testDiscoveredAsyncClientWithProfilingEnabled()
@@ -67,13 +57,8 @@ public function testDiscoveredAsyncClientWithProfilingEnabled()
6757

6858
$service = $container->get('httplug.auto_discovery.auto_discovered_async');
6959

70-
$this->assertInstanceOf(PluginClient::class, $service);
60+
$this->assertInstanceOf(ProfileClient::class, $service);
7161
$this->assertInstanceOf(HttpAsyncClient::class, NSA::getProperty($service, 'client'));
72-
73-
$plugins = NSA::getProperty($service, 'plugins');
74-
$this->assertCount(1, $plugins);
75-
$this->assertInstanceOf(StackPlugin::class, $plugins[0]);
76-
$this->assertEquals('auto_discovered_async', NSA::getProperty($plugins[0], 'client'));
7762
}
7863

7964
/**
@@ -92,9 +77,9 @@ public function testDiscovery()
9277
$httpClient = $container->get('httplug.auto_discovery.auto_discovered_client');
9378
$httpAsyncClient = $container->get('httplug.auto_discovery.auto_discovered_async');
9479

95-
$this->assertInstanceOf(PluginClient::class, $httpClient);
80+
$this->assertInstanceOf(ProfileClient::class, $httpClient);
9681
$this->assertSame(HttpClientDiscovery::find(), $httpClient);
97-
$this->assertInstanceOf(PluginClient::class, $httpAsyncClient);
82+
$this->assertInstanceOf(ProfileClient::class, $httpAsyncClient);
9883
$this->assertSame(HttpAsyncClientDiscovery::find(), $httpAsyncClient);
9984
}
10085

0 commit comments

Comments
 (0)