Skip to content

Commit

Permalink
bug #36811 [DependencyInjection] Fix register event listeners compile…
Browse files Browse the repository at this point in the history
…r pass (X-Coder264)

This PR was merged into the 4.4 branch.

Discussion
----------

[DependencyInjection] Fix register event listeners compiler pass

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       |
| License       | MIT
| Doc PR        |

I've wanted to use the simpler event listener registration syntax (https://symfony.com/blog/new-in-symfony-4-4-simpler-event-listeners) in my project and it didn't work so I'm sending this fix.

We use the `KnpPaginatorBundle` bundle which also [calls the `RegisterListenersPass` compiler pass](https://github.com/KnpLabs/KnpPaginatorBundle/blob/v5.2.0/src/DependencyInjection/Compiler/PaginatorConfigurationPass.php#L22) in order to register with the event dispatcher their custom tags for listeners and subscribers (`knp_paginator.listener` and `knp_paginator.subscriber`).

Their compiler pass is `TYPE_BEFORE_REMOVING` and priority zero which is the same type and priority as the pass that gets [added by FrameworkBundle](https://github.com/symfony/symfony/blob/v4.4.8/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php#L125). Since both the type and priority is the same the order of execution is `undefined` (because [that is how regular sort behaves in PHP which is used by default by `krsort`](https://github.com/symfony/symfony/blob/v4.4.8/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php#L264)) and the `RegisterListenersPass` currently removes the `eventAliasesParameter` parameter from the container if it is set (which is [set here](https://github.com/symfony/symfony/blob/v4.4.8/src/Symfony/Bundle/FrameworkBundle/Resources/config/services.xml#L9)). So what happens in my app is that the Knp compiler pass runs first, the `event_dispatcher.event_aliases` parameter is removed and then the FrameworkBundle registered compiler pass runs and since the aliases are not present anymore the events do not get aliased properly. The event dispatcher service in the compiled container looks like:

```php
$instance->addListener('Symfony\Component\HttpKernel\Event\RequestEvent', ...);
```

instead of the expected

```php
$instance->addListener('kernel.request', ...);
```

This means that my listener never gets called on the kernel request event.

Another potential fix would be to adjust the Knp compiler pass priority, but seeing as that would fix only that bundle (who knows how many bundles out there have the same problem) and that I don't see any drawback in letting the `event_dispatcher.event_aliases` parameter stay in the container I think that this is better to fix here.

Commits
-------

646878d Fix register event listeners compiler pass
  • Loading branch information
nicolas-grekas committed May 16, 2020
2 parents fb4c3f9 + 646878d commit ae67376
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 5 deletions.
Expand Up @@ -56,12 +56,12 @@ public function process(ContainerBuilder $container)
return;
}

$aliases = [];

if ($container->hasParameter($this->eventAliasesParameter)) {
$aliases = $container->getParameter($this->eventAliasesParameter);
$container->getParameterBag()->remove($this->eventAliasesParameter);
} else {
$aliases = [];
}

$definition = $container->findDefinition($this->dispatcherService);

foreach ($container->findTaggedServiceIds($this->listenerTag, true) as $id => $events) {
Expand Down
Expand Up @@ -213,17 +213,22 @@ public function testInvokableEventListener()
public function testAliasedEventListener(): void
{
$container = new ContainerBuilder();
$container->setParameter('event_dispatcher.event_aliases', [AliasedEvent::class => 'aliased_event']);
$eventAliases = [AliasedEvent::class => 'aliased_event'];
$container->setParameter('event_dispatcher.event_aliases', $eventAliases);
$container->register('foo', InvokableListenerService::class)->addTag('kernel.event_listener', ['event' => AliasedEvent::class, 'method' => 'onEvent']);
$container->register('bar', InvokableListenerService::class)->addTag('kernel.event_listener', ['event' => CustomEvent::class, 'method' => 'onEvent']);
$container->register('event_dispatcher');

$eventAliasPass = new AddEventAliasesPass([CustomEvent::class => 'custom_event']);
$customEventAlias = [CustomEvent::class => 'custom_event'];
$eventAliasPass = new AddEventAliasesPass($customEventAlias);
$eventAliasPass->process($container);

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

$this->assertTrue($container->hasParameter('event_dispatcher.event_aliases'));
$this->assertSame(array_merge($eventAliases, $customEventAlias), $container->getParameter('event_dispatcher.event_aliases'));

$definition = $container->getDefinition('event_dispatcher');
$expectedCalls = [
[
Expand Down

0 comments on commit ae67376

Please sign in to comment.