The Problem
ASP.NET MVC 2 introduces templated helpers. They're a convenient and type safe way to render model data. There is a potential problem in the way in which the system processes the lambda expression passed in to the helper. The problem causes overridden properties to be ignored and uses the base class' declaration of the property instead. This can have adverse reactions where the client side validation feature of ASP.NET MVC 2 is used.
An Example
Create a very simple BasePerson class:
public class BasePerson
{
public virtual string FirstName { get; set; }
}
Create a Person class that derives from the BasePerson class, but adds some data annotations (of course, you could do anything here that you required):
public class Person : BasePerson
{
[Required(ErrorMessage = "FirstName is required.")]
public override string FirstName { get; set; }
}
Add a very simple controller:
public class HomeController : Controller
{
public ActionResult Index()
{
var p = new Person();
return View(p);
}
}
Create the Index.htm view:
<%@ Page Language="C#" Inherits="System.Web.Mvc.ViewPage<MvcApplication1.Models.BasePerson>" %>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml" >
<head runat="server">
<title>Index</title>
<script src="/Scripts/MicrosoftAjax.js" type="text/javascript"></script>
<script src="/Scripts/MicrosoftMvcAjax.js" type="text/javascript"></script>
<script src="/Scripts/MicrosoftMvcValidation.js" type="text/javascript"></script>
</head>
<body>
<% Html.EnableClientValidation(); %>
<% Html.BeginForm(); %>
<div>
<%= Html.TextBoxFor(x=>x.FirstName) %>
<%= Html.ValidationMessageFor(x=>x.FirstName) %>
<input type='submit' value='submit' />
</div>
<% Html.EndForm(); %>
</body>
</html>
You could have made the ViewPage of type Person, instead of BasePerson, but the error would have still been present.
If you run the project now, the rendered html responsible for client side validation would look like this:
//<![CDATA[ if (!window.mvcClientValidationMetadata) { window.mvcClientValidationMetadata = []; } window.mvcClientValidationMetadata.push({"Fields":
[{"FieldName":"FirstName","ReplaceValidationMessageContents":true,
"ValidationMessageId":"form0_FirstName_validationMessage","ValidationRules":[]}],
"FormId":"form0","ReplaceValidationSummary":false}); //]]>
As you can see, the validation rules are empty and as such, there's no client side validation occurring. To see this, just click the submit button. The form will post back without any problems. That should not be happening if the FirstName property is empty.
The Reason
The cause of the bug is a single line of code in the method FromLambdaExpression in the file ModelMetaData.cs in the source code of MVC 2 RC 2. Here's the relevant portion of code:
MemberExpression memberExpression = (MemberExpression)expression.Body;
string propertyName = memberExpression.Member is PropertyInfo ? memberExpression.Member.Name : null;
Type containerType = memberExpression.Member.DeclaringType;
While this looks pretty normal, the third line is the culprit. The DeclaringType refers to the class that declares a member – it's not necessarily the type of the class that's in the Model of our ViewData. In our example, although we passed in a Person object as the model, the DeclaringType of FirstName will always be BasePerson. The consequence of using the DeclaringType member to get the containerType is that the attribute added to the overriden FirstName of the Person class is totally ignored. Any other attributes on an overridden property will also be ignored.
Note that if you were to use TextBoxFor(model=>model) or TextBox("FirstName"), then this would not be a problem until and unless
somethingFor(x=>x.FirstName)
is encountered.
Also, server side validation isn't affected by the bug as the bug appears to be centred on that specific third line of code.
A Possible Solution
The main reason for the problem is that FromLambdaExpression is not using the runtime type of the container. Using the runtime type can be achieved in a few lines of code. Let's first add a helper method:
private static ParameterExpression GetParameterExpression(Expression expression)
{
while (expression.NodeType == ExpressionType.MemberAccess)
{
expression = ((MemberExpression)expression).Expression;
}
if (expression.NodeType == ExpressionType.Parameter)
{
return (ParameterExpression)expression;
}
return null;
}
I've taken the GetParameterExpression method from here:
http://geekswithblogs.net/EltonStoneman/archive/
2009/11/05/retrieving-nested-properties-from-lambda-expressions.aspx
Credits for that go to Elton Stoneman
The last thing to do is to add a bit of code to retrieve the runtime type of the container:
MemberExpression memberExpression = (MemberExpression) expression.Body;
string propertyName = memberExpression.Member
is PropertyInfo ? memberExpression.Member.Name :
null;
Type containerType = memberExpression.Member.DeclaringType;
bool isContainerValueType = typeof (TParameter).IsValueType;
if(isContainerValueType || (isContainerValueType == false && container != null))
{
var parameter = GetParameterExpression(memberExpression.Expression);
var lambda = Expression.Lambda(memberExpression.Expression, parameter);
var compiled = lambda.Compile();
var containerInstance = compiled.DynamicInvoke(container);
containerType = containerInstance.GetType();
}
Running the page and viewing the source, we should see this:
//<![CDATA[ if (!window.mvcClientValidationMetadata) { window.mvcClientValidationMetadata = []; } window.mvcClientValidationMetadata.push({"Fields":
[{"FieldName":"FirstName","ReplaceValidationMessageContents":true,
"ValidationMessageId":"form0_FirstName_validationMessage",
"ValidationRules":[{"ErrorMessage":"FirstName is required.","ValidationParameters":{},
"ValidationType":"required"}]}],"FormId":"form0","ReplaceValidationSummary":false}); //]]>
As you can see, all the client validation info is now rendered.
Explanation
What we've done here isn't really complicated. We're basically taking the old expression (say x=>x.Dummy.Car.Name), getting a lambda delegate for all but the last part of the old expression (so x=>x.Dummy.Car). We're then invoking the delegate with viewData.Model (which is stored in the container variable by code earlier in the method). This gives us the instance of the entity holding the property to be displayed (so in this case, Model.Dummy.Car where the property to be displayed is Model.Dummy.Car.Name). After that, we call GetType() on the instance to get the runtime type. In our coded example, this gives Person and not BasePerson. As such, the validation script rendered honours the [Required] attribute on the overridden FirstName property of the Person class.
I've also added some checks to handle null values for the Model. This can happen if an invalid string is entered for a bool, for example. In these cases, we fall back to the expression derived type. Some unit tests fail without this check. Initially, I also checked to ensure that parameter was not null, but it seems that's not required as it's handled earlier (and all the unit tests pass without this check).
And yes, this method will work on complex types with deep nesting. Not just on direct members of the Model.
Possible Problem
I can think of one area where this method may cause problems. If you're encapsulating TempData into a viewmodel to be rendered, then this code will cause a read on the TempData, causing it to expire. As such it won't get rendered onto the page. Of course, if you didn't use this approach, then the TempData would vanish immediately after rendering to the page. So why would you want to use TempData for this in the first place? Use ViewModels and ViewData for rendering. Don't use TempData in the View.
Does It Break Anything
After making the changes to the MVC 2 RC 2 source code, all existing 2,281 unit tests of the ASP.NET MVC 2 RC 2 solution still pass. As such, I'm assuming nothing is getting broken.
Does It Solve Anything Else
I don't know. If there're other places that depend on FromLambdaExpression, this should fix them as well. Other than that, can't really say.
---------------------------------------------------------
Questions and comments relating to this article are welcome.
Comments completely unrelated to the article and posted with the sole intention of putting your link here are not.
If you spam, your comment will not be approved, will be deleted and your IP blocked. I maintain my site almost daily and such comments – even if they pass the spam filter – will get removed as soon as possible. If this gets too tedious, I may disable comments entirely. Please don't ruin it for everybody else.
---------------------------------------------------------
Share or Bookmark this post…