webdevilopers / php-ddd

PHP Symfony Doctrine Domain-driven Design
200 stars 11 forks source link

When do I create the UUID and how? #33

Closed ScreamingDev closed 7 years ago

ScreamingDev commented 7 years ago

I see everyone everywhere writing about DDD, CQRS, ES but no concrete examples especially when it comes to Symfony or Doctrine. So I am very puzzled about this becuase my requirements are very low:

class Customer {
    protected $customerId; // CustomerId Value Object - uuid5 using the username
    protected $username;
}

class Wallet {
    protected $uuid; // uuid5 with customer uuid as namespace and something random.
    protected $balance;
    protected $customer;
    // __construct($customer, $balance = 0)
}

What made me some headaches yesterday was:

When do I generate the UUID?

This issue might come down to some "best practice". But there are really some points I see with advantages and disadvantages.

I could do it while creating a new object in __construct (DDD-Layer). But in the app-layer I need some setter for doctrine when it loads the entity from the database. I surely don't want a setter for UUIDs - they shall never change.

This could also be done via a factory. Compatible with Doctrine when using a prePersist hook / generate uuid on the fly. But static methods are just bad solutions.

Speaking about on-the-fly I could also do it in the getter getUuid which checks if a uuid is already present and if not it generates a new one.

webdevilopers commented 7 years ago

Welcome @sourcerer-mike !

If you are using Commands and Handlers then you have no return values to redirect a User to a Details view for instance after an AR was created. In that and mostly CQRS related cases the Client should generate a UUID. For instance when populating the Command from Request parameters inside a Controller:

It is indeed a good practice to have an UUID before working on the Aggregate Root.

Some examples also have a nextIdentity() method on the Repository implementation that returns the UUID.

In the end you pass it to the AR:

If you have no way to generate an UUID before then you can still create it inside your AR constructor.

Just don't mix the creation with Infrastructure concerns like Doctrine.

ScreamingDev commented 7 years ago

Thank you very much for your reply! I really have some more issues with that but those are up to the app layer now ;) The more I do DDD the less the world seems be made for it :P

https://github.com/hautelook/AliceBundle/issues/331 (DDD and persistence layer must sync - douh) https://github.com/doctrine/doctrine2/issues/6360 (Embeded property reflection fails) https://github.com/nelmio/alice/issues/709 (Random selector goes wild)

Wonder what breaks next ;)

webdevilopers commented 7 years ago

Actually looking at those issues I see nothing that is broken by DDD itself. Mostly Doctrine problems. Never had any after decoupling my Domain Model from Persistence.

The more DDD I practice the more I see how most of the world fits!

Keep trying!

webdevilopers commented 5 years ago

BTW: If for some reason you cannot use a UUID and you have to rely on an e.g. SQL auto_increment value then you can still implement (an ORM) approach w/ DDD.

The Value Object could look something like this:

/**
 * Class BayWindowCalculationLegacyId
 * @deprecated Remove when using UUIDs only.
 * Represents ORM auto-increment integer.
 */
class BayWindowCalculationLegacyId
{
    /** @var int $id */
    private $id;

    /**
     * BayWindowCalculationLegacyId constructor.
     * @param int $id
     */
    private function __construct(int $id)
    {
        $this->id = $id;
    }

    /**
     * @param int $bayWindowCalculationId
     * @return BayWindowCalculationLegacyId
     */
    public static function fromInteger(int $bayWindowCalculationId)
    {
        return new self($bayWindowCalculationId);
    }

    /**
     * @return int
     */
    public function toInteger(): int
    {
        return $this->id;
    }
}

Normally - mostly when using CQRS and UUID - you want to pass this ID to your entity constructor. This is hard with an auto_increment but it is possible - but not reliable! Simply add a nextIdentity method to your repository that somehow "guesses" the next ID e.g.:

SELECT id+1 FROM `table` ORDER BY id DESC LIMIT 1

As mentioned: NOT RELIABLE!

Came from:

db306 commented 5 years ago

Hi @webdevilopers,

I think there is a better way, on the DoctrineEventSubscriber, if the Entity id = null, then update the id with the new generated id before dispatching the event.

webdevilopers commented 5 years ago

Welcome @db306 ! Could you please post your Entity so we can see the way you construct it?

I can only relate to the example by @JeffreyVerreckt:

I suggest the following changes:

<?php
namespace App\Domain\Model;

class Product
{
    /**
     * @var ProductId
     */
    private $id;

    /**
     * @var ProductName
     */
    private $name;

    private function __construct(ProductId $id, ProductName $name)
    {
        Assert::notEmpty($name);

        $this->id = $id; // ->toInteger() if you cannot use custom Doctrine types for value objects
        $this->name = $name;
    }
    public static function addToStock(ProductId $anId, ProductName $aName): Product
    {
        $self = new self($anId, $aName);

        return $self;
    }

    public function id(): ProductId()
    {
        return $this->id;
    }

No setters - no no no!! ;)

The event of course will look similar and use value objects instead of primitive types.

Then in your application service or handler:

$productId = $this->repository->nextIdentity();
$product = Product::addToStock($productId, $command->name());
$this->repository->add($product);

I hope this makes it more clear?

db306 commented 5 years ago

Hi ! thank you :)

I really don't like the nextIdentity function, perhaps getting the Id after being flushed and add it to the event just before it being dispatched ?

Perhaps I can do that by making the event implement an interface that forces the event to have a setId/getId function. I think i might be onto something

webdevilopers commented 5 years ago

I absolutely agree with you. I just wanted to demonstrate a technical workaround.

What I really wanted to point out is: From a DDD POV it is a bad practice to pass an Entity (or even "Aggregate Root" in this case) in general. They should always be referenced by a "Value Object" ID.

And the "nextIdentity" method itself is not a bad idea. You should add it to your repository interface and define the return type. And of course: I would always choose UUID. :)

Only if your application needs to pre-define the ID e.g. in your UI then you probably will not need the method anyway.

Original discussion can be found here:

Thanks for sharing your thoughts.

webdevilopers commented 5 years ago

P.S.: We migrated from auto_increment to UUID easily.

We just added an extra column to our table e.g. "id_uuid" and then used some migration files to keep them in sync. We created the "(Product)Id" value object. And we added an id() accessor for it and renamed the getId() to getLegacyId().

Though it was deprecated we still created an "(Product)LegacyId" value object for it to implement DDD already.

After the changes were applied to the application we simply removed the auto_increment field and the getLegacyId() method.

Though it was a huge monolith it worked out well and did not take than long.

It was more than worth it!