Friday, September 24, 2010

Separation of Concerns with Domain Events

Take a look at this nasty code:

[HttpPost, UnitOfWork]
public ActionResult Confirm(Order order)
{
    order.OrderStatus = OrderStatus.Created;

    if(order.ContactMe)
    {
        var mailingListSubscription = new MailingListSubscription
        {
            Contact = order.PostalContact,
            Email = order.Email,
            DateSubscribed = DateTime.Now
        };

        mailingListRepository.SaveOrUpdate(mailingListSubscription);
    }

    EmailOrder(order);
    basketService.CreateNewBasketFor(userService.CurrentUser);
    
    return this.RedirectToAction<OrderController>(c => c.Item(order.Id));
}

This is a controller action from Suteki Shop that is fired when the user confirms an order. There is no separation of concerns (SOC) here. In a single action method we change the status of the order, check to see if the user should be added to our mailing list, email the order confirmation, and finally create a new basket for the user. What we should really be doing here is simply asking the order to update its status:

[HttpPost, UnitOfWork]
public ActionResult Confirm(Order order)
{
    userService.CurrentUser.EnsureCanView(order);
    order.Confirm();

    return this.RedirectToAction<OrderController>(c => c.Item(order.Id));
}

This is much nicer. But the actions that occur as a result of the order confirmation still need to happen. We have a dilemma now, good design says we shouldn’t inject services into our domain entities (Order in this case), but in order to update the mailing list, send an email and create a new basket, we have no choice but to invoke those domain services. Even if we could access domain services from our entities, it would be another SOC violation to do it. Why should the order confirm method have to know about all the things that need to happen when an order is confirmed?

A really nice answer to this kind of issue is Udi Dahan’s Domain Event pattern. This pattern allows domain entities to raise events without having to be aware of what the events will trigger. So the Order.Confirm() method looks like this:

public virtual void Confirm()
{
    OrderStatus = OrderStatus.Created;
    DomainEvent.Raise(new OrderConfirmed(this));
}

It simply alters its state as required, in this case setting its status to Created and then raises an event to tell the world what’s happened.

Events can be handled by zero or more handlers. Handlers implement a simple interface:

public interface IHandle<TEvent> where TEvent : class, IDomainEvent
{
    void Handle(TEvent @event);
}

Here’s the AddToMailingListOnOrderConfirmed handler that adds the user to our mailing list:

public class AddToMailingListOnOrderConfirmed : IHandle<OrderConfirmed>
{
    readonly IRepository<MailingListSubscription> mailingListRepository;

    public AddToMailingListOnOrderConfirmed(IRepository<MailingListSubscription> mailingListRepository)
    {
        this.mailingListRepository = mailingListRepository;
    }

    public void Handle(OrderConfirmed orderConfirmed)
    {
        if (orderConfirmed == null)
        {
            throw new ArgumentNullException("orderConfirmed");
        }

        var order = orderConfirmed.Order;
        if (!order.ContactMe) return;

        var mailingListSubscription = new MailingListSubscription
        {
            Contact = order.PostalContact,
            Email = order.Email,
            DateSubscribed = DateTime.Now
        };

        mailingListRepository.SaveOrUpdate(mailingListSubscription);
    }
}

We simply implement the IHandle interface for a specific event, in this case OrderConfirmed. Because the handler is supplied by our IoC container, we can specify any services we require for the handler to do its job, in this case we require an IRepository<MailingListSubscription> in order for the handler to do its job.

There are similar handlers to send the confirmation email and create the new basket.

I’m not going to repeat Udi’s whole post here. Go and read it to see how the DomainEvent class is implemented. However I will repeat a crucial point: this is not about asynchronous messaging systems like MSMQ. The event pattern is simply a way to structure our software. When the order.Confirm() method is called, all the event handlers will fire synchronously before it returns.

Now there’s nothing to stop you from implementing an event handler that publishes an event onto an NServiceBus or MassTransit messaging system. Indeed, implementing domain events gives you the perfect hook for scaling your application as required by doing just that at a later date.

