From eb93b4e7b54b95627808f71e86bf0bdf78b3a8df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mariusz=20St=C4=99pie=C5=84?= Date: Wed, 13 Dec 2023 22:58:16 +0100 Subject: [PATCH] use IActionDescriptorCollectionProvider --- .../Internal/LoggerExtensions.cs | 39 ++- .../Internal/MethodCacheEntry.Factory.cs | 16 +- .../Internal/MethodCacheEntry.cs | 24 +- src/UriGeneration/Internal/MethodCacheKey.cs | 15 +- .../Internal/StringExtensions.cs | 16 -- src/UriGeneration/Internal/ValuesExtractor.cs | 231 ++++++------------ 6 files changed, 98 insertions(+), 243 deletions(-) delete mode 100644 src/UriGeneration/Internal/StringExtensions.cs diff --git a/src/UriGeneration/Internal/LoggerExtensions.cs b/src/UriGeneration/Internal/LoggerExtensions.cs index 30f2743..dbf7506 100644 --- a/src/UriGeneration/Internal/LoggerExtensions.cs +++ b/src/UriGeneration/Internal/LoggerExtensions.cs @@ -1,5 +1,8 @@ -using Microsoft.Extensions.Logging; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.Extensions.Logging; using System.Linq.Expressions; +using System.Reflection; namespace UriGeneration.Internal { @@ -17,8 +20,8 @@ internal static partial class LoggerExtensions [LoggerMessage(1004, LogLevel.Debug, "Successfully extracted Type of TController.", EventName = nameof(ControllerExtracted))] public static partial void ControllerExtracted(this ILogger logger); - [LoggerMessage(1005, LogLevel.Debug, "Successfully retrieved valid cache entry with values: {MethodName}, {ControllerName} and {AreaKey}, {ControllerAreaName}.", EventName = nameof(ValidCacheEntryRetrieved))] - public static partial void ValidCacheEntryRetrieved(this ILogger logger, string methodName, string controllerName, string areaKey, string controllerAreaName); + [LoggerMessage(1005, LogLevel.Debug, "Successfully retrieved valid cache entry with value: {ActionDescriptor}.", EventName = nameof(ValidCacheEntryRetrieved))] + public static partial void ValidCacheEntryRetrieved(this ILogger logger, ActionDescriptor actionDescriptor); [LoggerMessage(1006, LogLevel.Debug, "Successfully retrieved invalid cache entry: no values will be extracted (see previous log messages for further details).", EventName = nameof(InvalidCacheEntryRetrieved))] public static partial void InvalidCacheEntryRetrieved(this ILogger logger); @@ -28,35 +31,23 @@ internal static partial class LoggerExtensions [LoggerMessage(1008, LogLevel.Debug, "Expression must point to the method with the same declaring type as TController.", EventName = nameof(MethodDeclaringType))] public static partial void MethodDeclaringType(this ILogger logger); - - [LoggerMessage(1009, LogLevel.Debug, "Excluded method's parameter with position {MethodParameterPosition}: name cannot be null.", EventName = nameof(MethodParameterExcludedName))] - public static partial void MethodParameterExcludedName(this ILogger logger, int methodParameterPosition); - - [LoggerMessage(1010, LogLevel.Debug, "Excluded method's parameter: {MethodParameterName} cannot be of Type IFormFile, IEnumerable, CancellationToken or IFormCollection.", EventName = nameof(MethodParameterExcludedType))] - public static partial void MethodParameterExcludedType(this ILogger logger, string? methodParameterName); - [LoggerMessage(1011, LogLevel.Debug, "Excluded method's parameter: {MethodParameterName} cannot have FromBody, FromForm, FromHeader, FromServices or FromKeyedServices attribute specified.", EventName = nameof(MethodParameterExcludedAttribute))] - public static partial void MethodParameterExcludedAttribute(this ILogger logger, string? methodParameterName); + [LoggerMessage(1009, LogLevel.Debug, "Successfully extracted MethodInfo's ActionDescriptor.", EventName = nameof(ActionDescriptorExtracted))] + public static partial void ActionDescriptorExtracted(this ILogger logger); - [LoggerMessage(1012, LogLevel.Debug, "Successfully extracted method's name: {MethodName}.", EventName = nameof(MethodNameExtracted))] - public static partial void MethodNameExtracted(this ILogger logger, string methodName); - - [LoggerMessage(1013, LogLevel.Debug, "{MethodName} cannot have NonActionAttribute specified.", EventName = nameof(MethodNameNotExtracted))] - public static partial void MethodNameNotExtracted(this ILogger logger, string methodName); + [LoggerMessage(1010, LogLevel.Debug, "No ActionDescriptor with MethodInfo: {MethodInfo} found in ActionDescriptorCollection.", EventName = nameof(NoActionDescriptorFound))] + public static partial void NoActionDescriptorFound(this ILogger logger, MethodInfo methodInfo); - [LoggerMessage(1014, LogLevel.Debug, "Succesfully extracted controller's name: {ControllerName}.", EventName = nameof(ControllerNameExtracted))] - public static partial void ControllerNameExtracted(this ILogger logger, string controllerName); - - [LoggerMessage(1015, LogLevel.Debug, "{ControllerName} cannot have NonControllerAttribute specified.", EventName = nameof(ControllerNameNotExtracted))] - public static partial void ControllerNameNotExtracted(this ILogger logger, string controllerName); + [LoggerMessage(1011, LogLevel.Debug, "Parameter cannot have binding source: {BindingSource}.", EventName = nameof(BindingSource))] + public static partial void BindingSource(this ILogger logger, string? bindingSource); - [LoggerMessage(1016, LogLevel.Debug, "Successfully extracted route value: {Key}, {Value}.", EventName = nameof(RouteValueExtracted))] + [LoggerMessage(1012, LogLevel.Debug, "Successfully extracted route value: {Key}, {Value}.", EventName = nameof(RouteValueExtracted))] public static partial void RouteValueExtracted(this ILogger logger, string? key, object? value); - [LoggerMessage(1017, LogLevel.Debug, "Successfully extracted all values from expression.", EventName = nameof(ValuesExtracted))] + [LoggerMessage(1013, LogLevel.Debug, "Successfully extracted all values from expression.", EventName = nameof(ValuesExtracted))] public static partial void ValuesExtracted(this ILogger logger); - [LoggerMessage(1018, LogLevel.Debug, "Failed to extract one or more values from expression {Expression}: an exception occurred.", EventName = nameof(ValuesNotExtracted))] + [LoggerMessage(1014, LogLevel.Debug, "Failed to extract one or more values from expression {Expression}: an exception occurred.", EventName = nameof(ValuesNotExtracted))] public static partial void ValuesNotExtracted(this ILogger logger, LambdaExpression expression, Exception exception); } } diff --git a/src/UriGeneration/Internal/MethodCacheEntry.Factory.cs b/src/UriGeneration/Internal/MethodCacheEntry.Factory.cs index 3b6f452..fd4781a 100644 --- a/src/UriGeneration/Internal/MethodCacheEntry.Factory.cs +++ b/src/UriGeneration/Internal/MethodCacheEntry.Factory.cs @@ -1,4 +1,4 @@ -using System.Reflection; +using Microsoft.AspNetCore.Mvc.Controllers; namespace UriGeneration.Internal { @@ -8,18 +8,8 @@ internal partial class MethodCacheEntry new(isValid: false); public static MethodCacheEntry Valid( - string methodName, - string controllerName, - ParameterInfo[] includedMethodParameters, - string controllerAreaName) - { - return new( - isValid: true, - methodName, - controllerName, - includedMethodParameters, - controllerAreaName); - } + ControllerActionDescriptor actionDescriptor) => + new(isValid: true, actionDescriptor); public static MethodCacheEntry Invalid() => InvalidInstance; } diff --git a/src/UriGeneration/Internal/MethodCacheEntry.cs b/src/UriGeneration/Internal/MethodCacheEntry.cs index a589539..13cb545 100644 --- a/src/UriGeneration/Internal/MethodCacheEntry.cs +++ b/src/UriGeneration/Internal/MethodCacheEntry.cs @@ -1,5 +1,5 @@ -using System.Diagnostics.CodeAnalysis; -using System.Reflection; +using Microsoft.AspNetCore.Mvc.Controllers; +using System.Diagnostics.CodeAnalysis; namespace UriGeneration.Internal { @@ -7,15 +7,9 @@ internal partial class MethodCacheEntry { [MemberNotNullWhen( true, - nameof(MethodName), - nameof(ControllerName), - nameof(IncludedMethodParameters), - nameof(ControllerAreaName))] + nameof(ActionDescriptor))] public bool IsValid { get; } - public string? MethodName { get; } - public string? ControllerName { get; } - public ParameterInfo[]? IncludedMethodParameters { get; } - public string? ControllerAreaName { get; } + public ControllerActionDescriptor? ActionDescriptor { get; } private MethodCacheEntry(bool isValid) { @@ -24,16 +18,10 @@ private MethodCacheEntry(bool isValid) private MethodCacheEntry( bool isValid, - string methodName, - string controllerName, - ParameterInfo[] includedMethodParameters, - string controllerAreaName) + ControllerActionDescriptor actionDescriptor) { IsValid = isValid; - MethodName = methodName; - ControllerName = controllerName; - IncludedMethodParameters = includedMethodParameters; - ControllerAreaName = controllerAreaName; + ActionDescriptor = actionDescriptor; } } } diff --git a/src/UriGeneration/Internal/MethodCacheKey.cs b/src/UriGeneration/Internal/MethodCacheKey.cs index a517489..d4ac975 100644 --- a/src/UriGeneration/Internal/MethodCacheKey.cs +++ b/src/UriGeneration/Internal/MethodCacheKey.cs @@ -2,15 +2,8 @@ namespace UriGeneration.Internal { - internal record MethodCacheKey - { - public MethodInfo Method { get; } - public Type Controller { get; } - - public MethodCacheKey(MethodInfo method, Type controller) - { - Method = method; - Controller = controller; - } - } + internal record MethodCacheKey( + MethodInfo Method, + Type Controller, + int ActionDescriptorVersion); } diff --git a/src/UriGeneration/Internal/StringExtensions.cs b/src/UriGeneration/Internal/StringExtensions.cs deleted file mode 100644 index 488b85c..0000000 --- a/src/UriGeneration/Internal/StringExtensions.cs +++ /dev/null @@ -1,16 +0,0 @@ -namespace UriGeneration.Internal -{ - internal static class StringExtensions - { - public static string RemoveSuffix(this string value, string suffix) - { - if (!value.EndsWith(suffix)) - { - return value; - } - - int index = value.LastIndexOf(suffix); - return value.Remove(index); - } - } -} diff --git a/src/UriGeneration/Internal/ValuesExtractor.cs b/src/UriGeneration/Internal/ValuesExtractor.cs index f09e60f..eb06863 100644 --- a/src/UriGeneration/Internal/ValuesExtractor.cs +++ b/src/UriGeneration/Internal/ValuesExtractor.cs @@ -1,5 +1,7 @@ -using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.Controllers; +using Microsoft.AspNetCore.Mvc.Infrastructure; +using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.Caching.Memory; #if NET8_0_OR_GREATER @@ -17,8 +19,6 @@ namespace UriGeneration.Internal { internal class ValuesExtractor : IValuesExtractor { - private const string ControllerSuffix = "Controller"; - private const string AsyncSuffix = "Async"; private const string AreaKey = "area"; private static readonly MemoryCacheEntryOptions CacheEntryOptions = @@ -28,10 +28,12 @@ internal class ValuesExtractor : IValuesExtractor Expression.Parameter(typeof(object), "unused"); private readonly IMethodCacheAccessor _methodCacheAccessor; + private readonly IActionDescriptorCollectionProvider _actionDescriptorsProvider; private readonly ILogger _logger; public ValuesExtractor( IMethodCacheAccessor methodCacheAccessor, + IActionDescriptorCollectionProvider actionDescriptorsProvider, ILogger logger) { if (methodCacheAccessor == null) @@ -39,12 +41,18 @@ public ValuesExtractor( throw new ArgumentNullException(nameof(methodCacheAccessor)); } + if (actionDescriptorsProvider == null) + { + throw new ArgumentNullException(nameof(actionDescriptorsProvider)); + } + if (logger == null) { throw new ArgumentNullException(nameof(logger)); } _methodCacheAccessor = methodCacheAccessor; + _actionDescriptorsProvider = actionDescriptorsProvider; _logger = logger; } @@ -66,31 +74,33 @@ public bool TryExtractValues( var method = ExtractMethod(methodCall); var controller = ExtractController(); + var actionDescriptors = _actionDescriptorsProvider.ActionDescriptors; + var methodCache = _methodCacheAccessor.Cache; - var key = new MethodCacheKey(method, controller); + var key = new MethodCacheKey( + method, + controller, + actionDescriptors.Version); if (options?.BypassMethodCache is not true && methodCache.TryGetValue(key, out MethodCacheEntry? entry)) { if (entry!.IsValid) { - _logger.ValidCacheEntryRetrieved( - entry.MethodName, - entry.ControllerName, - AreaKey, - entry.ControllerAreaName); + _logger.ValidCacheEntryRetrieved(entry.ActionDescriptor); + + var entryActionDescriptor = entry.ActionDescriptor; var entryRouteValues = ExtractRouteValues( - entry.IncludedMethodParameters, + entryActionDescriptor, methodCall.Arguments, - entry.ControllerAreaName, options); _logger.ValuesExtracted(); values = new Values( - entry.MethodName, - entry.ControllerName, + entryActionDescriptor.ActionName, + entryActionDescriptor.ControllerName, entryRouteValues); return true; } @@ -102,8 +112,7 @@ public bool TryExtractValues( } if (!ValidateMethodConcreteness(method, controller) - || !TryExtractMethodName(method, out var methodName) - || !TryExtractControllerName(controller, out var controllerName)) + || !TryExtractActionDescriptor(method, actionDescriptors, out var actionDescriptor)) { if (options?.BypassMethodCache is not true) { @@ -114,33 +123,22 @@ public bool TryExtractValues( return false; } - var includedMethodParameters = ExtractIncludedMethodParameters( - method); - - string controllerAreaName = ExtractControllerAreaName( - controller); - var routeValues = ExtractRouteValues( - includedMethodParameters, + actionDescriptor, methodCall.Arguments, - controllerAreaName, options); if (options?.BypassMethodCache is not true) { - var validEntry = MethodCacheEntry.Valid( - methodName, - controllerName, - includedMethodParameters, - controllerAreaName); + var validEntry = MethodCacheEntry.Valid(actionDescriptor); methodCache.Set(key, validEntry, CacheEntryOptions); } _logger.ValuesExtracted(); values = new Values( - methodName, - controllerName, + actionDescriptor.ActionName, + actionDescriptor.ControllerName, routeValues); return true; } @@ -209,152 +207,50 @@ private bool ValidateMethodConcreteness( return true; } - private ParameterInfo[] ExtractIncludedMethodParameters( - MethodInfo method) - { - var includedMethodParameters = new List(); - var methodParameters = method.GetParameters(); - - foreach (var methodParameter in methodParameters) - { - if (IncludeMethodParameter(methodParameter, log: true)) - { - includedMethodParameters.Add(methodParameter); - } - } - - return includedMethodParameters.ToArray(); - } - - private bool IncludeMethodParameter( - ParameterInfo methodParameter, - bool log) - { - if (methodParameter.Name == null) - { - if (log) - { - _logger.MethodParameterExcludedName( - methodParameter.Position); - } - return false; - } - - var methodParameterType = methodParameter.ParameterType; - - if (methodParameterType.IsAssignableTo(typeof(IFormFile)) - || methodParameterType.IsAssignableTo(typeof(IEnumerable)) - || methodParameterType.IsAssignableTo(typeof(CancellationToken)) - || methodParameterType.IsAssignableTo(typeof(IFormCollection))) - { - if (log) - { - _logger.MethodParameterExcludedType(methodParameter.Name); - } - return false; - } - - var methodParameterAttributes = methodParameter - .GetCustomAttributes(inherit: true); - - if (methodParameterAttributes.Any(attr => attr is FromBodyAttribute - || attr is FromFormAttribute - || attr is FromHeaderAttribute - || attr is FromServicesAttribute -#if NET8_0_OR_GREATER - || attr is FromKeyedServicesAttribute -#endif - )) - { - if (log) - { - _logger.MethodParameterExcludedAttribute( - methodParameter.Name); - } - return false; - } - - return true; - } - - private bool TryExtractMethodName( + private bool TryExtractActionDescriptor( MethodInfo method, - [NotNullWhen(true)] out string? methodName) + ActionDescriptorCollection actionDescriptors, + [NotNullWhen(true)] out ControllerActionDescriptor? actionDescriptor) { - methodName = method.Name; + actionDescriptor = actionDescriptors.Items + .OfType() + .FirstOrDefault(descriptor => descriptor.MethodInfo == method); - if (method.IsDefined(typeof(NonActionAttribute), inherit: true)) + if (actionDescriptor is null) { - _logger.MethodNameNotExtracted(methodName); + _logger.NoActionDescriptorFound(method); return false; } - methodName = methodName.RemoveSuffix(AsyncSuffix); - - var actionNameAttribute = method - .GetCustomAttributes(inherit: true) - .FirstOrDefault(); - - if (actionNameAttribute != null) - { - methodName = actionNameAttribute.Name; - } - - _logger.MethodNameExtracted(methodName); - return true; - } - - private bool TryExtractControllerName( - Type controller, - [NotNullWhen(true)] out string? controllerName) - { - controllerName = controller.Name; - - if (controller.IsDefined( - typeof(NonControllerAttribute), - inherit: true)) - { - _logger.ControllerNameNotExtracted(controllerName); - return false; - } - - controllerName = controllerName.RemoveSuffix(ControllerSuffix); - - _logger.ControllerNameExtracted(controllerName); + _logger.ActionDescriptorExtracted(); return true; } - private string ExtractControllerAreaName(Type controller) - { - var areaAttribute = controller - .GetCustomAttributes(inherit: true) - .FirstOrDefault(); - - if (areaAttribute != null) - { - string controllerAreaName = areaAttribute.RouteValue; - _logger.RouteValueExtracted(AreaKey, controllerAreaName); - return controllerAreaName; - } - - return string.Empty; // don't use the 'ambient' value of area - } - private RouteValueDictionary ExtractRouteValues( - ParameterInfo[] includedMethodParameters, + ActionDescriptor actionDescriptor, ReadOnlyCollection methodCallArguments, - string controllerAreaName, UriOptions? options) { var routeValues = new RouteValueDictionary(); + var parameters = actionDescriptor.Parameters + .OfType(); - foreach (var includedMethodParameter in includedMethodParameters) + foreach (var parameter in parameters) { - // nullability validated in: IncludeMethodParameter - string key = includedMethodParameter.Name!; + var bindingSource = parameter.BindingInfo?.BindingSource; + + if (bindingSource is not null // Might be null in controllers with views. + && !bindingSource.CanAcceptDataFrom(BindingSource.Query) + && !bindingSource.CanAcceptDataFrom(BindingSource.Path)) + { + _logger.BindingSource(bindingSource?.Id); + continue; + } + + string key = parameter.Name; var methodCallArgument = methodCallArguments[ - includedMethodParameter.Position]; + parameter.ParameterInfo.Position]; object? value; @@ -371,8 +267,9 @@ private RouteValueDictionary ExtractRouteValues( _logger.RouteValueExtracted(key, value); } - routeValues.Add(AreaKey, controllerAreaName); - // logged in: ExtractControllerAreaName + string areaName = ExtractAreaName(actionDescriptor); + routeValues.Add(AreaKey, areaName); + _logger.RouteValueExtracted(AreaKey, areaName); return routeValues; } @@ -387,7 +284,7 @@ private RouteValueDictionary ExtractRouteValues( } else { - // see: CachedExpressionCompiler.Evaluate + // See: CachedExpressionCompiler.Evaluate. Expression> lambdaExpression = Expression.Lambda>( Expression.Convert(expression, typeof(object)), @@ -397,5 +294,17 @@ private RouteValueDictionary ExtractRouteValues( return func(null); } } + + private static string ExtractAreaName(ActionDescriptor actionDescriptor) + { + if (actionDescriptor.RouteValues.TryGetValue(AreaKey, out string? areaName)) + { + return areaName ?? string.Empty; + } + else + { + return string.Empty; // Don't use the 'ambient' value of area. + } + } } }