Applying domain events to aggregate roots when deserializing payload of old events break current value object constraint changes

[webdevilopers]

Came from:

• https://twitter.com/webdevilopers/status/1286211935407616000

Example:

• https://gist.github.com/webdevilopers/74c1f109c0f095ab65f7eef1f56a10f1

/cc @AntonStoeckl

Our FirstName value object has changed over time. It started with no constraints (FirstName_V1). That's whay event payload like the following was possible:

{"firstName":"Max 12345"}

Later constraints e.g. no numbers allowed were added (FirstName_V2). The names were fixed. Now the event history that has to applied to the Person aggregate root includes this:

{"firstName":"John 12345"},
{"firstName":"John Doe"},

When creating the event and serializing the data this will not cause any problem.

        $event = self::occur(
            $personId->toString(),
            [
                'personId' => $personId->toString(),
                'oldFirstName' => $oldFirstName->toString(),
                'newFirstName' => $newFirstName->toString(),
            ]
        );

But when applying it...

    protected function apply(AggregateChanged $event): void
    {
        switch (get_class($event)) {
            case NameChanged::class:
                /** @var NameChanged $event */
                $this->firstName = $event->newFirstName();

...it will break and throw the NameContainsIllegalCharacters exception.

    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);
    }

There are two solutions I can imagine so far:

(1) Somehow catch the date of the BC for the payload and convert the old breaking payload to the new constraints. Similar to "event upcasting".

(2) Add a named constructor e.g. fromPayload that skips the validation. Similar to value objects based on multiple properties that have an fromArray and toArray e.g.:

abstract class DomainMessage implements Message
{
    public static function fromArray(array $messageData): DomainMessage
    {
        MessageDataAssertion::assert($messageData);

        $messageRef = new \ReflectionClass(\get_called_class());

        /** @var $message DomainMessage */
        $message = $messageRef->newInstanceWithoutConstructor();

        $message->uuid = Uuid::fromString($messageData['uuid']);
        $message->messageName = $messageData['message_name'];
        $message->metadata = $messageData['metadata'];
        $message->createdAt = $messageData['created_at'];
        $message->setPayload($messageData['payload']);

        return $message;
    }
}

Source: https://github.com/prooph/common/blob/master/src/Messaging/DomainMessage.php#L49-L65

This example even uses reflection in order to skip the constructor.

Concrete solution:

/**
 * Character e.g. numbers were forbidden.
 */
final class FirstName_V2
{
    /** @var string $name */
    private $name;

    private function __construct()
    {
    }

    public static function fromString(string $aName): FirstName
    {
        $name = NameNormalizer::withString($aName);

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

        $self = new self();
        $self->name = $name;

        return $self;
    }

    public static function fromPayload(string $name): FirstName
    {
        $self = new self();
        $self->name = $name;

        return $self;
    }

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

Disadvantage: the validation is no longer inside the constructor. This is fine as long there is only a single named constructor that requires it. If there are multiple methods I guess you would have to put them into an extra validate method.

Please note:
Normally a first name is not a critical property that needs to be stored inside an aggregate root since it is not relevant when protecting the invariants when changing state. But let's assume it is in example. That's why I added the old first name to the event.

[webdevilopers]

Thoughts @prolic ?

@matthiasnoback maybe? This reminds me of your example in chapter "3.4. Using value objects with internal read models" of your new book "Web application architecture":

• https://leanpub.com/web-application-architecture

You suggest to use read model for a book price:

final class Price
{
    private int $priceInCents;
    
    private function __construct(int $priceInCents)
    {
        $this->priceInCents = $priceInCents;
    }

    public static function fromInt(int $priceInCents): self
    {
        return new self($priceInCents);
    }

    public function asInt(): int
    {
        return $this->priceInCents;
    }
}

Here we have multiple named constructors. And similar to my example it would get complex if someone suddenly added a "must be greater then zero" constraint to the constructor but for some reason negative numbers were valid in the past and these values still exist in the event stream for this read model.

[webdevilopers]

Interesting read by @heynickc :

When we replay these events to reconstitute the state of an Aggregate, if the Value Object has a new invariant that makes the Event invalid - how can we deserialize the Event into a Domain Model which guarantees its state to be valid at all times? We can’t.

• https://buildplease.com/pages/vos-in-events/

[webdevilopers]

Would love to hear how the @AxonFramework deals with these kind of "value object from deserialized event payload without revalidating" issue.

@abuijze Could you give us some feedback on this? Thanks in advance.

[webdevilopers]

Related to an older issue:

• Validating deserialized messages (e.g. Domain Events with payload) when re-constructing Domain Models #35

Thanks to @Ocramius for his feedback:

fromArray_v2 all the way: I avoid public function __construct in my systems, as data structures have multiple ways of being instantiated. Reusing private logic is fine, but the ctor should really not be public.

[webdevilopers]

My current suggestions would be:

    private function __construct()
    {}

    public static function fromString(string $name): FirstName
    {
        $name = NameNormalizer::withString($aName);

        $self = new self();
        $self->validate();
        $self->name = $name;

        return $self;
    }