I really like this pattern, but there tend to be two concerns that people have when they encounter it:

  1. Static methods suck! How can I test a static method buried in my domain entity? Yes, it’s not good OO practice to use static methods, but I think for certain infrastructure concerns, it’s OK. Here we have a single static method that can allow us to do some very nice refactoring, so I believe it’s a good payoff. Testability is not really an issue if you implement the DomainEvent class correctly.
  2. It’s not clear what exactly is going to happen when I call order.Confirm(). Yes there is a level of indirection here, but that is a good thing. You can see that a domain event is being raised, and it’s pretty easy to find all the handlers, so I'd argue that it’s not really a problem.

If you want to see more detail, you can browse my commit for this change in the Suteki Shop source code: http://code.google.com/p/sutekishop/source/detail?r=415

25 comments:

Ken Egozi said...

It still is not very easy to test understand this kind of code. Looking at a method that shows the orchestration of "things happening when confirming an event" is actually nice.

Plus you have a problem with controlling the order of the events execution.

and you need to somehow decide which handlers are crucial (fail should revert the transaction) and which are not (fail silently).

I do like the approach, but these questions has to be met when using it

Ken Egozi said...

and btw I do not like the commenting system of blogger not one bit

Mike Hadlow said...

Thanks Ken, very good points.

In my case, there's no need for the handlers to execute in any particular order, and if one fails they all fail, including the order.Confirm() call itself, since they all execute in the context of the action's transaction. But of course different use cases might have different requirements.

And yes, I don't think much of the blogger commenting system either :(

Josh Robb said...

Ken - I like your concerns - these are important things to think about. For OrderConfirmation I would definitely want anything which was specifically related to order confirmation included in one place. (e.g. checking credit availability, stock etc).

I think using events allows you to decouple pieces of "work" which are not the concern of the "order confirmation process". e,g, Adding a user to a mailing list.

This should actually allow you to see much more clearly the inherent process of order confirmation.

I think Mike's gone a little over board with his implementation.

I'd suggest that CreateNewBasketOnOrderConfirm and EmailOrderConfirmationOnOrderConfirmed should probably not be event handlers but should stay as single method calls in the Action.

Moving AddToMailingListOnOrderConfirmed to a handler is a win because this is not related to Order confirmation in any way. It's a little business rule and is something which is likely to change. (Client: "Can we add everyone who creates a basket to the Mailing list now?")

As with everything moderation.

Unknown said...

I'll be looking at this shortly. I think there could be scope for using Rx.

Unknown said...

We only use events to post a change to a different aggregate. For example addOrderItem would update the order item and fire an event, which our stock system would subscribe to update the core stocking system. Which then fires another event, which we can hook onto for any other crazy processes the business needs.

Also to get around the static methods simply inject the bus interface into your domain, then use your favourite IOC platform to inject the bus and ensure its a singleton.

Anonymous said...

"good design says we shouldn’t inject services into our domain entities"

where does good design say this???

Mike Hadlow said...

Hi Anonymous,

Injecting services into your domain entities has a number of difficulties. First is the question of where they get injected.

Do you get your ORM to do dependency injection? What happens when you have a detached entity?

Or do you inject them as arguments to state changing methods on your entity. So my order.Confirm() method would change to order.Confirm(basketService, mailingListService, emailService), which seems pretty awful.

The other difficulty is the issue of responsibility. Should my Order entity really have responsibility for orchestrating everything that happens when an order is confirmed? I'm especially uncomfortable with the idea that an entity is responsible for persisting another entity, which would have to be the case in my example.

Keeping entities free of non-domain dependencies makes them much simpler and easier to test. The nastiness of having a 'DomainEvent.Raise' static method is the price I'm willing to pay for this.

Karsten said...

Maybe I don't understand the problem fully, but why not just create the AddToMailingListOnOrderConfirmed-class and call it like

[HttpPost, UnitOfWork]
public ActionResult Confirm(Order order)
{
userService.CurrentUser.EnsureCanView(order);
order.Confirm();
this.addToMailingListOnOrderConfirmed.Handle(order); // injected into 'this'

return this.RedirectToAction(c => c.Item(order.Id));
}

Mike Hadlow said...

Hi Karsten,
Your solution is pretty much what I started with, except that now you've got a class that knows how to orchestrate all the follow on actions rather than explicitly executing them in the action method. Note that there are 3 handlers in all, I only pasted in the code for the first one as an example. The point of this refactoring is to decouple the act of moving the order on to 'Confirm' from the actions that need to happen as a consequence.

GeeBee said...

@ken
>>you have a problem with controlling the order of the events execution.

