Async Unit of Work in ASP.Net MVC

[Khalid Abuhakmeh]

Does anybody have any code that shows how to do the Unit of Work pattern correctly with an ASP.Net MVC application. The OnActionExecuting and OnActionExecuted don't allow for using the await keyword so I'm not sure the best way to approach this. I could use ContinueWith but not sure about that. Any advice would be greatly appreciated.

Thanks,

Khalid

[Khalid Abuhakmeh]

I have something like this so far, but it feels wrong.

public class HomeController : Controller

{

    public IAsyncDocumentSession Db { get; set; }

    public async Task<ActionResult> Index()

    {

        var person = new Person {Name = "Khalid Abuhakmeh"};

        await Db.StoreAsync(person);

        return View(person);

    }

    protected override void OnActionExecuting(ActionExecutingContext filterContext)

    {

        Db = MvcApplication.DocumentStore.OpenAsyncSession();

        base.OnActionExecuting(filterContext);

    }

    protected override void OnActionExecuted(ActionExecutedContext filterContext)

    {

        Db.SaveChangesAsync()

            .ContinueWith(x => { });

        base.OnActionExecuted(filterContext);

    }

    public class Person

    {

        public string Id { get; set; } 

        public string Name { get; set; }

    }

}

[Mircea Chirea]

The ContinueWith is useless there. You could use Wait, but if you're unlucky you'll deadlock.

Best way is to call SaveChangesAsync in your actions. That's what I do; it makes it explicit and it's obvious when you forget it.

[Chris Marisic]

And this would be one of the many reasons I still use the repository pattern. A controller has absolutely dead zero reason to have any knowledge of a unit of work.

On Monday, July 22, 2013 10:49:13 AM UTC-4, Khalid Abuhakmeh wrote:

[Mircea Chirea]

The repository pattern wouldn't solve the issue with async, either. Unless you make your equivalent of "Store" call "SaveChangesAsync", but then you're not exactly helping.

[Khalid Abuhakmeh]

yeah Chris the repository pattern has nothing to do with this particular implementation. You would have to either call the SaveChanges in your repository as Mircea suggested or figure out a way to put the unit of work pattern around the async part (which is till the issue).

@Mircea yeah the ContinueWith just starts execution of the task, without blocking. Wait would block and I wouldn't want that. Feels like something needs updating in ASP.Net MVC. The approach of calling SaveChangesAsync in the action would work, but seems as though would be repetitive.

[Oren Eini (Ayende Rahien)]

You can't really do that with the MVC API, but the WebAPI has a nice model for that, and it is what I would recommend.

--
You received this message because you are subscribed to the Google Groups "ravendb" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ravendb+u...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

[Khalid Abuhakmeh]

Yeah WebAPI was definitely designed with async in mind, but I develop mostly using ASP.NET MVC because it is a much more mature eco-system. It would make sense if I was doing SPA applications that were REST heavy, but I still like what ASP.NET MVC has to offer over what WebAPI does.

Also there is something strange about WebAPI that I still can't put my finger on. It looks like an MVC framework, but something smells. I can't tell you what, but it just doesn't feel right.

I guess @Mircea suggestion is probably the best suggestion.

Solution : "If you do async with ASP.Net MVC and want to use RavenDB, then you can't use the Unit of Work pattern since MVC doesn't allow you to."

