diff --git a/CHANGELOG.md b/CHANGELOG.md index 8948f8a9..bbbd0d38 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,16 @@ # Change Log +## UNRELEASED + +### Changed + +- All clients are registered with the PluginClient. (even in production) + +### Deprecated + +- `auto` value in `toolbar.enabled` config + ## 1.2.2 - 2016-07-19 ### Fixed diff --git a/ClientFactory/PluginClientFactory.php b/ClientFactory/PluginClientFactory.php index f0ff28dc..9213f878 100644 --- a/ClientFactory/PluginClientFactory.php +++ b/ClientFactory/PluginClientFactory.php @@ -13,15 +13,23 @@ final class PluginClientFactory { /** - * @param Plugin[] $plugins - * @param ClientFactory $factory - * @param array $config config to the client factory - * @param array $pluginClientOptions config forwarded to the PluginClient + * @param Plugin[] $plugins + * @param ClientFactory|callable $factory + * @param array $config config to the client factory + * @param array $pluginClientOptions config forwarded to the PluginClient * * @return PluginClient */ - public static function createPluginClient(array $plugins, ClientFactory $factory, array $config, array $pluginClientOptions = []) + public static function createPluginClient(array $plugins, $factory, array $config, array $pluginClientOptions = []) { - return new PluginClient($factory->createClient($config), $plugins, $pluginClientOptions); + if ($factory instanceof ClientFactory) { + $client = $factory->createClient($config); + } elseif (is_callable($factory)) { + $client = $factory($config); + } else { + throw new \RuntimeException(sprintf('Second argument to PluginClientFactory::createPluginClient must be a "%s" or a callable.', ClientFactory::class)); + } + + return new PluginClient($client, $plugins, $pluginClientOptions); } } diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index 90653dcd..d8430a5e 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -19,6 +19,23 @@ */ class Configuration implements ConfigurationInterface { + /** + * Whether to use the debug mode. + * + * @see https://github.com/doctrine/DoctrineBundle/blob/v1.5.2/DependencyInjection/Configuration.php#L31-L41 + * + * @var bool + */ + private $debug; + + /** + * @param bool $debug + */ + public function __construct($debug) + { + $this->debug = (bool) $debug; + } + /** * {@inheritdoc} */ @@ -75,12 +92,17 @@ public function getConfigTreeBuilder() ->end() ->arrayNode('toolbar') ->addDefaultsIfNotSet() - ->info('Extend the debug profiler with inforation about requests.') + ->info('Extend the debug profiler with information about requests.') ->children() - ->enumNode('enabled') - ->info('If "auto" (default), the toolbar is activated when kernel.debug is true. You can force the toolbar on and off by changing this option.') - ->values([true, false, 'auto']) - ->defaultValue('auto') + ->booleanNode('enabled') // @deprecated value auto in 1.3.0 + ->beforeNormalization() + ->ifString() + ->then(function ($v) { + return 'auto' === $v ? $this->debug : $v; + }) + ->end() + ->info('Turn the toolbar on or off. Defaults to kernel debug mode.') + ->defaultValue($this->debug) ->end() ->scalarNode('formatter')->defaultNull()->end() ->scalarNode('captured_body_length') diff --git a/DependencyInjection/HttplugExtension.php b/DependencyInjection/HttplugExtension.php index b38d0222..0e2ef183 100644 --- a/DependencyInjection/HttplugExtension.php +++ b/DependencyInjection/HttplugExtension.php @@ -5,10 +5,10 @@ use Http\Client\Common\FlexibleHttpClient; use Http\Client\Common\HttpMethodsClient; use Http\Client\Common\Plugin\AuthenticationPlugin; -use Http\Client\Common\PluginClient; use Http\Discovery\HttpAsyncClientDiscovery; use Http\Discovery\HttpClientDiscovery; use Http\HttplugBundle\ClientFactory\DummyClient; +use Http\HttplugBundle\ClientFactory\PluginClientFactory; use Http\HttplugBundle\Collector\DebugPlugin; use Http\Message\Authentication\BasicAuth; use Http\Message\Authentication\Bearer; @@ -39,21 +39,7 @@ public function load(array $configs, ContainerBuilder $container) $loader->load('services.xml'); $loader->load('plugins.xml'); - $toolbar = is_bool($config['toolbar']['enabled']) ? $config['toolbar']['enabled'] : $container->hasParameter('kernel.debug') && $container->getParameter('kernel.debug'); - - if ($toolbar) { - $loader->load('data-collector.xml'); - - if (!empty($config['toolbar']['formatter'])) { - // Add custom formatter - $container->getDefinition('httplug.collector.debug_collector') - ->replaceArgument(0, new Reference($config['toolbar']['formatter'])); - } - - $container->getDefinition('httplug.formatter.full_http_message') - ->addArgument($config['toolbar']['captured_body_length']); - } - + // Register default services foreach ($config['classes'] as $service => $class) { if (!empty($class)) { $container->register(sprintf('httplug.%s.default', $service), $class); @@ -65,9 +51,27 @@ public function load(array $configs, ContainerBuilder $container) $container->setAlias(sprintf('httplug.%s', $type), $id); } + // Configure toolbar + if ($config['toolbar']['enabled']) { + $loader->load('data-collector.xml'); + + if (!empty($config['toolbar']['formatter'])) { + // Add custom formatter + $container + ->getDefinition('httplug.collector.debug_collector') + ->replaceArgument(0, new Reference($config['toolbar']['formatter'])) + ; + } + + $container + ->getDefinition('httplug.formatter.full_http_message') + ->addArgument($config['toolbar']['captured_body_length']) + ; + } + $this->configurePlugins($container, $config['plugins']); - $this->configureClients($container, $config, $toolbar); - $this->configureAutoDiscoveryClients($container, $config, $toolbar); + $this->configureClients($container, $config); + $this->configureAutoDiscoveryClients($container, $config); } /** @@ -75,12 +79,10 @@ public function load(array $configs, ContainerBuilder $container) * * @param ContainerBuilder $container * @param array $config - * @param bool $enableCollector */ - private function configureClients(ContainerBuilder $container, array $config, $enableCollector) + private function configureClients(ContainerBuilder $container, array $config) { - // If we have a client named 'default' - $first = isset($config['clients']['default']) ? 'default' : null; + $first = null; foreach ($config['clients'] as $name => $arguments) { if ($first === null) { @@ -88,22 +90,16 @@ private function configureClients(ContainerBuilder $container, array $config, $e $first = $name; } - $this->configureClient($container, $name, $arguments, $enableCollector); + $this->configureClient($container, $name, $arguments, $config['toolbar']['enabled']); } // If we have clients configured if ($first !== null) { - if ($first !== 'default') { + // If we do not have a client named 'default' + if (!isset($config['clients']['default'])) { // Alias the first client to httplug.client.default $container->setAlias('httplug.client.default', 'httplug.client.'.$first); } - } elseif ($enableCollector) { - $serviceIdDebugPlugin = $this->registerDebugPlugin($container, 'default'); - // No client was configured. Make sure to configure the auto discovery client with the PluginClient. - $container->register('httplug.client', PluginClient::class) - ->addArgument(new Reference('httplug.client.default')) - ->addArgument([]) - ->addArgument(['debug_plugins' => [new Reference($serviceIdDebugPlugin)]]); } } @@ -212,144 +208,169 @@ private function configureAuthentication(ContainerBuilder $container, array $con * @param ContainerBuilder $container * @param string $name * @param array $arguments - * @param bool $enableCollector + * @param bool $profiling */ - private function configureClient(ContainerBuilder $container, $name, array $arguments, $enableCollector) + private function configureClient(ContainerBuilder $container, $name, array $arguments, $profiling) { $serviceId = 'httplug.client.'.$name; - $def = $container->register($serviceId, DummyClient::class); - - // If there are no plugins nor should we use the data collector - if (empty($arguments['plugins']) && !$enableCollector) { - $def->setFactory([new Reference($arguments['factory']), 'createClient']) - ->addArgument($arguments['config']); - } else { - $def - ->setFactory('Http\HttplugBundle\ClientFactory\PluginClientFactory::createPluginClient') - ->addArgument( - array_map( - function ($id) { - return new Reference($id); - }, - $arguments['plugins'] - ) - ) - ->addArgument(new Reference($arguments['factory'])) - ->addArgument($arguments['config']) + + $pluginClientOptions = []; + + if ($profiling) { + // Tell the plugin journal what plugins we used + $container + ->getDefinition('httplug.collector.plugin_journal') + ->addMethodCall('setPlugins', [$name, $arguments['plugins']]) ; - if ($enableCollector) { - $serviceIdDebugPlugin = $this->registerDebugPlugin($container, $name); - $def->addArgument(['debug_plugins' => [new Reference($serviceIdDebugPlugin)]]); + $debugPluginServiceId = $this->registerDebugPlugin($container, $serviceId); - // tell the plugin journal what plugins we used - $container->getDefinition('httplug.collector.plugin_journal') - ->addMethodCall('setPlugins', [$name, $arguments['plugins']]); - } + $pluginClientOptions['debug_plugins'] = [new Reference($debugPluginServiceId)]; } + $container + ->register($serviceId, DummyClient::class) + ->setFactory([PluginClientFactory::class, 'createPluginClient']) + ->addArgument( + array_map( + function ($id) { + return new Reference($id); + }, + $arguments['plugins'] + ) + ) + ->addArgument(new Reference($arguments['factory'])) + ->addArgument($arguments['config']) + ->addArgument($pluginClientOptions) + ; + + /* * Decorate the client with clients from client-common */ - if ($arguments['flexible_client']) { - $container->register($serviceId.'.flexible', FlexibleHttpClient::class) + $container + ->register($serviceId.'.flexible', FlexibleHttpClient::class) ->addArgument(new Reference($serviceId.'.flexible.inner')) ->setPublic(false) - ->setDecoratedService($serviceId); + ->setDecoratedService($serviceId) + ; } if ($arguments['http_methods_client']) { - $container->register($serviceId.'.http_methods', HttpMethodsClient::class) + $container + ->register($serviceId.'.http_methods', HttpMethodsClient::class) ->setArguments([new Reference($serviceId.'.http_methods.inner'), new Reference('httplug.message_factory')]) ->setPublic(false) - ->setDecoratedService($serviceId); + ->setDecoratedService($serviceId) + ; } } /** - * Create a new plugin service for this client. - * - * @param ContainerBuilder $container - * @param string $name - * - * @return string - */ - private function registerDebugPlugin(ContainerBuilder $container, $name) - { - $serviceIdDebugPlugin = 'httplug.client.'.$name.'.debug_plugin'; - $container->register($serviceIdDebugPlugin, DebugPlugin::class) - ->addArgument(new Reference('httplug.collector.debug_collector')) - ->addArgument($name) - ->setPublic(false); - - return $serviceIdDebugPlugin; - } - - /** - * Make sure we inject the debug plugin for clients found by auto discovery. + * Make the user can select what client is used for auto discovery. If none is provided, a service will be created + * by finding a client using auto discovery. * * @param ContainerBuilder $container * @param array $config - * @param bool $enableCollector */ - private function configureAutoDiscoveryClients(ContainerBuilder $container, array $config, $enableCollector) + private function configureAutoDiscoveryClients(ContainerBuilder $container, array $config) { $httpClient = $config['discovery']['client']; - if ($httpClient === 'auto') { - $httpClient = $this->registerAutoDiscoverableClientWithDebugPlugin( - $container, - 'client', - [HttpClientDiscovery::class, 'find'], - $enableCollector - ); - } elseif ($httpClient) { + + if (!empty($httpClient)) { + if ($httpClient === 'auto') { + $httpClient = $this->registerAutoDiscoverableClient( + $container, + 'auto_discovered_client', + [HttpClientDiscovery::class, 'find'], + $config['toolbar']['enabled'] + ); + } + $httpClient = new Reference($httpClient); } $asyncHttpClient = $config['discovery']['async_client']; - if ($asyncHttpClient === 'auto') { - $asyncHttpClient = $this->registerAutoDiscoverableClientWithDebugPlugin( - $container, - 'async_client', - [HttpAsyncClientDiscovery::class, 'find'], - $enableCollector - ); - } elseif ($asyncHttpClient) { - $asyncHttpClient = new Reference($httpClient); + + if (!empty($asyncHttpClient)) { + if ($asyncHttpClient === 'auto') { + $asyncHttpClient = $this->registerAutoDiscoverableClient( + $container, + 'auto_discovered_async', + [HttpAsyncClientDiscovery::class, 'find'], + $config['toolbar']['enabled'] + ); + } + + $asyncHttpClient = new Reference($asyncHttpClient); } - $container->getDefinition('httplug.strategy') + $container + ->getDefinition('httplug.strategy') ->addArgument($httpClient) - ->addArgument($asyncHttpClient); + ->addArgument($asyncHttpClient) + ; } /** + * Find a client with auto discovery and return a service Reference to it. + * * @param ContainerBuilder $container * @param string $name * @param callable $factory - * @param bool $enableCollector + * @param bool $profiling + * + * @return string service id + */ + private function registerAutoDiscoverableClient(ContainerBuilder $container, $name, $factory, $profiling) + { + $serviceId = 'httplug.auto_discovery.'.$name; + + $pluginClientOptions = []; + + if ($profiling) { + $debugPluginServiceId = $this->registerDebugPlugin($container, $serviceId); + + $pluginClientOptions['debug_plugins'] = [new Reference($debugPluginServiceId)]; + } + + $container + ->register($serviceId, DummyClient::class) + ->setFactory([PluginClientFactory::class, 'createPluginClient']) + ->setArguments([[], $factory, [], $pluginClientOptions]) + ; + + return $serviceId; + } + + /** + * Create a new plugin service for this client. + * + * @param ContainerBuilder $container + * @param string $serviceId * - * @return Reference + * @return string */ - private function registerAutoDiscoverableClientWithDebugPlugin(ContainerBuilder $container, $name, $factory, $enableCollector) + private function registerDebugPlugin(ContainerBuilder $container, $serviceId) { - $definition = $container->register('httplug.auto_discovery_'.$name.'.pure', DummyClient::class); - $definition->setPublic(false); - $definition->setFactory($factory); + $serviceIdDebugPlugin = $serviceId.'.debug_plugin'; - $pluginDefinition = $container - ->register('httplug.auto_discovery_'.$name.'.plugin', PluginClient::class) + $container + ->register($serviceIdDebugPlugin, DebugPlugin::class) + ->addArgument(new Reference('httplug.collector.debug_collector')) + ->addArgument(substr($serviceId, strrpos($serviceId, '.') + 1)) ->setPublic(false) - ->addArgument(new Reference('httplug.auto_discovery_'.$name.'.pure')) - ->addArgument([]) ; - if ($enableCollector) { - $serviceIdDebugPlugin = $this->registerDebugPlugin($container, 'auto_discovery_'.$name); - $pluginDefinition->addArgument(['debug_plugins' => [new Reference($serviceIdDebugPlugin)]]); - } + return $serviceIdDebugPlugin; + } - return new Reference('httplug.auto_discovery_'.$name.'.plugin'); + /** + * {@inheritdoc} + */ + public function getConfiguration(array $config, ContainerBuilder $container) + { + return new Configuration($container->getParameter('kernel.debug')); } } diff --git a/Resources/config/data-collector.xml b/Resources/config/data-collector.xml index d1815dfd..d42e93e5 100644 --- a/Resources/config/data-collector.xml +++ b/Resources/config/data-collector.xml @@ -5,6 +5,7 @@ + diff --git a/Tests/Resources/Fixtures/config/toolbar_auto.yml b/Tests/Resources/Fixtures/config/toolbar_auto.yml new file mode 100644 index 00000000..75ba9668 --- /dev/null +++ b/Tests/Resources/Fixtures/config/toolbar_auto.yml @@ -0,0 +1,3 @@ +httplug: + toolbar: + enabled: auto diff --git a/Tests/Unit/DependencyInjection/ConfigurationTest.php b/Tests/Unit/DependencyInjection/ConfigurationTest.php index 15764cbe..5fd30e15 100644 --- a/Tests/Unit/DependencyInjection/ConfigurationTest.php +++ b/Tests/Unit/DependencyInjection/ConfigurationTest.php @@ -11,6 +11,8 @@ */ class ConfigurationTest extends AbstractExtensionConfigurationTestCase { + private $debug = true; + protected function getContainerExtension() { return new HttplugExtension(); @@ -18,7 +20,7 @@ protected function getContainerExtension() protected function getConfiguration() { - return new Configuration(); + return new Configuration($this->debug); } public function testEmptyConfiguration() @@ -38,7 +40,7 @@ public function testEmptyConfiguration() ], 'clients' => [], 'toolbar' => [ - 'enabled' => 'auto', + 'enabled' => true, 'formatter' => null, 'captured_body_length' => 0, ], @@ -93,6 +95,7 @@ public function testEmptyConfiguration() 'config/empty.yml', 'config/empty.xml', 'config/empty.php', + 'config/toolbar_auto.yml', ]); foreach ($formats as $format) { diff --git a/Tests/Unit/DependencyInjection/HttplugExtensionTest.php b/Tests/Unit/DependencyInjection/HttplugExtensionTest.php index 54a2bd8f..2e0484d5 100644 --- a/Tests/Unit/DependencyInjection/HttplugExtensionTest.php +++ b/Tests/Unit/DependencyInjection/HttplugExtensionTest.php @@ -11,6 +11,13 @@ */ class HttplugExtensionTest extends AbstractExtensionTestCase { + protected function setUp() + { + parent::setUp(); + + $this->setParameter('kernel.debug', true); + } + protected function getContainerExtensions() { return [