Using async / await inside .Select lambda

Question

I am using Asp.Net Core Identity and trying to simplify some code that projects a list of users and their roles to a ViewModel. This code works, but in trying to simplify it I have gone into a crazy spiral of errors and curiosity.

Here is my working code:

        var allUsers = _userManager.Users.OrderBy(x => x.FirstName);
        var usersViewModel = new List<UsersViewModel>();

        foreach (var user in allUsers)
        {
            var tempVm = new UsersViewModel()
            {
                Id = user.Id,
                UserName = user.UserName,
                FirstName = user.FirstName,
                LastName = user.LastName,
                DisplayName = user.DisplayName,
                Email = user.Email,
                Enabled = user.Enabled,
                Roles = String.Join(", ", await _userManager.GetRolesAsync(user))
            };

            usersViewModel.Add(tempVm);
        }

In trying to simplify the code, I figured I could do something like this (broken code):

        var usersViewModel = allUsers.Select(user => new UsersViewModel
        {
            Id = user.Id,
            UserName = user.UserName,
            FirstName = user.FirstName,
            LastName = user.LastName,
            DisplayName = user.DisplayName,
            Email = user.Email,
            Enabled = user.Enabled,
            Roles = string.Join(", ", await _userManager.GetRolesAsync(user))
        }).ToList();

This breaks because I'm not using the async keyword in the lambda expression before user. However, when I do add async before user, I get yet another error that says "Async lambda expressions cannot be converted to expression trees"

My guess is that the GetRolesAsync() method is returning a Task and assigning it to Roles instead of the actual results of that task. What I can't seem to figure out for the life of me is how to make it work.

I have researched and tried many methods over the past day with no luck. Here are a few that I looked at:

Is it possible to call an awaitable method in a non async method?

https://blogs.msdn.microsoft.com/pfxteam/2012/04/12/asyncawait-faq/

Calling async method in IEnumerable.Select

How to await a list of tasks asynchronously using LINQ?

how to user async/await inside a lambda

How to use async within a lambda which returns a collection

Admittedly, I do not fully understand how async / await work, so that's probably part of the issue. My foreach code works, but I'd like to be able to understand how to make it work the way I'm trying to. Since I've spent so much time on it already I figured this would be a good first question.

Thanks!

Edit

I guess I have to explain what I did in each case of the articles I researched in order for this to not be flagged as a duplicate question - and I tried really hard to avoid that :-/. While the question sounds similar, the results are not. In the case of the article that was marked as an answer I tried the following code:

    public async Task<ActionResult> Users()
    {
        var allUsers = _userManager.Users.OrderBy(x => x.FirstName);
        var tasks = allUsers.Select(GetUserViewModelAsync).ToList();
        return View(await Task.WhenAll(tasks));
    }

    public async Task<UsersViewModel> GetUserViewModelAsync(ApplicationUser user)
    {
        return new UsersViewModel
        {
            Id = user.Id,
            UserName = user.UserName,
            FirstName = user.FirstName,
            LastName = user.LastName,
            DisplayName = user.DisplayName,
            Email = user.Email,
            Enabled = user.Enabled,
            Roles = String.Join(", ", await _userManager.GetRolesAsync(user))
        };
    }

I also tried using AsEnumerable like so:

    var usersViewModel = allUsers.AsEnumerable().Select(async user => new UsersViewModel
        {
            Id = user.Id,
            UserName = user.UserName,
            FirstName = user.FirstName,
            LastName = user.LastName,
            DisplayName = user.DisplayName,
            Email = user.Email,
            Enabled = user.Enabled,
            Roles = string.Join(", ", await _userManager.GetRolesAsync(user))
        }).ToList();

Both of these produce the error message: "InvalidOperationException: A second operation started on this context before a previous operation completed. Any instance members are not guaranteed to be thread safe."

At this point it seems like my original ForEach may be the best bet, but I'm still left wondering what would be the right way to do this if I were to do it using the async methods.

Edit 2 - with Answer Thanks to Tseng's comments (and some other research) I was able to make things work using the following code:

        var userViewModels = allUsers.Result.Select(async user => new UsersViewModel
        {
            Id = user.Id,
            UserName = user.UserName,
            FirstName = user.FirstName,
            LastName = user.LastName,
            DisplayName = user.DisplayName,
            Email = user.Email,
            Enabled = user.Enabled,
            Roles = string.Join(", ", await _userManager.GetRolesAsync(user))
        });
        var vms = await Task.WhenAll(userViewModels);
        return View(vms.ToList());

Although now that I have taken everyone's comments into consideration, I started looking closer at the SQL Profiler just to see how many hits the DB is actually getting - as Matt Johnson mentioned, it's a lot (N+1).

