From d49f7d63d339ca10b9d93b9914ecea634ac9ecfd Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Fri, 15 Jul 2016 17:01:03 +0200 Subject: [PATCH 1/9] Let user configure what do be found in discovery and if profiling should be used. --- Collector/PluginJournal.php | 6 +- DependencyInjection/Configuration.php | 7 +++ DependencyInjection/HttplugExtension.php | 70 ++++++++++++++++++++++++ Discovery/ConfiguredClientsStrategy.php | 18 +++++- Resources/config/services.xml | 1 - 5 files changed, 98 insertions(+), 4 deletions(-) diff --git a/Collector/PluginJournal.php b/Collector/PluginJournal.php index cb53d37d..cfc40e4d 100644 --- a/Collector/PluginJournal.php +++ b/Collector/PluginJournal.php @@ -21,7 +21,11 @@ final class PluginJournal */ public function getPlugins($clientName) { - return $this->data[$clientName]; + if (isset($this->data[$clientName])) { + return $this->data[$clientName]; + } + + return []; } /** diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index cf0ba826..8bc36f50 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -83,6 +83,8 @@ public function getConfigTreeBuilder() ->defaultValue('auto') ->end() ->scalarNode('formatter')->defaultNull()->end() + ->booleanNode('profile_discovered_client')->defaultTrue()->end() + ->booleanNode('profile_discovered_async_client')->defaultFalse()->end() ->scalarNode('captured_body_length') ->defaultValue(0) ->canNotBeEmpty() @@ -124,6 +126,11 @@ protected function configureClients(ArrayNodeDefinition $root) ->defaultFalse() ->info('Set to true to get the client wrapped in a HttpMethodsClient which emulates provides functions for HTTP verbs.') ->end() + ->enumNode('use_with_discovery') + ->values(['async_client', 'http_client', false]) + ->defaultFalse() + ->info('If set to "http_client" this will be the client return when using dicovery to find a HTTP client.') + ->end() ->arrayNode('plugins') ->info('A list of service ids of plugins. The order is important.') ->prototype('scalar')->end() diff --git a/DependencyInjection/HttplugExtension.php b/DependencyInjection/HttplugExtension.php index 09b6c6e4..ca732303 100644 --- a/DependencyInjection/HttplugExtension.php +++ b/DependencyInjection/HttplugExtension.php @@ -6,6 +6,7 @@ use Http\Client\Common\HttpMethodsClient; use Http\Client\Common\Plugin\AuthenticationPlugin; use Http\Client\Common\PluginClient; +use Http\Discovery\HttpAsyncClientDiscovery; use Http\HttplugBundle\ClientFactory\DummyClient; use Http\HttplugBundle\Collector\DebugPlugin; use Http\Message\Authentication\BasicAuth; @@ -65,6 +66,7 @@ public function load(array $configs, ContainerBuilder $container) $this->configurePlugins($container, $config['plugins']); $this->configureClients($container, $config); + $this->configureAutoDiscoveryClients($container, $config); } /** @@ -278,4 +280,72 @@ private function registerDebugPlugin(ContainerBuilder $container, $name) return $serviceIdDebugPlugin; } + + /** + * Make sure we inject the debug plugin for clients found by auto discovery. + * + * @param ContainerBuilder $container + * @param array $config + */ + private function configureAutoDiscoveryClients(ContainerBuilder $container, array $config) + { + $httpClient = null; + $asyncHttpClient = null; + + // Verify if any clients were specifucally set to function as auto discoverable. + foreach ($config['clients'] as $name => $arguments) { + if ($arguments['use_with_discovery'] === 'http_client') { + if ($httpClient !== null) { + throw new \LogicException('Only one client can configured with "use_with_discovery: http_client".'); + } + $httpClient = new Reference('httplug.client.'.$name); + } elseif ($arguments['use_with_discovery'] === 'async_client') { + if ($asyncHttpClient !== null) { + throw new \LogicException('Only one client can be configured with "use_with_discovery: async_client".'); + } + $asyncHttpClient = new Reference('httplug.client.'.$name); + } + } + + if ($httpClient === null) { + // Use auto discovery + if ($config['toolbar']['profile_discovered_client']) { + $httpClient = $this->registerAutoDiscoverableClientWithDebugPlugin($container, 'client'); + } + } + + if ($asyncHttpClient === null) { + // Use auto discovery + if ($config['toolbar']['profile_discovered_async_client']) { + $asyncHttpClient = $this->registerAutoDiscoverableClientWithDebugPlugin($container, 'async_client'); + } + } + + + $container->getDefinition('httplug.strategy') + ->addArgument($httpClient) + ->addArgument($asyncHttpClient); + } + + /** + * @param ContainerBuilder $container + * @param $name + * + * @return Reference + */ + private function registerAutoDiscoverableClientWithDebugPlugin(ContainerBuilder $container, $name) + { + $definition = $container->register('httplug.auto_discovery_'.$name.'.pure', DummyClient::class); + $definition->setPublic(false); + $definition->setFactory([HttpAsyncClientDiscovery::class, 'find']); + + $serviceIdDebugPlugin = $this->registerDebugPlugin($container, 'auto_discovery_'.$name); + $container->register('httplug.auto_discovery_'.$name.'.plugin', PluginClient::class) + ->setPublic(false) + ->addArgument(new Reference('httplug.auto_discovery_'.$name.'.pure')) + ->addArgument([]) + ->addArgument(['debug_plugins' => [new Reference($serviceIdDebugPlugin)]]); + + return new Reference('httplug.auto_discovery_'.$name.'.plugin'); + } } diff --git a/Discovery/ConfiguredClientsStrategy.php b/Discovery/ConfiguredClientsStrategy.php index d3a71eb1..89e5e317 100644 --- a/Discovery/ConfiguredClientsStrategy.php +++ b/Discovery/ConfiguredClientsStrategy.php @@ -3,6 +3,7 @@ namespace Http\HttplugBundle\Discovery; use Http\Client\HttpClient; +use Http\Client\HttpAsyncClient; use Http\Discovery\HttpClientDiscovery; use Http\Discovery\Strategy\DiscoveryStrategy; use Symfony\Component\Console\ConsoleEvents; @@ -24,11 +25,18 @@ class ConfiguredClientsStrategy implements DiscoveryStrategy, EventSubscriberInt private static $client; /** - * @param HttpClient $httpClient + * @var HttpAsyncClient */ - public function __construct(HttpClient $httpClient) + private static $asyncClient; + + /** + * @param HttpClient $httpClient + * @param HttpAsyncClient $asyncClient + */ + public function __construct(HttpClient $httpClient = null, HttpAsyncClient $asyncClient = null) { static::$client = $httpClient; + static::$asyncClient = $asyncClient; } /** @@ -42,6 +50,12 @@ public static function getCandidates($type) }]]; } + if (static::$asyncClient !== null && $type == HttpAsyncClient::class) { + return [['class' => function () { + return static::$asyncClient; + }]]; + } + return []; } diff --git a/Resources/config/services.xml b/Resources/config/services.xml index 78648a20..b785f3c7 100644 --- a/Resources/config/services.xml +++ b/Resources/config/services.xml @@ -5,7 +5,6 @@ - From c1f0982bba34925a427ce1a84b91c9c53881fd7e Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Mon, 18 Jul 2016 10:45:02 +0200 Subject: [PATCH 2/9] Use strict comparison. --- Discovery/ConfiguredClientsStrategy.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Discovery/ConfiguredClientsStrategy.php b/Discovery/ConfiguredClientsStrategy.php index 89e5e317..76ea6f1d 100644 --- a/Discovery/ConfiguredClientsStrategy.php +++ b/Discovery/ConfiguredClientsStrategy.php @@ -44,13 +44,13 @@ public function __construct(HttpClient $httpClient = null, HttpAsyncClient $asyn */ public static function getCandidates($type) { - if (static::$client !== null && $type == HttpClient::class) { + if (static::$client !== null && $type === HttpClient::class) { return [['class' => function () { return static::$client; }]]; } - if (static::$asyncClient !== null && $type == HttpAsyncClient::class) { + if (static::$asyncClient !== null && $type === HttpAsyncClient::class) { return [['class' => function () { return static::$asyncClient; }]]; From 5c015a250ad6ae59c3e20f1f40102cf0b343efa4 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Mon, 18 Jul 2016 10:56:35 +0200 Subject: [PATCH 3/9] fixed tests --- Tests/Unit/DependencyInjection/ConfigurationTest.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Tests/Unit/DependencyInjection/ConfigurationTest.php b/Tests/Unit/DependencyInjection/ConfigurationTest.php index 1a571c73..c4680db2 100644 --- a/Tests/Unit/DependencyInjection/ConfigurationTest.php +++ b/Tests/Unit/DependencyInjection/ConfigurationTest.php @@ -41,6 +41,8 @@ public function testEmptyConfiguration() 'enabled' => 'auto', 'formatter' => null, 'captured_body_length' => 0, + 'profile_discovered_client' => true, + 'profile_discovered_async_client' => false, ], 'plugins' => [ 'authentication' => [], @@ -116,6 +118,8 @@ public function testSupportsAllConfigFormats() 'enabled' => true, 'formatter' => 'my_toolbar_formatter', 'captured_body_length' => 0, + 'profile_discovered_client' => true, + 'profile_discovered_async_client' => false, ], 'plugins' => [ 'authentication' => [ From c8359e7499ce79e58f53fb5ffbdc004997f995a7 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Mon, 18 Jul 2016 14:17:43 +0200 Subject: [PATCH 4/9] Updated config --- DependencyInjection/Configuration.php | 16 +++++++-- DependencyInjection/HttplugExtension.php | 33 +++---------------- .../DependencyInjection/ConfigurationTest.php | 12 ++++--- 3 files changed, 26 insertions(+), 35 deletions(-) diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index 8bc36f50..023512fc 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -83,8 +83,6 @@ public function getConfigTreeBuilder() ->defaultValue('auto') ->end() ->scalarNode('formatter')->defaultNull()->end() - ->booleanNode('profile_discovered_client')->defaultTrue()->end() - ->booleanNode('profile_discovered_async_client')->defaultFalse()->end() ->scalarNode('captured_body_length') ->defaultValue(0) ->canNotBeEmpty() @@ -92,6 +90,20 @@ public function getConfigTreeBuilder() ->end() ->end() ->end() + ->arrayNode('discovery') + ->addDefaultsIfNotSet() + ->info('Control what clients should be found by the discovery.') + ->children() + ->scalarNode('client') + ->defaultValue('auto') + ->info('Set to "auto" to see auto discovered client in the web profiler. If provided a service id for a client then this client will be found by auto discovery.') + ->end() + ->scalarNode('async_client') + ->defaultNull() + ->info('Set to "auto" to see auto discovered client in the web profiler. If provided a service id for a client then this client will be found by auto discovery.') + ->end() + ->end() + ->end() ->end(); return $treeBuilder; diff --git a/DependencyInjection/HttplugExtension.php b/DependencyInjection/HttplugExtension.php index ca732303..c24f5258 100644 --- a/DependencyInjection/HttplugExtension.php +++ b/DependencyInjection/HttplugExtension.php @@ -289,39 +289,14 @@ private function registerDebugPlugin(ContainerBuilder $container, $name) */ private function configureAutoDiscoveryClients(ContainerBuilder $container, array $config) { - $httpClient = null; - $asyncHttpClient = null; - - // Verify if any clients were specifucally set to function as auto discoverable. - foreach ($config['clients'] as $name => $arguments) { - if ($arguments['use_with_discovery'] === 'http_client') { - if ($httpClient !== null) { - throw new \LogicException('Only one client can configured with "use_with_discovery: http_client".'); - } - $httpClient = new Reference('httplug.client.'.$name); - } elseif ($arguments['use_with_discovery'] === 'async_client') { - if ($asyncHttpClient !== null) { - throw new \LogicException('Only one client can be configured with "use_with_discovery: async_client".'); - } - $asyncHttpClient = new Reference('httplug.client.'.$name); - } + if ('auto' === $httpClient = $config['discovery']['client']) { + $httpClient = $this->registerAutoDiscoverableClientWithDebugPlugin($container, 'client'); } - if ($httpClient === null) { - // Use auto discovery - if ($config['toolbar']['profile_discovered_client']) { - $httpClient = $this->registerAutoDiscoverableClientWithDebugPlugin($container, 'client'); - } + if ('auto' === $asyncHttpClient = $config['discovery']['async_client']) { + $asyncHttpClient = $this->registerAutoDiscoverableClientWithDebugPlugin($container, 'async_client'); } - if ($asyncHttpClient === null) { - // Use auto discovery - if ($config['toolbar']['profile_discovered_async_client']) { - $asyncHttpClient = $this->registerAutoDiscoverableClientWithDebugPlugin($container, 'async_client'); - } - } - - $container->getDefinition('httplug.strategy') ->addArgument($httpClient) ->addArgument($asyncHttpClient); diff --git a/Tests/Unit/DependencyInjection/ConfigurationTest.php b/Tests/Unit/DependencyInjection/ConfigurationTest.php index c4680db2..15764cbe 100644 --- a/Tests/Unit/DependencyInjection/ConfigurationTest.php +++ b/Tests/Unit/DependencyInjection/ConfigurationTest.php @@ -41,8 +41,6 @@ public function testEmptyConfiguration() 'enabled' => 'auto', 'formatter' => null, 'captured_body_length' => 0, - 'profile_discovered_client' => true, - 'profile_discovered_async_client' => false, ], 'plugins' => [ 'authentication' => [], @@ -83,6 +81,10 @@ public function testEmptyConfiguration() 'stopwatch' => 'debug.stopwatch', ], ], + 'discovery' => [ + 'client' => 'auto', + 'async_client' => null, + ], ]; $formats = array_map(function ($path) { @@ -118,8 +120,6 @@ public function testSupportsAllConfigFormats() 'enabled' => true, 'formatter' => 'my_toolbar_formatter', 'captured_body_length' => 0, - 'profile_discovered_client' => true, - 'profile_discovered_async_client' => false, ], 'plugins' => [ 'authentication' => [ @@ -182,6 +182,10 @@ public function testSupportsAllConfigFormats() 'stopwatch' => 'debug.stopwatch', ], ], + 'discovery' => [ + 'client' => 'auto', + 'async_client' => null, + ], ]; $formats = array_map(function ($path) { From 5509df48eb88adc1ba5c4aa012204b72ab97025a Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Mon, 18 Jul 2016 14:29:26 +0200 Subject: [PATCH 5/9] Bugfix --- DependencyInjection/HttplugExtension.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/DependencyInjection/HttplugExtension.php b/DependencyInjection/HttplugExtension.php index c24f5258..45a3a544 100644 --- a/DependencyInjection/HttplugExtension.php +++ b/DependencyInjection/HttplugExtension.php @@ -289,12 +289,18 @@ private function registerDebugPlugin(ContainerBuilder $container, $name) */ private function configureAutoDiscoveryClients(ContainerBuilder $container, array $config) { - if ('auto' === $httpClient = $config['discovery']['client']) { + $httpClient = $config['discovery']['client']; + if ($httpClient === 'auto') { $httpClient = $this->registerAutoDiscoverableClientWithDebugPlugin($container, 'client'); + } elseif ($httpClient !== null) { + $httpClient = new Reference($httpClient); } - if ('auto' === $asyncHttpClient = $config['discovery']['async_client']) { + $asyncHttpClient = $config['discovery']['async_client']; + if ($asyncHttpClient === 'auto') { $asyncHttpClient = $this->registerAutoDiscoverableClientWithDebugPlugin($container, 'async_client'); + } elseif ($asyncHttpClient !== null) { + $asyncHttpClient = new Reference($httpClient); } $container->getDefinition('httplug.strategy') From a665340f32a025b75b5104cefa1b906e5d39fb03 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Mon, 18 Jul 2016 14:30:25 +0200 Subject: [PATCH 6/9] Removed code not used --- DependencyInjection/Configuration.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index 023512fc..90653dcd 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -138,11 +138,6 @@ protected function configureClients(ArrayNodeDefinition $root) ->defaultFalse() ->info('Set to true to get the client wrapped in a HttpMethodsClient which emulates provides functions for HTTP verbs.') ->end() - ->enumNode('use_with_discovery') - ->values(['async_client', 'http_client', false]) - ->defaultFalse() - ->info('If set to "http_client" this will be the client return when using dicovery to find a HTTP client.') - ->end() ->arrayNode('plugins') ->info('A list of service ids of plugins. The order is important.') ->prototype('scalar')->end() From 27b6238994398629d8b84a3114a8496e948f26d9 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Mon, 18 Jul 2016 17:30:32 +0200 Subject: [PATCH 7/9] Minor fix --- DependencyInjection/HttplugExtension.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/DependencyInjection/HttplugExtension.php b/DependencyInjection/HttplugExtension.php index 45a3a544..8cc63877 100644 --- a/DependencyInjection/HttplugExtension.php +++ b/DependencyInjection/HttplugExtension.php @@ -292,14 +292,14 @@ private function configureAutoDiscoveryClients(ContainerBuilder $container, arra $httpClient = $config['discovery']['client']; if ($httpClient === 'auto') { $httpClient = $this->registerAutoDiscoverableClientWithDebugPlugin($container, 'client'); - } elseif ($httpClient !== null) { + } elseif ($httpClient) { $httpClient = new Reference($httpClient); } $asyncHttpClient = $config['discovery']['async_client']; if ($asyncHttpClient === 'auto') { $asyncHttpClient = $this->registerAutoDiscoverableClientWithDebugPlugin($container, 'async_client'); - } elseif ($asyncHttpClient !== null) { + } elseif ($asyncHttpClient) { $asyncHttpClient = new Reference($httpClient); } From 5c1a5b9bc6e301d441a876aaf091536a5fd9c1b1 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Mon, 18 Jul 2016 22:35:20 +0200 Subject: [PATCH 8/9] evalute class type first --- Discovery/ConfiguredClientsStrategy.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Discovery/ConfiguredClientsStrategy.php b/Discovery/ConfiguredClientsStrategy.php index 76ea6f1d..b5b88ca6 100644 --- a/Discovery/ConfiguredClientsStrategy.php +++ b/Discovery/ConfiguredClientsStrategy.php @@ -44,13 +44,13 @@ public function __construct(HttpClient $httpClient = null, HttpAsyncClient $asyn */ public static function getCandidates($type) { - if (static::$client !== null && $type === HttpClient::class) { + if ($type === HttpClient::class && static::$client !== null) { return [['class' => function () { return static::$client; }]]; } - if (static::$asyncClient !== null && $type === HttpAsyncClient::class) { + if ($type === HttpAsyncClient::class && static::$asyncClient !== null) { return [['class' => function () { return static::$asyncClient; }]]; From cb37a485909a1cdd06bad41c301f83f796124f7c Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Mon, 18 Jul 2016 22:37:57 +0200 Subject: [PATCH 9/9] Removed `public="true"` --- Resources/config/services.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Resources/config/services.xml b/Resources/config/services.xml index b785f3c7..b5ad3efc 100644 --- a/Resources/config/services.xml +++ b/Resources/config/services.xml @@ -4,7 +4,7 @@ xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd"> - +