From 926034ac4eed763cf7baf4acaeccba3272738d46 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Sat, 8 Dec 2018 13:54:05 +0530 Subject: [PATCH] Re-enable extensibility points for ExpressionHelper.GetExpressionText, ExpressionMetadataProvider.FromStringExpression (#4494) * Re-enable extensibility points for ExpressionHelper.GetExpressionText, ExpressionMetadataProvider.FromStringExpression Fixes https://github.com/aspnet/Mvc/issues/8724 --- ...MvcViewFeaturesMvcCoreBuilderExtensions.cs | 5 +- .../ExpressionHelper.cs | 21 +-- .../ExpressionTextCache.cs | 124 ---------------- .../HtmlHelper.cs | 10 +- .../HtmlHelperOfT.cs | 133 +++++++++--------- .../LambdaExpressionComparer.cs | 113 +++++++++++++++ .../ModelExpressionProvider.cs | 66 +++++++-- .../ModelStateDictionaryExtensions.cs | 4 +- .../RazorPageActivatorTest.cs | 3 +- .../RazorPageCreateModelExpressionTest.cs | 5 +- .../RazorPageCreateTagHelperTest.cs | 2 +- .../ExpressionHelperTest.cs | 3 +- .../Rendering/DefaultTemplatesUtilities.cs | 4 +- .../ViewComponentResultTest.cs | 1 - 14 files changed, 258 insertions(+), 236 deletions(-) delete mode 100644 src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ExpressionTextCache.cs create mode 100644 src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/LambdaExpressionComparer.cs diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/DependencyInjection/MvcViewFeaturesMvcCoreBuilderExtensions.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/DependencyInjection/MvcViewFeaturesMvcCoreBuilderExtensions.cs index 01457ce718..e08abb884f 100644 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/DependencyInjection/MvcViewFeaturesMvcCoreBuilderExtensions.cs +++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/DependencyInjection/MvcViewFeaturesMvcCoreBuilderExtensions.cs @@ -166,8 +166,9 @@ namespace Microsoft.Extensions.DependencyInjection services.TryAddTransient(); services.TryAddTransient(typeof(IHtmlHelper<>), typeof(HtmlHelper<>)); services.TryAddSingleton(); - services.TryAddSingleton(); - services.TryAddSingleton(); + services.TryAddSingleton(); + // ModelExpressionProvider caches results. Ensure that it's re-used when the requested type is IModelExpressionProvider. + services.TryAddSingleton(s => s.GetRequiredService()); services.TryAddSingleton(); // diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ExpressionHelper.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ExpressionHelper.cs index aa2bd0df4e..8711728b51 100644 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ExpressionHelper.cs +++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ExpressionHelper.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.Collections.Concurrent; using System.Diagnostics; using System.Globalization; using System.Linq; @@ -13,18 +14,10 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures { internal static class ExpressionHelper { - public static string GetExpressionText(string expression) - { - // If it's exactly "model", then give them an empty string, to replicate the lambda behavior. - return string.Equals(expression, "model", StringComparison.OrdinalIgnoreCase) ? string.Empty : expression; - } + public static string GetUncachedExpressionText(LambdaExpression expression) + => GetExpressionText(expression, expressionTextCache: null); - public static string GetExpressionText(LambdaExpression expression) - { - return GetExpressionText(expression, expressionTextCache: null); - } - - public static string GetExpressionText(LambdaExpression expression, ExpressionTextCache expressionTextCache) + public static string GetExpressionText(LambdaExpression expression, ConcurrentDictionary expressionTextCache) { if (expression == null) { @@ -32,7 +25,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures } if (expressionTextCache != null && - expressionTextCache.Entries.TryGetValue(expression, out var expressionText)) + expressionTextCache.TryGetValue(expression, out var expressionText)) { return expressionText; } @@ -145,7 +138,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures Debug.Assert(!doNotCache); if (expressionTextCache != null) { - expressionTextCache.Entries.TryAdd(expression, string.Empty); + expressionTextCache.TryAdd(expression, string.Empty); } return string.Empty; @@ -202,7 +195,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures expressionText = builder.ToString(); if (expressionTextCache != null && !doNotCache) { - expressionTextCache.Entries.TryAdd(expression, expressionText); + expressionTextCache.TryAdd(expression, expressionText); } return expressionText; diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ExpressionTextCache.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ExpressionTextCache.cs deleted file mode 100644 index 6468769363..0000000000 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ExpressionTextCache.cs +++ /dev/null @@ -1,124 +0,0 @@ -// 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 System.Collections.Concurrent; -using System.Collections.Generic; -using System.Linq.Expressions; -using Microsoft.Extensions.Internal; - -namespace Microsoft.AspNetCore.Mvc.ViewFeatures -{ - /// - /// This class holds the cache for the expression text that is computed by ExpressionHelper. - /// - public class ExpressionTextCache - { - /// - public ConcurrentDictionary Entries { get; } = - new ConcurrentDictionary(LambdaExpressionComparer.Instance); - - // This comparer is tightly coupled with the logic of ExpressionHelper.GetExpressionText. - // It is not designed to accurately compare any two arbitrary LambdaExpressions. - private class LambdaExpressionComparer : IEqualityComparer - { - public static readonly LambdaExpressionComparer Instance = new LambdaExpressionComparer(); - - public bool Equals(LambdaExpression lambdaExpression1, LambdaExpression lambdaExpression2) - { - if (ReferenceEquals(lambdaExpression1,lambdaExpression2)) - { - return true; - } - // We will cache only pure member access expressions. Hence we compare two expressions - // to be equal only if they are identical member access expressions. - var expression1 = lambdaExpression1.Body; - var expression2 = lambdaExpression2.Body; - - while (true) - { - if (expression1 == null && expression2 == null) - { - return true; - } - - if (expression1 == null || expression2 == null) - { - return false; - } - - if (expression1.NodeType != expression2.NodeType) - { - return false; - } - - switch (expression1.NodeType) - { - 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; - - // 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.Ordinal)) - { - return false; - } - break; - - case ExpressionType.ArrayIndex: - // Shouldn't be cached. Just in case, ensure indexers are all different. - return false; - - 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; - } - } - } - - public int GetHashCode(LambdaExpression lambdaExpression) - { - var expression = lambdaExpression.Body; - var hashCodeCombiner = HashCodeCombiner.Start(); - - while (true) - { - if (expression != null && expression.NodeType == ExpressionType.MemberAccess) - { - var memberExpression = (MemberExpression)expression; - var memberName = memberExpression.Member.Name; - - if (memberName.Contains("__")) - { - break; - } - - hashCodeCombiner.Add(memberName, StringComparer.Ordinal); - expression = memberExpression.Expression; - } - else - { - break; - } - } - - return hashCodeCombiner.CombinedHash; - } - } - } -} diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/HtmlHelper.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/HtmlHelper.cs index 8d5c7554ed..016246f4a4 100644 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/HtmlHelper.cs +++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/HtmlHelper.cs @@ -321,7 +321,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures return GenerateDisplay( metadata, - htmlFieldName ?? ExpressionHelper.GetExpressionText(expression), + htmlFieldName ?? GetExpressionText(expression), templateName, additionalViewData); } @@ -366,7 +366,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures return GenerateEditor( modelExplorer, - htmlFieldName ?? ExpressionHelper.GetExpressionText(expression), + htmlFieldName ?? GetExpressionText(expression), templateName, additionalViewData); } @@ -1246,5 +1246,11 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures return selectList; } + + private static string GetExpressionText(string expression) + { + // If it's exactly "model", then give them an empty string, to replicate the lambda behavior. + return string.Equals(expression, "model", StringComparison.OrdinalIgnoreCase) ? string.Empty : expression; + } } } diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/HtmlHelperOfT.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/HtmlHelperOfT.cs index 34df7e0068..9dda5426aa 100644 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/HtmlHelperOfT.cs +++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/HtmlHelperOfT.cs @@ -15,7 +15,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures { public class HtmlHelper : HtmlHelper, IHtmlHelper { - private readonly ExpressionTextCache _expressionTextCache; + private readonly ModelExpressionProvider _modelExpressionProvider; /// /// Initializes a new instance of the class. @@ -27,7 +27,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures IViewBufferScope bufferScope, HtmlEncoder htmlEncoder, UrlEncoder urlEncoder, - ExpressionTextCache expressionTextCache) + ModelExpressionProvider modelExpressionProvider) : base( htmlGenerator, viewEngine, @@ -36,12 +36,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures htmlEncoder, urlEncoder) { - if (expressionTextCache == null) - { - throw new ArgumentNullException(nameof(expressionTextCache)); - } - - _expressionTextCache = expressionTextCache; + _modelExpressionProvider = modelExpressionProvider ?? throw new ArgumentNullException(nameof(modelExpressionProvider)); } /// @@ -102,10 +97,10 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures throw new ArgumentNullException(nameof(expression)); } - var modelExplorer = GetModelExplorer(expression); + var modelExpression = GetModelExpression(expression); return GenerateCheckBox( - modelExplorer, - GetExpressionName(expression), + modelExpression.ModelExplorer, + modelExpression.Name, isChecked: null, htmlAttributes: htmlAttributes); } @@ -122,10 +117,10 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures throw new ArgumentNullException(nameof(expression)); } - var modelExplorer = GetModelExplorer(expression); + var modelExpression = GetModelExpression(expression); return GenerateDropDown( - modelExplorer, - GetExpressionName(expression), + modelExpression.ModelExplorer, + modelExpression.Name, selectList, optionLabel, htmlAttributes); @@ -143,10 +138,10 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures throw new ArgumentNullException(nameof(expression)); } - var modelExplorer = GetModelExplorer(expression); + var modelExpression = GetModelExpression(expression); return GenerateDisplay( - modelExplorer, - htmlFieldName ?? GetExpressionName(expression), + modelExpression.ModelExplorer, + htmlFieldName ?? modelExpression.Name, templateName, additionalViewData); } @@ -159,8 +154,8 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures throw new ArgumentNullException(nameof(expression)); } - var modelExplorer = GetModelExplorer(expression); - return GenerateDisplayName(modelExplorer, GetExpressionName(expression)); + var modelExpression = GetModelExpression(expression); + return GenerateDisplayName(modelExpression.ModelExplorer, modelExpression.Name); } /// @@ -172,18 +167,11 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures throw new ArgumentNullException(nameof(expression)); } - var modelExplorer = ExpressionMetadataProvider.FromLambdaExpression( - expression, + var modelExpression = _modelExpressionProvider.CreateModelExpression( new ViewDataDictionary(ViewData, model: null), - MetadataProvider); + expression); - var expressionText = ExpressionHelper.GetExpressionText(expression, _expressionTextCache); - if (modelExplorer == null) - { - throw new InvalidOperationException(Resources.FormatHtmlHelper_NullModelMetadata(expressionText)); - } - - return GenerateDisplayName(modelExplorer, expressionText); + return GenerateDisplayName(modelExpression.ModelExplorer, modelExpression.Name); } /// @@ -209,10 +197,10 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures throw new ArgumentNullException(nameof(expression)); } - var modelExplorer = GetModelExplorer(expression); + var modelExpression = GetModelExpression(expression); return GenerateEditor( - modelExplorer, - htmlFieldName ?? GetExpressionName(expression), + modelExpression.ModelExplorer, + htmlFieldName ?? modelExpression.Name, templateName, additionalViewData); } @@ -227,11 +215,11 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures throw new ArgumentNullException(nameof(expression)); } - var modelExplorer = GetModelExplorer(expression); + var modelExpression = GetModelExpression(expression); return GenerateHidden( - modelExplorer, - GetExpressionName(expression), - modelExplorer.Model, + modelExpression.ModelExplorer, + modelExpression.Name, + modelExpression.Model, useViewData: false, htmlAttributes: htmlAttributes); } @@ -258,8 +246,8 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures throw new ArgumentNullException(nameof(expression)); } - var modelExplorer = GetModelExplorer(expression); - return GenerateLabel(modelExplorer, GetExpressionName(expression), labelText, htmlAttributes); + var modelExpression = GetModelExpression(expression); + return GenerateLabel(modelExpression.ModelExplorer, modelExpression.Name, labelText, htmlAttributes); } /// @@ -273,10 +261,10 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures throw new ArgumentNullException(nameof(expression)); } - var modelExplorer = GetModelExplorer(expression); - var name = GetExpressionName(expression); + var modelExpression = GetModelExpression(expression); + var name = modelExpression.Name; - return GenerateListBox(modelExplorer, name, selectList, htmlAttributes); + return GenerateListBox(modelExpression.ModelExplorer, name, selectList, htmlAttributes); } /// @@ -301,10 +289,10 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures throw new ArgumentNullException(nameof(expression)); } - var modelExplorer = GetModelExplorer(expression); + var modelExpression = GetModelExpression(expression); return GeneratePassword( - modelExplorer, - GetExpressionName(expression), + modelExpression.ModelExplorer, + modelExpression.Name, value: null, htmlAttributes: htmlAttributes); } @@ -325,10 +313,10 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures throw new ArgumentNullException(nameof(value)); } - var modelExplorer = GetModelExplorer(expression); + var modelExpression = GetModelExpression(expression); return GenerateRadioButton( - modelExplorer, - GetExpressionName(expression), + modelExpression.ModelExplorer, + modelExpression.Name, value, isChecked: null, htmlAttributes: htmlAttributes); @@ -346,8 +334,8 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures throw new ArgumentNullException(nameof(expression)); } - var modelExplorer = GetModelExplorer(expression); - return GenerateTextArea(modelExplorer, GetExpressionName(expression), rows, columns, htmlAttributes); + var modelExpression = GetModelExpression(expression); + return GenerateTextArea(modelExpression.ModelExplorer, modelExpression.Name, rows, columns, htmlAttributes); } /// @@ -361,15 +349,25 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures throw new ArgumentNullException(nameof(expression)); } - var modelExplorer = GetModelExplorer(expression); + var modelExpression = GetModelExpression(expression); return GenerateTextBox( - modelExplorer, - GetExpressionName(expression), - modelExplorer.Model, + modelExpression.ModelExplorer, + modelExpression.Name, + modelExpression.Model, format, htmlAttributes); } + private ModelExpression GetModelExpression(Expression> expression) + { + return _modelExpressionProvider.CreateModelExpression(ViewData, expression); + } + + /// + /// Gets the name for . + /// + /// The expression. + /// The expression name. protected string GetExpressionName(Expression> expression) { if (expression == null) @@ -377,9 +375,15 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures throw new ArgumentNullException(nameof(expression)); } - return ExpressionHelper.GetExpressionText(expression, _expressionTextCache); + return _modelExpressionProvider.GetExpressionText(expression); } + /// + /// Gets the for . + /// + /// The type of the result. + /// The expression. + /// The . protected ModelExplorer GetModelExplorer(Expression> expression) { if (expression == null) @@ -387,15 +391,8 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures throw new ArgumentNullException(nameof(expression)); } - var modelExplorer = - ExpressionMetadataProvider.FromLambdaExpression(expression, ViewData, MetadataProvider); - if (modelExplorer == null) - { - var expressionName = GetExpressionName(expression); - throw new InvalidOperationException(Resources.FormatHtmlHelper_NullModelMetadata(expressionName)); - } - - return modelExplorer; + var modelExpression = GetModelExpression(expression); + return modelExpression.ModelExplorer; } /// @@ -410,10 +407,10 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures throw new ArgumentNullException(nameof(expression)); } - var modelExplorer = GetModelExplorer(expression); + var modelExpression = GetModelExpression(expression); return GenerateValidationMessage( - modelExplorer, - GetExpressionName(expression), + modelExpression.ModelExplorer, + modelExpression.Name, message, tag, htmlAttributes); @@ -427,8 +424,8 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures throw new ArgumentNullException(nameof(expression)); } - var modelExplorer = GetModelExplorer(expression); - return GenerateValue(GetExpressionName(expression), modelExplorer.Model, format, useViewData: false); + var modelExpression = GetModelExpression(expression); + return GenerateValue(modelExpression.Name, modelExpression.Model, format, useViewData: false); } } } diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/LambdaExpressionComparer.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/LambdaExpressionComparer.cs new file mode 100644 index 0000000000..1e0dcc7fe7 --- /dev/null +++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/LambdaExpressionComparer.cs @@ -0,0 +1,113 @@ +// 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 System.Collections.Generic; +using System.Linq.Expressions; +using Microsoft.Extensions.Internal; + +namespace Microsoft.AspNetCore.Mvc.ViewFeatures +{ + // This comparer is tightly coupled with the logic of ExpressionHelper.GetExpressionText. + // It is not designed to accurately compare any two arbitrary LambdaExpressions. + internal class LambdaExpressionComparer : IEqualityComparer + { + public static readonly LambdaExpressionComparer Instance = new LambdaExpressionComparer(); + + public bool Equals(LambdaExpression lambdaExpression1, LambdaExpression lambdaExpression2) + { + if (ReferenceEquals(lambdaExpression1, lambdaExpression2)) + { + return true; + } + // We will cache only pure member access expressions. Hence we compare two expressions + // to be equal only if they are identical member access expressions. + var expression1 = lambdaExpression1.Body; + var expression2 = lambdaExpression2.Body; + + while (true) + { + if (expression1 == null && expression2 == null) + { + return true; + } + + if (expression1 == null || expression2 == null) + { + return false; + } + + if (expression1.NodeType != expression2.NodeType) + { + return false; + } + + switch (expression1.NodeType) + { + 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; + + // 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.Ordinal)) + { + return false; + } + break; + + case ExpressionType.ArrayIndex: + // Shouldn't be cached. Just in case, ensure indexers are all different. + return false; + + 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; + } + } + } + + public int GetHashCode(LambdaExpression lambdaExpression) + { + var expression = lambdaExpression.Body; + var hashCodeCombiner = HashCodeCombiner.Start(); + + while (true) + { + if (expression != null && expression.NodeType == ExpressionType.MemberAccess) + { + var memberExpression = (MemberExpression)expression; + var memberName = memberExpression.Member.Name; + + if (memberName.Contains("__")) + { + break; + } + + hashCodeCombiner.Add(memberName, StringComparer.Ordinal); + expression = memberExpression.Expression; + } + else + { + break; + } + } + + return hashCodeCombiner.CombinedHash; + } + } +} diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ModelExpressionProvider.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ModelExpressionProvider.cs index 9d4c73b745..10b0f54551 100644 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ModelExpressionProvider.cs +++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ModelExpressionProvider.cs @@ -2,40 +2,50 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Concurrent; using System.Linq.Expressions; using Microsoft.AspNetCore.Mvc.ModelBinding; namespace Microsoft.AspNetCore.Mvc.ViewFeatures { /// - /// A default implementation of . + /// Provides for expressions. /// public class ModelExpressionProvider : IModelExpressionProvider { private readonly IModelMetadataProvider _modelMetadataProvider; - private readonly ExpressionTextCache _expressionTextCache; + private readonly ConcurrentDictionary _expressionTextCache; /// - /// Creates a new . + /// Creates a new . /// /// The . - /// The . - public ModelExpressionProvider( - IModelMetadataProvider modelMetadataProvider, - ExpressionTextCache expressionTextCache) + public ModelExpressionProvider(IModelMetadataProvider modelMetadataProvider) { if (modelMetadataProvider == null) { throw new ArgumentNullException(nameof(modelMetadataProvider)); } - if (expressionTextCache == null) + _modelMetadataProvider = modelMetadataProvider; + _expressionTextCache = new ConcurrentDictionary(LambdaExpressionComparer.Instance); + } + + /// + /// Gets the name for . + /// + /// The model type. + /// The type of the result. + /// The expression. + /// The expression name. + public string GetExpressionText(Expression> expression) + { + if (expression == null) { - throw new ArgumentNullException(nameof(expressionTextCache)); + throw new ArgumentNullException(nameof(expression)); } - _modelMetadataProvider = modelMetadataProvider; - _expressionTextCache = expressionTextCache; + return ExpressionHelper.GetExpressionText(expression, _expressionTextCache); } /// @@ -53,7 +63,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures throw new ArgumentNullException(nameof(expression)); } - var name = ExpressionHelper.GetExpressionText(expression, _expressionTextCache); + var name = GetExpressionText(expression); var modelExplorer = ExpressionMetadataProvider.FromLambdaExpression(expression, viewData, _modelMetadataProvider); if (modelExplorer == null) { @@ -63,5 +73,37 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures return new ModelExpression(name, modelExplorer); } + + /// + /// Returns a instance describing the given . + /// + /// The type of the 's . + /// The containing the + /// against which is evaluated. + /// Expression name, relative to viewData.Model. + /// A new instance describing the given . + public ModelExpression CreateModelExpression( + ViewDataDictionary viewData, + string expression) + { + if (viewData == null) + { + throw new ArgumentNullException(nameof(viewData)); + } + + if (expression == null) + { + throw new ArgumentNullException(nameof(expression)); + } + + var modelExplorer = ExpressionMetadataProvider.FromStringExpression(expression, viewData, _modelMetadataProvider); + if (modelExplorer == null) + { + throw new InvalidOperationException( + Resources.FormatCreateModelExpression_NullModelMetadata(nameof(IModelMetadataProvider), expression)); + } + + return new ModelExpression(expression, modelExplorer); + } } } diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ModelStateDictionaryExtensions.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ModelStateDictionaryExtensions.cs index a80479e75d..b075c88818 100644 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ModelStateDictionaryExtensions.cs +++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ModelStateDictionaryExtensions.cs @@ -197,12 +197,12 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding if (IsConversionToObject(unaryExpression)) { - return ExpressionHelper.GetExpressionText(Expression.Lambda( + return ExpressionHelper.GetUncachedExpressionText(Expression.Lambda( unaryExpression.Operand, expression.Parameters[0])); } - return ExpressionHelper.GetExpressionText(expression); + return ExpressionHelper.GetUncachedExpressionText(expression); } private static bool IsConversionToObject(UnaryExpression expression) diff --git a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorPageActivatorTest.cs b/src/Mvc/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorPageActivatorTest.cs index 25f6072395..f4e3a8d97a 100644 --- a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorPageActivatorTest.cs +++ b/src/Mvc/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorPageActivatorTest.cs @@ -30,7 +30,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor HtmlEncoder = new HtmlTestEncoder(); JsonHelper = Mock.Of(); MetadataProvider = new EmptyModelMetadataProvider(); - ModelExpressionProvider = new ModelExpressionProvider(MetadataProvider, new ExpressionTextCache()); + ModelExpressionProvider = new ModelExpressionProvider(MetadataProvider); UrlHelperFactory = new UrlHelperFactory(); } @@ -245,7 +245,6 @@ namespace Microsoft.AspNetCore.Mvc.Razor var serviceProvider = new ServiceCollection() .AddSingleton(myService) .AddSingleton(htmlHelper) - .AddSingleton(new ExpressionTextCache()) .BuildServiceProvider(); var httpContext = new DefaultHttpContext diff --git a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorPageCreateModelExpressionTest.cs b/src/Mvc/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorPageCreateModelExpressionTest.cs index 861944f0bb..de2ceb9dd0 100644 --- a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorPageCreateModelExpressionTest.cs +++ b/src/Mvc/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorPageCreateModelExpressionTest.cs @@ -209,9 +209,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor private static IModelExpressionProvider CreateModelExpressionProvider() { var provider = new EmptyModelMetadataProvider(); - var modelExpressionProvider = new ModelExpressionProvider( - provider, - new ExpressionTextCache()); + var modelExpressionProvider = new ModelExpressionProvider(provider); return modelExpressionProvider; } @@ -222,7 +220,6 @@ namespace Microsoft.AspNetCore.Mvc.Razor var viewData = new ViewDataDictionary(provider, new ModelStateDictionary()); var serviceCollection = new ServiceCollection(); serviceCollection.AddSingleton(provider); - serviceCollection.AddSingleton(); var httpContext = new DefaultHttpContext { diff --git a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorPageCreateTagHelperTest.cs b/src/Mvc/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorPageCreateTagHelperTest.cs index 43d5e7e550..aeeae9732e 100644 --- a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorPageCreateTagHelperTest.cs +++ b/src/Mvc/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorPageCreateTagHelperTest.cs @@ -72,7 +72,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor private static TestRazorPage CreateTestRazorPage() { var modelMetadataProvider = new EmptyModelMetadataProvider(); - var modelExpressionProvider = new ModelExpressionProvider(modelMetadataProvider, new ExpressionTextCache()); + var modelExpressionProvider = new ModelExpressionProvider(modelMetadataProvider); var activator = new RazorPageActivator( modelMetadataProvider, new UrlHelperFactory(), diff --git a/src/Mvc/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ExpressionHelperTest.cs b/src/Mvc/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ExpressionHelperTest.cs index 4ad1ebdfe5..cbd005e5a0 100644 --- a/src/Mvc/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ExpressionHelperTest.cs +++ b/src/Mvc/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ExpressionHelperTest.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.Collections.Concurrent; using System.Collections.Generic; using System.Linq; using System.Linq.Expressions; @@ -11,7 +12,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures { public class ExpressionHelperTest { - private readonly ExpressionTextCache _expressionTextCache = new ExpressionTextCache(); + private readonly ConcurrentDictionary _expressionTextCache = new ConcurrentDictionary(LambdaExpressionComparer.Instance); public static TheoryData ExpressionAndTexts { diff --git a/src/Mvc/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Rendering/DefaultTemplatesUtilities.cs b/src/Mvc/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Rendering/DefaultTemplatesUtilities.cs index 0cd1f3fe4c..b0e0a7d2c1 100644 --- a/src/Mvc/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Rendering/DefaultTemplatesUtilities.cs +++ b/src/Mvc/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Rendering/DefaultTemplatesUtilities.cs @@ -252,8 +252,6 @@ namespace Microsoft.AspNetCore.Mvc.Rendering .Setup(f => f.GetUrlHelper(It.IsAny())) .Returns(urlHelper); - var expressionTextCache = new ExpressionTextCache(); - if (htmlGenerator == null) { htmlGenerator = HtmlGeneratorUtilities.GetHtmlGenerator(provider, urlHelperFactory.Object, options); @@ -290,7 +288,7 @@ namespace Microsoft.AspNetCore.Mvc.Rendering new TestViewBufferScope(), new HtmlTestEncoder(), UrlEncoder.Default, - expressionTextCache); + new ModelExpressionProvider(provider)); var viewContext = new ViewContext( actionContext, diff --git a/src/Mvc/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ViewComponentResultTest.cs b/src/Mvc/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ViewComponentResultTest.cs index 4650edd55e..03165321ab 100644 --- a/src/Mvc/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ViewComponentResultTest.cs +++ b/src/Mvc/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ViewComponentResultTest.cs @@ -562,7 +562,6 @@ namespace Microsoft.AspNetCore.Mvc var services = new ServiceCollection(); services.AddSingleton(diagnosticSource); services.AddSingleton(); - services.AddSingleton(); services.AddSingleton(Options.Create(new MvcViewOptions())); services.AddTransient(); services.AddSingleton();