Skip to content

Commit 8c98056

Browse files
fbourigaultfbourigault
authored
Merge pull request #154 from fbourigault/fix-missing-stack
Display discovered clients in the profiler
2 parents 5c2862c + 67b0210 commit 8c98056

File tree

2 files changed

+138
-44
lines changed

2 files changed

+138
-44
lines changed

‎DependencyInjection/HttplugExtension.php‎

Lines changed: 137 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use Http\Discovery\HttpClientDiscovery;
1111
use Http\HttplugBundle\ClientFactory\DummyClient;
1212
use Http\HttplugBundle\ClientFactory\PluginClientFactory;
13+
use Http\HttplugBundle\Collector\ProfileClientFactory;
1314
use Http\HttplugBundle\Collector\ProfilePlugin;
1415
use Http\Message\Authentication\BasicAuth;
1516
use Http\Message\Authentication\Bearer;
@@ -272,16 +273,7 @@ private function configureClient(ContainerBuilder $container, $clientName, array
272273
} elseif ('authentication' === $pluginName) {
273274
$plugins = array_merge($plugins, $this->configureAuthentication($container, $pluginConfig, $serviceId.'.authentication'));
274275
} else {
275-
$pluginServiceId = $serviceId.'.plugin.'.$pluginName;
276-
277-
$definition = class_exists(ChildDefinition::class)
278-
? new ChildDefinition('httplug.plugin'.'.'.$pluginName)
279-
: new DefinitionDecorator('httplug.plugin'.'.'.$pluginName)
280-
;
281-
282-
$this->configurePluginByName($pluginName, $definition, $pluginConfig, $container, $pluginServiceId);
283-
$container->setDefinition($pluginServiceId, $definition);
284-
$plugins[] = $pluginServiceId;
276+
$plugins[] = $this->configurePlugin($container, $serviceId, $pluginName, $pluginConfig);
285277
}
286278
}
287279

@@ -294,27 +286,12 @@ private function configureClient(ContainerBuilder $container, $clientName, array
294286

295287
//Decorate each plugin with a ProfilePlugin instance.
296288
foreach ($plugins as $pluginServiceId) {
297-
$container->register($pluginServiceId.'.debug', ProfilePlugin::class)
298-
->setDecoratedService($pluginServiceId)
299-
->setArguments([
300-
new Reference($pluginServiceId.'.debug.inner'),
301-
new Reference('httplug.collector.collector'),
302-
new Reference('httplug.collector.formatter'),
303-
$pluginServiceId,
304-
])
305-
->setPublic(false)
306-
;
289+
$this->decoratePluginWithProfilePlugin($container, $pluginServiceId);
307290
}
308291

309-
// Add the newstack plugin
310-
$definition = class_exists(ChildDefinition::class)
311-
? new ChildDefinition('httplug.plugin.stack')
312-
: new DefinitionDecorator('httplug.plugin.stack')
313-
;
314-
315-
$definition->addArgument($clientName);
316-
$container->setDefinition($serviceId.'.plugin.newstack', $definition);
317-
array_unshift($plugins, $serviceId.'.plugin.newstack');
292+
// To profile the requests, add a StackPlugin as first plugin in the chain.
293+
$stackPluginId = $this->configureStackPlugin($container, $clientName, $serviceId);
294+
array_unshift($plugins, $stackPluginId);
318295
}
319296

