ASP.NET MVC 3 HtmlHelper Exception does not recognize ModelMetadata on inherited interface
Asked Answered
E

6

18

After upgrading to MVC 3 RTM I get an exception where it previously worked.

Here is the scenario. I have several objects that use the same underlying interfaces IActivity and IOwned.

IActivity implements IOwned (another interface)

public interface IActivity:IOwned {...}

public interface IOwned 
{
    int? AuthorId {get;set;}
}

I have a partial view that uses IActivity for reuse from other concrete partials.

Here is the definition of the Activity Partial.

<%@ Control Language="C#" Inherits="System.Web.Mvc.ViewUserControl<IActivity>" %>
<%: Html.HiddenFor(item => item.AuthorId) %>

However, it throws an exception. It can not find AuthorId in the ModelMetadata.

I am guessing that in the previous version it looked at the interfaces IActivity implemented.

Any ideas, suggestions, short of duplicating similar interfaces everywhere?

Copied the stack trace below.

[ArgumentException: The property IActivity.AuthorId could not be found.]
   System.Web.Mvc.AssociatedMetadataProvider.GetMetadataForProperty(Func`1 modelAccessor, Type containerType, String propertyName) +498313
   System.Web.Mvc.ModelMetadata.GetMetadataFromProvider(Func`1 modelAccessor, Type modelType, String propertyName, Type containerType) +101
   System.Web.Mvc.ModelMetadata.FromLambdaExpression(Expression`1 expression, ViewDataDictionary`1 viewData) +393
   System.Web.Mvc.Html.InputExtensions.HiddenFor(HtmlHelper`1 htmlHelper, Expression`1 expression, IDictionary`2 htmlAttributes) +57
   System.Web.Mvc.Html.InputExtensions.HiddenFor(HtmlHelper`1 htmlHelper, Expression`1 expression) +51
   ASP.views_shared_activity_ascx.__Render__control1(HtmlTextWriter __w, Control parameterContainer) in c:\Users\...\Documents\Visual Studio 2010\Projects\ngen\trunk\...\Views\Shared\Activity.ascx:3
   System.Web.UI.Control.RenderChildrenInternal(HtmlTextWriter writer, ICollection children) +109
   System.Web.UI.Control.RenderChildren(HtmlTextWriter writer) +8
   System.Web.UI.Control.Render(HtmlTextWriter writer) +10
   System.Web.UI.Control.RenderControlInternal(HtmlTextWriter writer, ControlAdapter adapter) +27
   System.Web.UI.Control.RenderControl(HtmlTextWriter writer, ControlAdapter adapter) +100
   System.Web.UI.Control.RenderControl(HtmlTextWriter writer) +25
   System.Web.UI.Control.RenderChildrenInternal(HtmlTextWriter writer, ICollection children) +208
   System.Web.UI.Control.RenderChildren(HtmlTextWriter writer) +8
   System.Web.UI.Page.Render(HtmlTextWriter writer) +29
   System.Web.Mvc.ViewPage.Render(HtmlTextWriter writer) +43
   System.Web.UI.Control.RenderControlInternal(HtmlTextWriter writer, ControlAdapter adapter) +27
   System.Web.UI.Control.RenderControl(HtmlTextWriter writer, ControlAdapter adapter) +100
   System.Web.UI.Control.RenderControl(HtmlTextWriter writer) +25
   System.Web.UI.Page.ProcessRequestMain(Boolean includeStagesBeforeAsyncPoint, Boolean includeStagesAfterAsyncPoint) +3060
Experienced answered 15/1, 2011 at 22:24 Comment(2)
have you tried stepping through the framework code?Annetteannex
No, I have not step thru the Framework code. I was hoping not to ;) I think for now I can work around it by switching to <%: Html.Hidden("AuthorId") %> However this looks like a breaking change in the FX IMHO.Experienced
B
14

There has been a breaking change/bug in ASP.NET MVC 3 in the System.Web.Mvc.ModelMetadata. FromLambdaExpression method which explains the exception you are getting:

ASP.NET MVC 2.0:

...
case ExpressionType.MemberAccess:
{
    MemberExpression body = (MemberExpression) expression.Body;
    propertyName = (body.Member is PropertyInfo) ? body.Member.Name : null;
    containerType = body.Member.DeclaringType;
    flag = true;
    break;
}
...

ASP.NET MVC 3.0

...
case ExpressionType.MemberAccess:
{
    MemberExpression body = (MemberExpression) expression.Body;
    propertyName = (body.Member is PropertyInfo) ? body.Member.Name : null;
    containerType = body.Expression.Type;
    flag = true;
    break;
}
...

Notice how the containerType variable is assigned a different value. So in your case in ASP.NET MVC 2.0 it was assigned the value of IOwned which is the correct declaring type of the AuthorId property whereas in ASP.NET MVC 3.0 it is assigned to IActivity and later when the framework tries to find the property it crashes.

That's the cause. As far as the resolution is concerned I would wait for some official statement from Microsoft. I can't find any relevant information about this in the Release Notes document. Is it a bug or some feature which needs to be workarounded here?

For now you could either use the non-strongly typed Html.Hidden("AuthorId") helper or specify IOwned as type for your control (I know both suck).

Bubaline answered 15/1, 2011 at 23:28 Comment(3)
I agree. Yeah, I kinda came to the same conclusion on using the non-lambda helpers. And thanks for doing the diff on the source. Thanks again, very helpful.Experienced
@Dax70, I've submitted a ticket. You can follow it here: connect.microsoft.com/VisualStudio/feedback/details/636341/… I hope we will get some official statement or workaround.Bubaline
Wow! That change wasn't helpful. It's preventing us from upgrading to MVC 3. Hope it gets fixed soon!Amphibolous
O
15

From the MVC team:

Unfortunately, the code was actually exploiting a bug that was fixed, where the container of an expression for ModelMetadata purposes was inadvertently set to the declaring type instead of the containing type. This bug had to be fixed because of the needs of virtual properties and validation/model metadata.

Having interface-based models is not something we encourage (nor, given the limitations imposed by the bug fix, can realistically support). Switching to abstract base classes would fix the issue.

Oxen answered 30/3, 2011 at 19:53 Comment(6)
Thanks for answering the "why". I can see this change biting quite a few people. I can't think of any other situation where a lambda is passed in and it results in a property not found exception due to inheritance of interfaces.Risa
If we can prove that it's a big deal and affecting many folks, we can make a case to have it fixed/changed.Oxen
The reason for the interfaces was EF. Aside from immediately upgrading to EF 4.1 (Code First) Magic Unicorns ;). EF takes up the base class with EntityObject. And overriding the EF code gen even if it's with T4 just seems like a lot of work, interfaces seemed like a pragmatic approach to polymorphism. The workaround is fine, especially if it prevents other issues.Experienced
I'm not impressed that "Having interface-based models is not something we encourage". Am I supposed to toss the I from SOLID out the window?Alathia
I agree with @DavidAlpert. I tend to use a lot of partial views that use interfaces for their view models. The reason being it facilitates reuse of views where you would not otherwise be able to do it without a lot of mapping between view modelsGladi
I'm running into the same issue on a project that uses interfaces extensively. Casting to a base interface explicitly or adding my own ModelMetadataProvider feels like a hack to fix something that should just work in the ASP.NET MVC framework. Why can't the framework just check base interfaces last instead of not at all? Scott, I don't want to write this: @Html.HiddenFor(model => ((IUpdateEntityBase)model).UpdatedBy)Interpol
B
14

There has been a breaking change/bug in ASP.NET MVC 3 in the System.Web.Mvc.ModelMetadata. FromLambdaExpression method which explains the exception you are getting:

ASP.NET MVC 2.0:

...
case ExpressionType.MemberAccess:
{
    MemberExpression body = (MemberExpression) expression.Body;
    propertyName = (body.Member is PropertyInfo) ? body.Member.Name : null;
    containerType = body.Member.DeclaringType;
    flag = true;
    break;
}
...

ASP.NET MVC 3.0

...
case ExpressionType.MemberAccess:
{
    MemberExpression body = (MemberExpression) expression.Body;
    propertyName = (body.Member is PropertyInfo) ? body.Member.Name : null;
    containerType = body.Expression.Type;
    flag = true;
    break;
}
...

Notice how the containerType variable is assigned a different value. So in your case in ASP.NET MVC 2.0 it was assigned the value of IOwned which is the correct declaring type of the AuthorId property whereas in ASP.NET MVC 3.0 it is assigned to IActivity and later when the framework tries to find the property it crashes.

That's the cause. As far as the resolution is concerned I would wait for some official statement from Microsoft. I can't find any relevant information about this in the Release Notes document. Is it a bug or some feature which needs to be workarounded here?

For now you could either use the non-strongly typed Html.Hidden("AuthorId") helper or specify IOwned as type for your control (I know both suck).

Bubaline answered 15/1, 2011 at 23:28 Comment(3)
I agree. Yeah, I kinda came to the same conclusion on using the non-lambda helpers. And thanks for doing the diff on the source. Thanks again, very helpful.Experienced
@Dax70, I've submitted a ticket. You can follow it here: connect.microsoft.com/VisualStudio/feedback/details/636341/… I hope we will get some official statement or workaround.Bubaline
Wow! That change wasn't helpful. It's preventing us from upgrading to MVC 3. Hope it gets fixed soon!Amphibolous
E
7

With thanks to Burcephal, whos answer pointed me in the right direction

You can create a MetaDataProvider which will get around this issue, the code here adds to the code in the base class checking for the property on implemented interfaces of a model which is itself an interface.

public class MyMetadataProvider
    : EmptyModelMetadataProvider {

    public override ModelMetadata GetMetadataForProperty(
        Func<object> modelAccessor, Type containerType, string propertyName) {

        if (containerType == null) {
            throw new ArgumentNullException("containerType");
        }
        if (String.IsNullOrEmpty(propertyName)) {
            throw new ArgumentException(
                "The property &apos;{0}&apos; cannot be null or empty", "propertyName");
        }

        var property = GetTypeDescriptor(containerType)
            .GetProperties().Find(propertyName, true);
        if (property == null
            && containerType.IsInterface) {
            property = (from t in containerType.GetInterfaces()
                        let p = GetTypeDescriptor(t).GetProperties()
                            .Find(propertyName, true)
                        where p != null
                        select p
                        ).FirstOrDefault();
        }

        if (property == null) {
            throw new ArgumentException(
                String.Format(
                    CultureInfo.CurrentCulture,
                    "The property {0}.{1} could not be found",
                    containerType.FullName, propertyName));
        }

        return GetMetadataForProperty(modelAccessor, containerType, property);
    }
}

and as above, set your provider the global.asax Application_Start

ModelMetadataProviders.Current = new MyMetaDataProvider();
Eran answered 19/10, 2011 at 19:21 Comment(5)
This solved the problem in our application. If we encounter any bad side-effects, I will be sure to come back here to comment. Big Thanks!Ns
I know this is an old answer but we found it helpful at first. Unfortunately we discovered that our validation warnings lost the "friendly" name providing by using DisplayAttribute. So rather than seeing "The field Location is required", they might see something like "The field MyPropertyName is required." Not sure what the workaround is so we're going to use abstract classes rather than interfaces. Just a heads up to future visitors.Gropius
@Gropius could you elaborate on your comment? I've tried patching the MetaDataProvider to workaround the interface issue and it appears to be working with DisplayAttribute (though I'm not complaining!).Belch
@PhilCooper I'd have to go back and setup a test project to revisit this. Possible it's been addressed in subsequent updates (either to MVC specifically or to the runtime more broadly). Not sure I'll get to it but I will let you know if I do.Gropius
@Gropius yep it's possible. Good answer and comments though, I've implemented this workaround and it's working fine.Belch
P
6

f you are interrested, I have implemented a little work around for it in my app. As I was going through the MVC source code, I found that the FromLambdaExpression method named below will be calling the MetaDataProvider which is an overridable singleton. So we could just implement that class that will actually try the inhereted intefaces if the first one does not work. It will also go up the tree of intefaces.

public class MyMetaDataProvider : EmptyModelMetadataProvider
{
    public override ModelMetadata GetMetadataForProperty(Func<object> modelAccessor, Type containerType, string propertyName)
    {
        try
        {
            return base.GetMetadataForProperty(modelAccessor, containerType, propertyName);
        }
         catch(Exception ex)
        {
            //Try to go up to type tree
            var types = containerType.GetInterfaces();              
            foreach (var container in types)
            {
                if (container.GetProperty(propertyName) != null)
                {
                    try
                    {
                        return GetMetadataForProperty(modelAccessor, container, propertyName);
                    }
                    catch
                    {
                        //This interface did not work
                    }
                }
            }               
            //If nothing works, then throw the exception
            throw ex;
        }              
    }
}

and then, just cange the implementation of the MetaDataProvider in the global.asax Application_Start()

ModelMetadataProviders.Current = new MyMetaDataProvider();

It is not the best code ever, but it does the job.

Pervious answered 6/9, 2011 at 18:21 Comment(0)
A
5

Try using typecast. It works for my project although resharper highlights it as redundant.

For your code the solution would be

<%@ Control Language="C#" Inherits="System.Web.Mvc.ViewUserControl<IActivity>" %>
<%: Html.HiddenFor(item => ((IOwned)item).AuthorId) %>
Anhanhalt answered 21/3, 2011 at 7:56 Comment(0)
S
0

Furthering Anthony Johnston's answer, you might find that you get exceptions when using DataAnnotations, as the AssociatedValidatorProvider.GetValidatorsForProperty() method will try to use the inheriting interface as the container type rather than the base one and therefore fail to find the property again.

This is the reflected code from the GetValidatorsForProperty method (it is the second line which causes the propertyDescriptor variable to be null and thus the exception to be thrown) :

private IEnumerable<ModelValidator> GetValidatorsForProperty(ModelMetadata metadata, ControllerContext context)
{
    ICustomTypeDescriptor typeDescriptor = this.GetTypeDescriptor(metadata.ContainerType);
    PropertyDescriptor propertyDescriptor = typeDescriptor.GetProperties().Find(metadata.PropertyName, true);
    if (propertyDescriptor != null)
    {
        return this.GetValidators(metadata, context, propertyDescriptor.Attributes.OfType<Attribute>());
    }
    else
    {
        object[] fullName = new object[] { metadata.ContainerType.FullName, metadata.PropertyName };
        throw new ArgumentException(string.Format(CultureInfo.CurrentCulture, MvcResources.Common_PropertyNotFound, fullName), "metadata");
    }
}

If so, I believe that the following code may help, as it makes sure that the ContainerType is set to the type which the property is on, not the type of the view model.

Disclaimer: It seems to work fine, but I have not fully tested it yet so it might have undesired effects! I also understand it is not written perfectly, but I was trying to keep the format similar to the previous answer for ease of comparison. :)

public class MyMetadataProvider : DataAnnotationsModelMetadataProvider
{
    public override ModelMetadata GetMetadataForProperty(
        Func<object> modelAccessor, Type containerType, string propertyName)
    {

        if (containerType == null)
        {
            throw new ArgumentNullException("containerType");
        }
        if (String.IsNullOrEmpty(propertyName))
        {
            throw new ArgumentException(
                "The property &apos;{0}&apos; cannot be null or empty", "propertyName");
        }

        var containerTypeToUse = containerType;

        var property = GetTypeDescriptor(containerType)
            .GetProperties().Find(propertyName, true);
        if (property == null
            && containerType.IsInterface)
        {

            var foundProperty = (from t in containerType.GetInterfaces()
                        let p = GetTypeDescriptor(t).GetProperties()
                            .Find(propertyName, true)
                        where p != null
                        select (new Tuple<System.ComponentModel.PropertyDescriptor, Type>(p, t))
                        ).FirstOrDefault();

            if (foundProperty != null)
            {
                property = foundProperty.Item1;
                containerTypeToUse = foundProperty.Item2;
            }
        }


        if (property == null)
        {
            throw new ArgumentException(
                String.Format(
                    CultureInfo.CurrentCulture,
                    "The property {0}.{1} could not be found",
                    containerType.FullName, propertyName));
        }

        return GetMetadataForProperty(modelAccessor, containerTypeToUse, property);
    }
}
Sinkage answered 4/4, 2013 at 9:2 Comment(1)
Because I came across the problem and thought it useful to share a potential solution.Sinkage

© 2022 - 2024 — McMap. All rights reserved.