Where to call or pass a domain service?

[webdevilopers]

This is a screenshot from "The absolute beginner’s guide to DDD with Symfony" by @nealio82

Slides: https://speakerdeck.com/nealio82/the-absolute-beginners-guide-to-ddd-with-symfony?slide=105

My question was why the Domain Service slotConfirmationService was called inside the Command Handler instead of passing it to the named constructor of the Aggregate Root.

Our structure and code look very similar. Question about the domain service:
In similar use cases our application handler stays a pure orchestrator and instead would pass the service to the named constructor of the aggregate root. Thoughts?

Came from: https://twitter.com/webdevilopers/status/1426184442301800448

Current thread: https://twitter.com/webdevilopers/status/1426854214253400064

We had a similar use case a long time ago where we had to prevent employment contracts to overlap.
Those days we used the same approach calling a responsible unit in the handler. We did not have a service for it yet and used a repository and a separate Policy / Specification instead. Of course both could be moved to single service.

final class SignEmploymentContractHandler
{
    public function __invoke(SignEmploymentContract $command): void
    {
        $person = $this->personDetailsRepository->ofPersonId($command->personId()->toString());

        if (null === $person) {
            throw new \Exception('Person not found.');
        }

        /** @var ContractDetails[] $enteredContracts */
        $enteredContracts = $this->contractsDetailsRepository->ofPersonId($person->personId());

        if (!OverlappingEmploymentContractPolicy::isSatisfiedBy(
            $command->contractId(), $command->contractType(),
            $command->employmentPeriod(), $command->employerId(), $enteredContracts
        )) {
            throw new EmploymentPeriodOverlapsException();
        }

        // ...

        $contract = EmploymentContract::sign(/*...*/);
        $this->contractCollection->save($contract);
    }
}

In new projects we would prefer this approach:

final class ContractPeriodConfirmationService
{
    public function confirm(PersonId $personId, EmploymentPeriod $period, /*...*/)
    {
        $person = $this->personDetailsRepository->ofPersonId($command->personId()->toString());

        if (null === $person) {
            throw new \Exception('Person not found.');
        }

        /** @var ContractDetails[] $enteredContracts */
        $enteredContracts = $this->contractsDetailsRepository->ofPersonId($person->personId());

        if (!OverlappingEmploymentContractPolicy::isSatisfiedBy(
            $command->contractId(), $command->contractType(),
            $command->employmentPeriod(), $command->employerId(), $enteredContracts
        )) {
            throw new EmploymentPeriodOverlapsException();
        }
    }
}

final class SignEmploymentContractHandler
{
    public function __invoke(SignEmploymentContract $command): void
    {
        $contract = EmploymentContract::sign($command->contractId, $this->contractPeriodConfirmationService, /*...*/);
        $this->contractCollection->save($contract);
    }
}

final class EmploymentContract extends AggregateRoot
{
    public static function sign(ContractId $contractId, ContractPeriodConfirmationService $contractPeriodConfirmationService, /*...*/): EmploymentContract
    {
        try {
            $contractPeriodConfirmationService->(/*...*/);
        } catch (EmploymentPeriodOverlapsException $e) {
        }

        // ...
    }
}

Another solution would be to move the entire code to a separate Factory which would make the Application Service Command Handler very slim:

final class SignEmploymentContractHandler
{
    public function __invoke(SignEmploymentContract $command): void
    {
        $contract = $this->employmentContractFactory->sign($command->contractId(), /*...*/);
        $this->contractCollection->save($contract);
    }
}

Factories are recommended when creating an aggregate requires logic that does not naturally fit into the Aggregate Root or elsewhere.

Here are some very nice solutions suggested by @BrunoRommens:

• Passing read models (value objects representing state) / domain service to aggregate methods #50 (comment)

Possibly related:

• How to pass (which) data from Handler to Domain Service #22

[rubenrubiob]

I see no problem with passing the domain service to the named constructor of the aggregate root. Actually, I believe that is the best option, and I have been doing so for some years.

It works not only for aggregate roots, but for Value Objects too. Imagine you have a VO that represents a zip code, that needs to be valid. You have an interface in your domain that represents that validator service, because it might do some external calls, or check a local database… That would be done in infrastructure.

Then, in the CommandHandler you would pass that service to the named constructor of the zip code VO, that would validate itself.

ocramius has a blog post about it: https://ocramius.github.io/blog/on-aggregates-and-external-context-interactions/

[webdevilopers]

ocramius has a blog post about it: https://ocramius.github.io/blog/on-aggregates-and-external-context-interactions/

I know that post for sure! Also discussed on twitter:

• https://twitter.com/Ocramius/status/824352805318250504

And currently I prefer that approach too.

As @Ocramius mentions in the comments:

I got too accustomed to it when working with Axon, where command handlers could just be aggregate methods, as long as one of the command fields is marked as aggregate identifier. It's a neat shortcut that gets rid of loads of redundancy.

