webdevilopers / php-ddd

PHP Symfony Doctrine Domain-driven Design
201 stars 11 forks source link

How to deal with updating Entity (CRUD) and Domain Events using DDD? #31

Open maks-rafalko opened 7 years ago

maks-rafalko commented 7 years ago

I know that DDD is good with a Task-Based UI, but I'm refactoring a legacy app, where I have an Anemic Domain Model (many setters without the business logic).

One of the first steps was to make it reach model and add Domain Events. While adding events for creating (TaskCreated in constructor) and removing (TaskRemoved) the model is an easy process, I'm struggling with updating the model.

We have a RESTful API with PUT /tasks/{id} endpoint. Under the hood the framework maps the body of the response to Task object and then calls setters one by one:

$task->setText('new text');
$task->setStartDate($newStartDate);
// and so on

I want to listen some event when the task is updated and update it in e.g. Google Calendar. As you can imaging, if I record events in each setter (TextChanged, StartDateChanged) and listen all of them, I will end up with many API calls to the Google API, which is not what I want.

Question is: how should I work with Update operation in the right way? Should I replace all those setters calls with one ->update($newData) call and dispatch only one domain event there? How to make only one API call to google calendar after the task is updated?

PS: I've read tons of posts but didn't find useful information about Updating domain model. The real example I found in the dddinphp repository in UpdatePostHandler class, where they call updaters (setters) one by one: https://github.com/dddinphp/blog-cqrs/blob/master/src/CQRSBlog/BlogEngine/Command/UpdatePostHandler.php#L27

// UpdatePostHandler.php
$aPost->changeTitle($anUpdatePostCommand->getTitle());
$aPost->changeContent($anUpdatePostCommand->getContent());

// Post.php
public function changeTitle($aNewTitle)
{
    $this->applyAndRecordThat(
        new PostTitleWasChanged($this->postId, $aNewTitle)
    );
}
webdevilopers commented 7 years ago

I have a legacy app that handles Contracts. A Contract has general Details but also Invoicing Details. First of all these would totally belong into a different bounded context. But the legacy form forces me to handle everything on a single UI page. But anyway sometimes "grouping" helps to identify what methods you could design on your Aggregate.

