diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ExpressionHelper.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ExpressionHelper.cs index 335166e3ea..b09d359227 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ExpressionHelper.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ExpressionHelper.cs @@ -61,11 +61,14 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal switch (part.NodeType) { case ExpressionType.Call: + // Will exit loop if at Method().Property or [i,j].Property. In that case (like [i].Property), + // don't cache and don't remove ".Model" (if that's .Property). + containsIndexers = true; + lastIsModel = false; + var methodExpression = (MethodCallExpression)part; if (IsSingleArgumentIndexer(methodExpression)) { - containsIndexers = true; - lastIsModel = false; length += "[99]".Length; part = methodExpression.Object; segmentCount++; diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ExpressionTextCache.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ExpressionTextCache.cs index f6ff6d2bd4..c9845c644e 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ExpressionTextCache.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ExpressionTextCache.cs @@ -37,6 +37,11 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal while (true) { + if (expression1 == null && expression2 == null) + { + return true; + } + if (expression1 == null || expression2 == null) { return false; @@ -47,31 +52,41 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal return false; } - if (expression1.NodeType == ExpressionType.MemberAccess) + switch (expression1.NodeType) { - var memberExpression1 = (MemberExpression)expression1; - var memberName1 = memberExpression1.Member.Name; - expression1 = memberExpression1.Expression; + case ExpressionType.MemberAccess: + var memberExpression1 = (MemberExpression)expression1; + var memberName1 = memberExpression1.Member.Name; + expression1 = memberExpression1.Expression; - var memberExpression2 = (MemberExpression)expression2; - var memberName2 = memberExpression2.Member.Name; - expression2 = memberExpression2.Expression; + var memberExpression2 = (MemberExpression)expression2; + var memberName2 = memberExpression2.Member.Name; + expression2 = memberExpression2.Expression; - // If identifier contains "__", it is "reserved for use by the implementation" and likely compiler- - // or Razor-generated e.g. the name of a field in a delegate's generated class. - if (memberName1.Contains("__") && memberName2.Contains("__")) - { - return true; - } + // If identifier contains "__", it is "reserved for use by the implementation" and likely + // compiler- or Razor-generated e.g. the name of a field in a delegate's generated class. + if (memberName1.Contains("__") && memberName2.Contains("__")) + { + return true; + } - if (!string.Equals(memberName1, memberName2, StringComparison.OrdinalIgnoreCase)) - { + if (!string.Equals(memberName1, memberName2, StringComparison.OrdinalIgnoreCase)) + { + return false; + } + break; + + case ExpressionType.ArrayIndex: + // Shouldn't be cached. Just in case, ensure indexers are all different. return false; - } - } - else - { - return true; + + case ExpressionType.Call: + // Shouldn't be cached. Just in case, ensure indexers and other calls are all different. + return false; + + default: + // Everything else terminates name generation. Haven't found a difference so far... + return true; } } } diff --git a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/ExpressionHelperTest.cs b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/ExpressionHelperTest.cs index eb06df6954..2193f33026 100644 --- a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/ExpressionHelperTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/ExpressionHelperTest.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Linq.Expressions; using Xunit; @@ -12,7 +13,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal { private readonly ExpressionTextCache _expressionTextCache = new ExpressionTextCache(); - public static IEnumerable ExpressionAndTexts + public static TheoryData ExpressionAndTexts { get { @@ -80,11 +81,35 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal (Expression, int>>)(model => model[2].PreferredCategories[i].CategoryId), "[2].PreferredCategories[3].CategoryId" }, + { + (Expression, string>>)(model => model.FirstOrDefault().Name), + "Name" + }, + { + (Expression, string>>)(model => model.FirstOrDefault().Model), + "Model" + }, + { + (Expression, int>>)(model => model.FirstOrDefault().SelectedCategory.CategoryId), + "SelectedCategory.CategoryId" + }, + { + (Expression, string>>)(model => model.FirstOrDefault().SelectedCategory.CategoryName.MainCategory), + "SelectedCategory.CategoryName.MainCategory" + }, + { + (Expression, int>>)(model => model.FirstOrDefault().PreferredCategories.Count), + "PreferredCategories.Count" + }, + { + (Expression, int>>)(model => model.FirstOrDefault().PreferredCategories.FirstOrDefault().CategoryId), + "CategoryId" + }, }; } } - public static IEnumerable CachedExpressions + public static TheoryData CachedExpressions { get { @@ -104,7 +129,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal } } - public static IEnumerable IndexerExpressions + public static TheoryData IndexerExpressions { get { @@ -123,7 +148,29 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal } } - public static IEnumerable EquivalentExpressions + public static TheoryData UnsupportedExpressions + { + get + { + var i = 2; + var j = 3; + + return new TheoryData + { + // Indexers that have multiple arguments. + (Expression>)(model => model[23][3].Name), + (Expression>)(model => model[i][3].Name), + (Expression>)(model => model[23][j].Name), + (Expression>)(model => model[i][j].Name), + // Calls that aren't indexers. + (Expression, string>>)(model => model.FirstOrDefault().Name), + (Expression, string>>)(model => model.FirstOrDefault().SelectedCategory.CategoryName.MainCategory), + (Expression, int>>)(model => model.FirstOrDefault().PreferredCategories.FirstOrDefault().CategoryId), + }; + } + } + + public static TheoryData EquivalentExpressions { get { @@ -167,7 +214,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal } } - public static IEnumerable NonEquivalentExpressions + public static TheoryData NonEquivalentExpressions { get { @@ -253,7 +300,8 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal [Theory] [MemberData(nameof(IndexerExpressions))] - public void GetExpressionText_DoesNotCacheIndexerExpression(LambdaExpression expression) + [MemberData(nameof(UnsupportedExpressions))] + public void GetExpressionText_DoesNotCacheIndexerOrUnspportedExpression(LambdaExpression expression) { // Act - 1 var text1 = ExpressionHelper.GetExpressionText(expression, _expressionTextCache);