Updating/Binding Model Graphs with ASP.NET MVC

by Rudi24. February 2010 21:47

Introduction

This post describes issues and solutions involved when using 'complex' object graphs as models for ASP.NET MVC applications.

The ASP.NET MVC Controller class has a TryUpdateModel() method that is described in the API reference on MSDN as "Updates the specified model instance using values from the controller's current value provider":

protected internal bool TryUpdateModel<TModel>(TModel model)

This method does this correctly on the given model and even on nested objects (which is really how it should be). But when the object graph consists of collections (or dictionaries), the result may not be what you expect...

Applies to: this behavior has been observed on ASP.NET MVC2 RC2 (which is the latest version at the time of writing) and earlier versions.

The context

Note: the context described here is fictious and only meant to describe the problem type. Aspects as security and design are deliberately ignored ;)

Within an ASP.NET MVC application, I want to allow customers to update the quantities on a placed order. For this, I’ve a form that looks like:

<% Html.BeginForm("SaveOrder", "Order"); %>
<%= Html.HiddenFor(m => m.Id) %>
<%= Html.EditorFor(m => m.Customer.Phone) %>
<% for(int i=0; i<Model.Lines.Count; i++) { %>
<% // if (i==0) continue; %>
<p>
<%= Html.EditorFor(m => m.Lines[i].Quantity) %>
x <%= Model.Lines[i].UnitPrice) %>
of <%= Model.Lines[i].Product.Name %>
</p>
<% } %>
<input type="submit" value="Submit" />
<% Html.EndForm(); %>

So it shows the order lines, allows for editing the quantity of each line, and shows the name of the product associated with the line. It could have been possible to put all the information of the order in hidden fields, but this is deliberately not done as we do not want to divulgate all this information (security and privacy reasons).

To save the order, we need to retrieve the original order, then update the order with the values of the form postback, and then save the updated order. The SaveOrder action is implemented as follows:

public ActionResult SaveOrder(int id)
{
// Retrieve the original order from the service:
Order order = Service.GetOrder(id);
// Update the order by the form postback:
this.TryUpdateModel(order);
// If valid, save the updated order:
if (ModelState.IsValid)
{
order = Service.SaveOrder(order);
}
return RedirectToAction("Index", new { Id = order.Id });
}

This is what we would like to have. Unfortunately, it does not work...

Before the call to TryUpdateModel(), my order consists of OrderLines having quantity, unitprice and associated product. After the TryUpdateModel() call, though the quantities on my OrderLines have been updated, the unitprice and associated product have been removed !

The problem

The TryUpdateModel() method, to which a model is given, is supposed to update the given model and not to return a new model. It does this correctly for 'simple' models. Even for 'nested' models, this behavior is correct: I've added an EditorFor the phone number of the customer on the form and the TryUpdateModel() will correctly update the phone number of the provided customer object, leaving the customer’s name, email and address properties intact.

So it works fine for the root model object and all nested objects and can thus update a whole object graph. Except... when the object graph contains collections. While the Customer of my Order is correctly updated, the OrderLines of my Order are not, they are replaced with new instances containing only the properties of my form postback.

Not only is this behavior inconsistent with the behavior of parts in my object graph that are not beneath a collection, it is also contrary to the behavior I would expect from a method called TryUpdateModel(). If the method is to create new model objects instead of updating given ones, it should be named TryCreateModel() and should not require a model argument.

The cause

To find the cause of the problem, we need to delve deep into the implementation of the DefaultModelBinder class in System.Web.Mvc. After a thorough understanding of the problem and a few debugging sessions in the DefaultModelBinder implementation, I could pinpoint the issue. The DefaultModelBinder class uses an internal UpdateCollection() method to bind collection members. Within this method, an 'inner' ModelBindingContext is constructed for each collection member, to bind it. However, when constructing this inner ModelBindingContext, a "null" model value is passed to it (line 7 in the code below), such that binding has to happen on a new instance of the collection member instead of on the existing member...

The original code of the UpdateCollection() method (in MVC2 RC2) looks like this:

...
foreach (string currentIndex in indexes) {
...
ModelBindingContext innerContext = new ModelBindingContext() {
ModelMetadata = ModelMetadataProviders.Current.GetMetadataForType(
null, elementType),
ModelName = subIndexKey,
ModelState = bindingContext.ModelState,
PropertyFilter = bindingContext.PropertyFilter,
ValueProvider = bindingContext.ValueProvider
};
object thisElement = elementBinder.BindModel(controllerContext, innerContext);
...
}
...

