Merge "Doctrine Event Listeners and Subscribers" and "Entity Listeners"?

[ThomasLandauer]

Following up on #10195

I'm suggesting to merge https://symfony.com/doc/current/bundles/DoctrineBundle/entity-listeners.html and http://symfony.com/doc/current/doctrine/event_listeners_subscribers.html#creating-the-listener-class

Reason: Shouldn't the entity listeners be presented as the preferred way of doing it?
The note that they where introduced in Doctrine 2.4 suggests that they're a brand new feature - however, Doctrine 2.4 was released some 5 years ago ;-)

So since entity listeners have been around for some time, what's the use case for event listeners and subscribers at all anymore?

[weaverryan]

I agree! These should certainly live in just one doc. However, two important things:

1. The DoctrineBundle docs (even though they are displayed on symfony.com) are not in our control - they develop those docs separately from us in their repository. For sure we should try to make them work together as much as possible. What we should do first is make our docs how we want them. Then, if there is some duplication, we can propose to remove some things from the bundle.

2. Unrelated to the docs, I believe that thedoctrine.orm.entity_listener tag is NOT currently autoconfigured. We should really change that in the bundle if possible :)

[ThomasLandauer]

See #10209

One big question: What's the advantage of using listeners?? I mean: It doesn't make much of a difference if you do:

$userService->doWhateverPrePersist();
$em->persist($user);

...or just do $em->persist($user); and have a listener configured which does doWhateverPrePersist();
Does it make the code more organized (why/how?)? Is it just a matter of personal preference? If you're doing the persist in 10 different places (which is probably a sign of bad code organization anyway), is it about that the listener keeps you from forgetting to call doWhateverPrePersist(); in one of those 10 places?
I think this should be answered in the first paragraph. If there is no clear answer/advantage, then I'd just present listeners as an alternative way of calling services.

[xabbuh]

The "normal" event listeners are not bound to a certain entity. This allows for writing generic listeners (like some logging for example).

[xabbuh]

@weaverryan Entity listeners do not implement a common interface. That's why they cannot be autoconfigured.

[TomasVotruba]

@weaverryan Entity listeners do not implement a common interface. That's why they cannot be autoconfigured.

Sure, but why not subscribers that implement EventSubscriber interface?

services:
    # why this is not autoconfigured? :(
    Pehapkari\Doctrine\EventSubscriber\SetUploadDestinationOnPostLoadEventSubscriber:
        tags:
            - { name: doctrine.event_subscriber }

Now I have to do this manually. It's really WTF while all other common interfaces autoconfigured.

---

Unrelated to the docs, I believe that thedoctrine.orm.entity_listener tag is NOT currently autoconfigured. We should really change that in the bundle if possible :)

👍

---

Solution for Time Being

In case you have the same problem, put this in in your Kernel:

protected function configureContainer(ContainerBuilder $containerBuilder, LoaderInterface $loader): void
    {
        $containerBuilder->registerForAutoconfiguration(\Doctrine\Common\EventSubscriber::class)
            ->addTag('doctrine.event_subscriber');

[xabbuh]

IIRC the issue with auto configuring EventSubscriberInterface instances is that you do not know if the implementing class is an event subscriber for the ORM or any of the other implementations.

[weaverryan]

That is a problem - but one that could be overcome by adding a sub interface to the bundle - we’ve done that for fixtures. The problem is multiple entity managers. We would need to autoconfigure to the default one. If a user didn’t want that, they would need to add a tag manually with config for the other entity manager. That’s not a problem, but figuring out what’s going on in the compiler pass is challenging. Should the second tag override the first? Or does the user actually want listeners on both entity managers?

It’s a weird edge case that shouldn’t stop us from implementing autoconfigure, but it has so far. One idea is, for autoconfigure, add the tag and a special attribute - something like from_autoconfigure. Then, if the compiler pass detects 2 tags, but one has this attribute, it skips that one. It’s a clear way for us to know that the autoconfigure tag has been “overridden”.

[OskarStark]

Sounds like a straightforward idea from DX POV Ryan 👍🏻

[javiereguiluz]

Closing as "fixed" because we've merged all articles about Doctrine events into a single article: https://symfony.com/doc/current/doctrine/events.html The doc in the DoctrineBundle will be kept because all bundles provide some basic doc.

If we disagree, we can open an issue in the DoctrineBundle repo asking for the removal of their doc and to link instead to the new article in Symfony Docs.

Thanks.