webdevilopers / php-ddd

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

Where and how to handle command (superficial) and domain validation? #44

Open webdevilopers opened 4 years ago

webdevilopers commented 4 years ago

Originally posted by @gquemener :

Let's define domain related validation once and for all within well-defined VO and expose those errors to the world. > https://gist.github.com/gquemener/09b2bc303e63dfc3123f7d540e9891a0

It prevents to add extra constraint metadata! Is this too much magic?

@matthiasnoback and @AntonStoeckl joined the discussion.

@AntonStoeckl also blogged about a solution in GO:

On our case:

We decided to work w/ the #symfonymessenger usind the mentioned validation middleware based on symfony constraints. VOs are created from command getters. Every domain exception is caught by an listener and converted into human-readable messages.

Example:

# config/packages/messenger.yaml
framework:
  messenger:
    default_bus: messenger.bus.commands
    buses:
      messenger.bus.commands:
        middleware:
          - validation
# config/services.yaml
services:
    Acme\Common\Infrastructure\Symfony\EventListener\ExceptionListener:
        tags:
            - { name: kernel.event_listener, event: kernel.exception }
        arguments: ['@translator.default']
<?php

namespace Acme\Common\Infrastructure\Symfony\EventListener;

use DomainException;
use Prooph\EventStore\Exception\ConcurrencyException;
use Acme\Common\Presentation\Model\NotFoundException;
use Symfony\Component\HttpFoundation\JsonResponse;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Event\ExceptionEvent;
use Symfony\Component\Messenger\Exception\HandlerFailedException;
use Symfony\Component\Messenger\Exception\RuntimeException;
use Symfony\Component\Messenger\Exception\ValidationFailedException;
use Symfony\Component\Validator\ConstraintViolation;
use Symfony\Contracts\Translation\TranslatorInterface;

/**
 * Handles bad requests and returns a 400 status with human-readable errors to the client.
 * Returns a 500 status otherwise.
 */
final class ExceptionListener
{
    /** @var TranslatorInterface */
    private $translator;

    private $translationDomain = 'exception';

    private $locale = 'de';

    public function __construct(TranslatorInterface $translator)
    {
        $this->translator = $translator;
    }

    public function onKernelException(ExceptionEvent $event)
    {
        // You get the exception object from the received event
        $exception = $event->getThrowable();

        if (!$exception instanceof RuntimeException) {
            return;
        }

        $response = new JsonResponse();
        $response->setStatusCode(Response::HTTP_BAD_REQUEST);

        if ($exception instanceof ValidationFailedException) {
            // Handle domain specific exceptions - public to the client
            $errors = [];

            /** @var ConstraintViolation $violation */
            foreach ($exception->getViolations() as $violation) {
                $errors[] = [
                    'message' => $violation->getMessage(),
                    'path' => $violation->getPropertyPath()
                ];
            }

            $response->setContent(json_encode(['errors' => $errors]));
            $event->setResponse($response);

            return;
        }

        if ($exception instanceof HandlerFailedException) {
            // Handle presentation and domain specific exceptions - public to the client
            $errors = [];

            if ($exception->getPrevious() instanceof NotFoundException) {
                $errors = [
                    'message' => $exception->getMessage(),
                    'path' => null
                ];

                $response->setContent(json_encode(['errors' => $errors]));
                $response->setStatusCode(Response::HTTP_NOT_FOUND);

                $event->setResponse($response);

                return;
            }

            if ($exception->getPrevious() instanceof DomainException) {
                $errors[] = [
                    'message' => $this->translator->trans(
                        $exception->getPrevious()->getMessage(), [], $this->translationDomain, $this->locale
                    ),
                    'path' => null
                ];

                $response->setContent(json_encode(['errors' => $errors]));
                $event->setResponse($response);

                return;
            }

            // Handle individual server errors if relevant to the client
            switch (get_class($exception->getPrevious())) {
                case ConcurrencyException::class:
                    $errors = [
                        'message' => 'Duplicate entry',
                        'path' => null
                    ];

                    $response->setContent(json_encode(['errors' => $errors]));
                    $event->setResponse($response);

                    return;

                    break;
            }
        }
    }
}