So while this does answer my question, I am now reconsidering how to run the query and may just drop the roles in the main view and only pull them as each user is selected. I definitely learned a lot through this question though (and learned more of what I don't know), so thank you everyone.

Solution

I think you are mixing two things here. Expression trees and delegates. Lambda can be used to express both of them, but it depends on the type of parameter the method accepts in which one it will be turned.

A lambda passed to a method which as Action<T> or Func<T, TResult> will be converted into a delegate (basically an anonymous function/method).

When you pass lambda expression to a method accepting Expression<T>, you create an expression tree from the lambda. Expression trees are just code which describe code, but are not code themselves.

That being said, an expression tree can't be executed because it's converted to executable code. You can compile a expression tree at runtime and then execute it like a delegate.

ORM Frameworks use expression trees to allow you writing "code" which can be translated into something different (a database query for example) or to dynamically generate code at runtime.

For that reason you can't use async in methods that accept Expression<T>. The reason why it may work when you convert it to AsEnumerable() is because it returns an IEnumerable<T> and the LINQ methods on it accept Func<T, TResult>. But it essentially fetches the whole query and does the whole stuff in memory, so you can't use projections (or you have to fetch the data before using just expressions and projections), turn the filtered result into list and then filter it.

You could try something like this:

// Filter, sort, project it into the view model type and get the data as a list
var users = await allUsers.OrderBy(user => user.FirstName)
                             .Select(user => new UsersViewModel
    {
        Id = user.Id,
        UserName = user.UserName,
        FirstName = user.FirstName,
        LastName = user.LastName,
        DisplayName = user.DisplayName,
        Email = user.Email,
        Enabled = user.Enabled
    }).ToListAsync();

// now that we have the data, we iterate though it and 
// fetch the roles
var userViewModels = users.Select(async user => 
{
    user.Roles = string.Join(", ", await _userManager.GetRolesAsync(user))
});

The first part will be done fully on the database and you keep all your advantages (i.e. the order happens on database, so you don't have to do in-memory sorting after fetching the results and the limit calls limits the data fetched from the DB etc.).

The second part iterates through the result in memory and fetches the data for each temporary model and finally maps it into the view model.

Comments

[mason]

My guess is that the GetRolesAsync() method is returning a Task and assigning it to Roles instead of the actual results of that task. - probably not, because await will grab the result of the task.

[juharr]

Try putting an AsEnumerable before the Select so it will be run in LInq to Objects instead of trying to convert it to an expression tree for EF or whatever provider you use.

[Raquel]

var usersViewModels = (await Task.WhenAll(allUsers.AsEnumerable().Select(async user => new UsersViewModel { Id = user.Id, UserName = user.UserName, FirstName = user.FirstName, LastName = user.LastName, DisplayName = user.DisplayName, Email = user.Email, Enabled = user.Enabled, Roles = string.Join(", ", await _userManager.GetRolesAsync(user)) }))).ToList();

[Matt Johnson-Pint]

So you make one call to get all of the users, and then for each user you make a separate call to get their roles. This is the well-known anti-pattern called "Select N+1" (Google/Bing it). You really shouldn't do this. What happens when you have 10,000 users? Do you really want to make 10,001 database calls?

[poke]

Possible duplicate: Multi-async in Entity Framework 6?

[poke]

Essentially, since GetRolesAsync apparently also runs something on the database context, you cannot have multiple parallel GetRolesAsync calls. You would need to run them in sequence. As per Matt Johnson’s comment, you should consider a way to query multiple (all) user roles at once if you really need them. – btw. your original question completely left out the relevant parts of what you have tried with those linked questions, and now that you have edited that in, it’s not clear to me, why you didn’t search for the error message.

[FranksBrain]

Thanks for the feedback, everyone. Obviously I do not want to have a db call for every user - that's why I was attempting to fix this in the first place. I left out the EF6 link because I'm running EF7 - and since EF7 is a complete rewrite of Entity Framework I wasn't sure it was relevant. I know I could easily write this query using the db context, but the best practice is always given to use the Identity methods provided which is what I was trying to do. Perhaps this is more specific to EF7 / Identity. I'll keep chugging along - at least I am in no rush. Thanks.

[Admin]

If you don't know what EF is using expression trees for, you need to know. EF is a big magic box where you put data, but if you don't know how the magic works, you end up sacrificing your cat for no real reason.

[Theodor Zoulias]

allUsers.Result? Won't this block the current thread?

[Arash Ghasemi Rad]

@TheodorZoulias : Yes you are right. And many thanks for the comment. I used to use in this way, but now it's not a good idea. Do you know any correct solution which returns List<T> instead of Task<List<T>>?

[Theodor Zoulias]

Nope, I don't know. AFAIK the only correct solution is async all the way.

[Arash Ghasemi Rad]

@TheodorZoulias Async All the Way is not usefull when using in API controller which you better not to return Task<>s. I updated my answer to some better solution. Let's discuss about it.

[Theodor Zoulias]

If you are going to use await in the outer scope, then why not use it with allUsers too? = (await allUsers).Select(.... Using .Result is an admission of defeat IMHO.

[Arash Ghasemi Rad]

Again you are right, and my focus was on the return type. I updated the answer again and now it looks more compact.

[Jay]

What do you mean by 'carried deferred execution semantics'? The reason I return IEnumerable, is because the rest of the Linq library does it as well. I just want to be consistent. I do ToList() or ToArray() on the caller, whichever I need. Since this method works with both a source and a destination type, I prefer TSource and TDest. Tnx for the Linq.Async tip, though.

[Jay]

When you say 'deferral', you probably mean 'yield'. I never use it. In my use case, I have no need for it, afaik. Hence the fact that it also doesn't occur in my extension method. But you know what, I'll look into 'yield'. I might find a use for it, after I learn more about it.

[Jay]

That's why I've added it to my own collection of extension methods. It does exactly what I need it to. And what OP needs it to. So I thought I'd share.

[Jay]

At first it was about the naming of my types.

[Jay]

There is nothing misleading about it. The use of yield is not required to be used with an IEnumerable. I don't see how renaming a type will somehow make a 3 line code snippet better. But sure, I'll do a small edit, since you're offering.