Fix debug tool registration

Debug collectors and tools like that should not be configured when debug mode is disabled.
php-http · Jul 19, 2016 · fcdf81f · fcdf81f
1 parent bd4a71c
commit fcdf81f
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 25 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,13 @@
 # Change Log
 
 
+## 1.2.2 - 2016-07-19
+
+### Fixed
+
+- Do not register debug tools when debugging is disabled (eg. in prod mode)
+
+
 ## 1.2.1 - 2016-07-19
 
 ### Fixed

diff --git a/DependencyInjection/HttplugExtension.php b/DependencyInjection/HttplugExtension.php
@@ -39,10 +39,10 @@ public function load(array $configs, ContainerBuilder $container)
         $loader->load('services.xml');
         $loader->load('plugins.xml');
 
-        $enabled = is_bool($config['toolbar']['enabled']) ? $config['toolbar']['enabled'] : $container->hasParameter('kernel.debug') && $container->getParameter('kernel.debug');
-        if ($enabled) {
+        $toolbar = is_bool($config['toolbar']['enabled']) ? $config['toolbar']['enabled'] : $container->hasParameter('kernel.debug') && $container->getParameter('kernel.debug');
+
+        if ($toolbar) {
             $loader->load('data-collector.xml');
-            $config['_inject_collector_plugin'] = true;
 
             if (!empty($config['toolbar']['formatter'])) {
                 // Add custom formatter
@@ -66,17 +66,18 @@ public function load(array $configs, ContainerBuilder $container)
         }
 
         $this->configurePlugins($container, $config['plugins']);
-        $this->configureClients($container, $config);
-        $this->configureAutoDiscoveryClients($container, $config);
+        $this->configureClients($container, $config, $toolbar);
+        $this->configureAutoDiscoveryClients($container, $config, $toolbar);
     }
 
     /**
      * Configure client services.
      *
      * @param ContainerBuilder $container
      * @param array            $config
+     * @param bool             $enableCollector
      */
-    private function configureClients(ContainerBuilder $container, array $config)
+    private function configureClients(ContainerBuilder $container, array $config, $enableCollector)
     {
         // If we have a client named 'default'
         $first = isset($config['clients']['default']) ? 'default' : null;
@@ -87,7 +88,7 @@ private function configureClients(ContainerBuilder $container, array $config)
                 $first = $name;
             }
 
-            $this->configureClient($container, $name, $arguments, $config['_inject_collector_plugin']);
+            $this->configureClient($container, $name, $arguments, $enableCollector);
         }
 
         // If we have clients configured
@@ -96,7 +97,7 @@ private function configureClients(ContainerBuilder $container, array $config)
                 // Alias the first client to httplug.client.default
                 $container->setAlias('httplug.client.default', 'httplug.client.'.$first);
             }
-        } elseif (isset($config['_inject_collector_plugin'])) {
+        } 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)
@@ -218,14 +219,13 @@ private function configureClient(ContainerBuilder $container, $name, array $argu
         $serviceId = 'httplug.client.'.$name;
         $def = $container->register($serviceId, DummyClient::class);
 
-        // If there is no plugins nor should we use the data collector
+        // 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 {
-            $serviceIdDebugPlugin = $this->registerDebugPlugin($container, $name);
-
-            $def->setFactory('Http\HttplugBundle\ClientFactory\PluginClientFactory::createPluginClient')
+            $def
+                ->setFactory('Http\HttplugBundle\ClientFactory\PluginClientFactory::createPluginClient')
                 ->addArgument(
                     array_map(
                         function ($id) {
@@ -236,13 +236,17 @@ function ($id) {
                 )
                 ->addArgument(new Reference($arguments['factory']))
                 ->addArgument($arguments['config'])
-                ->addArgument(['debug_plugins' => [new Reference($serviceIdDebugPlugin)]]);
+            ;
 
-            // 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)]]);
 
+                // tell the plugin journal what plugins we used
+                $container->getDefinition('httplug.collector.plugin_journal')
+                    ->addMethodCall('setPlugins', [$name, $arguments['plugins']]);
+            }
+        }
 
         /*
          * Decorate the client with clients from client-common
@@ -287,15 +291,17 @@ private function registerDebugPlugin(ContainerBuilder $container, $name)
      *
      * @param ContainerBuilder $container
      * @param array            $config
+     * @param bool             $enableCollector
      */
-    private function configureAutoDiscoveryClients(ContainerBuilder $container, array $config)
+    private function configureAutoDiscoveryClients(ContainerBuilder $container, array $config, $enableCollector)
     {
         $httpClient = $config['discovery']['client'];
         if ($httpClient === 'auto') {
             $httpClient = $this->registerAutoDiscoverableClientWithDebugPlugin(
                 $container,
                 'client',
-                [HttpClientDiscovery::class, 'find']
+                [HttpClientDiscovery::class, 'find'],
+                $enableCollector
             );
         } elseif ($httpClient) {
             $httpClient = new Reference($httpClient);
@@ -306,8 +312,9 @@ private function configureAutoDiscoveryClients(ContainerBuilder $container, arra
             $asyncHttpClient = $this->registerAutoDiscoverableClientWithDebugPlugin(
                 $container,
                 'async_client',
-                [HttpAsyncClientDiscovery::class, 'find']
-                );
+                [HttpAsyncClientDiscovery::class, 'find'],
+                $enableCollector
+            );
         } elseif ($asyncHttpClient) {
             $asyncHttpClient = new Reference($httpClient);
         }
@@ -321,21 +328,27 @@ private function configureAutoDiscoveryClients(ContainerBuilder $container, arra
      * @param ContainerBuilder $container
      * @param string           $name
      * @param callable         $factory
+     * @param bool             $enableCollector
      *
      * @return Reference
      */
-    private function registerAutoDiscoverableClientWithDebugPlugin(ContainerBuilder $container, $name, $factory)
+    private function registerAutoDiscoverableClientWithDebugPlugin(ContainerBuilder $container, $name, $factory, $enableCollector)
     {
         $definition = $container->register('httplug.auto_discovery_'.$name.'.pure', DummyClient::class);
         $definition->setPublic(false);
         $definition->setFactory($factory);
 
-        $serviceIdDebugPlugin = $this->registerDebugPlugin($container, 'auto_discovery_'.$name);
-        $container->register('httplug.auto_discovery_'.$name.'.plugin', PluginClient::class)
+        $pluginDefinition = $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)]]);
+        ;
+
+        if ($enableCollector) {
+            $serviceIdDebugPlugin = $this->registerDebugPlugin($container, 'auto_discovery_'.$name);
+            $pluginDefinition->addArgument(['debug_plugins' => [new Reference($serviceIdDebugPlugin)]]);
+        }
 
         return new Reference('httplug.auto_discovery_'.$name.'.plugin');
     }