320297
$container
@@ -397,7 +374,12 @@ private function configureAutoDiscoveryClients(ContainerBuilder $container, arra
397374
$httpClient = $this->registerAutoDiscoverableClient(
398375
$container,
399376
'auto_discovered_client',
400-
[HttpClientDiscovery::class, 'find'],
377+
$this->configureAutoDiscoveryFactory(
378+
$container,
379+
HttpClientDiscovery::class,
380+
'auto_discovered_client',
381+
$config
382+
),
401383
$this->isConfigEnabled($container, $config['profiling'])
402384
);
403385
}
@@ -412,7 +394,12 @@ private function configureAutoDiscoveryClients(ContainerBuilder $container, arra
412394
$asyncHttpClient = $this->registerAutoDiscoverableClient(
413395
$container,
414396
'auto_discovered_async',
415-
[HttpAsyncClientDiscovery::class, 'find'],
397+
$this->configureAutoDiscoveryFactory(
398+
$container,
399+
HttpAsyncClientDiscovery::class,
400+
'auto_discovered_async',
401+
$config
402+
),
416403
$this->isConfigEnabled($container, $config['profiling'])
417404
);
418405
}
@@ -436,33 +423,46 @@ private function configureAutoDiscoveryClients(ContainerBuilder $container, arra
436423
/**
437424
* Find a client with auto discovery and return a service Reference to it.
438425
*
439-
* @param ContainerBuilder $container
440-
* @param string $name
441-
* @param callable $factory
442-
* @param bool $profiling
426+
* @param ContainerBuilder $container
427+
* @param string $name
428+
* @param Reference|callable $factory
429+
* @param bool $profiling
443430
*
444431
* @return string service id
445432
*/
446433
private function registerAutoDiscoverableClient(ContainerBuilder $container, $name, $factory, $profiling)
447434
{
448435
$serviceId = 'httplug.auto_discovery.'.$name;
449436

450-
$pluginClientOptions = [];
451-
437+
$plugins = [];
452438
if ($profiling) {
453-
// Tell the plugin journal what plugins we used
454-
$container
455-
->getDefinition('httplug.collector.plugin_journal')
456-
->addMethodCall('setPlugins', [$name, ['httplug.plugin.stopwatch']])
457-
;
439+
// To profile the requests, add a StackPlugin as first plugin in the chain.
440+
$plugins[] = $this->configureStackPlugin($container, $name, $serviceId);
441+
442+
$this->decoratePluginWithProfilePlugin($container, 'httplug.plugin.stopwatch');
443+
$plugins[] = 'httplug.plugin.stopwatch';
458444
}
459445

460446
$container
461447
->register($serviceId, DummyClient::class)
462448
->setFactory([PluginClientFactory::class, 'createPluginClient'])
463-
->setArguments([[new Reference('httplug.plugin.stopwatch')], $factory, [], $pluginClientOptions])
449+
->setArguments([
450+
array_map(
451+
function ($id) {
452+
return new Reference($id);
453+
},
454+
$plugins
455+
),
456+
$factory,
457+
[],
458+
])
464459
;
465460

461+
if ($profiling) {
462+
$collector = $container->getDefinition('httplug.collector.collector');
463+
$collector->replaceArgument(0, array_merge($collector->getArgument(0), [$name]));
464+
}
465+
466466
return $serviceId;
467467
}
468468

@@ -473,4 +473,98 @@ public function getConfiguration(array $config, ContainerBuilder $container)
473473
{
474474
return new Configuration($container->getParameter('kernel.debug'));
475475
}
476+
477+
/**
478+
* Configure a plugin using the parent definition from plugins.xml.
479+
*
480+
* @param ContainerBuilder $container
481+
* @param string $serviceId
482+
* @param string $pluginName
483+
* @param array $pluginConfig
484+
*
485+
* @return string configured service id
486+
*/
487+
private function configurePlugin(ContainerBuilder $container, $serviceId, $pluginName, array $pluginConfig)
488+
{
489+
$pluginServiceId = $serviceId.'.plugin.'.$pluginName;
490+
491+
$definition = class_exists(ChildDefinition::class)
492+
? new ChildDefinition('httplug.plugin.'.$pluginName)
493+
: new DefinitionDecorator('httplug.plugin.'.$pluginName);
494+
495+
$this->configurePluginByName($pluginName, $definition, $pluginConfig, $container, $pluginServiceId);
496+
$container->setDefinition($pluginServiceId, $definition);
497+
498+
return $pluginServiceId;
499+
}
500+
501+
/**
502+
* Decorate the plugin service with a ProfilePlugin service.
503+
*
504+
* @param ContainerBuilder $container
505+
* @param string $pluginServiceId
506+
*/
507+
private function decoratePluginWithProfilePlugin(ContainerBuilder $container, $pluginServiceId)
508+
{
509+
$container->register($pluginServiceId.'.debug', ProfilePlugin::class)
510+
->setDecoratedService($pluginServiceId)
511+
->setArguments([
512+
new Reference($pluginServiceId.'.debug.inner'),
513+
new Reference('httplug.collector.collector'),
514+
new Reference('httplug.collector.formatter'),
515+
$pluginServiceId,
516+
])
517+
->setPublic(false);
518+
}
519+
520+
/**
521+
* Configure a StackPlugin for a client.
522+
*
523+
* @param ContainerBuilder $container
524+
* @param string $clientName Client name to display in the profiler.
525+
* @param string $serviceId Client service id. Used as base for the StackPlugin service id.
526+
*
527+
* @return string configured StackPlugin service id
528+
*/
529+
private function configureStackPlugin(ContainerBuilder $container, $clientName, $serviceId)
530+
{
531+
$pluginServiceId = $serviceId.'.plugin.stack';
532+
533+
$definition = class_exists(ChildDefinition::class)
534+
? new ChildDefinition('httplug.plugin.stack')
535+
: new DefinitionDecorator('httplug.plugin.stack');
536+
537+
$definition->addArgument($clientName);
538+
$container->setDefinition($pluginServiceId, $definition);
539+
540+
return $pluginServiceId;
541+
}
542+
543+
/**
544+
* Configure the discovery factory when profiling is enabled to get client decorated with a ProfileClient.
545+
*
546+
* @param ContainerBuilder $container
547+
* @param string $discovery
548+
* @param string $name
549+
* @param array $config
550+
*
551+
* @return callable|Reference
552+
*/
553+
private function configureAutoDiscoveryFactory(ContainerBuilder $container, $discovery, $name, array $config)
554+
{
555+
$factory = [$discovery, 'find'];
556+
if ($this->isConfigEnabled($container, $config['profiling'])) {
557+
$factoryServiceId = 'httplug.auto_discovery.'.$name.'.factory';
558+
$container->register($factoryServiceId, ProfileClientFactory::class)
559+
->setPublic(false)
560+
->setArguments([
561+
$factory,
562+
new Reference('httplug.collector.collector'),
563+
new Reference('httplug.collector.formatter'),
564+
]);
565+
$factory = new Reference($factoryServiceId);
566+
}
567+
568+
return $factory;
569+
}
476570
}

‎Tests/Unit/DependencyInjection/HttplugExtensionTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ public function testClientPlugins()
116116
]);
117117

118118
$plugins = [
119-
'httplug.client.acme.plugin.newstack',
119+
'httplug.client.acme.plugin.stack',
120120
'httplug.plugin.stopwatch',
121121
'httplug.client.acme.plugin.decoder',
122122
'httplug.plugin.redirect',

0 commit comments

Comments
 (0)