[FrameworkBundle] Adding an exception to help with bad configuration and filling in some missing tests

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/ProfilerPass.php

@@ -35,7 +35,11 @@ class ProfilerPass implements CompilerPassInterface
         foreach ($container->findTaggedServiceIds('data_collector') as $id => $attributes) {
             $priority = isset($attributes[0]['priority']) ? $attributes[0]['priority'] : 0;
             $template = null;
-            if (isset($attributes[0]['template']) && isset($attributes[0]['id'])) {
+
+            if (isset($attributes[0]['template'])) {
+                if (!isset($attributes[0]['id'])) {
+                    throw new \InvalidArgumentException(sprintf('Data collector service "%s" must have an id attribute in order to specify a template', $id));
+                }
                 $template = array($attributes[0]['id'], $attributes[0]['template']);
             }
 

src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/ProfilerPassTest.php

@@ -0,0 +1,76 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien.potencier@symfony-project.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\Compiler;
+
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Definition;
+use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\ProfilerPass;
+
+class ProfilerPassTest extends \PHPUnit_Framework_TestCase
+{
+    /**
+     * Tests that collectors that specify a template but no "id" will throw
+     * an exception (both are needed if the template is specified). Thus,
+     * a fully-valid tag looks something like this:
+     *
+     *     <tag name="data_collector" template="YourBundle:Collector:templatename" id="your_collector_name" />
+     */
+    public function testTemplateNoIdThrowsException()
+    {
+        // one service, with a template key, but no id
+        $services = array(
+            'my_collector_service' => array(0 => array('template' => 'foo')),
+        );
+
+        $builder = $this->getMock('Symfony\Component\DependencyInjection\ContainerBuilder');
+        $builder->expects($this->atLeastOnce())
+            ->method('findTaggedServiceIds')
+            ->will($this->returnValue($services));
+
+        $this->setExpectedException('InvalidArgumentException');
+
+        $profilerPass = new ProfilerPass();
+        $profilerPass->process($builder);
+    }
+
+    public function testValidCollector()
+    {
+        // one service, with a template key, but no id
+        $services = array(
+            'my_collector_service' => array(0 => array('template' => 'foo', 'id' => 'my_collector')),
+        );
+
+        $container = $this->getMock('Symfony\Component\DependencyInjection\ContainerBuilder');
+        $container->expects($this->atLeastOnce())
+            ->method('findTaggedServiceIds')
+            ->will($this->returnValue($services));
+
+        // fake the getDefinition() to return a Profiler definition
+        $definition = new Definition('ProfilerClass');
+        $container->expects($this->atLeastOnce())
+            ->method('getDefinition')
+            ->will($this->returnValue($definition));
+
+        // assert that the data_collector.templates parameter should be set
+        $container->expects($this->once())
+            ->method('setParameter')
+            ->with('data_collector.templates', array('my_collector_service' => array('my_collector', 'foo')));
+
+        $profilerPass = new ProfilerPass();
+        $profilerPass->process($container);
+
+        // grab the method calls off of the "profiler" definition
+        $methodCalls = $definition->getMethodCalls();
+        $this->assertEquals(1, count($methodCalls));
+        $this->assertEquals('add', $methodCalls[0][0]); // grab the method part of the first call
+    }
+}