Because of the null passed as first argument of the GetMetaDataForType() method, model binding of each element of the collection has to be done towards a null object, hence a new instance is to be created. This issue can be solved by replacing the above implementation with:

...
foreach (string currentIndex in indexes) {
...
object model = null;
Func<object> modelAccessor = null;
if ((bindingContext.Model != null)
&& (true))
{
model = bindingContext.Model.GetType().GetProperty("Item")
.GetValue(bindingContext.Model, 
new object[] { Convert.ToInt32(currentIndex) });
modelAccessor = delegate() { return model; };
}
ModelBindingContext innerContext = new ModelBindingContext() {
ModelMetadata = ModelMetadataProviders.Current.GetMetadataForType(
modelAccessor, elementType),
ModelName = subIndexKey,
ModelState = bindingContext.ModelState,
PropertyFilter = bindingContext.PropertyFilter,
ValueProvider = bindingContext.ValueProvider
};
object thisElement = elementBinder.BindModel(controllerContext, innerContext);
...
}
...

But of course, to apply this solution we need to update the original DefaultModelBinder class implementation.

Solving the problem

Unless we work at Microsoft and can update the implementation of the DefaultModelBinder, the only thing we can do is implement our own 'Default' ModelBinder. The UpdateCollection() method is internal to DefaultModelBinder, and we cannot simply inherit of DefaultModelBinder to override that single method. We don’t want to rewrite the DefaultModelBinder from scratch either, as this is quite complex, it could break compatibility with later releases of ASP.NET MVC, and we really only want to change 0.3% of it’s code...

Fortunately, ASP.NET MVC is well designed and quite extensible, and it’s DefaultModelBinder recursively calls the overridable BindModel on each step in the model binding process. So what we can do, is specialize from DefaultModelBinder, override it’s BindModel method, and always delegate it to its base implementation, except when collections are to be bound.

The following implementation should do the trick:

/// <summary>
/// A DefaultModelBinder that can update complex model graphs.
/// </summary>
public class DefaultGraphModelBinder : DefaultModelBinder
{
public override object BindModel(ControllerContext controllerContext, ModelBindingContext bindingContext)
{
// Bind collections 'our way':
if ((bindingContext.Model.IsCollection())
&& (bindingContext.Model.CollectionGetCount() > 0))
return this.BindCollection(controllerContext, bindingContext);
else
return base.BindModel(controllerContext, bindingContext);
}
private object BindCollection(ControllerContext controllerContext, ModelBindingContext bindingContext)
{
object collection = bindingContext.Model;
Type collectionMemberType = typeof(Object);
if (collection.GetType().IsGenericType)
collectionMemberType = 
collection.GetType().GetGenericArguments()[0];
int count = collection.CollectionGetCount();
for (int index = 0; index < count; index++)
{
// Create a BindingContext for the collection member:
ModelBindingContext innerContext = new ModelBindingContext();
object member = collection.CollectionGetItem(index);
Type memberType = 
(member == null) ? collectionMemberType : member.GetType();
innerContext.ModelMetadata = 
ModelMetadataProviders.Current.GetMetadataForType(
delegate() { return member; },
memberType);
innerContext.ModelName =
String.Format("{0}[{1}]", bindingContext.ModelName, index);
innerContext.ModelState = bindingContext.ModelState;
innerContext.PropertyFilter = bindingContext.PropertyFilter;
innerContext.ValueProvider = bindingContext.ValueProvider;
// Bind the collection member:
IModelBinder binder = Binders.GetBinder(memberType);
object boundMember = 
binder.BindModel(controllerContext, innerContext) ?? member;
collection.CollectionSetItem(index, boundMember);
}
// Return the collection:
return collection;
}
}
internal static class DefaultModelGraphBinderCollectionExtensions
{
public static bool IsCollection(this object obj)
{
return (obj != null)
&& (obj.GetType() != typeof(String))
&& (typeof(System.Collections.IEnumerable).IsInstanceOfType(obj));
}
public static int CollectionGetCount(this object collection)
{
if (collection.GetType().IsArray)
return ((Array)collection).GetLength(0);
else
return (int)collection.GetType().GetProperty("Count")
.GetValue(collection, null);
}
public static object CollectionGetItem(this object collection, int index)
{
if (collection.GetType().IsArray)
return ((Array)collection).GetValue(index);
else
return collection.GetType().GetProperty("Item")
.GetValue(collection, new object[] { index });
}
public static void CollectionSetItem(this object collection, int index, object value)
{
if (collection.GetType().IsArray)
((Array)collection).SetValue(value, index);
else
collection.GetType().GetProperty("Item")
.SetValue(collection, value, new object[] { index });
}
}