The listener can be expanded for individual exceptions. Exception messages are simply translated. If it is a Symfony Constraint error message it normally was already translated. In addtion the path will be added.

The JSON result normally looks like this:

errors: {
    message: "Some error in the command DTO"
    path: "firstName"
}
errors: {
    message: "Some error in the domain e.g. thrown inside value object"
    path: null
}

AFAIK Symfony Messenger can be used without Symfony. You can use it as a standalone service bus. The Validation middleware is included too. Putting the "logic" of the listener elsewhere should be no problem.

WDYT?

Older possibly related issues:

gquemener commented 4 years ago

An onKernelException listener is clearly the way to go to convert command validation errors into a nice json response.

However, you could rely on the Symfony serializer to directly encode the ConstraintViolationListInterface instance. You would get RFC compliant response for free, if you would like (see https://github.com/symfony/serializer/blob/master/Normalizer/ConstraintViolationListNormalizer.php).

Another question that comes to mind is : how do you define command validation metadata ? I was suggesting to use one class constraint (instead of one property constraint for each properties) which would sequentially call all the command accessor (which creates the value objects) and aggregate the errors that way within the ConstraintViolationListInterface. Thus, this solution removes the need to define extra command validation metadata and rely solely on what's described within the value objects constructor.

webdevilopers commented 4 years ago

Stupid me, forgot the Command:

<?php

namespace Acme\PersonnelManagement\Application\Service\Person;

use Acme\Common\Domain\Model\BirthName;
use Acme\Common\Domain\Model\PlaceOfBirth;
use Acme\Common\Domain\Model\DateOfBirth;
use Acme\Common\Domain\Model\FirstName;
use Acme\Common\Domain\Model\LastName;
use Acme\Common\Domain\Model\PersonalName;
use Acme\Common\Domain\Model\PersonalName\NamePolicy;
use Acme\Common\Domain\Model\SocialSecurityNumber;
use Acme\Common\Domain\Model\Title;
use Acme\Common\Domain\Model\Address;
use Acme\PersonnelManagement\Domain\Model\BiographicInformation;
use Acme\Common\Domain\Model\ContactInformation;
use Acme\PersonnelManagement\Domain\Model\Person\PersonId;
use Symfony\Component\Validator\Constraints as Assert;

final class AddPerson
{
    /**
     * @var PersonId $personId
     * @Assert\NotNull()
     * @Assert\Uuid()
     */
    private $personId;

    /**
     * @var Title $title
     * @Assert\NotNull
     * @Assert\NotBlank
     * @Assert\Type(type="string")
     */
    private $title;

    /**
     * @var FirstName
     * @Assert\NotNull
     * @Assert\NotBlank
     * @Assert\Type(type="string")
     * @Assert\Regex(pattern=NamePolicy::ALLOWED_PATTERN, normalizer="trim")
     * @Assert\Regex(pattern=NamePolicy::FORBIDDEN_PATTERN, normalizer="trim", match=false)
     */
    private $firstName;

    /**
     * @var LastName $lastName
     * @Assert\NotNull
     * @Assert\NotBlank
     * @Assert\Type(type="string")
     * @Assert\Regex(pattern=NamePolicy::ALLOWED_PATTERN, normalizer="trim")
     * @Assert\Regex(pattern=NamePolicy::FORBIDDEN_PATTERN, normalizer="trim", match=false)
     */
    private $lastName;

    /**
     * @var BiographicInformation
     * @Assert\NotNull
     * @Assert\Type(type="array")
     * @Assert\Collection(
     *  fields = {
     *      "birthName" = {
     *          @Assert\NotBlank(allowNull=true),
     *          @Assert\Type(type="string")
     *      },
     *      "dateOfBirth" = {
     *          @Assert\DateTime(format="Y-m-d")
     *      },
     *      "placeOfBirth" = {
     *          @Assert\NotBlank(allowNull=true),
     *          @Assert\Type(type="string")
     *      },
     *  })
     */
    private $biographicInformation;

    /**
     * @var SocialSecurityNumber|null
     * @Assert\Type(type="string")
     */
    private $socialSecurityNumber;

    /**
     * @var ContactInformation $contactInformation
     * @Assert\NotNull,
     * @Assert\Type(type="array"),
     * @Assert\Collection(
     *  fields = {
     *      "email" = {
     *          @Assert\NotBlank(allowNull=true),
     *          @Assert\Email,
     *      },
     *      "phone" = {
     *          @Assert\NotBlank(allowNull=true),
     *          @Assert\Type(type="string")
     *      },
     *      "mobile" = {
     *          @Assert\NotBlank(allowNull=true),
     *          @Assert\Type(type="string")
     *      },
     *      "fax" = {
     *          @Assert\NotBlank(allowNull=true),
     *          @Assert\Type(type="string")
     *      },
     *  }
     * )
     */
    private $contactInformation;

    /**
     * @var Address
     * @Assert\NotNull,
     * @Assert\Type(type="array"),
     * @Assert\Collection(
     *  fields = {
     *      "street" = {
     *          @Assert\NotBlank(allowNull=true),
     *          @Assert\Type(type="string")
     *      },
     *      "postcode" = {
     *          @Assert\NotBlank(allowNull=true),
     *          @Assert\Type(type="string")
     *      },
     *      "city" = {
     *          @Assert\NotBlank(allowNull=true),
     *          @Assert\Type(type="string")
     *      },
     *      "countryCode" = {
     *          @Assert\NotBlank(allowNull=true),
     *          @Assert\Type(type="string"),
     *          @Assert\Length(min="2")
     *      },
     *  }
     * )
     */
    private $address;

    public function __construct(array $payload)
    {
        $this->personId = $payload['personId'];
        $this->title = $payload['title'];
        $this->firstName = $payload['firstName'];
        $this->lastName = $payload['lastName'];
        $this->biographicInformation = $payload['biographicInformation'];
        $this->socialSecurityNumber = $payload['socialSecurityNumber'];
        $this->contactInformation = $payload['contactInformation'];
        $this->address = $payload['address'];
    }

    public function personId(): PersonId
    {
        return PersonId::fromString($this->personId);
    }

    public function title(): Title
    {
        return Title::fromString($this->title);
    }

    public function personalName(): PersonalName
    {
        return new PersonalName(
            FirstName::fromString($this->firstName),
            null,
            LastName::fromString($this->lastName)
        );
    }

    public function biographicInformation(): BiographicInformation
    {
        return new BiographicInformation(
            null !== $this->biographicInformation['birthName'] ? BirthName::fromString($this->biographicInformation['birthName']) : null,
            null !== $this->biographicInformation['dateOfBirth'] ? DateOfBirth::fromString($this->biographicInformation['dateOfBirth']) : null,
            null !== $this->biographicInformation['placeOfBirth'] ? PlaceOfBirth::fromString($this->biographicInformation['placeOfBirth']) : null
        );
    }

    public function socialSecurityNumber(): ?SocialSecurityNumber
    {
        return null !== $this->socialSecurityNumber ? SocialSecurityNumber::fromString($this->socialSecurityNumber) : null;
    }

    public function contactInformation(): ContactInformation
    {
        return ContactInformation::fromArray($this->contactInformation);
    }

    public function address(): Address
    {
        return Address::fromArray($this->address);
    }
}

In this example the firstName is also later validated in the domain by a Policy / Specification:

<?php

namespace Acme\Common\Domain\Model\PersonalName;

final class NamePolicy
{
    public const ALLOWED_PATTERN = "/^[\p{L}\-\.\s\']+$/u";
    public const FORBIDDEN_PATTERN = "/[\p{Lu}]{2,}/u";

    public static function isSatisfiedBy(string $name): bool
    {
        if (empty($name)) {
            return false;
        }
        // Allowed are unicode letters only and `.`, `-`, `'`, no numbers.
        if (1 !== preg_match(self::ALLOWED_PATTERN, $name)) {
            return false;
        }

        // Continuous uppercase letters are not allowed.
        if (1 === preg_match(self::FORBIDDEN_PATTERN, $name)) {
            return false;
        }

        return true;
    }
}

It is checked inside the FirstName value object:

<?php

namespace Acme\Common\Domain\Model;

use ReflectionClass;
use Acme\Common\Domain\Model\PersonalName\Exception\NameContainsIllegalCharacters;
use Acme\Common\Domain\Model\PersonalName\NamePolicy;
use Acme\Common\Domain\Model\PersonalName\NameNormalizer;

final class FirstName
{
    /** @var string $name */
    private $name;

    private function __construct(string $aName)
    {
        $name = NameNormalizer::withString($aName);

        if (!NamePolicy::isSatisfiedBy($name)) {
            throw new NameContainsIllegalCharacters();
        }

        $this->name = $name;
    }

    public static function fromString(string $name): FirstName
    {
        return new self($name);
    }

    public static function fromPayload(string $name): FirstName
    {
        $firstNameRef = new ReflectionClass(\get_called_class());

        /** @var FirstName $firstName */
        $firstName = $firstNameRef->newInstanceWithoutConstructor();
        $firstName->name = $name;

        return $firstName;
    }

    public function toString(): string
    {
        return $this->name;
    }
}

That is a domain exception the listener can catch and translate to the user.

webdevilopers commented 4 years ago

Some details of the examples are inconsistent e.g. the doc block should hint to the primitive values. Please ignore.

gquemener commented 4 years ago

Allright, so you suggest to "duplicate" validation logic (eg: the forbbidden name pattern is checked through command validation, thanks to the Assert\Regex constraint AND through PersonalName instanciation).

Not saying that this is bad per se, I'm just wondering if it wouldn't be possible to only rely on validation-at-instanciation and provide the same level of information to the caller (aka http client), getting rid of extra framework-related work at the same time (the whole validation metadata definition) :thinking:

webdevilopers commented 4 years ago

I get your point. But I would not regard this a "duplication".

Each layer has its own validation. We have command DTOs inside the Application layer. Sometimes some "aggregate-spanning" validation e.g. "Unique Username" live here too. The Policy / Specification and other Domain Model (Value Object, Entities) validation lives inside the Domain layer.

BTW: The RegEx was easy since we could reuse the pattern from the Policy class.

One point is to give API client immediate feedback. If there were two errors e.g. socialSecurityNumber AND firstName he will get it at the same time.

The main idea is NOT to create the value object at all if the command has not the validation of the primitives. It is dispatched to the command bus but never really reaches the Domain and stays inside the Application layer.

Only if the primitives are valid the handler calls the getters on the command and starts orchestrating with the Domain.

In earlier scenarios we did not have the getters on the command DTO. The latter was reaching the handler after being deserialized. Then the VO factory methods were called directly in the handler. The code was not nice to look at.

At the bottom line most of the rules don't change often. There's not much more code. Value objects and Commands can be unit tested to ensure they do the same thing.

Just my...

    /**
     * @var int $cents
     * @Assert\NotNull()
     * @Assert\Min(min="2)
     */
    private $cents;

;)

webdevilopers commented 4 years ago

Possibly related:

In practice, it is often simpler to allow a degree of duplication rather than to strive for complete consistency.

@eulerfx

Or in the comments section on the article by @vkhorikov :

@plalx