What a bummer :(

[Mircea Chirea]

Personally I like making SaveChanges explicit. I put the call right after Store, so in most cases it's VERY obvious what's happening. I believe this isn't a bad thing to repeat all over the place, really.

With MVC and async you don't have any other alternative; and you can't open another session either, for change tracking. You could do something weird to run the task in a dedicated thread, but I think it'd be too complicated and probably would break in a million other ways.

[Oren Eini (Ayende Rahien)]

Note that the code that runs the action already handles async. find out where that is, then plug into that.

[Kijana Woodard]

I also call SaveChanges explicitly. I find that it makes things easier to understand and avoids some "oops scenarios" (like SaveChanges on GET when not intended).

However, there are some scenarios where the UoW is superior especially when working with decoupled code where no one piece is certain that it's time to call SaveChanges.

[Khalid Abuhakmeh]

Yeah agree with both of you, I just want to leverage the framework and not jump through hoops to do it. Sometimes I might just be trying to push a square peg through a round hole.

@Mircea by any chance could you paste an action that you've done this with? I'm just curious what it looks like in actual code and not pseudo code. Also when do you open the session and close the session? Is that also in the action?

[Mircea Chirea]

Well I guess ASP.NET MVC isn't really catered for those situations. It's quite an old design, relying on ASP.NET has some major issues for many apps. There not much your or Microsoft can do about it; that's why Web API exists. ServiceStack is still left in 3.5, so it doesn't support async; not sure about Nancy.

[Kijana Woodard]

Nancy has async in beta iirc. The community sort of pushed them into doing it.

So far, my forays into Async haven't really shown much benefit. There are a lot of hurdles and gotcahs and, at the end of the day, not much noticeable difference under "normal" loads. 

I was working on a Background Task processor, so it was a console app. It turned out that Parallel.For did a great job dealing with the i/o wait time.

Are you guys seeing enough benefit to justify the usage?

[Khalid Abuhakmeh]

Yeah I wouldn't mind seeing a breaking change to ASP.NET to support a more async mind set. There would be a lot of crying, but ultimately it would move it into the 21st century with the other frameworks like nodeJS.

[Khalid Abuhakmeh]

@Kijana that's interesting you say that about Parallel.For because I noticed the same thing with my services. It yielded much better performance because it makes the decision on whether it should do it on a separate thread or not. More often than not it throttles the amount of threads, where if you spin up Tasks then you incur that overhead.

I guess my point of view is this: If there are benefits to doing things in async and I can keep my code basically the same then why not use it? Async is the future of .NET and might as well learn how to drive this band wagon :).

I like ASP.Net MVC a lot, I think it is a great framework but I wish they would break things more to move the framework forward. Hoping that with ASP.Net MVC being on NuGet that it will be more the case that drastic changes happen sooner. I'd like to see a major ASP.Net MVC release once a year or at least every two years.

[Chris Marisic]

I would have a higher level abstraction than just the repository for this, a class that would manage the concurrent usages of the repertories and then commit/rollback the UOW as deemed necessary, and then return back to the controller.

[Khalid Abuhakmeh]

That's what my code needs, another abstraction. :)

http://ayende.com/blog/154209/limit-your-abstractions-refactoring-toward-reduced-abstractions

[Oren Eini (Ayende Rahien)]

That is infrastructure, you don't see that.

--

[Khalid Abuhakmeh]

what wouldn't I see? I'm lost.

[Oren Eini (Ayende Rahien)]

Shoving that to the infrastructure means that you deal with that once, you don't have to see it.

[Khalid Abuhakmeh]

Yeah I agree with you I try to keep my abstractions in the infrastructure, but I don't know if Chris' approach of yet another layer of infrastructure would solve the async issue, because ultimately the infrastructure it sits on top is ASP.NET MVC. I guess I need to see the code to be convinced, and I don't expect anyone to do that (we are all busy). Right now it is just postulation and theories about what "might" work.

This is about as abstract as I like to get since the MVC Pipeline can be dizzying if you get lost in it. The only issue with the code below is that failures would happen on a separate thread and could not be relayed to the user. I could also choose to run SaveChanges synchronously, but then whats the point?

 protected override void OnActionExecuted(ActionExecutedContext filterContext)

    {

        Db.SaveChangesAsync()

            .ContinueWith(x => { });

        base.OnActionExecuted(filterContext);

    }

Like @Mircea said, maybe the simplicity is the best approach here. Contain all the code in the action method; it might mean more code overall, but it would be much easier to keep track of and debug.

It would be nice to figure out a standard way to use RavenDB / Async in MVC to give other developers guidance as to how to approach the problem. Your post about the unit of work approach for synchronous use of RavenDB is really a great post.

[Chris Marisic]

My approach would be

Controller
HTTP GET {               return concurrentWorkManager.DoStuff()  }