To install this model binder as the default one, add the following line of code in the Application_Start method of the Global.asax.cs file:

ModelBinders.Binders.DefaultBinder = new DefaultGraphModelBinder();

Robustness

There are for sure some remarks to do on the given implementation. For instance it’s use of reflection and assumption that an instance of IEnumerable has properties Count and Item. The implementation will certainly have to grow whenever unsupported cases are met.

Furthermore, within this implementation we assume that the index is an integer starting at 0. Although the original DefaultModelBinder does not seem to make this assumption, it appears not to work when that’s not the case. For instance, if you uncomment the code in the following line in the view,

<% // if (i==0) continue; %>
then the original DefaultModelBinder does not succeed in binding the collection. The above implementation will successfully bind the other collection members.

Another story is the support of dictionaries which I haven’t tested so far.

Conclusions

Though the current ASP.NET MVC implementation still has some imperfections probably due to a rapid growth of supported features (also remember I used a release candidate version), thanks to it’s open and extensible architecture, it is possible to tailor the framework to your specific requirements with limited rework.

Download

The sample order application can be downloaded here. It simulates a basic order editing system and demonstrates the operation of the DefaultGraphModelBinder.

DefaultGraphModelBinder sample Order application
MvcGraphBindingSample.zip (20.94 kb)

Tags:

ASP.NET MVC

Comments (3) -

thekaido
thekaidoUnited States
2/25/2010 10:39:40 PM #

I was just trying to solve this exact problem and you solved it much cleaner than the road I was going down. Thanks!!  That said, I am using MVC 1 currently and had to make a couple of minor changes and thought I would share for others using mvc1.

MVC1 doesn't have the ModelMetadata attribute so that can be commented out and replaced with
//innerContext.ModelMetadata = ModelMetadataProviders.Current.GetMetadataForType(delegate() { return member; }, memberType);
innerContext.ModelType = memberType;
innerContext.Model = member;

The last line inside of BindCollection (collection.CollectionSetItem...) was blowing up when working with Linq to Sql EntitySet collections (I know, I should be using ViewModels and not the domain models directly ...). I didn't spend much time on it but I just commented the line out because I was binding reference types and thus automatically updated.  

Haacked
HaackedUnited States
2/25/2010 10:44:14 PM #

Some things to note. L2S and EF don't guarantee ordering unless you use an OrderBy clause so you could end up updating the wrong elements. Also, be aware of delete scenarios where the indexes change.

Rudi
RudiBelgium
2/26/2010 2:45:31 PM #

It appears the implementation lacks support for for instance an array of integers where you want to select values using checkboxes. Typically, you would then have HTML code as:
<input type="checkbox" value="1" />1<br/>
<input type="checkbox" value="2" />2<br/>
<input type="checkbox" value="3" selected="selected" />3<br/>
that matches an integer array with one element (the value 3).

To restore support on the binder for this, update the if condition on lines 9 and 10 of the DefaultGraphModelBinder class into:

if ((bindingContext.Model.IsCollection())
    && (bindingContext.Model.CollectionGetCount() > 0)
    && (!controllerContext.RequestContext.HttpContext.Request.Form.AllKeys.Contains(bindingContext.ModelName)))

The last condition part checks whether the modelname, which at this time is without indexes (i.e. "MyPerson.SelectedFriendIds" opposed to "MyPerson.SelectedFriendIds[0]" which is the modelname with indexes), exists within the list of posted form fieldnames.

Comments are closed

About me

Widget Month List not found.

The file '/blog/widgets/Month List/widget.ascx' does not exist.X

Widget Page List not found.

The file '/blog/widgets/Page List/widget.ascx' does not exist.X