• https://ocramius.github.io/blog/on-aggregates-and-external-context-interactions/#comment-3222187034

[webdevilopers]

But I also agree on the suggestions made by @ludofleury :

If we focus on DDD role of aggregate root: consistency boundaries (nested elements VO or entities ensure their invariants).
I would have designed another specific aggregate root checkout (might make sense according to business needs concerning checkouts).

• https://ocramius.github.io/blog/on-aggregates-and-external-context-interactions/#comment-4144724293

Moving e.g the OverlapDetector in my example or the slotConfirmationService by @nealio82 away from the AR would allow it to focus the true invariants.

I think we have to make distinction here!

• A username has to have a certain length etc. - those are invariantes protected by the AR or its VOs.
• A username must be unique across all ARs is a business rule - not an invariant.

Maybe that's what (Domain) Services or especially Factories are for.

My current approach would suggest to:

• Use the Command Handler as pure orchestrator. No "factory" logic either.
• Pass the Domain Services via Command Handler to a dedicated Factory.
• Let the Factory call the Domain Services and then call the named constructor on the AR.
• If all invariants are protected, create the AR and return it.
• The Command Handler passes the created AR to the repository.

At least it seems easier to test using a Factory instead of calling the services somewhere else. e.g. Command Handler or even the Controller as @skleanthous suggests on our twitter discussion:

The point of SRP covers the above, but also think about how you would test these. The setup for testing the aggregate methods would involve setting up different permutations of the domain service.
The aggregate also would need to accept data that the domain service needs simply to pass them along, polluting the contract and making it less clear.

I generally prefer to have the controller make the calls to the domain service and pass in the data to the other domain objects that need it. Why? A couple of reasons:

1. Dealing with errors
2. Ensuring that the domain methods only have a single responsibility

• https://twitter.com/SKleanthous/status/1426880635038220289

Dealing with errors it another interesting point.

In our application it does not matter who throws the exception - AR or a Service. We have a middleware on the Service Bus that does validation and error handling. It catches any exception and transforms it for the receiver e.g. Domain Exceptions to the Client.

[ludofleury]

Hey @webdevilopers, as said on twitter, I'm not sure of the context of this repo/thread.

On business rule, invariant, invariant business rule etc... I would recommend
https://medium.com/nick-tune-tech-strategy-blog/uncovering-hidden-business-rules-with-ddd-aggregates-67fb02abc4b

At the end of the day, despite their name, it's a set of "rules" vs our arbitrary architecture. We decide to define boundaries between complexity, correctness, integrity, performance etc (cf the article) but it's always an opinionated approach limited by our current understanding of the system or it's current requirements (that will change later).

The topic you cover in this thread is a classic "uncomfortable" DDD situation: rules requiring I/O.

In a simple situation, an aggregate is a unit of consistency. A safe space for the developer to make decision.
In an actual situation, an aggregate is a convenient unit of consistency that cannot satisfy every dimension of a problem.

Everything from there "is fine or possible". Injecting service, defining another aggregate etc.
Yet what matters the most to me is the maintainability cost. In this case:

1. I usually take a shortcut and relies on the handler + repository (naive, for the uniqueness of a username)
2. if the business rule is actually reused or complex (!= from premature abstraction "might be reused"): I do rely on a service to materialize the rule cross-aggregate (like your specification of overlapping contract)

"Pragmatism" is a cool word to solve all of our architectural problem :)

EDIT: the factory with business rule is often a strong indicator that another aggregate is to be uncovered.
$contract = $this->employmentContractFactory->sign($command->contractId(), /*...*/);
"sign" seems an interesting domain behavior with some rules. The fact that a factory "sign" sounds a little bit odd.
But maybe "your" BC don't really care about that and it's totally fine.

It would totally fit the #55 about "don't create AR"

[webdevilopers]

Thanks for your feedback and sorry again for the confusion.

"Pragmatism" is a cool word to solve all of our architectural problem :)

I really like this "approach".

I must confess that most of the time I am concerned about other devs working on the code might forget to us a Factory Class and directly call a named constructor on the aggregate. Passing a service to method will prevent this.

Personally I prefer your first approach too:

I usually take a shortcut and relies on the handler + repository (naive, for the uniqueness of a username)

It also makes testing the AR easier since I don't have to create a "InMemoryUserRepository" implementing all the methods of an "UserRepositoryInterface".

[skleanthous]

Hey all,

Just to note that there is a very specific reason why the aggregate is the actual consistency boundary: in the absence of distributed db transactions (which is the case with event sourcing for example) the aggregate and the stream it is stored in, represents the transactionaly boundary. Moving responsibility to domain services is fine, but unless these are aware of each stream version, we cannot use a domain service to implement cross-aggregate decisions. For example load A, make a check and if it passes, write on B -> this leaves us open to break consistency due to concurrency.

If we're in need to confirm data across aggregates we should do one of two things:

