CODEWITHSUNDEEP
c#.netasp.net-mvcentity-frameworkasync-await
eyeballpaul
eyeballpaul

Reputation: 1735

Is this the correct usage of async/await in MVC with service/repository layer?

Using VS2013, EF6.1.1, MVC5, .net 4.5.

I have just started looking into async/await for the first time and I am not sure if this is doing it properly. It appears to work, but could it be simpler? I seem to be sticking async/await in a lot of places for one method call down multiple layers.

All code has been simplified for brevity.

In my MVC controller action method I have:

public async Task<ActionResult> TestAction1()
{
    var testResponse = await _testService.TestMethod1(1);

    if (testResponse != null)
    {
        _unitOfWork.CommitAsync();
        return View(testResponse);
    }

    return RedirectToAction("TestAction2");
}

My service class is as follows:

public class TestService : ITestService
{
    public async Task<TestObject> TestMethod1()
    {
        var testObject1 = await privateMethod1Async();
        if (testObject1 != null) return testObject1;

        testObject1 = await privateMethod2Async();

        return testObject1;
    }

    private async Task<TestObject> privateMethod1Async()
    {
        return await _testRepository.FirstOrDefaultAsync();
    }

    private async Task<TestObject> privateMethod2Async()
    {
        return await _testRepository.FirstOrDefaultAsync();
    }
}

And my repository method:

public async Task<TEntity> FirstOrDefaultAsync()
{
    return await _entitySet.FirstOrDefaultAsync();
}

Basically, I have a controller method, that calls a service method. The service method is to call the database layer twice, in an async fashion. But I feel that I am changing every single method and layer to deal with async and I wasn't sure if what I have here is correct.

Secondly, in the controller method I am unsure how to call the unit of work commit method asynchronously. The line that has "_unitOfWork.CommitAsync();". I cannot stick an "await" before it as it is a void method.

Any thoughts?

EDIT 1

Here is the full version of the repository method call to EF:

public async Task<TEntity> FirstOrDefaultAsync(Expression<Func<TEntity, bool>> 
                                               predicate, params 
                                               Expression<Func<TEntity, object>>[]
                                               includeProperties)
{
    IQueryable<TEntity> query = EntitySet;
    if (includeProperties != null && includeProperties.Any())
    {
        query = IncludeProperties(query, includeProperties);
    }
    return await query.FirstOrDefaultAsync(predicate);
}

Upvotes: 3

Views: 2167

Answers (2)

Yuval Itzchakov
Yuval Itzchakov

Reputation: 149598

I see a reoccurring pattern in your code:

private async Task<TestObject> privateMethod2Async()
{
    return await _testRepository.FirstOrDefaultAsync();
}

When you have a single-liner which simply queries your DB for a Task<T>, you can avoid the state-machine allocation which await causes, and simply return the hot task to the caller (as he'll probably await on it anyway higher up the call-chain):

private Task<TestObject> privateMethod2Async()
{
    return _testRepository.FirstOrDefaultAsync();
}

Note that async-await will make you go "async all the way", that's the nature of async. Make sure that you group tasks that can run concurrently using Task.WhenAll if possible, for example (not sure this might be the best example):

public async Task<TestObject> TestMethod1()
{
    var testObject1 = await privateMethod1Async();
    if (testObject1 != null) return testObject1;

    testObject1 = await privateMethod2Async();

    return testObject1;
}

Might be able to turn into:

return Task.WhenAny(privateMethod1Async(), privateMethod2Async());

given that one of the methods is applicable as a return type.

Edit:

What is the general rule rule of thumb for adding the async/await? is it for a method that does other processing in it? Which, as you point out, this method doesn't?

You would want to use await when you'd want to be doing more processing on the returned Task. If all you want is to return the actual Task, there is no need to await it, you can simply return the hot task. That's the general rule i use. Also note that exception handling is different when you use return await vs simply return.

You can read more on that in At the end of an async method, should I return or await? and Any difference between "await Task.Run(); return;" and "return Task.Run()"?

Upvotes: 8

i3arnon
i3arnon

Reputation: 116636

async-await does have a tendency to crawl upwards in your code base and it's perfectly fine. You should let it rise up as long as it's possible (in a UI event handler it's possible to rise all the way to the top because of the combination of async-void and the UI SynchronizationContext)

When it isn't possible anymore (the root of a console application for example) you can simply Wait the returned task:

var task = RunAsync();
task.Wait();

Or even better, you can use Stephen Cleary's AsyncContext:

AsyncContext.Run(RunAsync);

Upvotes: 3

Related Questions