From 953567ee52579b11935fc41a461aff282a635634 Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Mon, 4 May 2020 15:10:17 -0700 Subject: [PATCH 1/7] Prototype of improved route value transformer See: #21471 This change just include the product-side changes and just for controllers. This addresses most the major gaps our partners have found with dynamic route value transformers. Doesn't include anything order-related. --- ...ontrollerEndpointRouteBuilderExtensions.cs | 32 ++++++++++- .../DynamicControllerEndpointMatcherPolicy.cs | 16 +++++- ...ControllerRouteValueTransformerMetadata.cs | 5 +- .../Routing/DynamicRouteValueTransformer.cs | 57 ++++++++++++++++++- 4 files changed, 104 insertions(+), 6 deletions(-) diff --git a/src/Mvc/Mvc.Core/src/Builder/ControllerEndpointRouteBuilderExtensions.cs b/src/Mvc/Mvc.Core/src/Builder/ControllerEndpointRouteBuilderExtensions.cs index 3d5915786793..4defbc5d2ada 100644 --- a/src/Mvc/Mvc.Core/src/Builder/ControllerEndpointRouteBuilderExtensions.cs +++ b/src/Mvc/Mvc.Core/src/Builder/ControllerEndpointRouteBuilderExtensions.cs @@ -473,6 +473,36 @@ public static void MapDynamicControllerRoute(this IEndpointRouteBu throw new ArgumentNullException(nameof(endpoints)); } + MapDynamicControllerRoute(endpoints, pattern, state: null); + } + + /// + /// Adds a specialized to the that will + /// attempt to select a controller action using the route values produced by . + /// + /// The to add the route to. + /// The URL pattern of the route. + /// A state object to provide to the instance. + /// The type of a . + /// + /// + /// This method allows the registration of a and + /// that combine to dynamically select a controller action using custom logic. + /// + /// + /// The instance of will be retrieved from the dependency injection container. + /// Register as transient in ConfigureServices. Using the transient lifetime + /// is required when using . + /// + /// + public static void MapDynamicControllerRoute(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(this IEndpointRouteBu }) .Add(b => { - b.Metadata.Add(new DynamicControllerRouteValueTransformerMetadata(typeof(TTransformer))); + b.Metadata.Add(new DynamicControllerRouteValueTransformerMetadata(typeof(TTransformer), state)); }); } diff --git a/src/Mvc/Mvc.Core/src/Routing/DynamicControllerEndpointMatcherPolicy.cs b/src/Mvc/Mvc.Core/src/Routing/DynamicControllerEndpointMatcherPolicy.cs index 118a5b8e848b..2cd1a7dfe3f3 100644 --- a/src/Mvc/Mvc.Core/src/Routing/DynamicControllerEndpointMatcherPolicy.cs +++ b/src/Mvc/Mvc.Core/src/Routing/DynamicControllerEndpointMatcherPolicy.cs @@ -97,13 +97,17 @@ public async Task ApplyAsync(HttpContext httpContext, CandidateSet candidates) // no realistic way this could happen. var dynamicControllerMetadata = endpoint.Metadata.GetMetadata(); var transformerMetadata = endpoint.Metadata.GetMetadata(); + + 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); + transformer.State = transformerMetadata.State; + dynamicValues = await transformer.TransformAsync(httpContext, originalValues); } else @@ -146,6 +150,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); diff --git a/src/Mvc/Mvc.Core/src/Routing/DynamicControllerRouteValueTransformerMetadata.cs b/src/Mvc/Mvc.Core/src/Routing/DynamicControllerRouteValueTransformerMetadata.cs index ebe6d07cc1c1..f632a69431b6 100644 --- a/src/Mvc/Mvc.Core/src/Routing/DynamicControllerRouteValueTransformerMetadata.cs +++ b/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; } } } diff --git a/src/Mvc/Mvc.Core/src/Routing/DynamicRouteValueTransformer.cs b/src/Mvc/Mvc.Core/src/Routing/DynamicRouteValueTransformer.cs index b858a9f62f40..6201c7095092 100644 --- a/src/Mvc/Mvc.Core/src/Routing/DynamicRouteValueTransformer.cs +++ b/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,17 +21,40 @@ namespace Microsoft.AspNetCore.Mvc.Routing /// /// The route values returned from a 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 - /// implementations such as . + /// will be considered as candidates, and may be further disambiguated by + /// as well as + /// implementations such as . + /// + /// + /// Operations on a instance will be called for each dynamic endpoint + /// in the following sequence: + /// + /// + /// is set + /// + /// + /// + /// + /// Implementations that are registered with the service collection as transient may safely use class + /// members to persist state across these operations. /// /// /// Implementations should be registered with the service /// collection as type . Implementations can use any service - /// lifetime. + /// lifetime. Implementations that make use of must be registered as transient. /// /// public abstract class DynamicRouteValueTransformer { + /// + /// Gets or sets a state value. An arbitrary value passed to the transformer from where was registered. + /// + /// + /// Implementations that make use of must be registered as transient with the service + /// collection. + /// + public object State { get; set; } + /// /// Creates a set of transformed route values that will be used to select an action. /// @@ -38,5 +62,32 @@ public abstract class DynamicRouteValueTransformer /// The route values associated with the current match. Implementations should not modify . /// A task which asynchronously returns a set of route values. public abstract ValueTask TransformAsync(HttpContext httpContext, RouteValueDictionary values); + + /// + /// Filters the set of endpoints that were chosen as a result of lookup based on the route values returned by + /// . + /// + /// The associated with the current request. + /// The route values returned from . + /// + /// The endpoints that were chosen as a result of lookup based on the route values returned by + /// . + /// + /// Asynchronously returns a list of endpoints to apply to the matches collection. + /// + /// + /// Implementations of may further + /// refine the list of endpoints chosen based on route value matching by returning a new list of endpoints based on + /// . + /// + /// + /// will not be called in the case + /// where zero endpoints were matched based on route values. + /// + /// + public virtual ValueTask> FilterAsync(HttpContext httpContext, RouteValueDictionary values, IReadOnlyList endpoints) + { + return new ValueTask>(endpoints); + } } } From f20a7a63ad8485c36ee7689323c1465549812f15 Mon Sep 17 00:00:00 2001 From: Javier Calvarro Nelson Date: Fri, 17 Jul 2020 05:35:53 -0700 Subject: [PATCH 2/7] Fix test --- .../Routing/DynamicControllerEndpointMatcherPolicyTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Mvc/Mvc.Core/test/Routing/DynamicControllerEndpointMatcherPolicyTest.cs b/src/Mvc/Mvc.Core/test/Routing/DynamicControllerEndpointMatcherPolicyTest.cs index 52e7dcece613..8c9060743cc6 100644 --- a/src/Mvc/Mvc.Core/test/Routing/DynamicControllerEndpointMatcherPolicyTest.cs +++ b/src/Mvc/Mvc.Core/test/Routing/DynamicControllerEndpointMatcherPolicyTest.cs @@ -1,4 +1,4 @@ -// 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; @@ -58,7 +58,7 @@ public DynamicControllerEndpointMatcherPolicyTest() _ => Task.CompletedTask, new EndpointMetadataCollection(new object[] { - new DynamicControllerRouteValueTransformerMetadata(typeof(CustomTransformer)), + new DynamicControllerRouteValueTransformerMetadata(typeof(CustomTransformer), null), }), "dynamic"); From fe7dc75dcbb4bea9bd13ff1b9e0e41ba252d28c7 Mon Sep 17 00:00:00 2001 From: Javier Calvarro Nelson Date: Tue, 21 Jul 2020 12:53:17 -0700 Subject: [PATCH 3/7] Added support for razor pages, added tests and functional tests to validate the implementation --- ...amicControllerEndpointMatcherPolicyTest.cs | 182 ++++++++++++++- ...azorPagesEndpointRouteBuilderExtensions.cs | 30 ++- .../DynamicPageEndpointMatcherPolicy.cs | 16 +- ...ynamicPageRouteValueTransformerMetadata.cs | 7 +- .../DynamicPageEndpointMatcherPolicyTest.cs | 207 ++++++++++++++++-- .../Mvc.FunctionalTests/RoutingDynamicTest.cs | 37 +++- .../WebSites/RoutingWebSite/DynamicVersion.cs | 10 + .../RoutingWebSite/StartupForDynamic.cs | 21 +- 8 files changed, 469 insertions(+), 41 deletions(-) create mode 100644 src/Mvc/test/WebSites/RoutingWebSite/DynamicVersion.cs diff --git a/src/Mvc/Mvc.Core/test/Routing/DynamicControllerEndpointMatcherPolicyTest.cs b/src/Mvc/Mvc.Core/test/Routing/DynamicControllerEndpointMatcherPolicyTest.cs index 8c9060743cc6..ed95acba222f 100644 --- a/src/Mvc/Mvc.Core/test/Routing/DynamicControllerEndpointMatcherPolicyTest.cs +++ b/src/Mvc/Mvc.Core/test/Routing/DynamicControllerEndpointMatcherPolicyTest.cs @@ -58,7 +58,7 @@ public DynamicControllerEndpointMatcherPolicyTest() _ => Task.CompletedTask, new EndpointMetadataCollection(new object[] { - new DynamicControllerRouteValueTransformerMetadata(typeof(CustomTransformer), null), + new DynamicControllerRouteValueTransformerMetadata(typeof(CustomTransformer), State), }), "dynamic"); @@ -71,7 +71,8 @@ public DynamicControllerEndpointMatcherPolicyTest() services.AddScoped(s => { var transformer = new CustomTransformer(); - transformer.Transform = (c, values) => Transform(c, values); + transformer.Transform = (c, values, state) => Transform(c, values, state); + transformer.Filter = (c, values, state, candidates) => Filter(c, values, state, candidates); return transformer; }); Services = services.BuildServiceProvider(); @@ -91,7 +92,11 @@ public DynamicControllerEndpointMatcherPolicyTest() private IServiceProvider Services { get; } - private Func> Transform { get; set; } + private Func> Transform { get; set; } + + private Func, ValueTask>> Filter { get; set; } = (_, __, ___, e) => new ValueTask>(e); + + private object State { get; } = new object(); [Fact] public async Task ApplyAsync_NoMatch() @@ -106,7 +111,7 @@ public async Task ApplyAsync_NoMatch() var candidates = new CandidateSet(endpoints, values, scores); candidates.SetValidity(0, false); - Transform = (c, values) => + Transform = (c, values, state) => { throw new InvalidOperationException(); }; @@ -135,7 +140,7 @@ public async Task ApplyAsync_HasMatchNoEndpointFound() var candidates = new CandidateSet(endpoints, values, scores); - Transform = (c, values) => + Transform = (c, values, state) => { return new ValueTask(new RouteValueDictionary()); }; @@ -166,7 +171,7 @@ public async Task ApplyAsync_HasMatchFindsEndpoint_WithoutRouteValues() var candidates = new CandidateSet(endpoints, values, scores); - Transform = (c, values) => + Transform = (c, values, state) => { return new ValueTask(new RouteValueDictionary(new { @@ -212,12 +217,13 @@ public async Task ApplyAsync_HasMatchFindsEndpoint_WithRouteValues() var candidates = new CandidateSet(endpoints, values, scores); - Transform = (c, values) => + Transform = (c, values, state) => { return new ValueTask(new RouteValueDictionary(new { controller = "Home", action = "Index", + state })); }; @@ -242,13 +248,162 @@ public async Task ApplyAsync_HasMatchFindsEndpoint_WithRouteValues() { Assert.Equal("controller", kvp.Key); Assert.Equal("Home", kvp.Value); - }, + }, + kvp => + { + Assert.Equal("slug", kvp.Key); + Assert.Equal("test", kvp.Value); + }, + kvp => + { + Assert.Equal("state", kvp.Key); + Assert.Same(State, kvp.Value); + }); + Assert.True(candidates.IsValidCandidate(0)); + } + + [Fact] + public async Task ApplyAsync_CanDiscardFoundEndpoints() + { + // Arrange + var policy = new DynamicControllerEndpointMatcherPolicy(Selector, Comparer); + + var endpoints = new[] { DynamicEndpoint, }; + var values = new RouteValueDictionary[] { new RouteValueDictionary(new { slug = "test", }), }; + var scores = new[] { 0, }; + + var candidates = new CandidateSet(endpoints, values, scores); + + Transform = (c, values, state) => + { + return new ValueTask(new RouteValueDictionary(new + { + controller = "Home", + action = "Index", + state + })); + }; + + Filter = (c, values, state, endpoints) => + { + return new ValueTask>(Array.Empty()); + }; + + var httpContext = new DefaultHttpContext() + { + RequestServices = Services, + }; + + // Act + await policy.ApplyAsync(httpContext, candidates); + + // Assert + Assert.False(candidates.IsValidCandidate(0)); + } + + [Fact] + public async Task ApplyAsync_CanReplaceFoundEndpoints() + { + // Arrange + var policy = new DynamicControllerEndpointMatcherPolicy(Selector, Comparer); + + var endpoints = new[] { DynamicEndpoint, }; + var values = new RouteValueDictionary[] { new RouteValueDictionary(new { slug = "test", }), }; + var scores = new[] { 0, }; + + var candidates = new CandidateSet(endpoints, values, scores); + + Transform = (c, values, state) => + { + return new ValueTask(new RouteValueDictionary(new + { + controller = "Home", + action = "Index", + state + })); + }; + + Filter = (c, values, state, endpoints) => new ValueTask>(new[] + { + new Endpoint((ctx) => Task.CompletedTask, new EndpointMetadataCollection(Array.Empty()), "ReplacedEndpoint") + }); + + var httpContext = new DefaultHttpContext() + { + RequestServices = Services, + }; + + // Act + await policy.ApplyAsync(httpContext, candidates); + + // Assert + Assert.Collection( + candidates[0].Values.OrderBy(kvp => kvp.Key), + kvp => + { + Assert.Equal("action", kvp.Key); + Assert.Equal("Index", kvp.Value); + }, + kvp => + { + Assert.Equal("controller", kvp.Key); + Assert.Equal("Home", kvp.Value); + }, kvp => { Assert.Equal("slug", kvp.Key); Assert.Equal("test", kvp.Value); + }, + kvp => + { + Assert.Equal("state", kvp.Key); + Assert.Same(State, kvp.Value); }); + Assert.Equal("ReplacedEndpoint", candidates[0].Endpoint.DisplayName); + Assert.True(candidates.IsValidCandidate(0)); + } + + [Fact] + public async Task ApplyAsync_CanExpandTheListOfFoundEndpoints() + { + // Arrange + var policy = new DynamicControllerEndpointMatcherPolicy(Selector, Comparer); + + var endpoints = new[] { DynamicEndpoint, }; + var values = new RouteValueDictionary[] { new RouteValueDictionary(new { slug = "test", }), }; + var scores = new[] { 0, }; + + var candidates = new CandidateSet(endpoints, values, scores); + + Transform = (c, values, state) => + { + return new ValueTask(new RouteValueDictionary(new + { + controller = "Home", + action = "Index", + state + })); + }; + + Filter = (c, values, state, endpoints) => new ValueTask>(new[] + { + ControllerEndpoints[1], ControllerEndpoints[2] + }); + + var httpContext = new DefaultHttpContext() + { + RequestServices = Services, + }; + + // Act + await policy.ApplyAsync(httpContext, candidates); + + // Assert + Assert.Equal(2, candidates.Count); Assert.True(candidates.IsValidCandidate(0)); + Assert.True(candidates.IsValidCandidate(1)); + Assert.Same(ControllerEndpoints[1], candidates[0].Endpoint); + Assert.Same(ControllerEndpoints[2], candidates[1].Endpoint); } private class TestDynamicControllerEndpointSelector : DynamicControllerEndpointSelector @@ -261,11 +416,18 @@ public TestDynamicControllerEndpointSelector(EndpointDataSource dataSource) private class CustomTransformer : DynamicRouteValueTransformer { - public Func> Transform { get; set; } + public Func> Transform { get; set; } + + public Func, ValueTask>> Filter { get; set; } public override ValueTask TransformAsync(HttpContext httpContext, RouteValueDictionary values) { - return Transform(httpContext, values); + return Transform(httpContext, values, State); + } + + public override ValueTask> FilterAsync(HttpContext httpContext, RouteValueDictionary values, IReadOnlyList endpoints) + { + return Filter(httpContext, values, State, endpoints); } } } diff --git a/src/Mvc/Mvc.RazorPages/src/Builder/RazorPagesEndpointRouteBuilderExtensions.cs b/src/Mvc/Mvc.RazorPages/src/Builder/RazorPagesEndpointRouteBuilderExtensions.cs index 289da17e6dcc..a1f48fc00ee2 100644 --- a/src/Mvc/Mvc.RazorPages/src/Builder/RazorPagesEndpointRouteBuilderExtensions.cs +++ b/src/Mvc/Mvc.RazorPages/src/Builder/RazorPagesEndpointRouteBuilderExtensions.cs @@ -1,4 +1,4 @@ -// 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; @@ -299,6 +299,30 @@ public static IEndpointConventionBuilder MapFallbackToAreaPage( /// public static void MapDynamicPageRoute(this IEndpointRouteBuilder endpoints, string pattern) where TTransformer : DynamicRouteValueTransformer + { + MapDynamicPageRoute(endpoints, pattern, state: null); + } + + /// + /// Adds a specialized to the that will + /// attempt to select a page using the route values produced by . + /// + /// The to add the route to. + /// The URL pattern of the route. + /// A state object to provide to the instance. + /// The type of a . + /// + /// + /// This method allows the registration of a and + /// that combine to dynamically select a page using custom logic. + /// + /// + /// The instance of will be retrieved from the dependency injection container. + /// Register with the desired service lifetime in ConfigureServices. + /// + /// + public static void MapDynamicPageRoute(this IEndpointRouteBuilder endpoints, string pattern, object state) + where TTransformer : DynamicRouteValueTransformer { if (endpoints == null) { @@ -316,14 +340,14 @@ public static void MapDynamicPageRoute(this IEndpointRouteBuilder GetOrCreateDataSource(endpoints).CreateInertEndpoints = true; endpoints.Map( - pattern, + pattern, context => { throw new InvalidOperationException("This endpoint is not expected to be executed directly."); }) .Add(b => { - b.Metadata.Add(new DynamicPageRouteValueTransformerMetadata(typeof(TTransformer))); + b.Metadata.Add(new DynamicPageRouteValueTransformerMetadata(typeof(TTransformer), state)); }); } diff --git a/src/Mvc/Mvc.RazorPages/src/Infrastructure/DynamicPageEndpointMatcherPolicy.cs b/src/Mvc/Mvc.RazorPages/src/Infrastructure/DynamicPageEndpointMatcherPolicy.cs index 9f1faa2baa3e..62f8c1ad410b 100644 --- a/src/Mvc/Mvc.RazorPages/src/Infrastructure/DynamicPageEndpointMatcherPolicy.cs +++ b/src/Mvc/Mvc.RazorPages/src/Infrastructure/DynamicPageEndpointMatcherPolicy.cs @@ -1,4 +1,4 @@ -// 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; @@ -105,13 +105,15 @@ public async Task ApplyAsync(HttpContext httpContext, CandidateSet candidates) // no realistic way this could happen. var dynamicPageMetadata = endpoint.Metadata.GetMetadata(); var transformerMetadata = endpoint.Metadata.GetMetadata(); + DynamicRouteValueTransformer transformer = null; if (dynamicPageMetadata != null) { dynamicValues = dynamicPageMetadata.Values; } else if (transformerMetadata != null) { - var transformer = (DynamicRouteValueTransformer)httpContext.RequestServices.GetRequiredService(transformerMetadata.SelectorType); + transformer = (DynamicRouteValueTransformer)httpContext.RequestServices.GetRequiredService(transformerMetadata.SelectorType); + transformer.State = transformerMetadata.State; dynamicValues = await transformer.TransformAsync(httpContext, originalValues); } else @@ -154,6 +156,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); diff --git a/src/Mvc/Mvc.RazorPages/src/Infrastructure/DynamicPageRouteValueTransformerMetadata.cs b/src/Mvc/Mvc.RazorPages/src/Infrastructure/DynamicPageRouteValueTransformerMetadata.cs index 00c5dea04590..a3ab9dd64f1e 100644 --- a/src/Mvc/Mvc.RazorPages/src/Infrastructure/DynamicPageRouteValueTransformerMetadata.cs +++ b/src/Mvc/Mvc.RazorPages/src/Infrastructure/DynamicPageRouteValueTransformerMetadata.cs @@ -1,4 +1,4 @@ -// 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; @@ -9,7 +9,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure { internal class DynamicPageRouteValueTransformerMetadata : IDynamicEndpointMetadata { - public DynamicPageRouteValueTransformerMetadata(Type selectorType) + public DynamicPageRouteValueTransformerMetadata(Type selectorType, object state) { if (selectorType == null) { @@ -24,10 +24,13 @@ public DynamicPageRouteValueTransformerMetadata(Type selectorType) } SelectorType = selectorType; + State = state; } public bool IsDynamic => true; + public object State { get; } + public Type SelectorType { get; } } } diff --git a/src/Mvc/Mvc.RazorPages/test/Infrastructure/DynamicPageEndpointMatcherPolicyTest.cs b/src/Mvc/Mvc.RazorPages/test/Infrastructure/DynamicPageEndpointMatcherPolicyTest.cs index 1cea019f5ccd..ff3510144792 100644 --- a/src/Mvc/Mvc.RazorPages/test/Infrastructure/DynamicPageEndpointMatcherPolicyTest.cs +++ b/src/Mvc/Mvc.RazorPages/test/Infrastructure/DynamicPageEndpointMatcherPolicyTest.cs @@ -1,14 +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.Text; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Abstractions; -using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.AspNetCore.Mvc.Routing; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.Matching; @@ -30,6 +28,7 @@ public DynamicPageEndpointMatcherPolicyTest() { ["page"] = "/Index", }, + DisplayName = "/Index", }, new PageActionDescriptor() { @@ -37,6 +36,7 @@ public DynamicPageEndpointMatcherPolicyTest() { ["page"] = "/About", }, + DisplayName = "/About" }, }; @@ -50,7 +50,7 @@ public DynamicPageEndpointMatcherPolicyTest() _ => Task.CompletedTask, new EndpointMetadataCollection(new object[] { - new DynamicPageRouteValueTransformerMetadata(typeof(CustomTransformer)), + new DynamicPageRouteValueTransformerMetadata(typeof(CustomTransformer), State), }), "dynamic"); @@ -63,21 +63,35 @@ public DynamicPageEndpointMatcherPolicyTest() services.AddScoped(s => { var transformer = new CustomTransformer(); - transformer.Transform = (c, values) => Transform(c, values); + transformer.Transform = (c, values, state) => Transform(c, values, state); + transformer.Filter = (c, values, state, endpoints) => Filter(c, values, state, endpoints); return transformer; }); Services = services.BuildServiceProvider(); Comparer = Services.GetRequiredService(); - LoadedEndpoint = new Endpoint(_ => Task.CompletedTask, EndpointMetadataCollection.Empty, "Loaded"); + LoadedEndpoints = new[] + { + new Endpoint(_ => Task.CompletedTask, EndpointMetadataCollection.Empty, "Test1"), + new Endpoint(_ => Task.CompletedTask, EndpointMetadataCollection.Empty, "Test2"), + new Endpoint(_ => Task.CompletedTask, EndpointMetadataCollection.Empty, "ReplacedLoaded") + }; var loader = new Mock(); loader .Setup(l => l.LoadAsync(It.IsAny())) - .Returns(Task.FromResult(new CompiledPageActionDescriptor() { Endpoint = LoadedEndpoint, })); + .Returns(descriptor => Task.FromResult(new CompiledPageActionDescriptor + { + Endpoint = descriptor.DisplayName switch + { + "/Index" => LoadedEndpoints[0], + "/About" => LoadedEndpoints[1], + "/ReplacedEndpoint" => LoadedEndpoints[2], + _ => throw new InvalidOperationException($"Invalid endpoint '{descriptor.DisplayName}'.") + } + })); Loader = loader.Object; - } private EndpointMetadataComparer Comparer { get; } @@ -88,15 +102,19 @@ public DynamicPageEndpointMatcherPolicyTest() private Endpoint DynamicEndpoint { get; } - private Endpoint LoadedEndpoint { get; } + private Endpoint [] LoadedEndpoints { get; } private PageLoader Loader { get; } private DynamicPageEndpointSelector Selector { get; } + private object State { get; } + private IServiceProvider Services { get; } - private Func> Transform { get; set; } + private Func> Transform { get; set; } + + private Func, ValueTask>> Filter { get; set; } = (_, __, ___, e) => new ValueTask>(e); [Fact] public async Task ApplyAsync_NoMatch() @@ -111,7 +129,7 @@ public async Task ApplyAsync_NoMatch() var candidates = new CandidateSet(endpoints, values, scores); candidates.SetValidity(0, false); - Transform = (c, values) => + Transform = (c, values, state) => { throw new InvalidOperationException(); }; @@ -140,7 +158,7 @@ public async Task ApplyAsync_HasMatchNoEndpointFound() var candidates = new CandidateSet(endpoints, values, scores); - Transform = (c, values) => + Transform = (c, values, state) => { return new ValueTask(new RouteValueDictionary()); }; @@ -171,7 +189,7 @@ public async Task ApplyAsync_HasMatchFindsEndpoint_WithoutRouteValues() var candidates = new CandidateSet(endpoints, values, scores); - Transform = (c, values) => + Transform = (c, values, state) => { return new ValueTask(new RouteValueDictionary(new { @@ -188,7 +206,7 @@ public async Task ApplyAsync_HasMatchFindsEndpoint_WithoutRouteValues() await policy.ApplyAsync(httpContext, candidates); // Assert - Assert.Same(LoadedEndpoint, candidates[0].Endpoint); + Assert.Same(LoadedEndpoints[0], candidates[0].Endpoint); Assert.Collection( candidates[0].Values.OrderBy(kvp => kvp.Key), kvp => @@ -211,11 +229,12 @@ public async Task ApplyAsync_HasMatchFindsEndpoint_WithRouteValues() var candidates = new CandidateSet(endpoints, values, scores); - Transform = (c, values) => + Transform = (c, values, state) => { return new ValueTask(new RouteValueDictionary(new { page = "/Index", + state })); }; @@ -228,7 +247,7 @@ public async Task ApplyAsync_HasMatchFindsEndpoint_WithRouteValues() await policy.ApplyAsync(httpContext, candidates); // Assert - Assert.Same(LoadedEndpoint, candidates[0].Endpoint); + Assert.Same(LoadedEndpoints[0], candidates[0].Endpoint); Assert.Collection( candidates[0].Values.OrderBy(kvp => kvp.Key), kvp => @@ -240,10 +259,155 @@ public async Task ApplyAsync_HasMatchFindsEndpoint_WithRouteValues() { Assert.Equal("slug", kvp.Key); Assert.Equal("test", kvp.Value); + }, + kvp => + { + Assert.Equal("state", kvp.Key); + Assert.Same(State, kvp.Value); }); Assert.True(candidates.IsValidCandidate(0)); } + [Fact] + public async Task ApplyAsync_CanDiscardFoundEndpoints() + { + // Arrange + var policy = new DynamicPageEndpointMatcherPolicy(Selector, Loader, Comparer); + + var endpoints = new[] { DynamicEndpoint, }; + var values = new RouteValueDictionary[] { new RouteValueDictionary(new { slug = "test", }), }; + var scores = new[] { 0, }; + + var candidates = new CandidateSet(endpoints, values, scores); + + Transform = (c, values, state) => + { + return new ValueTask(new RouteValueDictionary(new + { + page = "/Index", + state + })); + }; + + Filter = (c, values, state, endpoints) => + { + return new ValueTask>(Array.Empty()); + }; + + var httpContext = new DefaultHttpContext() + { + RequestServices = Services, + }; + + // Act + await policy.ApplyAsync(httpContext, candidates); + + // Assert + Assert.False(candidates.IsValidCandidate(0)); + } + + [Fact] + public async Task ApplyAsync_CanReplaceFoundEndpoints() + { + // Arrange + var policy = new DynamicPageEndpointMatcherPolicy(Selector, Loader, Comparer); + + var endpoints = new[] { DynamicEndpoint, }; + var values = new RouteValueDictionary[] { new RouteValueDictionary(new { slug = "test", }), }; + var scores = new[] { 0, }; + + var candidates = new CandidateSet(endpoints, values, scores); + + Transform = (c, values, state) => + { + return new ValueTask(new RouteValueDictionary(new + { + page = "/Index", + state + })); + }; + + Filter = (c, values, state, endpoints) => new ValueTask>(new[] + { + new Endpoint((ctx) => Task.CompletedTask, new EndpointMetadataCollection(new PageActionDescriptor() + { + DisplayName = "/ReplacedEndpoint", + }), "ReplacedEndpoint") + }); + + var httpContext = new DefaultHttpContext() + { + RequestServices = Services, + }; + + // Act + await policy.ApplyAsync(httpContext, candidates); + + // Assert + Assert.Equal(1, candidates.Count); + Assert.Collection( + candidates[0].Values.OrderBy(kvp => kvp.Key), + kvp => + { + Assert.Equal("page", kvp.Key); + Assert.Equal("/Index", kvp.Value); + }, + kvp => + { + Assert.Equal("slug", kvp.Key); + Assert.Equal("test", kvp.Value); + }, + kvp => + { + Assert.Equal("state", kvp.Key); + Assert.Same(State, kvp.Value); + }); + Assert.Equal("ReplacedLoaded", candidates[0].Endpoint.DisplayName); + Assert.True(candidates.IsValidCandidate(0)); + } + + [Fact] + public async Task ApplyAsync_CanExpandTheListOfFoundEndpoints() + { + // Arrange + var policy = new DynamicPageEndpointMatcherPolicy(Selector, Loader, Comparer); + + var endpoints = new[] { DynamicEndpoint, }; + var values = new RouteValueDictionary[] { new RouteValueDictionary(new { slug = "test", }), }; + var scores = new[] { 0, }; + + var candidates = new CandidateSet(endpoints, values, scores); + + Transform = (c, values, state) => + { + return new ValueTask(new RouteValueDictionary(new + { + page = "/Index", + state + })); + }; + + Filter = (c, values, state, endpoints) => new ValueTask>(new[] + { + PageEndpoints[0], PageEndpoints[1] + }); + + var httpContext = new DefaultHttpContext() + { + RequestServices = Services, + }; + + // Act + await policy.ApplyAsync(httpContext, candidates); + + // Assert + Assert.Equal(2, candidates.Count); + Assert.True(candidates.IsValidCandidate(0)); + Assert.True(candidates.IsValidCandidate(1)); + Assert.Same(LoadedEndpoints[0], candidates[0].Endpoint); + Assert.Same(LoadedEndpoints[1], candidates[1].Endpoint); + } + private class TestDynamicPageEndpointSelector : DynamicPageEndpointSelector { public TestDynamicPageEndpointSelector(EndpointDataSource dataSource) @@ -254,11 +418,18 @@ public TestDynamicPageEndpointSelector(EndpointDataSource dataSource) private class CustomTransformer : DynamicRouteValueTransformer { - public Func> Transform { get; set; } + public Func> Transform { get; set; } + + public Func, ValueTask>> Filter { get; set; } + + public override ValueTask> FilterAsync(HttpContext httpContext, RouteValueDictionary values, IReadOnlyList endpoints) + { + return Filter(httpContext, values, State, endpoints); + } public override ValueTask TransformAsync(HttpContext httpContext, RouteValueDictionary values) { - return Transform(httpContext, values); + return Transform(httpContext, values, State); } } } diff --git a/src/Mvc/test/Mvc.FunctionalTests/RoutingDynamicTest.cs b/src/Mvc/test/Mvc.FunctionalTests/RoutingDynamicTest.cs index da6392f9fcc0..a55c3f92f8e6 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/RoutingDynamicTest.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/RoutingDynamicTest.cs @@ -1,4 +1,4 @@ -// 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.Linq; @@ -59,7 +59,7 @@ public async Task DynamicPage_CanGet404ForMissingAction() public async Task DynamicController_CanSelectControllerInArea() { // Arrange - var url = "http://localhost/dynamic/area%3Dadmin,controller%3Ddynamic,action%3Dindex"; + var url = "http://localhost/v1/dynamic/area%3Dadmin,controller%3Ddynamic,action%3Dindex"; var request = new HttpRequestMessage(HttpMethod.Get, url); // Act @@ -71,11 +71,25 @@ public async Task DynamicController_CanSelectControllerInArea() Assert.Equal("Hello from dynamic controller: /link_generation/dynamic/index", content); } + [Fact] + public async Task DynamicController_CanFilterResultsBasedOnState() + { + // Arrange + var url = "http://localhost/v2/dynamic/area%3Dadmin,controller%3Ddynamic,action%3Dindex"; + var request = new HttpRequestMessage(HttpMethod.Get, url); + + // Act + var response = await Client.SendAsync(request); + + // Assert + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } + [Fact] public async Task DynamicController_CanSelectControllerInArea_WithActionConstraints() { // Arrange - var url = "http://localhost/dynamic/area%3Dadmin,controller%3Ddynamic,action%3Dindex"; + var url = "http://localhost/v1/dynamic/area%3Dadmin,controller%3Ddynamic,action%3Dindex"; var request = new HttpRequestMessage(HttpMethod.Post, url); // Act @@ -91,7 +105,7 @@ public async Task DynamicController_CanSelectControllerInArea_WithActionConstrai public async Task DynamicPage_CanSelectPage() { // Arrange - var url = "http://localhost/dynamicpage/page%3D%2FDynamicPage"; + var url = "http://localhost/v1/dynamicpage/page%3D%2FDynamicPage"; var request = new HttpRequestMessage(HttpMethod.Get, url); // Act @@ -103,6 +117,21 @@ public async Task DynamicPage_CanSelectPage() Assert.Equal("Hello from dynamic page: /DynamicPage", content); } + [Fact] + public async Task DynamicPage_CanFilterBasedOnState() + { + // Arrange + var url = "http://localhost/v2/dynamicpage/page%3D%2FDynamicPage"; + var request = new HttpRequestMessage(HttpMethod.Get, url); + + // Act + var response = await Client.SendAsync(request); + var content = await response.Content.ReadAsStringAsync(); + + // Assert + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } + [Fact] public async Task AppWithDynamicRouteAndMapRazorPages_CanRouteToRazorPage() { diff --git a/src/Mvc/test/WebSites/RoutingWebSite/DynamicVersion.cs b/src/Mvc/test/WebSites/RoutingWebSite/DynamicVersion.cs new file mode 100644 index 000000000000..398f4c4bb3a4 --- /dev/null +++ b/src/Mvc/test/WebSites/RoutingWebSite/DynamicVersion.cs @@ -0,0 +1,10 @@ +// 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. + +namespace RoutingWebSite +{ + public class DynamicVersion + { + public string Version { get; set; } + } +} diff --git a/src/Mvc/test/WebSites/RoutingWebSite/StartupForDynamic.cs b/src/Mvc/test/WebSites/RoutingWebSite/StartupForDynamic.cs index 21d44d447d96..3bc3039f0c58 100644 --- a/src/Mvc/test/WebSites/RoutingWebSite/StartupForDynamic.cs +++ b/src/Mvc/test/WebSites/RoutingWebSite/StartupForDynamic.cs @@ -1,6 +1,8 @@ // 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.Threading.Tasks; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; @@ -32,8 +34,10 @@ public void Configure(IApplicationBuilder app) app.UseRouting(); app.UseEndpoints(endpoints => { - endpoints.MapDynamicControllerRoute("dynamic/{**slug}"); - endpoints.MapDynamicPageRoute("dynamicpage/{**slug}"); + endpoints.MapDynamicControllerRoute("v1/dynamic/{**slug}", new DynamicVersion { Version = "V1" }); + endpoints.MapDynamicControllerRoute("v2/dynamic/{**slug}", new DynamicVersion { Version = "V2" }); + endpoints.MapDynamicPageRoute("v1/dynamicpage/{**slug}", new DynamicVersion { Version = "V1" }); + endpoints.MapDynamicPageRoute("v2/dynamicpage/{**slug}", new DynamicVersion { Version = "V2" }); endpoints.MapControllerRoute("link", "link_generation/{controller}/{action}/{id?}"); @@ -59,8 +63,21 @@ public override ValueTask TransformAsync(HttpContext httpC results[split[0]] = split[1]; } + results["version"] = ((DynamicVersion)State).Version; + return new ValueTask(results); } + + public override ValueTask> FilterAsync(HttpContext httpContext, RouteValueDictionary values, IReadOnlyList endpoints) + { + var version = ((DynamicVersion)State).Version; + if (version == "V2" && version == (string)values["version"]) + { + // For v1 routes this transformer will work fine, for v2 routes, it will filter them. + return new ValueTask>(Array.Empty()); + } + return base.FilterAsync(httpContext, values, endpoints); + } } } } From 7fdf04e501bd27245007c4979866b37a9895d64d Mon Sep 17 00:00:00 2001 From: Javier Calvarro Nelson Date: Wed, 22 Jul 2020 04:07:18 -0700 Subject: [PATCH 4/7] Address feedback --- src/Mvc/Mvc.Core/src/Resources.resx | 3 ++ .../DynamicControllerEndpointMatcherPolicy.cs | 7 +++- ...amicControllerEndpointMatcherPolicyTest.cs | 33 ++++++++++++++++++- .../DynamicPageEndpointMatcherPolicy.cs | 4 +++ src/Mvc/Mvc.RazorPages/src/Resources.resx | 3 ++ .../DynamicPageEndpointMatcherPolicyTest.cs | 32 +++++++++++++++++- 6 files changed, 79 insertions(+), 3 deletions(-) diff --git a/src/Mvc/Mvc.Core/src/Resources.resx b/src/Mvc/Mvc.Core/src/Resources.resx index 2450503a4310..4afac908e6d0 100644 --- a/src/Mvc/Mvc.Core/src/Resources.resx +++ b/src/Mvc/Mvc.Core/src/Resources.resx @@ -519,4 +519,7 @@ A container cannot be specified when the ModelMetada is of kind '{0}'. + + Transformer '{0}' was retrieved from dependency injection with a state value. State can only be specified using the MapDynamicControllerRoute method's state argument and a transient transformer. '{0}' may not be registered as transient in dependency injection. + \ No newline at end of file diff --git a/src/Mvc/Mvc.Core/src/Routing/DynamicControllerEndpointMatcherPolicy.cs b/src/Mvc/Mvc.Core/src/Routing/DynamicControllerEndpointMatcherPolicy.cs index 2cd1a7dfe3f3..5f30a03eb744 100644 --- a/src/Mvc/Mvc.Core/src/Routing/DynamicControllerEndpointMatcherPolicy.cs +++ b/src/Mvc/Mvc.Core/src/Routing/DynamicControllerEndpointMatcherPolicy.cs @@ -1,4 +1,4 @@ -// 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; @@ -6,6 +6,7 @@ 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; @@ -106,6 +107,10 @@ public async Task ApplyAsync(HttpContext httpContext, CandidateSet candidates) else if (transformerMetadata != null) { 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); diff --git a/src/Mvc/Mvc.Core/test/Routing/DynamicControllerEndpointMatcherPolicyTest.cs b/src/Mvc/Mvc.Core/test/Routing/DynamicControllerEndpointMatcherPolicyTest.cs index ed95acba222f..03dbeab793c2 100644 --- a/src/Mvc/Mvc.Core/test/Routing/DynamicControllerEndpointMatcherPolicyTest.cs +++ b/src/Mvc/Mvc.Core/test/Routing/DynamicControllerEndpointMatcherPolicyTest.cs @@ -68,7 +68,7 @@ public DynamicControllerEndpointMatcherPolicyTest() var services = new ServiceCollection(); services.AddRouting(); - services.AddScoped(s => + services.AddTransient(s => { var transformer = new CustomTransformer(); transformer.Transform = (c, values, state) => Transform(c, values, state); @@ -205,6 +205,37 @@ public async Task ApplyAsync_HasMatchFindsEndpoint_WithoutRouteValues() Assert.True(candidates.IsValidCandidate(0)); } + [Fact] + public async Task ApplyAsync_ThrowsForTransformerWithInvalidLifetime() + { + // Arrange + var policy = new DynamicControllerEndpointMatcherPolicy(Selector, Comparer); + + var endpoints = new[] { DynamicEndpoint, }; + var values = new RouteValueDictionary[] { new RouteValueDictionary(new { slug = "test", }), }; + var scores = new[] { 0, }; + + var candidates = new CandidateSet(endpoints, values, scores); + + Transform = (c, values, state) => + { + return new ValueTask(new RouteValueDictionary(new + { + controller = "Home", + action = "Index", + state + })); + }; + + var httpContext = new DefaultHttpContext() + { + RequestServices = new ServiceCollection().AddScoped(sp => new CustomTransformer { State = "Invalid" }).BuildServiceProvider(), + }; + + // Act & Assert + await Assert.ThrowsAsync(() => policy.ApplyAsync(httpContext, candidates)); + } + [Fact] public async Task ApplyAsync_HasMatchFindsEndpoint_WithRouteValues() { diff --git a/src/Mvc/Mvc.RazorPages/src/Infrastructure/DynamicPageEndpointMatcherPolicy.cs b/src/Mvc/Mvc.RazorPages/src/Infrastructure/DynamicPageEndpointMatcherPolicy.cs index 62f8c1ad410b..337fe3e5f0ff 100644 --- a/src/Mvc/Mvc.RazorPages/src/Infrastructure/DynamicPageEndpointMatcherPolicy.cs +++ b/src/Mvc/Mvc.RazorPages/src/Infrastructure/DynamicPageEndpointMatcherPolicy.cs @@ -113,6 +113,10 @@ public async Task ApplyAsync(HttpContext httpContext, CandidateSet candidates) else if (transformerMetadata != null) { 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); } diff --git a/src/Mvc/Mvc.RazorPages/src/Resources.resx b/src/Mvc/Mvc.RazorPages/src/Resources.resx index 2d9e11cd4144..194a4389cb86 100644 --- a/src/Mvc/Mvc.RazorPages/src/Resources.resx +++ b/src/Mvc/Mvc.RazorPages/src/Resources.resx @@ -153,4 +153,7 @@ The model type for '{0}' is of type '{1}' which is not assignable to its declared model type '{2}'. + + Transformer '{0}' was retrieved from dependency injection with a state value. State can only be specified using the MapDynamicControllerRoute method's state argument and a transient transformer. '{0}' may not be registered as transient in dependency injection. + \ No newline at end of file diff --git a/src/Mvc/Mvc.RazorPages/test/Infrastructure/DynamicPageEndpointMatcherPolicyTest.cs b/src/Mvc/Mvc.RazorPages/test/Infrastructure/DynamicPageEndpointMatcherPolicyTest.cs index ff3510144792..7781158e0455 100644 --- a/src/Mvc/Mvc.RazorPages/test/Infrastructure/DynamicPageEndpointMatcherPolicyTest.cs +++ b/src/Mvc/Mvc.RazorPages/test/Infrastructure/DynamicPageEndpointMatcherPolicyTest.cs @@ -60,7 +60,7 @@ public DynamicPageEndpointMatcherPolicyTest() var services = new ServiceCollection(); services.AddRouting(); - services.AddScoped(s => + services.AddTransient(s => { var transformer = new CustomTransformer(); transformer.Transform = (c, values, state) => Transform(c, values, state); @@ -268,6 +268,36 @@ public async Task ApplyAsync_HasMatchFindsEndpoint_WithRouteValues() Assert.True(candidates.IsValidCandidate(0)); } + [Fact] + public async Task ApplyAsync_Throws_ForTransformersWithInvalidLifetime() + { + // Arrange + var policy = new DynamicPageEndpointMatcherPolicy(Selector, Loader, Comparer); + + var endpoints = new[] { DynamicEndpoint, }; + var values = new RouteValueDictionary[] { new RouteValueDictionary(new { slug = "test", }), }; + var scores = new[] { 0, }; + + var candidates = new CandidateSet(endpoints, values, scores); + + Transform = (c, values, state) => + { + return new ValueTask(new RouteValueDictionary(new + { + page = "/Index", + state + })); + }; + + var httpContext = new DefaultHttpContext() + { + RequestServices = new ServiceCollection().AddScoped(sp => new CustomTransformer() { State = "Invalid" }).BuildServiceProvider() + }; + + // Act & Assert + await Assert.ThrowsAsync(() => policy.ApplyAsync(httpContext, candidates)); + } + [Fact] public async Task ApplyAsync_CanDiscardFoundEndpoints() { From 3dcca5b6ca62c82f5f845e7774191ec790917bcc Mon Sep 17 00:00:00 2001 From: Javier Calvarro Nelson Date: Wed, 22 Jul 2020 13:27:55 +0200 Subject: [PATCH 5/7] Update src/Mvc/Mvc.Core/src/Routing/DynamicRouteValueTransformer.cs Co-authored-by: James Newton-King --- src/Mvc/Mvc.Core/src/Routing/DynamicRouteValueTransformer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Mvc/Mvc.Core/src/Routing/DynamicRouteValueTransformer.cs b/src/Mvc/Mvc.Core/src/Routing/DynamicRouteValueTransformer.cs index 6201c7095092..0b3adfe5a177 100644 --- a/src/Mvc/Mvc.Core/src/Routing/DynamicRouteValueTransformer.cs +++ b/src/Mvc/Mvc.Core/src/Routing/DynamicRouteValueTransformer.cs @@ -47,7 +47,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing public abstract class DynamicRouteValueTransformer { /// - /// Gets or sets a state value. An arbitrary value passed to the transformer from where was registered. + /// Gets or sets a state value. An arbitrary value passed to the transformer from where it was registered. /// /// /// Implementations that make use of must be registered as transient with the service From 603f567a6630be03b09cdc3057e0c2fbb5c52a0d Mon Sep 17 00:00:00 2001 From: Javier Calvarro Nelson Date: Wed, 22 Jul 2020 06:02:12 -0700 Subject: [PATCH 6/7] Update E2E tests --- src/Mvc/test/WebSites/RoutingWebSite/StartupForDynamic.cs | 2 +- .../WebSites/RoutingWebSite/StartupForDynamicAndRazorPages.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Mvc/test/WebSites/RoutingWebSite/StartupForDynamic.cs b/src/Mvc/test/WebSites/RoutingWebSite/StartupForDynamic.cs index 3bc3039f0c58..7b22a48dce0a 100644 --- a/src/Mvc/test/WebSites/RoutingWebSite/StartupForDynamic.cs +++ b/src/Mvc/test/WebSites/RoutingWebSite/StartupForDynamic.cs @@ -23,7 +23,7 @@ public void ConfigureServices(IServiceCollection services) .AddNewtonsoftJson() .SetCompatibilityVersion(CompatibilityVersion.Latest); - services.AddSingleton(); + services.AddTransient(); // Used by some controllers defined in this project. services.Configure(options => options.ConstraintMap["slugify"] = typeof(SlugifyParameterTransformer)); diff --git a/src/Mvc/test/WebSites/RoutingWebSite/StartupForDynamicAndRazorPages.cs b/src/Mvc/test/WebSites/RoutingWebSite/StartupForDynamicAndRazorPages.cs index 7aa4e9f609b6..1048a972ea9a 100644 --- a/src/Mvc/test/WebSites/RoutingWebSite/StartupForDynamicAndRazorPages.cs +++ b/src/Mvc/test/WebSites/RoutingWebSite/StartupForDynamicAndRazorPages.cs @@ -20,7 +20,7 @@ public void ConfigureServices(IServiceCollection services) .AddMvc() .SetCompatibilityVersion(CompatibilityVersion.Latest); - services.AddSingleton(); + services.AddTransient(); // Used by some controllers defined in this project. services.Configure(options => options.ConstraintMap["slugify"] = typeof(SlugifyParameterTransformer)); From 766594282c147a5a328424c1affd9703507cdc81 Mon Sep 17 00:00:00 2001 From: Javier Calvarro Nelson Date: Wed, 22 Jul 2020 23:17:18 +0200 Subject: [PATCH 7/7] Apply suggestions from code review Cleaned up error messages. Thanks @jamesnk, I totally overlooked the content. Co-authored-by: James Newton-King --- src/Mvc/Mvc.Core/src/Resources.resx | 4 ++-- src/Mvc/Mvc.RazorPages/src/Resources.resx | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Mvc/Mvc.Core/src/Resources.resx b/src/Mvc/Mvc.Core/src/Resources.resx index 4afac908e6d0..93966b49f766 100644 --- a/src/Mvc/Mvc.Core/src/Resources.resx +++ b/src/Mvc/Mvc.Core/src/Resources.resx @@ -520,6 +520,6 @@ A container cannot be specified when the ModelMetada is of kind '{0}'. - Transformer '{0}' was retrieved from dependency injection with a state value. State can only be specified using the MapDynamicControllerRoute method's state argument and a transient transformer. '{0}' may not be registered as transient in dependency injection. + 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. - \ No newline at end of file + diff --git a/src/Mvc/Mvc.RazorPages/src/Resources.resx b/src/Mvc/Mvc.RazorPages/src/Resources.resx index 194a4389cb86..f6a9a1805991 100644 --- a/src/Mvc/Mvc.RazorPages/src/Resources.resx +++ b/src/Mvc/Mvc.RazorPages/src/Resources.resx @@ -154,6 +154,6 @@ The model type for '{0}' is of type '{1}' which is not assignable to its declared model type '{2}'. - Transformer '{0}' was retrieved from dependency injection with a state value. State can only be specified using the MapDynamicControllerRoute method's state argument and a transient transformer. '{0}' may not be registered as transient in dependency injection. + Transformer '{0}' was retrieved from dependency injection with a state value. State can only be specified when the dynamic route is mapped using MapDynamicPageRoute'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. - \ No newline at end of file +