Skip to content

Commit

Permalink
minor #22039 Skip abstract definitions in compiler passes (chalasr)
Browse files Browse the repository at this point in the history
This PR was merged into the 3.3-dev branch.

Discussion
----------

Skip abstract definitions in compiler passes

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #21761
| License       | MIT
| Doc PR        | n/a

Commits
-------

fc1ba0d Skip abstract definitions in compiler passes
  • Loading branch information
fabpot committed Mar 17, 2017
2 parents 4836007 + fc1ba0d commit 207d068
Show file tree
Hide file tree
Showing 7 changed files with 18 additions and 30 deletions.
Expand Up @@ -79,7 +79,7 @@ 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));
continue;
}

$em->addMethodCall('addEventSubscriber', array(new Reference($id)));
Expand All @@ -95,7 +95,7 @@ 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));
continue;
}

$em->addMethodCall('addEventListener', array(
Expand Down
Expand Up @@ -18,9 +18,6 @@

class RegisterEventListenersAndSubscribersPassTest extends TestCase
{
/**
* @expectedException \InvalidArgumentException
*/
public function testExceptionOnAbstractTaggedSubscriber()
{
$container = $this->createBuilder();
Expand All @@ -32,12 +29,10 @@ public function testExceptionOnAbstractTaggedSubscriber()
$container->setDefinition('a', $abstractDefinition);

$this->process($container);
$this->assertSame(array(), $container->getDefinition('doctrine.dbal.default_connection.event_manager')->getMethodCalls());
}

/**
* @expectedException \InvalidArgumentException
*/
public function testExceptionOnAbstractTaggedListener()
public function testAbstractTaggedListenerIsSkipped()
{
$container = $this->createBuilder();

Expand All @@ -48,6 +43,7 @@ public function testExceptionOnAbstractTaggedListener()
$container->setDefinition('a', $abstractDefinition);

$this->process($container);
$this->assertSame(array(), $container->getDefinition('doctrine.dbal.default_connection.event_manager')->getMethodCalls());
}

public function testProcessEventListenersWithPriorities()
Expand Down
Expand Up @@ -62,10 +62,6 @@ public function visibilityProvider()
);
}

/**
* @expectedException \InvalidArgumentException
* @expectedExceptionMessage The service "my-command" tagged "console.command" must not be abstract.
*/
public function testProcessThrowAnExceptionIfTheServiceIsAbstract()
{
$container = new ContainerBuilder();
Expand All @@ -77,6 +73,8 @@ public function testProcessThrowAnExceptionIfTheServiceIsAbstract()
$container->setDefinition('my-command', $definition);

$container->compile();

$this->assertSame(array(), $container->getParameter('console.command.ids'));
}

/**
Expand Down
Expand Up @@ -32,7 +32,7 @@ public function process(ContainerBuilder $container)
$definition = $container->getDefinition($id);

if ($definition->isAbstract()) {
throw new \InvalidArgumentException(sprintf('The service "%s" tagged "console.command" must not be abstract.', $id));
continue;
}

$class = $container->getParameterBag()->resolveValue($definition->getClass());
Expand Down
Expand Up @@ -50,11 +50,7 @@ public function visibilityProvider()
);
}

/**
* @expectedException \InvalidArgumentException
* @expectedExceptionMessage The service "my-command" tagged "console.command" must not be abstract.
*/
public function testProcessThrowAnExceptionIfTheServiceIsAbstract()
public function testProcessSkipAbstractDefinitions()
{
$container = new ContainerBuilder();
$container->setResourceTracking(false);
Expand All @@ -66,6 +62,8 @@ public function testProcessThrowAnExceptionIfTheServiceIsAbstract()
$container->setDefinition('my-command', $definition);

$container->compile();

$this->assertSame(array(), $container->getParameter('console.command.ids'));
}

/**
Expand Down
Expand Up @@ -63,7 +63,7 @@ public function process(ContainerBuilder $container)
foreach ($container->findTaggedServiceIds($this->listenerTag) as $id => $events) {
$def = $container->getDefinition($id);
if ($def->isAbstract()) {
throw new InvalidArgumentException(sprintf('The service "%s" must not be abstract as event listeners are lazy-loaded.', $id));
continue;
}

foreach ($events as $event) {
Expand All @@ -90,7 +90,7 @@ public function process(ContainerBuilder $container)
foreach ($container->findTaggedServiceIds($this->subscriberTag) as $id => $attributes) {
$def = $container->getDefinition($id);
if ($def->isAbstract()) {
throw new InvalidArgumentException(sprintf('The service "%s" must not be abstract as event subscribers are lazy-loaded.', $id));
continue;
}

// We must assume that the class value has been correctly filled, even if the service is created by a factory
Expand Down
Expand Up @@ -87,10 +87,6 @@ public function testValidEventSubscriber()
$registerListenersPass->process($builder);
}

/**
* @expectedException \InvalidArgumentException
* @expectedExceptionMessage The service "foo" must not be abstract as event listeners are lazy-loaded.
*/
public function testAbstractEventListener()
{
$container = new ContainerBuilder();
Expand All @@ -99,20 +95,20 @@ public function testAbstractEventListener()

$registerListenersPass = new RegisterListenersPass();
$registerListenersPass->process($container);

$this->assertSame(array(), $container->getDefinition('event_dispatcher')->getMethodCalls());
}

/**
* @expectedException \InvalidArgumentException
* @expectedExceptionMessage The service "foo" must not be abstract as event subscribers are lazy-loaded.
*/
public function testAbstractEventSubscriber()
public function testAbstractEventSubscriberIsSkipped()
{
$container = new ContainerBuilder();
$container->register('foo', 'stdClass')->setAbstract(true)->addTag('kernel.event_subscriber', array());
$container->register('event_dispatcher', 'stdClass');

$registerListenersPass = new RegisterListenersPass();
$registerListenersPass->process($container);

$this->assertSame(array(), $container->getDefinition('event_dispatcher')->getMethodCalls());
}

public function testEventSubscriberResolvableClassName()
Expand Down

0 comments on commit 207d068

Please sign in to comment.