diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ExpressionMetadataProvider.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ExpressionMetadataProvider.cs index 0649c8033c..38d89f80bc 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ExpressionMetadataProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ExpressionMetadataProvider.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Diagnostics; using System.Globalization; using System.Linq.Expressions; using System.Reflection; @@ -60,7 +61,12 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal return FromModel(viewData, metadataProvider); } - containerType = memberExpression.Expression.Type; + // memberExpression.Expression can be null when this is a static field or property. + // + // This can be the case if the expression is like (m => Person.Name) where Name is a static field + // or property on the Person type. + containerType = memberExpression.Expression?.Type; + legalExpression = true; break; @@ -86,16 +92,8 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal } }; - ModelMetadata metadata; - if (propertyName == null) - { - // Ex: - // m => 5 (arbitrary expression) - // m => foo (arbitrary expression) - // m => m.Widgets[0] (expression ending with non-property-access) - metadata = metadataProvider.GetMetadataForType(typeof(TResult)); - } - else + ModelMetadata metadata = null; + if (containerType != null && propertyName != null) { // Ex: // m => m.Color (simple property access) @@ -103,6 +101,22 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal // m => m.Widgets[0].Size (expression ending with property-access) metadata = metadataProvider.GetMetadataForType(containerType).Properties[propertyName]; } + + if (metadata == null) + { + // Ex: + // m => 5 (arbitrary expression) + // m => foo (arbitrary expression) + // m => m.Widgets[0] (expression ending with non-property-access) + // + // This can also happen for any case where we cannot retrieve a model metadata. + // This will happen for: + // - fields + // - statics + // - non-visibility (internal/private) + metadata = metadataProvider.GetMetadataForType(typeof(TResult)); + Debug.Assert(metadata != null); + } return viewData.ModelExplorer.GetExplorerForExpression(metadata, modelAccessor); } diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/HtmlGenerationTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/HtmlGenerationTest.cs index 3773ced1be..d2d6a1349d 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/HtmlGenerationTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/HtmlGenerationTest.cs @@ -523,6 +523,32 @@ Products: Music Systems, Televisions (3)"; Assert.Equal(expected, response, ignoreLineEndingDifferences: true); } + // We want to make sure that for 'wierd' model expressions involving: + // - fields + // - statics + // - private + // + // These tests verify that we don't throw, and can evaluate the expression to get the model + // value. One quirk of behavior for these cases is that we can't return a correct model metadata + // instance (this is true for anything other than a public instance property). We're not overly + // concerned with that, and so the accuracy of the model metadata is is not verified by the test. + [Theory] + [InlineData("GetWeirdWithHtmlHelpers")] + [InlineData("GetWeirdWithTagHelpers")] + public async Task WeirdModelExpressions_CanAccessModelValues(string action) + { + // Arrange + var url = "http://localhost/HtmlGeneration_WeirdExpressions/" + action; + + // Act + var response = await Client.GetStringAsync(url); + + // Assert + Assert.Contains("Hello, Field World!", response); + Assert.Contains("Hello, Static World!", response); + Assert.Contains("Hello, Private World!", response); + } + private static HttpRequestMessage RequestWithLocale(string url, string locale) { var request = new HttpRequestMessage(HttpMethod.Get, url); diff --git a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/ExpressionMetadataProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/ExpressionMetadataProviderTest.cs index 2b912cfa67..44d421c376 100644 --- a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/ExpressionMetadataProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/ExpressionMetadataProviderTest.cs @@ -10,6 +10,12 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal { public class ExpressionMetadataProviderTest { + private string PrivateProperty { get; set; } + + public static string StaticProperty { get; set; } + + public string Field = "Hello"; + [Fact] public void FromLambdaExpression_GetsExpectedMetadata_ForIdentityExpression() { @@ -148,6 +154,72 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal Assert.Same(myModel, metadata.Container.Model); } + // A private property can't be found by the model metadata provider, so return the property's type + // as a best-effort mechanism. + [Fact] + public void FromLambdaExpression_ForPrivateProperty_ReturnsMetadataOfExpressionType() + { + // Arrange + var provider = new EmptyModelMetadataProvider(); + var viewData = new ViewDataDictionary(provider); + + // Act + var explorer = ExpressionMetadataProvider.FromLambdaExpression( + m => m.PrivateProperty, + viewData, + provider); + + // Assert + Assert.NotNull(explorer); + Assert.NotNull(explorer.Metadata); + Assert.Equal(ModelMetadataKind.Type, explorer.Metadata.MetadataKind); + Assert.Equal(typeof(string), explorer.ModelType); + } + + // A static property can't be found by the model metadata provider, so return the property's type + // as a best-effort mechanism. + [Fact] + public void FromLambdaExpression_ForStaticProperty_ReturnsMetadataOfExpressionType() + { + // Arrange + var provider = new EmptyModelMetadataProvider(); + var viewData = new ViewDataDictionary(provider); + + // Act + var explorer = ExpressionMetadataProvider.FromLambdaExpression( + m => ExpressionMetadataProviderTest.StaticProperty, + viewData, + provider); + + // Assert + Assert.NotNull(explorer); + Assert.NotNull(explorer.Metadata); + Assert.Equal(ModelMetadataKind.Type, explorer.Metadata.MetadataKind); + Assert.Equal(typeof(string), explorer.ModelType); + } + + // A field can't be found by the model metadata provider, so return the field's type + // as a best-effort mechanism. + [Fact] + public void FromLambdaExpression_ForField_ReturnsMetadataOfExpressionType() + { + // Arrange + var provider = new EmptyModelMetadataProvider(); + var viewData = new ViewDataDictionary(provider); + + // Act + var explorer = ExpressionMetadataProvider.FromLambdaExpression( + m => m.Field, + viewData, + provider); + + // Assert + Assert.NotNull(explorer); + Assert.NotNull(explorer.Metadata); + Assert.Equal(ModelMetadataKind.Type, explorer.Metadata.MetadataKind); + Assert.Equal(typeof(string), explorer.ModelType); + } + private class TestModel { public Category SelectedCategory { get; set; } diff --git a/test/WebSites/HtmlGenerationWebSite/Controllers/HtmlGeneration_WeirdExpressions.cs b/test/WebSites/HtmlGenerationWebSite/Controllers/HtmlGeneration_WeirdExpressions.cs new file mode 100644 index 0000000000..7281b6cce1 --- /dev/null +++ b/test/WebSites/HtmlGenerationWebSite/Controllers/HtmlGeneration_WeirdExpressions.cs @@ -0,0 +1,22 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using HtmlGenerationWebSite.Models; +using Microsoft.AspNetCore.Mvc; + +namespace HtmlGenerationWebSite.Controllers +{ + public class HtmlGeneration_WeirdExpressionsController : Controller + { + public IActionResult GetWeirdWithHtmlHelpers() + { + return View(new WeirdModel()); + } + + public IActionResult GetWeirdWithTagHelpers() + { + return View(new WeirdModel()); + } + } +} \ No newline at end of file diff --git a/test/WebSites/HtmlGenerationWebSite/Models/WeirdModel.cs b/test/WebSites/HtmlGenerationWebSite/Models/WeirdModel.cs new file mode 100644 index 0000000000..8dce437889 --- /dev/null +++ b/test/WebSites/HtmlGenerationWebSite/Models/WeirdModel.cs @@ -0,0 +1,14 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.ComponentModel.DataAnnotations; + +namespace HtmlGenerationWebSite.Models +{ + public class WeirdModel + { + public string Field = "Hello, Field World!"; + + public static string StaticProperty { get; set; } = "Hello, Static World!"; + } +} \ No newline at end of file diff --git a/test/WebSites/HtmlGenerationWebSite/Views/HtmlGeneration_WeirdExpressions/GetWeirdWithHtmlHelpers.cshtml b/test/WebSites/HtmlGenerationWebSite/Views/HtmlGeneration_WeirdExpressions/GetWeirdWithHtmlHelpers.cshtml new file mode 100644 index 0000000000..f192c5d84f --- /dev/null +++ b/test/WebSites/HtmlGenerationWebSite/Views/HtmlGeneration_WeirdExpressions/GetWeirdWithHtmlHelpers.cshtml @@ -0,0 +1,27 @@ +@addTagHelper *, Microsoft.AspNetCore.Mvc.TagHelpers +@using HtmlGenerationWebSite.Models +@model WeirdModel + +@functions{ + private string PrivateProperty { get; set; } = "Hello, Private World!"; +} + + + +
+

