romaricdrigon / romaricdrigon.github.io

My personal blog
https://romaricdrigon.github.io/
2 stars 0 forks source link

2019/08/09/domain-events #7

Open romaricdrigon opened 5 years ago

romaricdrigon commented 5 years ago

Please comment below! Comments will automatically be published on the blog, too.

curtchan commented 5 years ago

Nice article and approach but how does it deal with missing ID issue? I can see that you create UserCreated event on it's construct, where id is null (for example if ID is generated by doctrine during flush), so i can quickly tell that it will generate issues.

Safe approach would be to work with uuid's instead of id's, which may exist during preUpdate and prePersist + call the events twice (somehow), so you can hook to preUserCreated and postUserUpdated if you want to (maybe via decorator that would append .post / .pre to event name).

romaricdrigon commented 5 years ago

Good catch, I will update my example! UUID, set in constructor, are definitely the best option. My type hints are wrong, and I should add at least a comment.

mickaelandrieu commented 5 years ago

Hello,

about that:

Doctrine listeners have many technical limitations regarding how you can create new entities and whether you can or can not modify relationships, long-term, it will be a burden.

As I used to use that and haven't met any issue yet, I'm wondering if you could share with us a list of potential limitations?

Thanks for your article

romaricdrigon commented 5 years ago

Hello @mickaelandrieu,

The biggest limitation is that you have to call Unit of work to re-compute changeset if you modify an entity or create a new one. It is not literally impossible to do, but then your listeners know how the calling context, and it is a pretty big issue regarding code simplicity and ability to re-use.

Regarding relationships, I remember having such an issue, but it was a long time, I don't have the details at hand anymore. I will try to dig in my memories!

curtchan commented 5 years ago

Never had an issue (never had to recompute changeset) although i noticed someone recently posting some doctrine listener on Sf slack with that being called, after longer discussion it turns out to be needed, but only if you modify a not-yet-tracked entity.

For example: we have entity A and entity B, A is oneToMany B, now if we want to modify A via listener when B is modified, if we just modify something in A it will not be noticed by unit of work - because it isn't considering it being modified during current flush flow. Haven't actually tested that, but logic tells me that it is the issue happening. As i said, never had such problem and never had to re-compute changeset, so i can't be sure.

romaricdrigon commented 5 years ago

