I am having a problem with custom model binders in the release candidate.  I have a class decorated with the [ModelBinder(typeof(UserProfileBinder))] attribute.  The UserProfileBinder class implements the IModelBinder interface as seen below.  When i use the UserProfile class in strongly typed view via the Html.TextBox helper i get a nullreference  excpetion.  according to the stack trace the error occurs in the System.Web.Mvc.HtmlHelper.GetModelStateValue method.  Any ideas for work arounds

public class UserProfileBInders : IModelBinder{  public object BindModel(ControllerContext controllerContext, ModelBindingContext bindingContext){

 

     UserProfile p = new UserProfile();

     p.FirstName = controllerContext.HttpContext.Request.Form["FirstName"];

     p.LastName = controllerContext.HttpContext.Request.Form["LastName"];

     return p;

}

}

 

I believe you need to call SetModelValue for each property.

I've seen this exception when AddModelError() is called without calling SetModelValue(), but I've never seen this when neither method is called. :(  Is the sample code posted above complete?  We'll continue looking into this.

Thanks!

Some clarification on my above post and the explanation behind the exception -

If you have a custom binder that calls AddModelError() without calling SetModelValue(), the HTML helpers will sometimes throw a NullReferenceException when you try to use them.  The reason is that a call to AddModelError() without a call to SetModelValue() doesn't really mean anything.  Consider that model errors are due to the user submitting bad form data, and by using the HTML helpers you're implicitly telling us that you want them to redisplay the user's invalid submitted form data, but the binder never told us what the bad data was!  It only gave the helpers half the data necessary to render themselves properly; it told them that there was an error, but it didn't provide the expected old value.

Obviously it's not really desirable behavior for us to throw this exception.  We're looking into ways to address this, but choosing an answer isn't as trivial as it seems.  We could silently fall back to a blank value, we could throw a different exception, etc., but in any case we're going to confuse and irritate developers who wrote custom binders and wonder why the users' bad values aren't being propagated between requests.

Now back to the original post -

If you're seeing a NullReferenceException even though your binder doesn't modify the ModelStateDictionary, we're definitely interested in investigating this further, since there are no known issues regarding this particular scenario at this time.

I am not calling SetModelValue.  In my controller I am adding a model error in my validation ie

ViewData.ModelState.AddModelError("FirstName", "Required field");  

 I don't want to call setmodel value from my modelbinder.  My model binder handles loading the model class.  Once I get to my controller i perform my validations then business logic.

hvossos:

I don't want to call setmodel value from my modelbinder.  My model binder handles loading the model class.

Why don't you want to call it from here?  This seems like the appropriate place for it.  Perhaps we're just miscommunicating what the SetModelValue() method does.  It simply says "this is what the user typed in for this field", nothing more.  It doesn't say whether the value is good or bad, only what the user gave us.

 I've experienced this behaviour when not using SetModelValue() after I've added an error with AddModelError() and I looked through the source code to find why it throw a NullReferenceException and as you mentioned it's because the key is in the modelstatedictionary but there is no "attempted value".

The method SetModelValue(string key, ValueProviderResult value) isn't very clear to me. It doesn't express that it's supposed to be the attempted value. The "previous" method SetAttemptedValue(string key, string attemptedValue) was much better in that sense. My second opinion about the method SetModelValue(string key, ValueProviderResult result) is the last parameter. The ValueProviderResult is also not good at expressing how it should be used. The constructor looks like this: ValueProviderResult(object rawValue, string attemptedValue, CultureInfo culture). I had to look through the code to find out what object rawValue should be, is it the attemptedValue but casted into an object? I'm probably missing the point with this class here. The other two parameters I can understand. Except that when I look through the source code for MVC the attemptedValue is barely used.

 Anyhow, those were my opinions. My suggestion to answering how MVC should behave when it doesn't have all the information it needs as you clarified in your previous post is to merge those two methods. Perhaps with an overload AddModelError(string key, string errorMessage, string attemptedValue), this way there won't be a state where the helpers only will have the half of the data and cannot continue. That's just my opinion, in the meanwhile I'll just create an extension method to do it for me.

bjn,

I believe the overloaded AddModelError method that you suggested existed pre-beta (or at least in some of the previews).  I never really understood why they changed it.  I totally agree about the ValueProviderResult, it seems like more work and more confusing and I don't understand what benefits it provides.

bradleylandis:

I believe the overloaded AddModelError method that you suggested existed pre-beta (or at least in some of the previews).  I never really understood why they changed it.

It was changed because the code which calls SetModelValue() and the code which calls AddModelError() aren't necessarily in the same method and aren't necessarily aware of one another.  SetModelValue() is meant to be called by value-binding methods, while AddModelError() is meant to be called by validation methods.  The methods are meant to be callable in any order because we don't force value-binding or validation to take place in any order.

bradleylandis:

I totally agree about the ValueProviderResult, it seems like more work and more confusing and I don't understand what benefits it provides.

The main benefit is that it encapsulates a CultureInfo, as values coming from the Route have to be culture-invariant and values coming from the Form have to be culture-aware.  The RawValue property is what is consumed by the ConvertTo() method, while the AttemptedValue property is what is consumed by the validation system to provide an error message.  To see an example of this, break into one of your action methods and inspect the Controller.ValueProvider entries.  Some of the RawValue entries will be strings (like those from Routing), some of them will be string arrays (like those from QueryString or Form), etc.  The reason for the differentiation between RawValue and AttemptedValue is that RawValue supports passing array types to the ConvertTo() method.  We can't convert to a multi-element array from a single AttemptedValue, but we can convert to a multi-element array from a RawValue that is itself an array.  This supports the infrastructure to allow binding to collection types.

I agree about the unfortunate naming, though unfortunately this isn't likely to change. :(

levib,

Thanks for the information.  I understand there are cases when the extra functionality is needed, but I'd like to see a simpler overload for when I don't need it.  Also, why do I have to set the culture for every single input?  I can't think of a situation where I would have 1 form with different cultures for different inputs.  I certainly wouldn't think it the most common scenario.

In general, you should never be creating a ValueProviderResult; you should be getting it from the dictionary provided by ModelBindingContext.ValueProvider.  The reason that the culture is specified per-value isn't that textbox A has Culture A and textbox B has Culture B; it's that we need to know where the value came from.

Consider the case of ValueProvider["someDate"].ConvertTo(typeof(DateTime)).  If someDate is specified in the URL (as it would be if it were a Routing parameter), this conversion should be done in a culture-invariant fashion.  After all, the U in URL is for uniform.  If I were in the U.S. and IMed my friend in the U.K. a URL containing a date, it shouldn't produce a 404 for him just because he's coming from a different country.  However, if someDate were a form value (that is, we read it from Request.Form instead of Routing), you want to perform the conversion in a culture-aware fashion, because the user should be able to input form entries using his current locale's settings.  In general, values from Routing and the query string (which together make the URL) are InvariantCulture, while values from Form are CurrentCulture.

OK - so this explains the CultureInfo, but what about the RawValue property?  This has to do with the way in which we fetch values from the underlying mechanism - Routing, QueryString, or Form.  Routing uses a RouteValueDictionary, a dictionary in which the entries are of type Object.  QueryString and Form are internally NameValueCollections, and we call GetValues(), which returns a string array.  This is what the RawValue returns - the result as given to us by the underlying mechanism, unmodified.

The RawValue and CultureInfo are both consumed by the ConvertTo() method, which can perform the conversion using the correct culture and give you the type you wanted.  The AttemptedValue is only used by the validation framework; it's used for providing an error message if a submitted value is incorrect.

Thanks for the information. To me it doesn't really make sense not to have the AddModelError and SetModelValue together in a method. (Like the binder as you suggested to hvossos in an earlier post). OK, it's good that you don't want to force where/how value-binding and validation takes place. However, splitting up this into two different parts of your application, would be very confusing to developers. First you had to go look in the validation code, looking for an AddModelError() call and then search the value-binding for a SetModelValue() with the same key. And, why would I be going into the value-binding method if my validation fails? It's kind of hard to bind to a DateTime when it's not possible to convert it to a DateTime :) Big thank you for explaining about the CultureInfo, it didn't make any sense and when looking through the ASP.NET MVC code it was always invoked with null (so it would use the culture-invariant).

In my opinion, there shouldn't be any "Model Value", all the model value should explicitly come from the controller.

When I wrote <%=Html.TextBox("Name") %>, I mean find the "Name" property/field from ViewPage.Model (or ViewData dictionary), and retrieve the value, and use it as the value for html input tag. Why the hell should I call SetModelValue after I modify the model object which will be passed to the View? Otherwise I have to keep track of every modification of the model object -- it's possible that the class which modifies the model object knows nothing about System.Web.Mvc.