Skip to content

Fix erroneous order of events for multi events per listener #1885

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: 2.14.x
Choose a base branch
from
Open

Fix erroneous order of events for multi events per listener #1885

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 5 additions & 8 deletions src/DependencyInjection/Compiler/EntityListenerPass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
use Doctrine\Bundle\DoctrineBundle\Mapping\EntityListenerServiceResolver;
use Symfony\Component\DependencyInjection\ChildDefinition;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\Compiler\PriorityTaggedServiceTrait;
use Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Definition;
Expand All @@ -17,6 +16,7 @@
use function method_exists;
use function sprintf;
use function substr;
use function usort;

/**
* Class for Symfony bundles to register entity listeners
Expand All @@ -25,18 +25,15 @@
*/
class EntityListenerPass implements CompilerPassInterface
{
use PriorityTaggedServiceTrait;

/** @return void */
public function process(ContainerBuilder $container)
{
$resolvers = $this->findAndSortTaggedServices('doctrine.orm.entity_listener', $container);

$lazyServiceReferencesByResolver = [];

foreach ($resolvers as $reference) {
$id = $reference->__toString();
foreach ($container->getDefinition($id)->getTag('doctrine.orm.entity_listener') as $attributes) {
foreach ($container->findTaggedServiceIds('doctrine.orm.entity_listener') as $id => $tags) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The next line will sort the tags within the service by priority but the services are now simply in the order they are returned by $container->findTaggedServiceIds(), i.e., in the order they were defined.
But for the listeners to be registered in the correct order the tags of all services need to be ordered together.

This probably needs to restructure the code to a single foreach over all tags of all returned services?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So something like this:

        $serviceTags = [];
        foreach ($container->findTaggedServiceIds('doctrine.orm.entity_listener') as $id => $tags) {
            foreach ($tags as $attributes) {
                $serviceTags[] = [
                    'serviceId' => $id,
                    'attributes' => $attributes,
                ];
            }
        }

        usort($serviceTags, static fn (array $a, array $b) => ($b['attributes']['priority'] ?? 0) <=> ($a['attributes']['priority'] ?? 0));

        foreach ($serviceTags as $tag) {
            $id            = $tag['serviceId'];
            $attributes    = $tag['attributes'];

usort($tags, static fn (array $a, array $b) => ($a['priority'] ?? 0) <=> ($b['priority'] ?? 0));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be

Suggested change
usort($tags, static fn (array $a, array $b) => ($a['priority'] ?? 0) <=> ($b['priority'] ?? 0));
usort($tags, static fn (array $a, array $b) => ($b['priority'] ?? 0) <=> ($a['priority'] ?? 0));

i.e. tags with a larger number as priority come first!?

see PriorityTaggedServiceTrait#L113


foreach ($tags as $attributes) {
$name = $attributes['entity_manager'] ?? $container->getParameter('doctrine.default_entity_manager');
$entityManager = sprintf('doctrine.orm.%s_entity_manager', $name);

Expand Down
49 changes: 49 additions & 0 deletions tests/DependencyInjection/Compiler/EntityListenerPassTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,43 @@ public function provideEvents(): iterable
yield 'With event and custom method' => [Events::postLoad, 'postLoadHandler', 'postLoadHandler'];
yield 'With event and no matching method' => [Events::postLoad, null, '__invoke'];
}

public function testMultipleAttributesOnSameClassKeepTheCorrectOrder(): void
{
$container = new ContainerBuilder();
$container->addCompilerPass(new EntityListenerPass());

$container->setParameter('doctrine.default_entity_manager', 'default');
$container->register('doctrine.orm.default_entity_manager', EntityManager::class);
$container->register('doctrine.orm.default_entity_listener_resolver', ContainerEntityListenerResolver::class);
$container->register('doctrine.orm.default_listeners.attach_entity_listeners', AttachEntityListenersListener::class)
->setPublic(true);

$container->register(TestListener::class)
->addTag('doctrine.orm.entity_listener', ['entity' => stdClass::class, 'event' => Events::prePersist])
->addTag('doctrine.orm.entity_listener', ['entity' => stdClass::class, 'event' => Events::postPersist]);
$container->register(TestListener2::class)
->addTag(
'doctrine.orm.entity_listener',
['entity' => stdClass::class, 'event' => Events::prePersist, 'priority' => 1],
)
->addTag(
'doctrine.orm.entity_listener',
['entity' => stdClass::class, 'event' => Events::postPersist, 'priority' => -1],
);

$container->compile();

$this->assertSame(
[
['addEntityListener', ['stdClass', TestListener::class, 'prePersist']],
['addEntityListener', ['stdClass', TestListener::class, 'postPersist']],
['addEntityListener', ['stdClass', TestListener2::class, 'postPersist']],
['addEntityListener', ['stdClass', TestListener2::class, 'prePersist']],
],
$container->getDefinition('doctrine.orm.default_listeners.attach_entity_listeners')->getMethodCalls(),
);
}
}

class TestListener
Expand All @@ -84,3 +121,15 @@ public function __invoke(): void
{
}
}


class TestListener2
{
public function prePersist(): void
{
}

public function postPersist(): void
{
}
}
Loading