ConcurrentWorkManager
DoStuff {

Parallel
Parallel
Parallel

Do i like things? SaveChanges()

return results

[Khalid Abuhakmeh]

not sure what programming language that is?

[Mircea Chirea]

So you just moved all the code from one function to another.

[Khalid Abuhakmeh]

My point exactly, you are still stuck with the initial problem.

[Mircea Chirea]

Khalid, just call SaveChangesAsync in the action after you are done with writing to the database. Sometimes copy-pasting is good; this is one of those cases. This way you'll know at a glance whether and when your changes are persisted to the database. I mean:

public async Task<HttpStatusCodeResult> Confirm(ConfirmOrderModel model)

{

    if (ModelState.IsValid)

    {

        var order = await Raven.LoadAsync<Order>(model.Id);

        if (order == null)

            return HttpNotFound();

        if (order.Status == Order.StatusType.Accepted)

            return Ok();

        order.Status           = Order.StatusType.Accepted;

        order.AcceptedOn       = DateTime.UtcNow;

        order.DeliveryDuration = model.DeliveryDuration;

        await Raven.SaveChangesAsync();

        await Tasks.PushAsync(new OrderStatusChangeMessage

        {

            Order = order.Id

        });

        return Ok();

    }

    else

    {

        return BadRequest();

    }

}

If I were to call SaveChanges automatically in OnActionExecuted I'd have to peek into the request here, to see if the status code is 2XX, or worse, modify my code to throw exceptions (which would make me very unhappy during debugging, with VS breaking on those exceptions, which I personally hate).

In this case there'd be a nice race condition: what if SaveChangesAsync throws but PushAsync doesn't? I'd still notify the customer of the order status change, when in fact it wasn't. The above code might not notify the customer, but it always ensures that the status change has been persisted before doing so.

[Khalid Abuhakmeh]

That is pretty clean looking code. I think you won me over. By the way, why are you returning HttpStatusCodeResult, are you utilizing ASP.NET MVC as a REST back end?

[Chris Marisic]

I don't agree, this is not merely moved, this is actually taking infrastructure out. SaveChanges is not an infrastructure concern, it is a core business concern. It shouldn't just happen in OnActionExecuted this is pretty much a sure fire way to guarnetee at some point your system will be making unintended changes your data. Load(foo) modify foo, do logic, exception happens, infrastructure commits ragged foo anyway.

[Mircea Chirea]

That is an AJAX action, so yes. I stuffed all my AJAX methods into an MVC Area; with AttributeRouting it's painless to set up.

[Chris Marisic]

Atleast this isn't infrastructural code, but...

On Tuesday, July 23, 2013 1:30:20 PM UTC-4, Khalid Abuhakmeh wrote:

That is pretty clean looking code. I think you won me over. By the way, why are you returning HttpStatusCodeResult, are you utilizing ASP.NET MVC as a REST back end?

On Tuesday, July 23, 2013 1:15:13 PM UTC-4, Mircea Chirea wrote:

Khalid, just call SaveChangesAsync in the action after you are done with writing to the database. Sometimes copy-pasting is good; this is one of those cases. This way you'll know at a glance whether and when your changes are persisted to the database. I mean:

public async Task<HttpStatusCodeResult> Confirm(ConfirmOrderModel model)

{

    if (ModelState.IsValid)

    {

        var order = await Raven.LoadAsync<Order>(model.Id);

        if (order == null)

            return HttpNotFound();

        if (order.Status == Order.StatusType.Accepted)

            return Ok();

        order.Status           = Order.StatusType.Accepted;

        order.AcceptedOn       = DateTime.UtcNow;

        order.DeliveryDuration = model.DeliveryDuration;

        await Raven.SaveChangesAsync();

        await Tasks.PushAsync(new OrderStatusChangeMessage

        {

            Order = order.Id

        });

        return Ok();

    }

    else

    {

        return BadRequest();

    }

}

All that in red is pure business logic, business logic does not belong in a controller. Controllers are to be route drivers, not business logic executors. 

The yellow hightlighted code is the fine line of acceptable logic in a controller this gives you your model that you can construct view responses to the client whether they're 200, 404, etc.

[Khalid Abuhakmeh]

@Mircea AttributeRouting, oh well it WAS good... just kidding. I maintain Restful Routing, and both frameworks are better than what you get out of the box. Thanks for pasting your code, it really helps see a concrete example. Much better than my approach of return a JSON object with ok = true or false. I think I'll steal that from you :).

I started migrating all my DateTime properties to DateTimeOffset. Having the offset makes a world of difference.

[Mircea Chirea]

Yeah well, sometimes you have to stop with all the architecture and get work done. Believe me, I have MUCH worse examples. I don't have a good place to stick all business logic into; creating a model with all the functionality will not really work as in some places I need the result of about three queries so it won't be easy to construct; besides I'm trying to get as close to the database as possible, I did make the mistake of abstracting everything to the point where you couldn't see where the work was done.

Here you go: https://privatepaste.com/6e769db1f7

Feel free to pick on my code; I'll appreciate any feedback.

[Mircea Chirea]

Context: this is my first real software project that went into production. I worked on the server-side parts alone. I'm starting the second year of college in October.

[Khalid Abuhakmeh]

The code looks pretty solid, the only criticism I would have is that your controller is doing too much in one controller (style/preference issue). I like to separate my controllers based on conceptual resources. Looking at your code I would have a controller for the following:

Restaurants

Orders

Suspensions

Restful Routing is opinionated in this way, and it helps keep your actions small while making it easy to find code that your wrote. Again, just personal preference, but I found it makes adding and removing code easier.

"Yeah well, sometimes you have to stop with all the architecture and get work done", I learned this after having to program on my own dime. DRY and YAGNI aren't a license to go nuts with layers and abstraction. Like Oren said, limit your abstractions and KISS. 

You have a bright future ahead of you, and you'll be one to watch, but I'm not here to inflate your ego :)

