VaughnVernon / IDDD_Samples

These are the sample Bounded Contexts from the book "Implementing Domain-Driven Design" by Vaughn Vernon: http://vaughnvernon.co/?page_id=168
Other
3.76k stars 904 forks source link

sprint.commit vs. backlogItem.commitTo #41

Open killer-it44 opened 3 years ago

killer-it44 commented 3 years ago

I'm trying to understand the eventing and am a bit puzzled cause when I follow the invocations starting from RabbitMQBacklogItemCommitedListener, it calls SprintApplicationService, which calls sprint.commit(BacklogItem bli). It doesn't publish any event.

Then on the BacklogItem entity I also see a "commitTo" method, which does publish an event, but this method is only called from the tests.

That looks like something is wrong/inconsistent here...

shreddish commented 3 months ago

yeah something is off. My guess is that the sprint.commit() should be accepting a BacklogItemCommited class. And the flow should be BacklogItem.commitTo(sprint) -> backlogItemCommitedEvent() -> sprint.commit(backlogItemCommited)

VaughnVernon commented 3 months ago

You don't send events into the model. Events are translated to corresponding commands. The Sprint is accepting that a BacklogItem has been committed to it.

The commit() command could take on additional responsibility. For example, the Sprint might know the total commitment that can be taken on. If, for example, any given BacklogItem causes over-commitment (too much work for the Sprint), it could emit SprintOverCommitted, which could lead to a sub-process that helps the team understand how they might most effectively uncommit specific BacklogItems.

Still, that doesn't necessarily mean that the Sprint must emit a SprintCommittedBacklogItem. It depends on the downstream interests or possibly terminating an orchestration. But this is a simple in-context choreography that need not be managed and thus terminated. The commit() could still emit SprintCommittedBacklogItem, but apparently, at least for now, no one is interested in knowing that. (And this context is using KV storage, not Event Sourcing.)

shreddish commented 3 months ago

You don't send events into the model. Events are translated to corresponding commands. The Sprint is accepting that a BacklogItem has been committed to it.

The commit() command could take on additional responsibility. For example, the Sprint might know the total commitment that can be taken on. If, for example, any given BacklogItem causes over-commitment (too much work for the Sprint), it could emit SprintOverCommitted, which could lead to a sub-process that helps the team understand how they might most effectively uncommit specific BacklogItems.

Still, that doesn't necessarily mean that the Sprint must emit a SprintCommittedBacklogItem. It depends on the downstream interests or possibly terminating an orchestration. But this is a simple in-context choreography that need not be managed and thus terminated. The commit() could still emit SprintCommittedBacklogItem, but apparently, at least for now, no one is interested in knowing that. (And this context is using KV storage, not Event Sourcing.)

I apologize, I wasn't saying to send events into the model but rather the action of BacklogItem.commitTo() emits an event. A handler would listen to that event and then utilize the commit command on the sprint model to inform the sprint model that a backlog item is to be added to the sprint. What confused me a bit was that the sprint took a BacklogItem instead of a CommittedBackLogItem as the parameter. The backlog item being the parameter at first glance makes it seem that calling this would commit the backlog item to the sprint (which technically it does? So it appears like theres two places to commit a backlog item to a sprint). But doesn't this then bypass the model invariants in the BacklogItem that are checked when you call commitTo with a given sprint?

EDIT: Would adding a check in the sprint model's commit method that checks that the backlogItem's sprintId is assigned to the same sprint its being committed to make sense here?

VaughnVernon commented 3 months ago

Passing in CommittedBacklogItem would leak business logic into the application service. I suggest that if anything, the commit(BacklogItem backlogItem) language could be improved by this:

public void assignCommitted(BacklogItem backlogItem) {
}

And, no, this operation does not break the BacklogItem invariants, because the BacklogItem is not modified by the Sprint.

shreddish commented 3 months ago

Passing in CommittedBacklogItem would leak business logic into the application service. I suggest that if anything, the commit(BacklogItem backlogItem) language could be improved by this:

public void assignCommitted(BacklogItem backlogItem) {
}

And, no, this operation does not break the BacklogItem invariants, because the BacklogItem is not modified by the Sprint.

Okay that makes sense - however shouldn't the sprint be checking that the backlog item is in fact committed to itself? otherwise if called out of order you have a sprint that thinks a backlog item is committed to it and a backlog item that thinks it has not been committed to a sprint?