[Mvc] Improved route value transformer (#21481)

* Apply suggestions from code review

Cleaned up error messages. Thanks @JamesNK, I totally overlooked the content.

Co-authored-by: James Newton-King <[email protected]>

Co-authored-by: Javier Calvarro Nelson <[email protected]>
Co-authored-by: James Newton-King <[email protected]>

Author: 3 people

src/Mvc/Mvc.Core/src/Builder/ControllerEndpointRouteBuilderExtensions.cs

@@ -473,6 +473,36 @@ public static void MapDynamicControllerRoute<TTransformer>(this IEndpointRouteBu
                 throw new ArgumentNullException(nameof(endpoints));
             }
 
+            MapDynamicControllerRoute<TTransformer>(endpoints, pattern, state: null);
+        }
+
+        /// <summary>
+        /// Adds a specialized <see cref="RouteEndpoint"/> to the <see cref="IEndpointRouteBuilder"/> that will
+        /// attempt to select a controller action using the route values produced by <typeparamref name="TTransformer"/>.
+        /// </summary>
+        /// <param name="endpoints">The <see cref="IEndpointRouteBuilder"/> to add the route to.</param>
+        /// <param name="pattern">The URL pattern of the route.</param>
+        /// <param name="state">A state object to provide to the <typeparamref name="TTransformer" /> instance.</param>
+        /// <typeparam name="TTransformer">The type of a <see cref="DynamicRouteValueTransformer"/>.</typeparam>
+        /// <remarks>
+        /// <para>
+        /// This method allows the registration of a <see cref="RouteEndpoint"/> and <see cref="DynamicRouteValueTransformer"/>
+        /// that combine to dynamically select a controller action using custom logic.
+        /// </para>
+        /// <para>
+        /// The instance of <typeparamref name="TTransformer"/> will be retrieved from the dependency injection container.
+        /// Register <typeparamref name="TTransformer"/> as transient in <c>ConfigureServices</c>. Using the transient lifetime
+        /// is required when using <paramref name="state" />.
+        /// </para>
+        /// </remarks>
+        public static void MapDynamicControllerRoute<TTransformer>(this IEndpointRouteBuilder endpoints, string pattern, object state)
+            where TTransformer : DynamicRouteValueTransformer
+        {
+            if (endpoints == null)
+            {
+                throw new ArgumentNullException(nameof(endpoints));
+            }
+
             EnsureControllerServices(endpoints);
 
             // Called for side-effect to make sure that the data source is registered.
@@ -486,7 +516,7 @@ public static void MapDynamicControllerRoute<TTransformer>(this IEndpointRouteBu
                 })
                 .Add(b =>
                 {
-                    b.Metadata.Add(new DynamicControllerRouteValueTransformerMetadata(typeof(TTransformer)));
+                    b.Metadata.Add(new DynamicControllerRouteValueTransformerMetadata(typeof(TTransformer), state));
                 });
         }
 

src/Mvc/Mvc.Core/src/Resources.resx

@@ -519,4 +519,7 @@
   <data name="ValidationVisitor_ContainerCannotBeSpecified" xml:space="preserve">
     <value>A container cannot be specified when the ModelMetada is of kind '{0}'.</value>
   </data>
-</root>
+  <data name="StateShouldBeNullForRouteValueTransformers" xml:space="preserve">
+    <value>Transformer '{0}' was retrieved from dependency injection with a state value. State can only be specified when the dynamic route is mapped using MapDynamicControllerRoute's state argument together with transient lifetime transformer. Ensure that '{0}' doesn't set its own state and that the transformer is registered with a transient lifetime in dependency injection.</value>
+  </data>
+</root>

src/Mvc/Mvc.Core/src/Routing/DynamicControllerEndpointMatcherPolicy.cs

@@ -1,11 +1,12 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// 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;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Mvc.Core;
 using Microsoft.AspNetCore.Routing;
 using Microsoft.AspNetCore.Routing.Matching;
 using Microsoft.Extensions.DependencyInjection;
@@ -97,13 +98,21 @@ public async Task ApplyAsync(HttpContext httpContext, CandidateSet candidates)
                 // no realistic way this could happen.
                 var dynamicControllerMetadata = endpoint.Metadata.GetMetadata<DynamicControllerMetadata>();
                 var transformerMetadata = endpoint.Metadata.GetMetadata<DynamicControllerRouteValueTransformerMetadata>();
+
+                DynamicRouteValueTransformer transformer = null;
                 if (dynamicControllerMetadata != null)
                 {
                     dynamicValues = dynamicControllerMetadata.Values;
                 }
                 else if (transformerMetadata != null)
                 {
-                    var transformer = (DynamicRouteValueTransformer)httpContext.RequestServices.GetRequiredService(transformerMetadata.SelectorType);
+                    transformer = (DynamicRouteValueTransformer)httpContext.RequestServices.GetRequiredService(transformerMetadata.SelectorType);
+                    if (transformer.State != null)
+                    {
+                        throw new InvalidOperationException(Resources.FormatStateShouldBeNullForRouteValueTransformers(transformerMetadata.SelectorType.Name));
+                    }
+                    transformer.State = transformerMetadata.State;
+
                     dynamicValues = await transformer.TransformAsync(httpContext, originalValues);
                 }
                 else
@@ -146,6 +155,16 @@ public async Task ApplyAsync(HttpContext httpContext, CandidateSet candidates)
                     }
                 }
 
+                if (transformer != null)
+                {
+                    endpoints = await transformer.FilterAsync(httpContext, values, endpoints);
+                    if (endpoints.Count == 0)
+                    {
+                        candidates.ReplaceEndpoint(i, null, null);
+                        continue;
+                    }
+                }
+
                 // Update the route values
                 candidates.ReplaceEndpoint(i, endpoint, values);
 