1. Use a saga (with rollback)
2. Reshape our aggregates (it's costly to do)

I know this is slightly outside of the discussion where to use domain service, but I think it's quite an important point seeing the examples and discussion above.

For a more practical example, in the last samples in #60 (comment) an new entry could have been added after the contractPeriodConfirmationService is called in the EmploymentContract.

[skleanthous]

Just to note that I am very much OK with pragmatism, especially considering that most of what we have are guidelines and heuristics. However, in this case above we have a clear violation of our business rule. The aggregate as a transactional boundary is very real in the sense of consistency and we should pay quite a bit of attention to what we do (again in the absence of an ambiend trasnaction spanning multiple aggregates).

[ludofleury]

Totally agreed. The post was mentioning uniqueness of username as well. Where in this case it might be overkill. On the overlap employee contract, I didn’t took the time to read the other comment with schema and diagram. The problem seems complex and I’m not sure of the bounded context. My first instinct would be designing a new AR. But as you rightly pointed out, it might be costly in a maintenance scenario.

[skleanthous]

Specifically about domain services and where they are called I think the fact that the services throw exception is something you can utilize to allow to deal with errors as a cross-cutting concern, and if that works for you, then great! No further arguments there. If you find yourself in the need to do something custom, then this would be a point that would apply, but you're not forced into adopting this pattern across the board, so do what works for you.

However, the SRP and testing arguments still stand though, but do depend on how you test. For example I prefer to test my business logic on it's own to make sure that it works as expected. This includes the domain services which are part of the domain as per DDD. Testing each on it's own guarantees that we have a significantly smaller number of tests to run due to the permutations we need to run if we consider the domain service + aggregate as a whole.

[skleanthous]

Regarding uniqueness of username, it depends if it's simple or not. For example you could use the username as a natural primary key (which it is), but if combined with event sourcing, it becomes a bit "weird". If you use the primary key as a stream name, and rely on a repository to enforce that it would work, but it needs support from the infrastructure to be able to rename the stream transactionally (which a LOT of servers don't do). Another option is to migrate the stream on username change. It works as well again, but still it's a bit of overhead to do and not a lot of people are aware of such solutions (I mean the total things necessary to look out for).

Generally, I recommend splitting the requirements into two aggregates in such cases to be honest.

Perfectly fine to rely on a natural key for uniquness checks in other settings, but I prefer to avoid enforcing unique checks on anything other than the ID as these are business rules and should "sit" alongside other business rules in code for maintainability (so we shouldn't have some in code and some in unique constraints in a sql db).

[webdevilopers]

I know this is slightly outside of the discussion where to use domain service, but I think it's quite an important point seeing the examples and discussion above.

Actually I think we should re-define the subject of this discussion. It's great to discover "the big picture".

[webdevilopers]

On the overlap employee contract,...
My first instinct would be designing a new AR. But as you rightly pointed out, it might be costly in a maintenance scenario.

I agree and I agree! We started with the detector in the Command Handler. We then moved it to the AR.
By now - having at least some experiences with what Event Sourcing really brings to us - we started thinking in other aggregates that hold responsibilities e.g. "Open Slot Manager", "Available Periods when scheduling" etc.!

And yes, we couldn't give it a real try yet because of it being costly.

[skleanthous]

One thing to note though, it doesn't really matter where the "detector" is called from, it matters whether some data used in the decision-making process are checked for concurrency at the same time as the decision itself. If these are both in the same stream (as is normally in an aggregate where one stream is used to rehydrate an aggregate state that is used to emit events that are to be appended on top) then this is guaranteed. HOWEVER, when you read from multiple or other streams and write to another one, where you make the decision is not important, as you have a potential concurrency issue.

[webdevilopers]

Regarding uniqueness of username,...

I think this has become a "classic" example.

1. Use the username as primary key. Indeed this seems weird.
2. Store the username inside an extra table w/ unique key, catch exception. Try to save the AR. If it fails rollback. Since the repositories for usernames and AR can live in the same transaction / unit of work.
3. Use a separate AR to clear the responsibility.

The latter seems the purest event-driven solution. But I always wondered if changing two ARs in a single transaction is recommended.
Of course AR1 could throw an event that again holds all the data the original Command had. But that feels like unneccessary "forwarding".

[skleanthous]

With point 3 in #60 (comment) you don't need to change 2 aggregate roots in one transaction. You'll have one aggregate that only guarantees the "usernames have to be unique" and another aggregate that guarantees that OTHER changes to the user aggregate are consistently applied. The aggregate that guarantees the name is unique can be backed by a sql store, or it could be a stream with all user names (so consistency is guaranteed using optimistic concurrency on write on the same stream), and the other could be what you normally do.

One thing we all need to break away from is to consider aggregates as semantic groups; there is no "user" aggregate. The aggregates exist to enforce consistency of data being changed due to business decisions. We have usernames, and then we have "other consistent user data". These are two things, not one.