    public static function fromWhateverWithValidationRequired(string $name): FirstName
    {
        $name = NameNormalizer::withString($aName);

        $self = new self();
        $self->validate($name);
        $self->name = $name;

        return $self;
    }

    private function validate(string $name): void
    {
        if (!NamePolicy::isSatisfiedBy($name)) {
            throw new NameContainsIllegalCharacters();
        }
    }

    public function fromPayload(string $payload): FirstName // or `fromEventPayload` or `deserialize`?!
    {
        $self = new self();
        $self->name = $name;

        return $self;
    }

or

    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 // or `fromEventPayload` or `deserialize`?!
    {
        $firstNameRef = new \ReflectionClass(\get_called_class());

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

        return $firstName;
    }

Thoughts?

[prolic]

You can use a special constructor for this that is marked @internal

[webdevilopers]

Other options related to the library or framework you use could be versioning of events as suggested by @Mattin. Options would be:

• Upcasting the event
• Using a different value object e.g. "LegacyFirstName" or different named constructor on the object e.g. "FirstName::fromLegacyPayload". But this makes the "data" command the domain, add extra files and puzzle your domain.

Regarding versioning of events in @prooph you could simply add your version inside your events and then store it inside the metadata just like the "aggregate_version".

Related:

• Versioning events in PROOPH prooph/proophessor#80 created by @Arkowsky
• http://docs.getprooph.org/event-store/upcasting.html
• https://groups.google.com/forum/#!topic/dddinphp/zGgSJLuftJ4 commented by @codeliner
• https://www.michielrook.nl/2016/09/event-versioning/ by @mrook

[webdevilopers]

At the bottom line a totally I agree with @AntonStoeckl here:

Upcasting imho is: Adding new information where old Events will be valid with a default. Or only changing the structure.

• https://twitter.com/TonyBologni/status/1286282849516236803

This example is not critical and the old data does not have to "fixed". Otherwise e.g. with GDPR a change of the event stream may be neccessary. As suggested by @bwaidelich.

Here applying old events to the aggregate root should not break anything. The EVENT itself has not changed - that why Event Upasting is not really the best approach. The DOMAIN changed.

As explained by @AntonStoeckl here:

I don’t get why this simple problem needs a complex solution. Events are facts, projecting them should not fail (exceptions exist). Command handling applies all validations. Semantic, invariants, policies.

• https://twitter.com/TonyBologni/status/1286585852584497153

[bwaidelich]

I think this question can only be answered if you know the intentions of the change:

a) It's a critical new invariant that has to be enforced by all means
=> event upcasting
=> or a new VO constructor that enforces its new constraints by changing the payload
=> or re-published events

b) It's a new rule that should be enforced during the creation of new events but can be ignored when consuming them
=> create a new VO that is used in the command side
=> or add additional validation in the UI

[webdevilopers]

b) It's a new rule that should be enforced during the creation of new events but can be ignored when consuming them
=> create a new VO that is used in the command side
=> or add additional validation in the UI

But why create a new value object? The constraints can simply be added to the existing ones. The are added NOW. If the event stream is applied to the aggregate root OLD and NEW data use a named constructor e.g. "fromPayload" that DOES NOT RE-VALIDATE. Since it has already been validated the day they were created and WERE VALID at that time.

The fromPayload method (see suggestions above) may look risky and could be used by a developer (not a client) to create invalid data. But it is required for smoothly deserializing event payload.

I like the idea of marking it as @internal as suggested by @prolic .

[bwaidelich]

But why create a new value object?

Because, if you ignore the new constraints you don't gain anything from them.
You might reply: Yes, I'll gain that the new invariants are enforced when a user creates an instance (i.e. in the command side).
But now your domain logic needs to know where your VO comes from to know whether it follows the new rules or not.

Also it would get really confusing if you later add constraints that should be always enforced..

I'm still not perfectly sure whether I got the reason for adding the new constraints right. But it seems it's all about the write side anyways (prevent users from creating new names with numbers).
If that's the case I would not even suggest to create a new VO, just enforce the constraints during the write side.

In the UI:

<input type="text" pattern="[a-zA-Z]{2,20}" required />

or in the command (handler).

And, btw, this was probably just an example. But Elon Musks daughter wouldn't be allowed to use your service ;)

[webdevilopers]

But now your domain logic needs to know where your VO comes from to know whether it follows the new rules or not.

That is the point! The domain logic does not have to know it. NEW data will be passed to the VO and validated for sure.
OLD data is just event payload that has been valid the day it was created. BUT the serialized primitve content e.g. John 1234 has to be deserialized from the event payload and transformed back into the VO in order to use it for applying it to the aggregate root state.

    protected function apply(AggregateChanged $event): void
    {
        switch (get_class($event)) {
            case NameChanged::class:
                /** @var NameChanged $event */
                $this->firstName = $event->newFirstName();

These primitives must NOT be re-validated since:

1. they already were validated and are facts that happened.
2. the validation would throw an exception and the aggregate root can not apply the event and state. The system would break.

At the bottom line it is just a matter of de-/serialization of primitives in event payload. This can be achieved by the suggestions above.

Hope this helps to nail the original issue. :)