2
\$\begingroup\$

It's a question which lies me for years on the tongue.

Given a LoginViewModel:

public class LoginViewModel : ViewModelBase
{
    public string Email { get; set; }
    public string Password { get; set; }
    public bool RememberMe { get; set; }
    public string ReturnUrl { get; set; }

    public IEnumerable<AuthenticationScheme> AuthenticationSchemes { get; set; }
}

Typically in Mvc, you need this ViewModel in a GET Request, and (if the request failed) in a POST request.

So here we have a simple LoginController:

public class LoginController : Controller
{
    private readonly ConmaniaSignInManager _signInManager;

    public LoginController(ConmaniaSignInManager signInManager)
    {
        _signInManager = signInManager;
    }

    [HttpGet]
    public async Task<IActionResult> Index(string returnUrl = null)
    {
        var model = new LoginViewModel
        {
            ReturnUrl = returnUrl,
            AuthenticationSchemes = await _signInManager.GetExternalAuthenticationSchemesAsync() //call 1
        };
        return View(model);
    }

    [HttpPost]
    public async Task<IActionResult> Index(LoginViewModel model)
    {
        if (!ModelState.IsValid)
        {
            model.AuthenticationSchemes = await _signInManager.GetExternalAuthenticationSchemesAsync(); //call 2
            return View(model);
        }
        var result = await _signInManager.PasswordSignInAsync(model.Email, model.Password, model.RememberMe, true);
        if (result.Succeeded)
        {
            return LocalRedirect(Url.GetLocalUrl(Url.GetLocalUrl(model.ReturnUrl)));
        }

        //... Removed for readability

        model.AuthenticationSchemes = await _signInManager.GetExternalAuthenticationSchemesAsync(); //call 3
        ModelState.AddModelError(string.Empty, "Invalid login attempt.");
        return View(model);
    }
}

The first 4 properties of the view model doesn't matter, since they are fields which are getting populated on Post by the ModelBinder.

However the property AuthenticationSchemes gets populated 3 times, even in this simple Login case. Other ViewModels in a typical application can have many more such properties.

In previous Versions of Mvc I have cheated by retrieving the data in the ViewModel by using the getter. That was ofcorse bad design.

So my concern is, is there any Design Pattern to make my controller less painfull (DRY etc)?

I thought about making a Method void PopulateLoginViewModel(LoginViewModel model), and calling this 3 times* instead, but I am not sure if this would be correct.

I also thought about a Service which is injected into the View which has a GetExternalAuthenticationSchemesAsync() method, so the ViewModel does not need to know it. But from what I have read, that is also bad practise.

* I do not want to populate the ViewModel if the user will get redirected anyway.

\$\endgroup\$
  • \$\begingroup\$ I'm voting to close this question as off-topic because OP removed some parts of the code. \$\endgroup\$ – t3chb0t Jun 12 '18 at 6:20
2
\$\begingroup\$

More of a flow problem. Populating the property is necessary for the GET. No getting around that even if you were to create a function to do it for you. The POST however can be restructured to make only one call.

public class LoginController : Controller {
    private readonly ConmaniaSignInManager _signInManager;

    public LoginController(ConmaniaSignInManager signInManager) {
        _signInManager = signInManager;
    }

    private async Task populateLoginViewModel(LoginViewModel model) {
        model.AuthenticationSchemes = await _signInManager.GetExternalAuthenticationSchemesAsync();
    }

    [HttpGet]
    public async Task<IActionResult> Index(string returnUrl = null) {
        var model = new LoginViewModel {
            ReturnUrl = returnUrl
        };
        await populateLoginViewModel(model); //call 1
        return View(model);
    }

    [HttpPost]
    public async Task<IActionResult> Index(LoginViewModel model) {
        if (ModelState.IsValid) {
            var result = await _signInManager.PasswordSignInAsync(model.Email, model.Password, model.RememberMe, true);
            if (result.Succeeded) {
                return LocalRedirect(Url.GetLocalUrl(Url.GetLocalUrl(model.ReturnUrl)));
            }

            ModelState.AddModelError(string.Empty, "Invalid login attempt.");
        }
        //...if we get this far something was wrong. repopulate model and return
        await populateLoginViewModel(model); //call 2
        return View(model);
    }
}

If the model state is not valid then ModelState is already populated. If the login attempt was unsuccessful then we populate the model state and continue to the end of the action.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.