The Weekly Source Code 38 - ASP.NET MVC Beta Obscurity - ModelState.IsValid is False because ModelBinder pulls values from RouteData
I found a bug in an application I'm working on. I couldn't decide if it's a bug in the ASP.NET MVC Framework or a feature. I knew this, however. Both their code and my code runs exactly as it was written. ;)
First, the behavior I'm seeing, then a pile of unnecessary technical context because I like to hear myself talk, then my conclusion. Regardless, it's great fun!
UPDATE: I've been promptly teased/beaten by the MVC team, as they pointed out, pointedly, is the same behavior pointed out by Ayende like 15 years ago. It was prompted fixed/changed, and the new behavior changes the order (listed below) to 3, 1, 2 as I understand it. I hang my head in shame. ;) It's been totally fixed in the Release Candidate, so this post acts only as my own CSI:ASPNETMVC.
The Behavior
My application does CRUD (Create, Read, Update, Delete) on Dinners. When you are entering a new Dinner object, you fill out a form and POST your HTML form.
We take in the Dinner and save it (I've removed goo for clarity):
[AcceptVerbs(HttpVerbs.Post)]
[Authorize]
public ActionResult New([Bind(Prefix = "")]Dinner item)
{
if (ModelState.IsValid)
{
item.UserName = User.Identity.Name;
_dinnerRepository.Add(item);
_dinnerRepository.Save();
TempData["Message"] = item.Title + " Created";
return RedirectToAction("List");
}
}
The behavior I'm seeing is that ModelState.IsValid is ALWAYS false.
Note at this point that I'm not clever enough to go digging into the ModelState object for exactly why. More on my stupidity later. ;)
The Context
The Form POST looks like this (RAW HTTP):
Title=Foo&EventDate=2008-12-10&EventTime=21%3A50&Description=Bar&HostedBy=shanselman&LocationName=SUBWAY&MapString=1050+SW+Baseline+St+Ste+A1%2C+Hillsboro%2C+OR&ContactPhone=%28503%29+601-0307&LocationLatitude=45.519978&LocationLongitude=-123.001934
See how there's a bunch of Name/Value pairs in there? They mostly line up with the names of properties in my class as seen below.
However, note that ID isn't in the Form POST. It's not there because it's an identity, and it'll be created when we save the Dinner to the database.
The New() method takes a Dinner as a parameter. That Dinner is create by the system because using the DefaultModelBinder. That binder looks at the values in the HTTP POST and tries to line them up with the properties on the object. Notice that there's no "ID" in the Form POST. Why is ModelState.IsValid false?
If I look in the ModelState.Values, I can see that the first value says "A value is required." ModelState.Keys tells me that's "ID" and that "" was passed in.
Where did MVC get an ID from and why is it ""? I don't need an ID, I'm in the middle of making a Dinner, not editing one.
Well, it turns out that the DefaultValueProvider, the thing that, ahem, provides values, to the DefaultModelBinder looks in a few places for its values. From the source on CodePlex, note the nice comments:
public virtual ValueProviderResult GetValue(string name) {
if (String.IsNullOrEmpty(name)) {
throw new ArgumentException(MvcResources.Common_NullOrEmpty, "name");
}
// Try to get a value for the parameter. We use this order of precedence:
// 1. Values from the RouteData (could be from the typed-in URL or from the route's default values)
// 2. URI query string
// 3. Request form submission (should be culture-aware)
object rawValue = null;
CultureInfo culture = CultureInfo.InvariantCulture;
string attemptedValue = null;
if (ControllerContext.RouteData != null && ControllerContext.RouteData.Values.TryGetValue(name, out rawValue)) {
attemptedValue = Convert.ToString(rawValue, CultureInfo.InvariantCulture);
}
else {
HttpRequestBase request = ControllerContext.HttpContext.Request;
if (request != null) {
if (request.QueryString != null) {
rawValue = request.QueryString.GetValues(name);
attemptedValue = request.QueryString[name];
}
if (rawValue == null && request.Form != null) {
culture = CultureInfo.CurrentCulture;
rawValue = request.Form.GetValues(name);
attemptedValue = request.Form[name];
}
}
}
return (rawValue != null) ? new ValueProviderResult(rawValue, attemptedValue, culture) : null;
}
Seems that while I assumed that the Form POST was the only place that a Model Binder would go looking, in fact, it looks in three places:
- Values from the RouteData
- URI query string
- Request form submission (should be culture-aware)
The emphasis is mine. At this point I know I'm not seeing a bug, but rather an uncomfortable side-effect of my overly generic naming. I created an ID property on Dinner, but is also have the default route in my Global.asax.cs:
routes.MapRoute(
"Default", // Route name
"{controller}/{action}/{id}", // URL with parameters
new { controller = "Home", action = "List", id = "" } // Parameter defaults
);
Note the default value for ID. This can be confirmed in the debugger at the same breakpoint I used before. The RouteData collection shows an ID name/value pair, where the value is "".
The DefaultModelBinder saw that an ID was available, but it was set to "", which is different than complete omission. If it wasn't there at all, it wouldn't have been required. If it was set to "A" as in /controller/action/A then I'd have seen a different error of "The value 'A' is invalid." as "A" isn't an int.
The Conclusion
I'm going to change my model to use Dinner.DinnerID instead of Dinner.ID so that there's no reuse of tokens between the Route/URL and the Model's property names. Fun stuff! It was so nice to just check the ASP.NET MVC source code to solve my problem. Kudos also to the ASP.NET MVC team for making the source easy to read.
About Scott
Scott Hanselman is a former professor, former Chief Architect in finance, now speaker, consultant, father, diabetic, and Microsoft employee. He is a failed stand-up comic, a cornrower, and a book author.
About Newsletter
Ayende ran into a similar problem a month ago :)
http://ayende.com/Blog/archive/2008/11/07/reproducing-a-bug.aspx
Regards,
Bogdan
1. Values from the RouteData (could be from the typed-in URL)
2. URI query string
3. Request form submission (should be culture-aware)
4. Values from the RouteData default values.
The only time it should use the RouteData's default values is if it can't be found anywhere else. Otherwise, you model is influenced by the technology that you are using. It's one thing to be influenced because of persistence, but a web framework mucking with you model?
http://www.codeplex.com/aspnet/WorkItem/View.aspx?WorkItemId=2549
We're object oriented right? A dinner isn't the same thing as an input form that represents a dinner. We should model reality and model the form itself, bind from it and pass it as a message into a service that manages Dinners.
You have allowed the technical implementation of form inputs to impact your domain model in a way that makes changing your model object (Dinner) difficult. Even worse, to a new developer working on the Dinner object, the fact that its identifier must be named DinnerId so that the UI works is non-obvious. After gaining nothing but brittleness, you lose reuse as deriving your model objects from a layer supertype (to share an Id property for example) becomes impossible.
By using a ViewModel form object you gain more: now the UI can use authorization and validation etc. to protect the domain model from corruption.
I ended up doing _exactly_ what you did, and as somewhat of an object bigot, I don't like constraints forced upon my objects. :)
We already checked a change into our repository based on his report. Scott, you could have simply asked us, you're part of the Big Blue Monster now, whether you like it or not. Heck, you have access to look at the source in progress if you want.
I'm going to change my model to use Dinner.DinnerID instead of Dinner.ID so that there's no reuse of tokens between the Route/URL and the Model's property names.
Instead you should change how DefaultValueProvider behaves or override default behaviour with ScottValueProvider.
You don't want to rewrite your business model and the whole application just because of there's inconsistency in some framework.
Cheers.
The way I solved it was to declare a concrete route for my Save action and don't include the 'id' part in it.
Comments are closed.