MVC async method in constructor controller

Question

I'm trying to make dynamic menu (stored in DB), that is showing on all web app pages. Using Google I found that it is better to make menu view as a part of Master View (_Layout.cshtml). And because of that, every action method of the controller must contain data with the menu model. To avoid code duplication I found the solution to create a base controller and provide data using its constructor:

https://learn.microsoft.com/en-us/aspnet/mvc/overview/older-versions-1/views/passing-data-to-view-master-pages-cs

Also, I'm trying to use async/await possibilities and my PageService (menu) is using ToListAsync() to get data from DB. So now I have a problem, that BaseController constructor has an async method:

public class BaseController : AsyncController, IBaseController
{
    private readonly IPageService _pageService;

    public BaseController(IPageService pageService)
    {
        _pageService = pageService;
        SetBaseViewModelAsync();
    }

    private async Task SetBaseViewModelAsync()
    {
        ViewData["Pages"] = await _pageService.GetAllAsync();
    }
}

I know that this is BAD CODE, but I don't know how to design this situation properly. Maybe there is another better way to create the dynamic menu or another way to get data asynchronously?

Also, I found this article, but I don't know if I can apply its solutions because I don't know if I can handle controller instance creation:

http://blog.stephencleary.com/2013/01/async-oop-2-constructors.html

That's bad design, you are correct there. You should be looking at a middle-ware or action filter instead — Camilo Terevinto
– Camilo Terevinto, Commented Dec 22, 2017 at 17:29
Thanks for the response! Can you please provide some links to articles with proper designs? Because I googled for like 1-2 hours to find how to implement the dynamic menu. — Dima Glushko
– Dima Glushko, Commented Dec 22, 2017 at 17:33
This sounds like a good candidate for a Child Action. See also this question stackoverflow.com/questions/21948909/… — Jasen
– Jasen, Commented Dec 22, 2017 at 17:38

Erik Philips · Accepted Answer · 2017-12-22 17:39:01Z

6

Instead of deriving everything from a base controller (which can be a lot of extra work and testing) you can just create a controller called MenuController, create a method called Default and then call it from your Layout:

[ChildActionOnly]
public Default()
{
  var viewModel = _pageService.GetAllAsync();
  return Partial(viewModel);
}

in your layout:

@{Html.RenderAction("Default", "Menu");}

This is really the easiest and cleanest solution. The biggest PRO is that you can control the cache for the menu separate from the method calling. There are no good solution for asp.net-mvc (1-5) to run Async code in this fashion. (ActionFilters can't be async and (Render)Partials can't be async. You can still call an async method, it will just run Sync.

Render vs Non-Render Performance.

answered Dec 22, 2017 at 17:39

Erik Philips

54.8k11 gold badges133 silver badges158 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

Dima Glushko Over a year ago

Thanks for the response! I am new to MVC, so I will read about RenderAction and will try your solution. Also, cache possibility is great here!

Dima Glushko Over a year ago

Applied your solution, but it has some nuances: child actions can't be async: stackoverflow.com/questions/24072720/… So I decided not to make the action as ChildAction. Is it necessary to make it child only? Can you please also modify your answer, so acceptance would be proper? Because now your example has async code in ChildAction

Erik Philips Over a year ago

Async code has async/await keywords. Mine does not use those keywords. It should compile with a warning. Otherwise you could just call async code within a sync method.

Dima Glushko Over a year ago

Calling async code within sync method is a bad idea. IMO I will better remove ChildAction attribute and remain code async

Dima Glushko Over a year ago

I was wrong, that was just my cache, that returned the correct result to me. I removed the cache and I'm getting the same as here error: stackoverflow.com/questions/24072720/… So I can't use RenderAction with async Actions... Then I will just call sync method. Seems like it's impossible to realize my problem with the async code.

Dima Glushko · Accepted Answer · 2017-12-24 20:56:26Z

I changed functionality to call Html.RenderAction in my View, as Erik Philips suggested:

@{
    Html.RenderAction("Index", "Pages");
}

and controller:

public class PagesController : AsyncController, IPagesController
{
    private readonly IPagesService _pagesService;

    public PagesController(IPagesService pagesService)
    {
        _pagesService = pagesService;
    }

    [HttpGet]
    [Route("")]
    public async Task<ActionResult> IndexAsync()
    {
        var viewModel = await _pagesService.GetAllAsync();
        return PartialView("MenuPartial", viewModel);
    }
}

But RenderAction doesn't work with async controller Actions:

Async PartialView causes "HttpServerUtility.Execute blocked..." exception

So seems like sync call is the only one possible here.

Collectives™ on Stack Overflow

MVC async method in constructor controller

2 Answers 2

5 Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

5 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related