[Oren Eini (Ayende Rahien)]

Here is what you want to do:

http://ayende.com/blog/163170/building-async-unit-of-work-with-mvc-4?key=6e88c4b8426947b2bbca7c8ff0f6430a

--

[Mircea Chirea]

Right, well the thing is the page for the restaurant is the same accepting the order. If I were to move the POST action to another controller I'd need to find a way to keep the URL if an error is catched server side (that is important for me). Not pretty this way, but I had to do it and didn't have much time available back then. Suspensions are going to be split and the rate-limiting part will go into Redis. I am working on an API and will eventually rewrite the web app to use the API directly, having web and mobile apps using the same code, but for now this'll have to do.

That said, since this is working production, money-making code, I'm VERY careful about touching it. It's the most critical part of all the app :)

[Kijana Woodard]

SRP > DRY :-]

--

[Felipe Leusin]

Kinda offtopic but I might chime in, I`ve used an approach on NancyFX (the async branch) that has been a joy to work with. We made a RESTish API and were really careful with the HTTP status code we used, so we created a convention (implemented as a After handler):

- If the method is a POST/PUT/DELETE and the status code is in the 200 range we call SaveChanges. Everything else we just ignore... All the logging (errors and what not) gets pushed to something akin of the TaskExecutor we see in Ayende`s code.

We`ve been very pleased with this approach but I always wondered if we missed anything.

[Khalid Abuhakmeh]

For sure, if it is making money then don't touch it. Let me say that again to others reading. If it works and is making money fight every impulse you have to "fix" it.

I am finding out the same issue with urls on the client. It would be really cool if if I could generate a RouteTable similar to the one used by ASP.NET MVC / WebAPI and spit out a RouteResolver that you can use in your JavaScript. that way you can do things like.

url.action("action", "controller", { id, param1, param2 })

This is probably a leaky abstraction, so not sure if it would work out. Right now I send a url with every model so it can figure out where to post and put to. Following the resource based approach makes it so that url is the same with the verb changing.

[Mircea Chirea]

That is probably what I'm going to do on the API (I am using Nancy for it at well), but will still call SaveChanges manually, though will check the status code and do the appropriate logging.

[Mircea Chirea]

Khalid, you owe me a crate of cold beers: http://codepaste.net/wbppyt

[Khalid Abuhakmeh]

How do you use this thing? What would a controller that implements this look?

[Mircea Chirea]

An example would be this:

using System;

using System.Collections.Generic;

using System.Linq;

using System.Text;

using System.Threading.Tasks;

using EuMananc.Data.Raven.Documents;

using Raven.Client;

namespace EuMananc.Web.Mvc

{

    public abstract partial class Controller : ControllerBase

    {

        /*\ *** *** *** *** *** Properties *** *** *** *** *** \*/

        new public Account User

        {

            get;

            private set;

        }

        /*\ *** *** *** *** *** Database Properties *** *** *** *** *** \*/

        protected IAsyncDocumentSession Raven

        {

            get;

            private set;

        }

        /*\ *** *** *** *** *** Constructor *** *** *** *** *** \*/

        protected Controller()

        {

            Raven = Application.Raven.OpenAsyncSession();

        }

        /*\ *** *** *** *** *** Protected Methods *** *** *** *** *** \*/

        protected override async Task OnBeforeExecuteAsync()

        {

            if (!Application.IsAuthenticated())

                return;

            User = await Raven.LoadAsync<Account>(Application.User.Id);

        }

        protected override void Dispose(bool disposing)

        {

            if (disposing)

            {

                using (Raven)

                    Raven = null;

            }

            base.Dispose(disposing);

        }

    }

}

And then derive your real controllers from it. When an action will execute you'll have the User property populated :)