src/Mvc/Mvc.Core/src/Routing/DynamicControllerRouteValueTransformerMetadata.cs

@@ -8,7 +8,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing
 {
     internal class DynamicControllerRouteValueTransformerMetadata : IDynamicEndpointMetadata
     {
-        public DynamicControllerRouteValueTransformerMetadata(Type selectorType)
+        public DynamicControllerRouteValueTransformerMetadata(Type selectorType, object state)
         {
             if (selectorType == null)
             {
@@ -23,10 +23,13 @@ public DynamicControllerRouteValueTransformerMetadata(Type selectorType)
             }
 
             SelectorType = selectorType;
+            State = state;
         }
 
         public bool IsDynamic => true;
 
         public Type SelectorType { get; }
+
+        public object State { get; }
     }
 }

src/Mvc/Mvc.Core/src/Routing/DynamicRouteValueTransformer.cs

@@ -1,6 +1,7 @@
 // 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.Collections.Generic;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Routing;
@@ -20,23 +21,73 @@ namespace Microsoft.AspNetCore.Mvc.Routing
     /// <para>
     /// The route values returned from a <see cref="TransformAsync(HttpContext, RouteValueDictionary)"/> implementation
     /// will be used to select an action based on matching of the route values. All actions that match the route values
-    /// will be considered as candidates, and may be further disambiguated by <see cref="IEndpointSelectorPolicy" />
-    /// implementations such as <see cref="HttpMethodMatcherPolicy" />.
+    /// will be considered as candidates, and may be further disambiguated by 
+    /// <see cref="FilterAsync(HttpContext, RouteValueDictionary, IReadOnlyList{Endpoint})" /> as well as 
+    /// <see cref="IEndpointSelectorPolicy" /> implementations such as <see cref="HttpMethodMatcherPolicy" />.
+    /// </para>
+    /// <para>
+    /// Operations on a <see cref="DynamicRouteValueTransformer" /> instance will be called for each dynamic endpoint
+    /// in the following sequence:
+    /// 
+    /// <list type="bullet">
+    ///   <item><description><see cref="State" /> is set</description></item>
+    ///   <item><description><see cref="TransformAsync(HttpContext, RouteValueDictionary)"/></description></item>
+    ///   <item><description><see cref="FilterAsync(HttpContext, RouteValueDictionary, IReadOnlyList{Endpoint})" /></description></item>
+    /// </list>
+    /// 
+    /// Implementations that are registered with the service collection as transient may safely use class
+    /// members to persist state across these operations.
     /// </para>
     /// <para>
     /// Implementations <see cref="DynamicRouteValueTransformer" /> should be registered with the service
     /// collection as type <see cref="DynamicRouteValueTransformer" />. Implementations can use any service
-    /// lifetime.
+    /// lifetime. Implementations that make use of <see cref="State" /> must be registered as transient.
     /// </para>
     /// </remarks>
     public abstract class DynamicRouteValueTransformer
     {
+        /// <summary>
+        /// Gets or sets a state value. An arbitrary value passed to the transformer from where it was registered.
+        /// </summary>
+        /// <remarks>
+        /// Implementations that make use of <see cref="State" /> must be registered as transient with the service
+        /// collection.
+        /// </remarks>
+        public object State { get; set; }
+
         /// <summary>
         /// Creates a set of transformed route values that will be used to select an action.
         /// </summary>
         /// <param name="httpContext">The <see cref="HttpContext" /> associated with the current request.</param>
         /// <param name="values">The route values associated with the current match. Implementations should not modify <paramref name="values"/>.</param>
         /// <returns>A task which asynchronously returns a set of route values.</returns>
         public abstract ValueTask<RouteValueDictionary> TransformAsync(HttpContext httpContext, RouteValueDictionary values);
+
+        /// <summary>
+        /// Filters the set of endpoints that were chosen as a result of lookup based on the route values returned by 
+        /// <see cref="TransformAsync(HttpContext, RouteValueDictionary)" />. 
+        /// </summary>
+        /// <param name="httpContext">The <see cref="HttpContext" /> associated with the current request.</param>
+        /// <param name="values">The route values returned from <see cref="TransformAsync(HttpContext, RouteValueDictionary)" />.</param>
+        /// <param name="endpoints">
+        /// The endpoints that were chosen as a result of lookup based on the route values returned by
+        /// <see cref="TransformAsync(HttpContext, RouteValueDictionary)" />.
+        /// </param>
+        /// <returns>Asynchronously returns a list of endpoints to apply to the matches collection.</returns>
+        /// <remarks>
+        /// <para>
+        /// Implementations of <see cref="FilterAsync(HttpContext, RouteValueDictionary, IReadOnlyList{Endpoint})" /> may further
+        /// refine the list of endpoints chosen based on route value matching by returning a new list of endpoints based on 
+        /// <paramref name="endpoints" />.
+        /// </para>
+        /// <para>
+        /// <see cref="FilterAsync(HttpContext, RouteValueDictionary, IReadOnlyList{Endpoint})" /> will not be called in the case
+        /// where zero endpoints were matched based on route values.
+        /// </para>
+        /// </remarks>
+        public virtual ValueTask<IReadOnlyList<Endpoint>> FilterAsync(HttpContext httpContext, RouteValueDictionary values, IReadOnlyList<Endpoint> endpoints)
+        {
+            return new ValueTask<IReadOnlyList<Endpoint>>(endpoints);
+        }
     }
 }