E.g. we have to static methods "changeDetails" (or changeName, change Description depending on your desired UI) and "changeInspectionDetails". These methods only receive some of the data to be updated. As I mentioned: when you moved them to a different `Contract" aggregate in a different BC you could simply re-use the methods.

BTW: These methods should be named using the UL. There probably are better words than "change" or "update".

So far we separated the "update" process into single processes. What really helps here is using Commands. Create a Command with each of those data groups and write a Handler that calls the change* method on the aggregate. E.g.:

maks-rafalko commented 7 years ago

Create a Command with each of those data groups and write a Handler that calls the change* method on the aggregate

Using such commands are useful when we change separately different parts of the Entity, but here is the question about PUT (not PATCH) HTTP verb - when multiple properties can be changed at the same time (just like Symfony form when we edit all editable fields and submit the form).

Now I'm not sure it's possible to do it with DDD in mind at all, but what do you guys suggest in the situation when CRUD-like update is required for legacy code that is being refactored with DDD principles?

Literally the question is: 1) should I dispatch many events in each updater (setter) method or should I dispatch only 1 event when the whole entity is changed and successfully saved?

Feel free to point me to some other links/resources in case these questions are trivial/not relevant

webdevilopers commented 7 years ago

Why not take my example and prepare all those methods for a non-legacy scenario but calling them all in one handler - just like the setters you suggested. Then one DTO / command / symfony data_class with all the data. Later you add multiple commands, still have NO setters but a ready domain model.

webdevilopers commented 7 years ago

But remember when refactoring your domain model later: keep your aggregates small. Maybe having too much data to change in an Entity should make you rethink your aggregate. As I mentioned maybe some parts should belong to other contexts.

webdevilopers commented 7 years ago

Final note: Removing the setters and having good domain model will move you away from CRUD like style. Since you are talking about HTTP we are not talking about CRUD per se but REST as an infrastructure solution, right? CRUD != REST. If your domain model is ready then why not later add multiple PUT actions each one handled by a single command / handler as suggested above?

And in your final step I gues your single Entity (Domain Model / AR) will become multiple aggregates indeed.

Some useful REST w/ DDD examples by @olivergierke:

maks-rafalko commented 7 years ago

thanks for your answers

Why not take my example and prepare all those methods for a non-legacy scenario but calling them all in one handler - just like the setters you suggested

It's not a problem, so instead of setName, setDescription I have changeName, changeDescription (and so on) for Task domain model. We can imagine that I have also created separate commands and handlers for these actions, but renaming the methods does not solve the problem I described in the first message with multiple events when only 1 event is expected

If your domain model is ready then why not later add multiple PUT actions each one handled by a single command / handler as suggested above?

It's not possible, because:


We can think about PUT like "Update entity form" where we have all editable fields.

The real problem I was talking about is about multiple domain events dispatched with such update method:

$command = UpdateTaskCommand('new name', 'new description', ...);

$commandBus->handle($command);

// command handler internally calls
class UpdateTaskHandler {
    public function __invoke()
    {
        $task->changeName('new name'); // dispatches TaskNameChanged
        $task->changeDescription('new description'); // dispatches TaskDescriptionChanged
    }
}

Having the code above, I don't know how to make only one API call to the 3rd-party API (Google Calendar) and update the task, because there is no TaksChanged event, only separate multiple events for each update method.

Do you see the problem? :)

I thought about adding a condition to the change* methods to dispatch events only if the data differs from what is currently in the domain model:

public function changeName($newName)
{
    if ($this->name !== $newName) {
        $this->name = $newName;
        $this->record(TaskNameChagehanged());
    }
}

but this looks strange to me.

maks-rafalko commented 7 years ago

Seems like I found the good suggestion here: http://mattallan.org/2016/rest-and-ddd/

[...] think about mapping resources to domain actions, not entities. A PATCH request usually has a couple of different intentions. PATCH /users/12 with the payload { "phone_number" : "727-555-1234"} could correspond to the action UpdateContactInfo, which PATCH /users/12 with the payload { "password": "newPassw0rd"} would coorespond to the action UpdatePassword. Think about what these intentions are and focus on mapping those.

webdevilopers commented 7 years ago

I think there was a similar example in the talk I linked previously @olivergierke: https://www.youtube.com/watch?v=1OgvUIsv96o

Looks like a good attempt by @yuloh.

jasonycw commented 6 years ago

Hi @borNfreee @webdevilopers I come across this issue when researching on how to maintain single update domain event for multiple setters. Although we are not writing php, the issue with our case is very similar. (multiple setters, want only 1 updated event)

What are your final finding on how to handle your case? Seems that you settle with 1 domain action per API, am I understand correctly?

We have no luck finding anything like "grouping domain events" where we can still keep the events being raised in their own update action and handle multiple updated events at once.

webdevilopers commented 6 years ago

TLDR After having a short look at the discussion I'm wondering why I ever considered "setters" resp. using multiple setters when handling a single command.

After some projects with Event Sourcing all my UI tasks are broken down to a single command. A single command has one dedicated handler and calls only one dedicated method on the domain model.

E.g.:

class ChangeDetailsHandler
{
    public function __invoke(ChangeDetails $command)
    {
        // get model
        $model->changeDetails($command->title(), $command->description());

No multiple setters, just a dedicated method. Mostly named constructors when creating the objects with a private __construct() method. Please notet that these methods can indeed create multiple domain events.

If you have a command handler that handles different tasks e.g. "changing details AND uploading a file" than you should re-think your UI and break it down to multiple commands.

Sometimes we even had situations where we had two commands "changeTitle" and "changeDescription" because the UI separated them into two HTML forms send via AJAX.

I think this is one of the biggest lessons I learned those days: If you are thinking that there is too much going on, separate the concerns. SRP can also be the key when designing commands & handlers.

I hope this helps!

In the case of @borNfreee where you have to put multiple tasks into a single command, you should still be able to use a single method on the domain model aka Entity and use $this->title = $aTitle; instead of $this->setTitle($aTitle). Of course a setter can do some extra domain validation but at least it should be private then. Don't allow the external world to change the state of your object through setters. Use dedicated methods!

jasonycw commented 6 years ago

Thanks @webdevilopers for your reply.

Indeed we should follow SRP when design the command and triggering multiple domain events. However, sometime we may need to handle multiple events with a single handler. e.g.
"changeTitle" and "changeDescription" is emitted at the same time from the UI save button (maybe), and now a "somethingChanged" event handler may want to handle both events and send out one email about 2 things is changed

In this case, is there any good practice or suggestion? Ours finding leads to event storing and will need to maintain another "message queue-ish" system which may not ideal.

arielmoraes commented 6 years ago

I prefer the one-event-per-set way, because the update action sometimes is just an use case, imagine that you will have to, later in the domain, change just one field of an entity because of an external process, you shall have a command representing that use case and not an entire update. Now the question arises: how do I track down the events that were fired based on a single use case? Simple, use some kind of correlation identifier, that could be the request Id for example. To achieve that you may use a list that holds the domain events and, when you save your transaction, you dispatch the events and, as they were created within the same logical context, they will have the same correlation Id.

laudirbispo commented 6 years ago

I was wondering the same thing. What is the best way to implement something like this. For example, update an article. In my case, I created a single module for tasks and in my model I separated the tasks.

We retrieved the last valid state of our model

// Article Model
public static function reconstituteFrom(AggregateHistory $anAggregateHistory)
{
    $aPost = static::createEmptyPostWith($anAggregateHistory->getAggregateId());

    foreach ($anAggregateHistory as $anEvent) {
        $aPost->apply($anEvent);
    }

    return $aPost;
}

Can I have a handler to handle all data at once.

$task = ChangeArticleHandler(Repository....);
$task->handler(Command....);

...

// Article model
public function changeTitle($aNewTitle)
{
    $this->applyAndRecordThat(
        new PostTitleWasChanged($this->postId, $aNewTitle)
    );
}
public function changeContent($aNewContent)
{
    $this->applyAndRecordThat(
        new PostContentWasChanged($this->postId, $aNewContent)
    );
}
private function applyPostTitleWasChanged(PostTitleWasChanged $event)
{
    $this->title = $event->getTitle();
}
private function applyPostContentWasChanged(PostContentWasChanged $event)
{
    $this->content = $event->getContent();
}

Now I can also create specific handlers for each action separately.

$task = ChangeArticleTitle(Repository...);
$task->handler(Command...);

...

$task = ChangeArticleContent(Repository...);
$task->handler(Command...);

The separation of tasks in our model is the x of the question.

wazum commented 3 years ago

@jasonycw

"changeTitle" and "changeDescription" is emitted at the same time from the UI save button (maybe), and now a "somethingChanged" event handler may want to handle both events and send out one email about 2 things is changed In this case, is there any good practice or suggestion?

If you're interested in the fact that multiple updates have finished, I would suggest to emit another event (e.g. "EntityXYUpdateComplete") inside the command handler and let your event listeners wait for that to send this one email.