It may have changed, because I definitely remember it being needed a few years ago. It may also depend upon which events you use, and how (lifecycle hooks, or listeners...). Anyhow, it means there may be some unexpected surprises, which is not great :(

ghost commented 4 years ago

Hmmm, are those Events in your implementation stored anywhere?

romaricdrigon commented 3 years ago

Hmmm, are those Events in your implementation stored anywhere?

Not here, they are just temporary in a PHP array. You could easily persist those in DomainEventsCollector, though.

PS: sorry for the very slow answer! I somehow lost track of notifications.

Kalyse commented 3 years ago

@romaricdrigon How would this approach work if the Entity needed knowledge of a state of a model outside of the current entity?

For example..

A user who's account is not active, who places as order.

You may want to raise a domain event for something like, rather than jsut simple, created update removed.

dykyi-roman commented 3 years ago

Who call dispatchCollectedEvents method?

romaricdrigon commented 3 years ago

@romaricdrigon How would this approach work if the Entity needed knowledge of a state of a model outside of the current entity?

For example..

A user who's account is not active, who places as order.

You may want to raise a domain event for something like, rather than jsut simple, created update removed. from @Kalyse

sorry for the late answer - I got kind behind my blog. So in your case, if Order and User are 2 totally unrelated entities, an event raised by an entity wouldn't work. Event should be raised from the controller, or a service, which has access to both. But just as a note, in this specific example, I think you likely want to store a reference to the "buyer" in your Order.

Who call dispatchCollectedEvents method? from @dykyi-roman

Just below the code snippet options are detailed: either it has top be called manually (from controller, etc.), either by a Symfony listener listening to request.terminate. I personally prefer the first option, and at the end of the article I propose to add ForgottenDomainEventsSubscriber (in dev/test only), to make sure no developper forgets to call it.

ludekbenedik commented 3 years ago

Hi, thank for the great article. I would like to read your opinion about our solution of dispatching collected events. The solution doesn't require manually dispatching of events or automated dispatching during KernelEvents::RESPONSE and ConsoleEvents::TERMINATE Symfony events.

I thing the problem that we can't call EntityManager::flush() inside postFlush event is that postFlush events are dispatched before cleaning of UnitOfWork, see UnitOfWork::commit.

The solution is to decorate EntityManagerInterface and dispatch domain events after EntityManagerInterface::flush.

<?php declare(strict_types = 1);

namespace App\Doctrine;

use App\EventSubscriber\DomainEventsCollector;
use Doctrine\ORM\Decorator\EntityManagerDecorator;
use Doctrine\ORM\EntityManagerInterface;

class EventEntityManager extends EntityManagerDecorator
{
    private DomainEventsCollector $collector;

    public function __construct(EntityManagerInterface $wrapped, DomainEventsCollector $collector)
    {
        parent::__construct($wrapped);
        $this->collector = $collector;
    }

    public function flush($entity = null): void
    {
        parent::flush($entity);
        $this->collector->dispatchCollectedEvents();
    }
}
# services.yaml

services:
    # Decorate all entity managers
    app.doctrine.orm.default_entity_manager:
        class: App\Doctrine\EventEntityManager
        decorates: doctrine.orm.default_entity_manager
romaricdrigon commented 3 years ago

Hello @ludekbenedik

If that works for you, I think it is a quite good and quite convenient solution! I considered listing it in the article in the past, as third option. In the end I didn't because of some possible edge effects. For instance, if you have multiple entity manager ; or some tools as steevanb/doctrine-stats decorates EntityManager too so I'm not sure how it will play out. But if those does not matter to you, that's a good one.

md81 commented 3 years ago

I'm wondering how to deal with dispatch collected domain events in the same transaction? Let's assume that we use CRQS using Messenger with middleware DoctrineTransactionMiddleware. Do you have any suggestions how to do this properly?

ludekbenedik commented 3 years ago

Hi @md81, see this article.

You can create a subscriber with postPersist, preUpdate, postUpdate and postDelete event listeners (only these events are executed in transaction). In the listeners you use the connection from the entity manager from the event and insert serialized events to a table. Then in a cron you will read, deserialize and send events to a messenger.

md81 commented 3 years ago

Hi @ludekbenedik

thank you for suggestion, it looks nice. But I have found inspiration here: https://github.com/SimpleBus/doctrine-orm-bridge/blob/main/src/EventListener/CollectsEventsFromEntities.php It collects and dispatches events just before flush. If some will fail all transaction is rollbacked.

Thanks anyway.

jackbentley commented 2 years ago

Domain events are great. I wouldn't recommend your "How they are thrown" approach.

  1. Domain events are not part of the domain. They represent domain operations. They describe changes to the domain.
  2. Which means they shouldn't be stored in the domain model/entity - ever.
  3. Your solution implements events in the domain (which is bad as explained above), discourages use of Doctrine events... and then uses Doctrine events because of the limitations of your solution. Which immediately proves the point of events are not part of the domain and raises questions as to why we should be doing this if we're using Doctrine events anyway.
  4. Transactions will be painful with this approach. You're calculating events on the fly, not as part of a change set.
  5. You still have to manually identify and dispatch events. This might not line up with actual changes flushed to the DB and adds unnecessary code bloat.
  6. Doctrine listeners have many technical limitations regarding how you can create new entities and whether you can or can not modify relationships

    All this is saying is that you need to choose the correct event - which seems reasonable. It's not saying you can't create new entities or you can't modify relationships in Doctrine events - because you can.

  7. Your "continuous enhancement" argument is void. After re-writing doctrine events in your own code, you've then coupled it to another event system (Symfony events). Using this approach or using Doctrine events will both be equally as coupled - except you'll have lots of custom code with little documentation of how it works or how to use it.

In short, this approach doesn't feel adequate or suitable for domain events.

Doctrine does provide EntityListeners which gives fine-grained access to events - incase that is part of your concern about using Doctrine events.

The main difference between this approach and using Doctrine's event system is that:

Finally, you don't want to be firing all events in postFlush. You want to integrate them into the flush routine. Doctrine is intended to be transactional. I.e. do a single flush instead of lots of execute()s. Doing everything in postFlush is similar to the later. Everything you say you can't do in postFlush you can do in onFlush.

payourself2 commented 2 years ago

Hellow. Have you solve the problem with transactions? $em->beginTransaction(); try { $em->flush(); $em->flush(); $em->commit(); } catch (Exception $e) { $em->rollback(); } if we have rollback after second flush - we musnt apply event after first one. so in collector we must check transaction level. If we do so, events now will not apply on succes