Open webdevilopers opened 3 years ago
Please do not @ people who have not explicitly consented to it. Thanks.
(Copy/Pasted from https://gist.github.com/webdevilopers/687c8b34d68e97a8f93df5db09242406 )
Disclaimer: I'm no expert or authority, I might be "wrong" on every line below ;)
Thoughts regarding terminology/structure:
... Personally, I would probably do something like:
public function __invoke(SignEmploymentContract $command): void
{
$enteredContracts = $this->contractsDetailsRepository->ofPersonId($person->personId());
if(!OverlappingEmploymentContractPolicy::isSatisfiedBy($command->... , $enteredContracts))
{
throw new EmploymentPeriodOverlapsException();
}
$person = $this->personDetailsRepository->ofPersonId($command->personId()->toString());
$contract = EmploymentContract::sign($command->... , $person->... )
$this->contractCollection->save($contract);
EDIT: you could probably clean it up by injecting the "policy" as a dependency, which encapsulates the 'contractsDetailsRepository' (as long as you don't need it to be 'pure'):
public function __invoke(SignEmploymentContract $command): void
{
$constraintSatisfied = $this->newEmploymentContractMayNotOverlap->isSatisfiedBy($command);
if(!$constraintSatisfied)
{
throw new EmploymentPeriodOverlapsException();
}
$person = $this->personDetailsRepository->ofPersonId($command->personId()->toString());
$contract = EmploymentContract::sign($command->... , $person->... )
$this->contractCollection->save($contract);
Thoughts regarding consistency/invariants: Be aware that all code in this gist so far does not guarantee that 2 contracts for the same person do not overlap periods. It "only" guarantees it as long as that person does not sign up for 2 different contracts at the same time - It's probably only a window of a few milliseconds, and more a theoretical edge case than anything worth coding around. Besides "how probably is it", I find another heuristic for whether the code should care, is the severity of the consequence of such invariant failing. If there is a way to identify and compensate for a theoretical edge case without much blowback, and the race-condition would probably never happen, I would suggest not complicating things with saga/process-manager/policies.
Just be aware that you have a guard, not an absolute guarantee ;)
Please do not @ people who have not explicitly consented to it. Thanks.
Sorry @udidahan for this faux-pas. Same for twitter. Just wanted to ask you for your feedback on this regarding your suggestion in your mentioned article.
Came from:
/cc @JulianMay
An
EmploymentContract
is an event-sourced Aggregate Root (A+ES). It holds a reference to a A+ESPerson
.Employment periods of contracts for a person must NOT overlap. This is ensured by a
OverlappingEmploymentContractPolicy
that currently is called inside the command handler. The handler has to get the existing contracts of a Person from a read model repository.The idea is to move the creation of the contract to the Read Model for the Person as demonstrated in PersonReadModel.
Based on this article by @udidahan:
Or the example from "Implementing Domain-Driven Design" by @VaughnVernon:
as mentioned by @sofiaguyang:
The new handler would then look like this:
This scenario is made for an application that lives in a single microservice. But even if Person and Contracts were dedicated services the Contracts service could consume
PersonHired
events and create a "local copy" of persons and use them as a read model. Or you would move the logic back to the command handler.But THEN I would indeed recommend to make everything event-driven and create separate events that may result in a
ContractCancelledDueToOverlapping
event.