Can this MVC code be refactored using a design pattern?

Question

I've got controller code like this all over my ASP.NET MVC 3 site:

[HttpPost]
public ActionResult Save(PostViewModel viewModel)
{
   // VM -> Domain Mapping. Definetely belongs here. Happy with this.
   var post = Mapper.Map<PostViewModel, Post>(viewModel);

   // Saving. Again, fine. Controllers job to update model.
   _postRepository.Save(post);

   // No. Noooo..caching, thread spawning, something about a user?? Why....
   Task.Factory.StartNew(() => {
       _cache.RefreshSomeCache(post);
       _cache2.RefreshSomeOtherCache(post2);
       _userRepository.GiveUserPoints(post.User);
       _someotherRepo.AuditThisHappened();
   });

   // This should be the 3rd line in this method.
   return RedirectToAction("Index");
}

Basically, i'm referring to the code in the threading block. All things need to happen, but the user doesn't need to wait for them (good case for a background thread, right?).

Just to be clear, i use caching (regular ASP.NET data cache) all over the site, and most of this has a "no expire" cache policy, so i manually evict it when required (like the above).

And the user part is basically giving user rep for doing something (like Stack).

So let's recap: we have caching, user reputation handling, auditing, all in one. Doesn't really belong in one spot does it. Hence the problem with the current code, and the problem with trying to figure out how to move it away.

The reason i want to refactor this is for a few reasons:

1. Difficult to unit test. Multithreading and unit testing doesn't really play nice.
2. Readability. It's hard to read. Messy.
3. SRP. Controller doing/knowing too much.

I solved 1) by wrapping the thread spawning code into an interface, and just mocking/faking that out.

But i would like to do some kind of pattern, where my code could look like this:

[HttpPost]
public ActionResult Save(PostViewModel viewModel)
{
   // Map.
   var post = Mapper.Map<PostViewModel, Post>(viewModel);

   // Save.
   _postRepository.Save(post);

   // Tell someone about this.
   _eventManager.RaiseEvent(post);

   // Redirect.
   return RedirectToAction("Index");
}

Basically, putting the onus on "something else" to react, not the controller.

I've heard/read about Tasks, Commands, Events, etc but have yet to see one implemented in the ASP.NET MVC space.

First thoughts would tell me to create some kind of "event manager". But then i thought, where does this go? In the domain? Well then how does it handle interactions with the cache, which is an infrastructure concern. And then threading, which is also an infrastructure concern. And what if i want to do is synchronously, instead of async? What makes that decision?

I don't want to have to just pile all this logic somewhere else. It ideally should be re factored into manageable and meaningful components, not shifted responsbility, if that makes sense.

Any advice?

Answer 1

First thoughts would tell me to create some kind of "event manager". But then i thought, where does this go? In the domain?

It's the way I solve the problem. I see the event manager as infrastructure. But the actual events belongs in the domain.

Well then how does it handle interactions with the cache, which is an infrastructure concern. And then threading, which is also an infrastructure concern. And what if i want to do is synchronously, instead of async? What makes that decision?

Async is nice, but makes transaction handling complex. If you use an IoC container you already have a well defined scope and a transaction which can be used during the event propagation.

imho it's up to the subscriber to schedule/thread it's task if it knows that it's event handling will take time.

Proposed solution:

Use your IoC container to publish the events. I would let the repository publish the events (either PostUpdated or EntityUpdated depending on what you want to do with the event) rather than the controller (to reduce code duplication).

I've made an IoC implementation for autofac which allows you to:

DomainEventDispatcher.Current.Dispatch(new EntityUpdated(post));

Subscription:

public class CacheService : IAutoSubscriberOf<EntityUpdated>
{
    public void Handle(EntityUpdated domainEvent) {};
}

https://github.com/sogeti-se/Sogeti.Pattern/wiki/Domain-events

Typical usage

1. Implement IServiceResolver (for your container)
2. Assign it: ServiceResolver.Assign(new yourResolver(yourContainer))
3. Use as described here.

Answer 2

You could use aspect oriented programming on this particular problem. Commonly used product in the .NET world is PostSharp.

The idea would be that you add an attribute above the method. The attribute will tell what particular actions should be done (in your case cache refreshing, points to increase etc.) and when it should happen (in your case when exiting the method for example).

You could also separate these to different attributes, so you could make different kind of combinations.

Answer 3

perhaps the post object should update itself, and be passed an IRepository (which itself was passed to the controller.) (this is basic dependency injection/IOC, and keeps the controller skinnier)

//in controller:
var post = Mapper.Map<PostViewModel, Post>(viewModel);
post.Update(_postRepository);

//inside Post.cs:
public Update(IRepository rep){
//update db with the repo    
//give points
}

Answer 4

There is a message bus implementation in MvcContrib, as part of the PortableArea feature. Its main purpose is to allow independently implemented features to trigger and listen for events, which sounds pretty much like what you desire.

I'm not sure if its the best option out there, as the state of MvcContrib is somewhat undocumented and sketchy. Some parts are actively maintained while others are obsolete.

Another option to consider is ZeroMQ, but it may be overkill for your needs.

Answer 5

You may want to consider implementing a service bus system such as NServiceBus if you are dealing with cache busting or auditing (that may be better served asynchronously).

With a message bus, event messages may be published to any number of event handlers (e.g. a cache handler) asynchronously, so your app can fire off a message, and then continue serving up synchronous pages quickly.