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

[webdevilopers]

When deserializing messages (e.g. Domain Events inside the same context) that hold value objects, do you still validate the given data or do you simply trust the incoming data?

Currently my objects offer a toArray() and fromArray() (see fromArray_v2) method. But the latter could theoretically be used by a developer to create "invalid state".

Especially when value objects do some custom logic e.g. price calculation it gets tricky. Maybe the calculation has changed when replaying the Domain Event. Alternatively I could go through the constructor again. But then the calculation is again executed though it may has changed.

Which version do you prefer? Or are there any additional checks or other factory method suggestions?

final class CustomPrice
{
    /** @var int $quantity */
    private $quantity;

    /** @var float $subtotal */
    private $subtotal;

    /** @var float $total */
    private $total;

    /**
     * CustomPrice constructor.
     * @param int $quantity
     * @param float $subtotal
     */
    public function __construct(int $quantity, float $subtotal)
    {
        $this->quantity = $quantity;
        $this->subtotal = $subtotal;
        $this->total = $quantity*$subtotal;
    }

    /**
     * @param array $array
     * @return CustomPrice
     */
    public static function fromArray(array $array): CustomPrice
    {
        Assert::keyExists($array, 'quantity');
        Assert::integer($array['quantity']);
        Assert::keyExists($array, 'subtotal');
        Assert::float($array['quantity']);

        return new self($array['quantity'], $array['subtotal']);
    }

    public static function fromArray_v2(array $array): CustomPrice
    {
        Assert::keyExists($array, 'quantity');
        Assert::integer($array['quantity']);
        Assert::keyExists($array, 'subtotal');
        Assert::float($array['quantity']);
        Assert::keyExists($array, 'total');
        Assert::float($array['total']);

        $self = new self();
        $self->quantity = $array['quantity'];
        $self->subtotal = $array['subtotal'];
        $self->total = $array['total'];

        return $self;
    }

    /**
     * @return array
     */
    public function toArray(): array
    {
        return [
            'quantity' => $this->quantity,
            'subtotal' => $this->subtotal,
            'total' => $this->total
        ];
    }

Original discussion started here:

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

P.S.: The value object shown here is stored inside the payload of a Domain Event. The payload uses primitives only. In order to use on a event handler method on the aggregate Root it has to be re-constructed from those primitives.

[Ocramius]

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]

Thanks for the feedback! Same here regarding private and named ctors. The example was just for demonstration.

At the bottom line do you even think about other developers of your team using the fromArray_v2 method to create invalid data? It maybe sounds a little bit paranoid though. :)

[Ocramius]

        $self = new self();         $self->quantity = $array['quantity'];         $self->subtotal = $array['subtotal'];         $self->total = $array['total'];         return $self;

This bit can be in any private method, if duplicate.

[webdevilopers]

Sorry, but could you explain what you mean? I missed the point. :/

[Ocramius]

I took a snippet from the above code: you can make it a private method if validation duplication is a concern.

[webdevilopers]

But how would you init the object then for instance in an event like this?

    public function customPrices(): CustomPrices
    {
        if (null === $this->customPrices) {
            $this->customPrices = new CustomPrices(array_map(function(array $rate) {
                return CustomPrice::fromArray($rate);
            }, $this->payload['customPrices']));
        }

        return $this->customPrices;
    }

[Ocramius]

I'd do that at initialisation, not when that data is requested. Or even better: keep it as value objects as much as possible as long as it's in memory

[webdevilopers]

Thanks for explaining!

BTW the example was taken from the older version of @prooph event store. In the current version they skipped the serialization of Domain Event value objects:

final class Basket extends AggregateRoot
{
    public static function startShoppingSession(
        ShoppingSession $shoppingSession,
        BasketId $basketId)
    {
        //Start new aggregate lifecycle by creating an "empty" instance
        $self = new self();

        //Record the very first domain event of the new aggregate
        //Note: we don't pass the value objects directly to the event but use their
        //primitive counterparts. This makes it much easier to work with the events later
        //and we don't need complex serializers when storing events.
        $self->recordThat(ShoppingSessionStarted::occur($basketId->toString(), [
            'shopping_session' => $shoppingSession->toString()
        ]));

        //Return the new aggregate
        return $self;
    }

If I'm right either way the value object stays in memory until the apply method on the AR.