webdevilopers
Are CQRS commands part of the domain model?
Came from: https://twitter.com/webdevilopers/status/1374292878713831428 by @domainascode
A classic discussion. And it really depends on how you define your commands. For instance our „commands“ start as Application DTO and convert primitives to Value Objects and are then passed to the Handler. If your commands go directly to the Model, then Domain is a good place.
Refs:
- https://github.com/webdevilopers/php-ddd/issues/44
- https://github.com/webdevilopers/php-ddd/issues/14
This is our current approach using the Symfony Messenger as Command Bus incl. the Doctrine Middleware.
Controller
Here User JSON is passed to the Command. In addition the User ID ("accountId") is added from the security layer.
<?php
namespace Trexxon\Places\Infrastructure\Symfony\Controller;
use Symfony\Component\HttpFoundation\JsonResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
use Symfony\Component\Messenger\MessageBusInterface;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
use Trexxon\Places\Application\Service\Spot\AddSpot;
final class AddSpotController
{
private AuthorizationCheckerInterface $authorizationChecker;
private TokenStorageInterface $tokenStorage;
private MessageBusInterface $commandBus;
public function __construct(
AuthorizationCheckerInterface $authorizationChecker,
TokenStorageInterface $tokenStorage,
MessageBusInterface $commandBus
)
{
$this->authorizationChecker = $authorizationChecker;
$this->tokenStorage = $tokenStorage;
$this->commandBus = $commandBus;
}
public function __invoke(string $placeId, Request $request): Response
{
if (!$this->authorizationChecker->isGranted('ROLE_HOST')) {
throw new AccessDeniedHttpException();
}
$command = AddSpot::fromPayload(array_merge(
[
'accountId' => $this->tokenStorage->getToken()->getUser()->userId()->toString(),
'placeId' => $placeId
],
json_decode($request->getContent(), true)
));
$this->commandBus->dispatch($command);
return new JsonResponse(null, Response::HTTP_CREATED);
}
}
Trait
This trait allows the command to accept any payload e.g. a User JSON request and populate the command automatically if the property is defined there.
<?php declare(strict_types=1);
namespace Trexxon\Common\Application\Services;
trait PayloadMessage
{
private function __construct()
{}
/**
* @param array $payload
*
* @return static
* @todo Return static.
* @see https://stitcher.io/blog/php-8-before-and-after#static-instead-of-doc-blocks
*/
public static function fromPayload(array $payload)
{
$self = new static();
foreach (get_class_vars(get_class($self)) as $varName => $varValue) {
if (array_key_exists($varName, $payload)) {
$self->$varName = $payload[$varName];
}
}
return $self;
}
}
Command
Here the "allowed" properties are defined. Together with annotations for the Validation Middleware that is fired when the message is dispatched. Only primitives are used. But for convenience the Command returns Domain Value Objects via getters instead of building them inside the Command Handler.
<?php
namespace Trexxon\Places\Application\Service\Spot;
use Symfony\Component\Validator\Constraints as Assert;
use Trexxon\Account\Domain\Model\Account\AccountId;
use Trexxon\Common\Application\Services\PayloadMessage;
use Trexxon\Places\Domain\Model\Place\PlaceId;
use Trexxon\Places\Domain\Model\Spot\SpotId;
use Trexxon\Places\Domain\Model\Spot\SpotName;
final class AddSpot
{
use PayloadMessage;
/**
* @Assert\NotBlank()
* @Assert\Uuid()
*/
private string $accountId;
/**
* @Assert\NotBlank()
* @Assert\Uuid()
*/
private string $placeId;
/**
* @Assert\NotBlank()
* @Assert\Uuid()
*/
private string $spotId;
/**
* @Assert\NotBlank()
* @Assert\Type(type="string")
*/
private string $name;
public function accountId(): AccountId
{
return AccountId::fromString($this->accountId);
}
public function placeId(): PlaceId
{
return PlaceId::fromString($this->placeId);
}
public function spotId(): SpotId
{
return SpotId::fromString($this->spotId);
}
public function name(): SpotName
{
return SpotName::fromString($this->name);
}
}
As @matthiasnoback states:
My general rule is to have only immediately serializable data in a DTO (that is, primitive types). I know some people who add "getters" to a DTO which produce actual (domain) objects based on those primitive values. I don't do it like that, but always explicitly convert to objects inside the command handler.
- https://github.com/webdevilopers/php-ddd/issues/14#issuecomment-226955268
Or:
Also, a command is a DTO, which isn't a domain model. It just represents the data you need to handle it. In that sense it supports having alternative implementations in the infrastructure layer (e.g. a web controller, and a Behat FeatureContext) which can use it.
- https://twitter.com/matthiasnoback/status/1101098710338060289
/cc @mrook @kapitancho @nicolaibaaring
(= @stochnagara)
So from what I see, if we have let's say AddSpot, ChangeSpot, DisableSpot and maybe a few more, then all of them would have the very same function
public function spotId(): SpotId { return SpotId::fromString($this->spotId); }
Would you consider this a DRY violation and if yes, how would you overcome it?
That indeed is another discussion. Personally I never regard this code duplication. It is the same code technically. But I to prefer to look at the contexts the method exists in. And these are different use cases: "adding", "changing" etc..
On the other hand these methods will fairly never change. Opening the class shows you all you need. I'm not a fan of clicking through abstract classes to find some shared base methods. ;)
On the other side if you are using PHP 8 you can also shorten it a little bit this way:
final class AddSpot {
public function __construct(
private string $accountId,
private string $placeId,
private string $spotId,
private string $name
) { }
/// + getters
}
$payload = ['accountId' => 'A1', 'placeId' => 'P1', 'spotId' => 'S1', 'name' => 'N1'];
$addSpot = new AddSpot(...$payload);
That indeed is another discussion. Personally I never regard this code duplication. It is the same code technically. But I to prefer to look at the contexts the method exists in. And these are different use cases: "adding", "changing" etc..
On the other hand these methods will fairly never change. Opening the class shows you all you need. I'm not a fan of clicking through abstract classes to find some shared base methods. ;)
Well I am fully with you there. Many people seem to over-abstract and that's not good in general. The only reasonable alternative I see is by using composition but still, the simplicity is often our best friend :)
On the other side if you are using PHP 8 you can also shorten it a little bit this way:
Nice! The next project will start with PHP8. I will take your example into account! Thanks.
So I think the only question remaining is "what happens if you have an invalid value in any of the properties (e.g. spotId) and the VO initialization fails?
So I think the only question remaining is "what happens if you have an invalid value in any of the properties (e.g. spotId) and the VO initialization fails?
Good question, I forgot to mention!
The Validation Middleware ensures that the primitives match some basic rules e.g. types and fires an InvalidArgumentException. That prevents a few cases where a value object would fail on creation.
When the value object itself fails it throws a DomainException.
Both exception types are catched by a listener that converts them into readable formats for the user.
That is shown in here:
- https://github.com/webdevilopers/php-ddd/issues/44
So I think the only question remaining is "what happens if you have an invalid value in any of the properties (e.g. spotId) and the VO initialization fails?
Good question, I forgot to mention!
The Validation Middleware ensures that the primitives match some basic rules e.g. types and fires an
InvalidArgumentException. That prevents a few cases where a value object would fail on creation.When the value object itself fails it throws a
DomainException.Both exception types are catched by a listener that converts them into readable formats for the user.
That is shown in here:
* #44
Cool, that's interesting. So you decided to split the input and the domain validations and you use a "validation framework" for the first part. I believe this is perfectly fine since the domain has a sort of an API and it expects a valid input in order to provide a valid output (e.g event or rejection). Something I was unable to see in #44 is - do you double the validations within the VOs?
For instance you have
"countryCode" = {
* @Assert\NotBlank(allowNull=true),
* @Assert\Type(type="string"),
* @Assert\Length(min="2")
* },
and then you call this:
Address::fromArray($this->address);
@webdevilopers I build the exact same structure of code, but I guess the part that is new to me is keeping the properties of the command as primitives and only getting value objects from the getters. I typically build value objects in a static named constructor and have a set of serialization methods on the object. It's surely more code than your example, so that's the downside, but the command will be valid from construction and it doesn't make a new value object each time you get it from the command. I will surely consider the ease of your way.
What do you think is the right balance?
What do you think is the right balance?
I like the explanation by @matthiasnoback here:
My general rule is to have only immediately serializable data in a DTO (that is, primitive types). I know some people who add "getters" to a DTO which produce actual (domain) objects based on those primitive values. I don't do it like that, but always explicitly convert to objects inside the command handler. Also, a command is a DTO, which isn't a domain model. It just represents the data you need to handle it. In that sense it supports having alternative implementations in the infrastructure layer (e.g. a web controller, and a Behat FeatureContext) which can use it.
There are other solutions where Commands are not DTOs and are directly passed to the Aggregate instead of the Command Handler. A method on the aggregate method can then be annotated as @CommandHandler.
Sure those types of Commands should be rich in Domain Models and kept inside the Domain Layer.
I guess this "pure Command vs. Command DTO" is a general misunderstanding when it comes to discussions like these. ;)
Something I was unable to see in #44 is - do you double the validations within the VOs?
Indeed we sometimes "duplicate" validation - again: technically. But from the context POV these are different validations.
- Command -> "Superfical Validation" - checking primitives and preventing an invalid DTO to be dispatched by the Command Bus.
- Command Handler / Value Object -> "Domain Validation" - the DTO has now reached the Domain and should be checked again for types etc. plus business rules.
And don't forget: what if someone throws away your commands and instead directly acts on the Aggregate? You will at least need the validation on the Domain. "Superficial Validation" lives in another Context and Layer.
Again - as you said - using abstraction here could be a pain. At the bottom line it is a single annotation for checking for an integer. And thanks to type-hinted attributes some of the "Superficial Validation" can even be left out.
It is so interesting how everybody finds a different approach. This is one of the beautiful things in our job - the creativity!
About the validation - so we have it in the Domain in the Commands .. we may also have it for the web-input (e.g. OpenApi) and we usually have it in the GUI as well... so it comes along everywhere :)
One last thing - I see you are (ab)using the DomainException. Take care - it extends LogicException which is explained as "Exception that represents error in the program logic. This kind of exception should lead directly to a fix in your code." It should be instead an exception that comes from RuntimeException ;)
One last thing - I see you are (ab)using the DomainException. Take care - it extends LogicException which is explained as "Exception that represents error in the program logic. This kind of exception should lead directly to a fix in your code." It should be instead an exception that comes from RuntimeException ;)
Absolutely. We are still playing around with the exceptions. There are also FooNotFound Exceptions. Some are for internal use only. Others are for the user and should produce a 404 http status.
But whenever we have an error caused by business logic restrictions we think a DomainException is a good idea.
I think @matthiasnoback wrote a blog about this too.