5

When MVC runs an ActionMethod it will populate the ModelState dictionary and uses a ModelBinder to build the ActionMethod parameter(s) if any. It does this for both GET and POST. Which makes sense.

After the ActionMethod successfully has been ran, the view is rendered using the razor supplied, which in my case uses as much HtmlHelper calls as possible. Until now, you might think, 'Yes I know how MVC works'. Hold on, I'm getting there.

When we use e.g. @Html.TextFor(m => m.Name) MVC uses the code that can be here to render the tag.

The interesting part is at the end of the file where we find:

switch (inputType)
{
    case InputType.CheckBox:
        // ... removed for brevity
    case InputType.Radio:
    // ... removed for brevity
    case InputType.Password:
    // ... removed for brevity
    default:
        string attemptedValue = 
               (string)htmlHelper.GetModelStateValue(fullName, typeof(string));
        tagBuilder.MergeAttribute("value", attemptedValue ?? 
               ((useViewData) 
                  ? htmlHelper.EvalString(fullName, format) 
                  : valueParameter), isExplicitValue);
        break;
} 

What this means is that the Modelstate is used to get a value over a supplied Model. And this makes sense for a POST because when there are validation errors you want MVC to render the view with the values already supplied by the user. The documentation also states this behavior and there are several posts here on StackOverflow confirming this. But it is stated only for POST, as can be seen on this thread for example

However this code is also used when a view is rendered for a GET! And this makes no sense at all to me.

Consider the following action method:

public ActionResult GetCopy(Guid id)
{
    var originalModel = this.repository.GetById(id);
    var copiedModel = new SomeModel
    {
        Id = Guid.NewGuid(),
        Name = originalModel.Name,
    };

    return this.View(copiedModel);
}

with this simple razor markup:

    @Html.TextBoxFor(m => m.Id)
    @Html.TextBoxFor(m => m.Name)

I would expect the view to show the newly generated GUID instead of the Id of the original model. However the view shows the Id passed to the actionmethod on the GET request because the ModelState dictionary contains a key with the name Id.

My question is not on how to solve this, because that is fairly easy:

  • rename the ActionMethod parameter to something different
  • Clear the ModelState before returning the ActionResult using Modelstate.Clear()
  • Replace the value in the ModelState dictionary and don't supply a Model to the view at all.

My question is: Why is this proces the same for both GET and POST. Is there any valid use case for using the populated ModelState over the supplied Model on a GET.

Ric .Net
  • 5,540
  • 1
  • 20
  • 39
  • The `DefaultModelBinder` does not differentiate between a GET or POST. But why do you think the behavior should be different? If I made a request to a method and passed it a value, then I would expect the value I passed to be used, not some completely different value –  Jan 11 '18 at 10:21
  • 1
    @StephenMuecke In general I would expect that if I pass an object to a method, it will be used, not data from the ambient context. So `this.View(model)` will use the data from `model` from my perspective. – Ric .Net Jan 11 '18 at 10:48
  • And I would expect that if I had (say) a form making a GET, (say to pass some 'search' values used to filter records), that those values would be returned back again. (I'm a bit unsure what to do with this question - up-vote because it very well written and researched, and/or vote to close it because its asking for an opinion :) –  Jan 11 '18 at 10:55
  • The question is if there are any valid use cases – Ric .Net Jan 11 '18 at 11:33
  • And I gave you my opinion of one in my previous comment :) –  Jan 11 '18 at 11:38
  • @StephenMuecke: Sorry that comment was not complete, my app crashed. The question was about understanding the design choices made by the MVC team. Because I couldn't think of any valid use cases. But you provided a valid use case. If you'll post that as an answer, I'll accept it. Thanks! And thanks for the complement about the quality of the question! – Ric .Net Jan 11 '18 at 11:43
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/162978/discussion-between-stephen-muecke-and-ric-net). –  Jan 11 '18 at 11:43

1 Answers1

3

The DefaultModelBinder does not make a distinction between GET and POST requests. While only the MVC team can confirm why they made that decision, there are valid use cases why ModelState should be used in a GET (in the same way its used in a POST) since you can also submit a form to a GET method.

Take for example an airline booking app, where you have a model with properties to select departure and arrival locations, a date, and the number of passengers (an int property), plus a collection property for the filtered results. The GET method signature might look like

public ActionResult Search(SearchViewModel model)

The view includes a that makes a GET back to the method and includes a textbox for the number of passengers.

A (not too bright) user who has disabled javascript enters "TWO" in the textbox and submits the form. Your method returns the view without populating the collection of available flights because ModelState is invalid (the value "TWO" is not valid for an int) and now the user sees an error message stating The field Passengers must be a number.

If the model value was used rather that the ModelState value, then the associated textbox would be displaying 0 instead of "Two", making the error message confusing (but 0 is a number! - and what happened to what I entered?)

  • Thank you for the detailed answer! I guess there are enough valid use cases to make this indeed the desired behaviour in most scenarios. Although I still would have liked an overload with some bool flag making it clear and possible to determine which values to use. – Ric .Net Jan 12 '18 at 06:58