Do you mean controling the order in which the event handlers are executed? If so you could argue that EventHandler2 does not infact repond to the same Event as EventHandler1, but rather handles a second Event fired off from whatever is being executed in EventHandler1

Karsten said...

Hi Mike

Thanks for your answer, it helped a bit, but brings up another question. It seems that you are at least two steps ahead of me. Can you explain why is it bad that the class knows how to do the orchestration? I would argue, that when the implementation is put in separated classes, it's good SoC?

Anonymous said...

Thanks for the reply Mike.

Really I was suggesting that that "not using DI for entities" is a still a matter of opinion, and not necessarily "good design".

It seems there is still some debate to be had on this topic. If anyone is interested in this stuff, there is a good debate on this here:

http://www.lostechies.com/blogs/derickbailey/archive/2010/09/22/domain-models-dependencies-and-fighting-anemia.aspx

and someone concluding that DI on entities works pretty well for them here:

http://danhaywood.com/2010/04/30/accessing-domain-services-from-entities/

Marco Paul said...

One thing to keep in mind with the implementation of the DomainEvent class is the use of ThreadStatic, which does not always work as expected in a web environment. I believe Udi's implementation was under the assumption the domain model was running in its own process on an app server for example.

Mike Hadlow said...

Karsten, Anonymous,
Another way of thinking about SOC is to ask the question, 'would I need to change any of the existing code in order to add some action to order confirmation?' By using Domain events I can add to, or change, those actions without changing a single line of existing code. That's SOC :)

Unknown said...

I really enjoyed the post. Thank you for including the link to Udi's blog. This is exactly what I have been looking for. The beauty of this solution is its simplicity. I have tried to implement event aggregators using Microsoft and my own home grown solutions. Each time I found that these solutions added noise and unnecessary complexity to the domain. This is a SOLID solution keep up the good work. Thanks...

Anonymous said...

Mike,

I am little concerned about the order of events here.

How do you orchestrate the events if you want to send the email after saving the order?


Mike Hadlow said...

Hi Periop IT,

The simple answer is 'you don't'. This implementation says nothing about the order that the handlers get fired. If one handler is dependent on another handler, then, as GeeBee says, it should subscribe to an event published by the handler it depends on, not the original event.

As for the order in which things are saved, we really shouldn't care about persistence per-se. This all happens as a unit in a single transaction.

Anonymous said...

But if you already sent a mail, how can you rollback the sent mail if the save order call fails?

I am writing the series of actions to explain the confusion

a. you are setting a property value of Order

b. And that property fires domain event(s)

c. one of the domain event sends an email (before order persisted?)

d. controller action method persists the order. What if save operation fails here?

Mike Hadlow said...

Hi Anonymous, yes you are right, in this example the email sending isn't enrolled in the transaction, so that if the save fails, the email would still have been sent.

GeeBee said...

@Anonymous,

Three ways around this spring to mind - although Im not sure how I feel about them!

1. The property fires the event but an email handler is NOT subscribed to this event - when the repository perists the Order it publishes an event which the Email Handler subscribes to instead and send the email.

2. The email handler subscribes to the Order's property event and calls "send" on an email service. This email service however does not actually send the email, it just "queues" it. Assuming you are doing a web app the email service could then be flushed in the request_end handler in global.aspx.

3. You go the full SOA approach - when the Order propery event is handled by the Email Handler this infact just registers a message with a servicebus which will send the email at somepoint in the future

Mike Hadlow said...

Thanks GeeBee,

Sending an email during a synchronous web request is not a great idea in any case. As you said, it would be far better to queue the email processing using some kind of ServiceBus.

A simple implementation would be to write a database record for each email and then have some background process that polled the table and sent emails. This has the added advantage that the email record insertion would be part of the MVC action's transaction and would also rollback if the order update failed.

The "Mar" said...

Mike,

I will appreciate you can advise me on a SO question I posted today.
http://stackoverflow.com/questions/3925184/which-layer-for-domain-events-eventhandlers-dispatcher

Regards,

Mar

Daniel González said...

Regarding your concerns about "Here we have a single static method" inside your entity, what I have done in the past if have a good-old .Net event fired, which will be "promoted" to a domain event by the one listeing to that event.
It has worked very well for me and it is easily testable.

Mike Hadlow said...

Hi Daniel,

The question then becomes: where do you wire up your events?