Simplify parameter transformer usage

rynowak · rynowak · commit c9887e027a97 · 2018-11-27T09:01:55.000-08:00
diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ActionModel.cs b/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ActionModel.cs
@@ -50,6 +50,7 @@ public ActionModel(ActionModel other)
 
             ActionMethod = other.ActionMethod;
             ActionName = other.ActionName;
+            RouteParameterTransformer = other.RouteParameterTransformer;
 
             // Not making a deep copy of the controller, this action still belongs to the same controller.
             Controller = other.Controller;
@@ -93,6 +94,18 @@ public ActionModel(ActionModel other)
 
         public IList<ParameterModel> Parameters { get; }
 
+        /// <summary>
+        /// Gets or sets an <see cref="IOutboundParameterTransformer"/> that will be used to transform 
+        /// built-in route parameters such as <c>action</c>, <c>controller</c>, and <c>area</c> as well as
+        /// additional parameters specified by <see cref="RouteValues"/> into static segments in the route template.
+        /// </summary>
+        /// <remarks>
+        /// <para>
+        /// This feature only applies when using endpoint routing.
+        /// </para>
+        /// </remarks>
+        public IOutboundParameterTransformer RouteParameterTransformer { get; set; }
+
         /// <summary>
         /// Gets a collection of route values that must be present in the 
         /// <see cref="RouteData.Values"/> for the corresponding action to be selected.
diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ApplicationModelFactory.cs b/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ApplicationModelFactory.cs
@@ -159,20 +159,17 @@ private static void ReplaceAttributeRouteTokens(
                     routeValues.TryAdd(kvp.Key, kvp.Value);
                 }
 
-                action.Properties.TryGetValue(typeof(IOutboundParameterTransformer), out var obj);
-                var transformer = obj as IOutboundParameterTransformer;
-
                 selector.AttributeRouteModel.Template = AttributeRouteModel.ReplaceTokens(
                     selector.AttributeRouteModel.Template,
                     routeValues,
-                    transformer);
+                    action.RouteParameterTransformer);
 
                 if (selector.AttributeRouteModel.Name != null)
                 {
                     selector.AttributeRouteModel.Name = AttributeRouteModel.ReplaceTokens(
                         selector.AttributeRouteModel.Name,
                         routeValues,
-                        transformer);
+                        action.RouteParameterTransformer);
                 }
             }
             catch (InvalidOperationException ex)
diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/RouteTokenTransformerConvention.cs b/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/RouteTokenTransformerConvention.cs
@@ -33,7 +33,7 @@ public void Apply(ActionModel action)
         {
             if (ShouldApply(action))
             {
-                action.Properties[typeof(IOutboundParameterTransformer)] = _parameterTransformer;
+                action.RouteParameterTransformer = _parameterTransformer;
             }
         }
 
diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/ApplicationModels/PageRouteModel.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/ApplicationModels/PageRouteModel.cs
@@ -55,6 +55,7 @@ public PageRouteModel(PageRouteModel other)
             RelativePath = other.RelativePath;
             ViewEnginePath = other.ViewEnginePath;
             AreaName = other.AreaName;
+            RouteParameterTransformer = other.RouteParameterTransformer;
 
             Properties = new Dictionary<object, object>(other.Properties);
             Selectors = new List<SelectorModel>(other.Selectors.Select(m => new SelectorModel(m)));
@@ -113,5 +114,17 @@ public PageRouteModel(PageRouteModel other)
         /// </para>
         /// </remarks>
         public IDictionary<string, string> RouteValues { get; }
+
+        /// <summary>
+        /// Gets or sets an <see cref="IOutboundParameterTransformer"/> that will be used to transform 
+        /// built-in route parameters such as <c>action</c>, <c>controller</c>, and <c>area</c> as well as
+        /// additional parameters specified by <see cref="RouteValues"/> into static segments in the route template.
+        /// </summary>
+        /// <remarks>
+        /// <para>
+        /// This feature only applies when using endpoint routing.
+        /// </para>
+        /// </remarks>
+        public IOutboundParameterTransformer RouteParameterTransformer { get; set; }
     }
 }
diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/ApplicationModels/PageRouteTransformerConvention.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/ApplicationModels/PageRouteTransformerConvention.cs
@@ -33,7 +33,7 @@ public void Apply(PageRouteModel model)
         {
             if (ShouldApply(model))
             {
-                model.Properties[typeof(IOutboundParameterTransformer)] = _parameterTransformer;
+                model.RouteParameterTransformer = _parameterTransformer;
             }
         }
 
diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageActionDescriptorProvider.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageActionDescriptorProvider.cs
@@ -113,11 +113,8 @@ private void AddActionDescriptors(IList<ActionDescriptor> actions, PageRouteMode
 
         private static string TransformPageRoute(PageRouteModel model, SelectorModel selectorModel)
         {
-            model.Properties.TryGetValue(typeof(IOutboundParameterTransformer), out var transformer);
-            var pageRouteTransformer = transformer as IOutboundParameterTransformer;
-
             // Transformer not set on page route
-            if (pageRouteTransformer == null)
+            if (model.RouteParameterTransformer == null)
             {
                 return selectorModel.AttributeRouteModel.Template;
             }
@@ -134,7 +131,7 @@ private static string TransformPageRoute(PageRouteModel model, SelectorModel sel
             var segments = pageRouteMetadata.PageRoute.Split('/');
             for (var i = 0; i < segments.Length; i++)
             {
-                segments[i] = pageRouteTransformer.TransformOutbound(segments[i]);
+                segments[i] = model.RouteParameterTransformer.TransformOutbound(segments[i]);
             }
 
             var transformedPageRoute = string.Join("/", segments);
diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModels/ActionModelTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModels/ActionModelTest.cs
@@ -7,6 +7,8 @@
 using Microsoft.AspNetCore.Mvc.ActionConstraints;
 using Microsoft.AspNetCore.Mvc.Filters;
 using Microsoft.AspNetCore.Mvc.Routing;
+using Microsoft.AspNetCore.Routing;
+using Moq;
 using Xunit;
 
 namespace Microsoft.AspNetCore.Mvc.ApplicationModels
@@ -75,6 +77,7 @@ public void CopyConstructor_CopiesAllProperties()
                 (typeof(TestController).GetTypeInfo(),
                 new List<object>());
             action.Filters.Add(new MyFilterAttribute());
+            action.RouteParameterTransformer = Mock.Of<IOutboundParameterTransformer>();
             action.RouteValues.Add("key", "value");
             action.Properties.Add(new KeyValuePair<object, object>("test key", "test value"));
 
diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModels/RouteTokenTransformerConventionTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModels/RouteTokenTransformerConventionTest.cs
@@ -3,11 +3,10 @@
 
 using System;
 using System.Reflection;
-using Microsoft.AspNetCore.Mvc.ApplicationModels;
 using Microsoft.AspNetCore.Routing;
 using Xunit;
 
-namespace Microsoft.AspNetCore.Mvc.Test.ApplicationModels
+namespace Microsoft.AspNetCore.Mvc.ApplicationModels
 {
     public class RouteTokenTransformerConventionTest
     {
@@ -28,8 +27,7 @@ public void Apply_HasAttributeRouteModel_SetRouteTokenTransformer()
             convention.Apply(model);
 
             // Assert
-            Assert.True(model.Properties.TryGetValue(typeof(IOutboundParameterTransformer), out var routeTokenTransformer));
-            Assert.Equal(transformer, routeTokenTransformer);
+            Assert.Same(transformer, model.RouteParameterTransformer);
         }
 
         [Fact]
@@ -49,7 +47,7 @@ public void Apply_ShouldApplyFalse_NoOp()
             convention.Apply(model);
 
             // Assert
-            Assert.False(model.Properties.TryGetValue(typeof(IOutboundParameterTransformer), out _));
+            Assert.Null(model.RouteParameterTransformer);
         }
 
         private MethodInfo GetMethodInfo()
diff --git a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/ApplicationModels/PageRouteTransformerConventionTest.cs b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/ApplicationModels/PageRouteTransformerConventionTest.cs
@@ -24,8 +24,7 @@ public void Apply_SetTransformer()
             convention.Apply(model);
 
             // Assert
-            Assert.True(model.Properties.TryGetValue(typeof(IOutboundParameterTransformer), out var routeTokenTransformer));
-            Assert.Equal(transformer, routeTokenTransformer);
+            Assert.Same(transformer, model.RouteParameterTransformer);
         }
 
         [Fact]
@@ -41,7 +40,7 @@ public void Apply_ShouldApplyFalse_NoOp()
             convention.Apply(model);
 
             // Assert
-            Assert.False(model.Properties.TryGetValue(typeof(IOutboundParameterTransformer), out _));
+            Assert.Null(model.RouteParameterTransformer);
         }
 
         private class TestParameterTransformer : IOutboundParameterTransformer
diff --git a/test/WebSites/RoutingWebSite/ControllerRouteTokenTransformerConvention.cs b/test/WebSites/RoutingWebSite/ControllerRouteTokenTransformerConvention.cs
@@ -8,31 +8,24 @@
 
 namespace RoutingWebSite
 {
-    public class ControllerRouteTokenTransformerConvention : IApplicationModelConvention
+    public class ControllerRouteTokenTransformerConvention : RouteTokenTransformerConvention
     {
         private readonly Type _controllerType;
-        private readonly IOutboundParameterTransformer _parameterTransformer;
 
         public ControllerRouteTokenTransformerConvention(Type controllerType, IOutboundParameterTransformer parameterTransformer)
+            : base(parameterTransformer)
         {
             if (parameterTransformer == null)
             {
                 throw new ArgumentNullException(nameof(parameterTransformer));
             }
 
             _controllerType = controllerType;
-            _parameterTransformer = parameterTransformer;
         }
 
-        public void Apply(ApplicationModel application)
+        protected override bool ShouldApply(ActionModel action)
         {
-            foreach (var controller in application.Controllers.Where(c => c.ControllerType == _controllerType))
-            {
-                foreach (var action in controller.Actions)
-                {
-                    action.Properties[typeof(IOutboundParameterTransformer)] = _parameterTransformer;
-                }
-            }
+            return action.Controller.ControllerType == _controllerType;
         }
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -159,20 +159,17 @@ private static void ReplaceAttributeRouteTokens(`
`159`	`159`	`routeValues.TryAdd(kvp.Key, kvp.Value);`
`160`	`160`	`}`
`161`	`161`
`162`		`- action.Properties.TryGetValue(typeof(IOutboundParameterTransformer), out var obj);`
`163`		`- var transformer = obj as IOutboundParameterTransformer;`
`164`		`-`
`165`	`162`	`selector.AttributeRouteModel.Template = AttributeRouteModel.ReplaceTokens(`
`166`	`163`	`selector.AttributeRouteModel.Template,`
`167`	`164`	`routeValues,`
`168`		`- transformer);`
	`165`	`+ action.RouteParameterTransformer);`
`169`	`166`
`170`	`167`	`if (selector.AttributeRouteModel.Name != null)`
`171`	`168`	`{`
`172`	`169`	`selector.AttributeRouteModel.Name = AttributeRouteModel.ReplaceTokens(`
`173`	`170`	`selector.AttributeRouteModel.Name,`
`174`	`171`	`routeValues,`
`175`		`- transformer);`
	`172`	`+ action.RouteParameterTransformer);`
`176`	`173`	`}`
`177`	`174`	`}`
`178`	`175`	`catch (InvalidOperationException ex)`
Original file line number	Diff line number	Diff line change
`@@ -33,7 +33,7 @@ public void Apply(ActionModel action)`
`33`	`33`	`{`
`34`	`34`	`if (ShouldApply(action))`
`35`	`35`	`{`
`36`		`- action.Properties[typeof(IOutboundParameterTransformer)] = _parameterTransformer;`
	`36`	`+ action.RouteParameterTransformer = _parameterTransformer;`
`37`	`37`	`}`
`38`	`38`	`}`
`39`	`39`
Original file line number	Diff line number	Diff line change
`@@ -33,7 +33,7 @@ public void Apply(PageRouteModel model)`
`33`	`33`	`{`
`34`	`34`	`if (ShouldApply(model))`
`35`	`35`	`{`
`36`		`- model.Properties[typeof(IOutboundParameterTransformer)] = _parameterTransformer;`
	`36`	`+ model.RouteParameterTransformer = _parameterTransformer;`
`37`	`37`	`}`
`38`	`38`	`}`
`39`	`39`
Original file line number	Diff line number	Diff line change
`@@ -113,11 +113,8 @@ private void AddActionDescriptors(IList<ActionDescriptor> actions, PageRouteMode`
`113`	`113`
`114`	`114`	`private static string TransformPageRoute(PageRouteModel model, SelectorModel selectorModel)`
`115`	`115`	`{`
`116`		`- model.Properties.TryGetValue(typeof(IOutboundParameterTransformer), out var transformer);`
`117`		`- var pageRouteTransformer = transformer as IOutboundParameterTransformer;`
`118`		`-`
`119`	`116`	`// Transformer not set on page route`
`120`		`- if (pageRouteTransformer == null)`
	`117`	`+ if (model.RouteParameterTransformer == null)`
`121`	`118`	`{`
`122`	`119`	`return selectorModel.AttributeRouteModel.Template;`
`123`	`120`	`}`
`@@ -134,7 +131,7 @@ private static string TransformPageRoute(PageRouteModel model, SelectorModel sel`
`134`	`131`	`var segments = pageRouteMetadata.PageRoute.Split('/');`
`135`	`132`	`for (var i = 0; i < segments.Length; i++)`
`136`	`133`	`{`
`137`		`- segments[i] = pageRouteTransformer.TransformOutbound(segments[i]);`
	`134`	`+ segments[i] = model.RouteParameterTransformer.TransformOutbound(segments[i]);`
`138`	`135`	`}`
`139`	`136`
`140`	`137`	`var transformedPageRoute = string.Join("/", segments);`
Original file line number	Diff line number	Diff line change
`@@ -3,11 +3,10 @@`
`3`	`3`
`4`	`4`	`using System;`
`5`	`5`	`using System.Reflection;`
`6`		`-using Microsoft.AspNetCore.Mvc.ApplicationModels;`
`7`	`6`	`using Microsoft.AspNetCore.Routing;`
`8`	`7`	`using Xunit;`
`9`	`8`
`10`		`-namespace Microsoft.AspNetCore.Mvc.Test.ApplicationModels`
	`9`	`+namespace Microsoft.AspNetCore.Mvc.ApplicationModels`
`11`	`10`	`{`
`12`	`11`	`public class RouteTokenTransformerConventionTest`
`13`	`12`	`{`
`@@ -28,8 +27,7 @@ public void Apply_HasAttributeRouteModel_SetRouteTokenTransformer()`
`28`	`27`	`convention.Apply(model);`
`29`	`28`
`30`	`29`	`// Assert`
`31`		`- Assert.True(model.Properties.TryGetValue(typeof(IOutboundParameterTransformer), out var routeTokenTransformer));`
`32`		`- Assert.Equal(transformer, routeTokenTransformer);`
	`30`	`+ Assert.Same(transformer, model.RouteParameterTransformer);`
`33`	`31`	`}`
`34`	`32`
`35`	`33`	`[Fact]`
`@@ -49,7 +47,7 @@ public void Apply_ShouldApplyFalse_NoOp()`
`49`	`47`	`convention.Apply(model);`
`50`	`48`
`51`	`49`	`// Assert`
`52`		`- Assert.False(model.Properties.TryGetValue(typeof(IOutboundParameterTransformer), out _));`
	`50`	`+ Assert.Null(model.RouteParameterTransformer);`
`53`	`51`	`}`
`54`	`52`
`55`	`53`	`private MethodInfo GetMethodInfo()`
Original file line number	Diff line number	Diff line change
`@@ -24,8 +24,7 @@ public void Apply_SetTransformer()`
`24`	`24`	`convention.Apply(model);`
`25`	`25`
`26`	`26`	`// Assert`
`27`		`- Assert.True(model.Properties.TryGetValue(typeof(IOutboundParameterTransformer), out var routeTokenTransformer));`
`28`		`- Assert.Equal(transformer, routeTokenTransformer);`
	`27`	`+ Assert.Same(transformer, model.RouteParameterTransformer);`
`29`	`28`	`}`
`30`	`29`
`31`	`30`	`[Fact]`
`@@ -41,7 +40,7 @@ public void Apply_ShouldApplyFalse_NoOp()`
`41`	`40`	`convention.Apply(model);`
`42`	`41`
`43`	`42`	`// Assert`
`44`		`- Assert.False(model.Properties.TryGetValue(typeof(IOutboundParameterTransformer), out _));`
	`43`	`+ Assert.Null(model.RouteParameterTransformer);`
`45`	`44`	`}`
`46`	`45`
`47`	`46`	`private class TestParameterTransformer : IOutboundParameterTransformer`
Original file line number	Diff line number	Diff line change
`@@ -8,31 +8,24 @@`
`8`	`8`
`9`	`9`	`namespace RoutingWebSite`
`10`	`10`	`{`
`11`		`- public class ControllerRouteTokenTransformerConvention : IApplicationModelConvention`
	`11`	`+ public class ControllerRouteTokenTransformerConvention : RouteTokenTransformerConvention`
`12`	`12`	`{`
`13`	`13`	`private readonly Type _controllerType;`
`14`		`- private readonly IOutboundParameterTransformer _parameterTransformer;`
`15`	`14`
`16`	`15`	`public ControllerRouteTokenTransformerConvention(Type controllerType, IOutboundParameterTransformer parameterTransformer)`
	`16`	`+ : base(parameterTransformer)`
`17`	`17`	`{`
`18`	`18`	`if (parameterTransformer == null)`
`19`	`19`	`{`
`20`	`20`	`throw new ArgumentNullException(nameof(parameterTransformer));`
`21`	`21`	`}`
`22`	`22`
`23`	`23`	`_controllerType = controllerType;`
`24`		`- _parameterTransformer = parameterTransformer;`
`25`	`24`	`}`
`26`	`25`
`27`		`- public void Apply(ApplicationModel application)`
	`26`	`+ protected override bool ShouldApply(ActionModel action)`
`28`	`27`	`{`
`29`		`- foreach (var controller in application.Controllers.Where(c => c.ControllerType == _controllerType))`
`30`		`- {`
`31`		`- foreach (var action in controller.Actions)`
`32`		`- {`
`33`		`- action.Properties[typeof(IOutboundParameterTransformer)] = _parameterTransformer;`
`34`		`- }`
`35`		`- }`
	`28`	`+ return action.Controller.ControllerType == _controllerType;`
`36`	`29`	`}`
`37`	`30`	`}`
`38`	`31`	`}`