bug #11160 [DoctrineBridge] Abstract Doctrine Subscribers with tags (…

…merk)

This PR was merged into the 2.3 branch.

Discussion
----------

[DoctrineBridge] Abstract Doctrine Subscribers with tags

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | this one
| License       | MIT
| Doc PR        | N/A

I've hit a problem with some doctrine listeners, built by decorating an abstract definition.

I want the abstract definition to hold the tag, however because the RegisterEventListenersAndSubscribersPass runs before abstract definitions are removed, they get added as method calls to the EventManager definition, which once the abstract service is removed, we end up with a method call that breaks the container.

I don't know if this is the best approach, it might be better not to return abstract services when calling `findTaggedServiceIds` instead?

Commits
-------

cbcf513 Disallow abstract definitions from doctrine event listener registration

src/Symfony/Bridge/Doctrine/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPass.php

@@ -68,6 +68,10 @@ public function process(ContainerBuilder $container)
 
             uasort($subscribers, $sortFunc);
             foreach ($subscribers as $id => $instance) {
+                if ($container->getDefinition($id)->isAbstract()) {
+                    throw new \InvalidArgumentException(sprintf('The abstract service "%s" cannot be tagged as a doctrine event subscriber.', $id));
+                }
+
                 $em->addMethodCall('addEventSubscriber', array(new Reference($id)));
             }
         }
@@ -78,6 +82,10 @@ public function process(ContainerBuilder $container)
 
             uasort($listeners, $sortFunc);
             foreach ($listeners as $id => $instance) {
+                if ($container->getDefinition($id)->isAbstract()) {
+                    throw new \InvalidArgumentException(sprintf('The abstract service "%s" cannot be tagged as a doctrine event listener.', $id));
+                }
+
                 $em->addMethodCall('addEventListener', array(
                     array_unique($instance['event']),
                     isset($instance['lazy']) && $instance['lazy'] ? $id : new Reference($id),

src/Symfony/Bridge/Doctrine/Tests/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPassTest.php

@@ -13,6 +13,7 @@
 
 use Symfony\Bridge\Doctrine\DependencyInjection\CompilerPass\RegisterEventListenersAndSubscribersPass;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Definition;
 
 class RegisterEventListenersAndSubscribersPassTest extends \PHPUnit_Framework_TestCase
 {
@@ -23,6 +24,38 @@ protected function setUp()
         }
     }
 
+    /**
+     * @expectedException InvalidArgumentException
+     */
+    public function testExceptionOnAbstractTaggedSubscriber()
+    {
+        $container = $this->createBuilder();
+
+        $abstractDefinition = new Definition('stdClass');
+        $abstractDefinition->setAbstract(true);
+        $abstractDefinition->addTag('doctrine.event_subscriber');
+
+        $container->setDefinition('a', $abstractDefinition);
+
+        $this->process($container);
+    }
+
+    /**
+     * @expectedException InvalidArgumentException
+     */
+    public function testExceptionOnAbstractTaggedListener()
+    {
+        $container = $this->createBuilder();
+
+        $abstractDefinition = new Definition('stdClass');
+        $abstractDefinition->setAbstract(true);
+        $abstractDefinition->addTag('doctrine.event_listener', array('event' => 'test'));
+
+        $container->setDefinition('a', $abstractDefinition);
+
+        $this->process($container);
+    }
+
     public function testProcessEventListenersWithPriorities()
     {
         $container = $this->createBuilder();