Persisting Large/Complex Entities with the Command Pattern — Am I doing it right?

Question

I am in the process of designing and building a large-scale inventory management software-as-a-service that will, hopefully, live a long and fruitful life. Therefore I am exerting a lot of effort up-front to ensure the application will be maintainable and horizontally scabale. I really enjoyed this talk by Mathias Verraes on decoupling which inspired to try out the Command Pattern as it lends itself to highly decoupled and very explicit code.

In terms of technology, I'm using Laravel 5 and Doctrine 2 backed by a MySQL database.

Although Mathias speaks on the subject of "getting out of the CRUD mindset", I think we can all agree that many situations exist where the business language does in fact reflect CRUD. For example, the logistics manager says "I need to CREATE a Purchase Order which I will then SUBMIT to the Supplier". The "SUBMIT" portion lights up as a great use-case for a Command, but not so much the CREATE part.

Let me explain my position in hopes someone can agree/disagree and maybe point me in a better direction.

I feel as though the Command Pattern is not particularly well-suited for the act of handling a CREATE request for a complex object, like a Purchase Order. In my domain, the Purchase Order holds information such as:

• User-defined Order Number / Identifier (e.g., PO1234)
• Supplier for which the Order will be submitted to
• The Ship To Address
• The Bill To Address
• The Requested Shipping Method and Courier
• NET Terms
• The Monetary Currency
• A Sale Order and Customer if the order is to be Drop Shipped
• One or more Line Items

If I understand correctly, the Command Object should essentially be a relatively simple DTO that provides a contract to the outside world to interact with the application. "If you want to CREATE a new Purchase Order, populate this object with data and send it down the bus". As a developer, it is beautifully explicit to issue a CreatePurchaseOrderCommand to the application-- I like how that sounds, but when it comes to execution it just feels clumsy.

By clumsy, I mean it seems awkward to extrapolate all of the above data from the Request object and new-up a CreatePurchaseOrderCommand with no fewer than 9 arguments, some of which would be arrays, maybe a ValueObject or two (or would that introduce coupling??).

I've mocked up some code that looks like this:

EDIT I've updated the OrderController to reflect my needs of outputting a JSON encoded object after a successful CREATE in order to satisfy the needs of my client-side framework. Is this considered a good approach? I have no issue with injecting the repository into the Controller as it will almost certainly be needed for other read methods.

OrderController.php

public function __construct(ValidatingCommandBus $commandBus, OrderRepositoryInterface $repository) { .. }
public function create(CreateOrderRequest $request){
     $uuid = UUID::generate();
     $command = new CreatePurchaseOrderCommand($uuid, $request->all());

     try
     {
         $this->commandBus->execute($command);
         $order = $this->repository->findByUuid($uuid);
         return $this->response()->json([
             'success' => true,
             'data'    => $order->jsonSerialize()
         ]);
     }catch(OrderValidationException $e)
     {
         return $this->response()->json([
             'success' => false,
             'message' => $e->getMessage()
         ]);
     }
}

ValidatingCommandBus.php (decorates BaseCommandBus)

public function __construct(BaseCommandBus $baseCommandBus, IoC $container, CommandTranslator $translator) { .. }
public function execute(Command $command){
    // string manipulation to CreatePurchaseOrderValidator
    $validator = $this->translator->toValidator($command);

    // build validator class from IOC container
    // validates the command's data, might throw exception
    // does *not* validate set constraints e.g., uniqueness of order number
    // this is answering "does this look like a valid command?"
    $this->container->make($validator)->validate($command) 

    // pass off to the base command bus to execute
    // invokes CreatePurchaseOrderCommandHandler->handle($command)
    $this->baseCommandBus->execute($command)
}

CreatePurchaseOrderCommandHandler.php

public function __construct(PurchaseOrderBuilderService $builder, PurchaseOrderRepositoryInterface $repository){ .. }
public function handle(CreatePurchaseOrderCommand $command){

    // this again? i'm pulling the same data as I pulled from the
    // Request object back in the Controller, now I'm just getting
    // the same data out of the Command object. Seems repetitive...
    $order = $this->builder->build([
       $command->order_number,
       $command->supplier_id,
    ]);

    // now maybe I should handle set constraints?
    // ensure order number is unique, order is not stale... etc.
    $orderNumberIsUnique = new OrderNumberIsUniqueSpecification($this->repository);
    if ( ! $orderNumberIsUnique->isSatisfiedBy($order) ){
        throw new \ValidationException("The Order Number is not unique");
    }

    // ok now I can persist the entity...
    try
    {
        // start a transaction
        $this->repository->persist($order);
    }catch(SomeDbException $e)
    {
        // roll back transaction
        // cant return anything so i'll throw another exception?
        throw new ErrorException('Something went wrong', $e);
    }

    // no return here as that breaks the CommandBus pattern :|
}

From a code perspective I think that seems like the logical way to go about things in terms of placement of validation and such. But, at the end of the day, it doesn't feel like the right solution.

I also don't like that I can't return a value from the Command Bus which leaves me with the choice of switching to generating a UUID before persisting (currently relying on AUTO INC from MySQL) or performing a lookup on the other unique identifier (the order number) which may not work for all entities (i.e., not every entity/aggregate will have some other form of unique identifier besides the database ID).

Am I using and understanding the Command Pattern correctly? Are my concerns valid (see comments in code examples). Am I missing something about the Command Pattern? Any input would be great!!

Answer 1

Keep your DTO's simple

You are right that having a command with 9 arguments in the constructor is ugly. But do you really HAVE to put those things into the constructor? Make your fields public, create your command with an empty constructor and just assign to the fields. The point of constructor arguments is to have guaranteed valid entities, since you are just using a DTO and validation will be performed by another entity there is not much point in hiding data or having complicated constructors. Public fields all the way for commands.

Nothing wrong with value-DTO's

If you have a Command that is composed of a set of fields that belong together, feel free to map these into a value object that belongs to your DTO. There is no hard rule that says you shouldn't do this.

Commands do not return anything

A command is a change to objects in your domain. You should have no need to return anything from your commandbus. Why would you need to return the id? You created the order, as requested by the command, and that's it. There is one exception you might consider however and that is...

Validation

Simple validation such as not null/not empty, valid dates, regex-for-email all belongs in a validator. The call to this validator is chained before the call to your commandhandler to the handler is guaranteed to be receiving a valid command.

For complex validation, there are different opinions you can look into. Some people say that even complex validation that requires you do touch the database ('username already taken') should not belong on the validator, some prefer to have all this validation on the validator (which requires repositories) and perform no validation on the command whatsoever. In the end, you have to decide on a strategy to handle failures to validate commands.

Depending on your preference, this might lead you to ValidationResults being returned from your commandbus, trapping and displaying CommandValidationExceptions when you call the commandbus...

Answer 2

In my app, the logic flow goes

• Controller builds DTO
• Controller dispatches to Command Bus
• Command Bus Calls Events
• Event Handlers dispatch to the Command Bus as well
• Command Handler returns a response DTO to the controller
• Controller generates a response to the client based on the response

Your application seems similar. Decoupling is also an issue for me. However, the problem of chaining (or nesting) commands or events hasn't been an issue. Keep a list of generated UUIDs and associated objects in every command chain, if possible.