Field

+ @Html.LabelFor(m => m.Field) + @Html.TextBoxFor(m => m.Field) +
+
+

Static Property

+ @Html.LabelFor(m => WeirdModel.StaticProperty) + @Html.TextBoxFor(m => WeirdModel.StaticProperty) +
+
+

Private Property

+ @Html.LabelFor(m => PrivateProperty) + @Html.TextBoxFor(m => PrivateProperty) +
+ + \ No newline at end of file diff --git a/test/WebSites/HtmlGenerationWebSite/Views/HtmlGeneration_WeirdExpressions/GetWeirdWithTagHelpers.cshtml b/test/WebSites/HtmlGenerationWebSite/Views/HtmlGeneration_WeirdExpressions/GetWeirdWithTagHelpers.cshtml new file mode 100644 index 0000000000..8bf8575cdc --- /dev/null +++ b/test/WebSites/HtmlGenerationWebSite/Views/HtmlGeneration_WeirdExpressions/GetWeirdWithTagHelpers.cshtml @@ -0,0 +1,27 @@ +@addTagHelper *, Microsoft.AspNetCore.Mvc.TagHelpers +@using HtmlGenerationWebSite.Models +@model WeirdModel + +@functions{ + private string PrivateProperty { get; set; } = "Hello, Private World!"; +} + + + +
+

Field

+
+
+

Static Property

+
+
+

Private Property

+
+ + \ No newline at end of file