From 85e92ab3cc4ec28a5386a15bf2a8df5c228d30cc Mon Sep 17 00:00:00 2001 From: Nate McMaster Date: Thu, 28 Jun 2018 16:48:57 -0700 Subject: [PATCH 1/3] Added support for route constraints in Dispatcher world --- .../Benchmarks/StartupUsingDispatcher.cs | 1 + .../Matchers/MatcherBenchmarkBase.cs | 15 +- .../EndsWithStringMatchProcessor.cs | 40 ++++ samples/DispatcherSample.Web/Startup.cs | 54 ++++- .../DefaultInlineConstraintResolver.cs | 2 +- .../DefaultLinkGenerator.cs | 51 ++++- .../RoutingServiceCollectionExtensions.cs | 3 + .../Internal/NullRouter.cs | 26 +++ .../Matchers/DefaultMatchProcessorFactory.cs | 156 ++++++++++++++ .../Matchers/MatchProcessor.cs | 18 ++ .../Matchers/MatchProcessorBase.cs | 42 ++++ .../Matchers/MatchProcessorFactory.cs | 10 + .../Matchers/MatchProcessorReference.cs | 86 ++++++++ .../Matchers/MatcherEndpoint.cs | 35 ++++ .../Matchers/OptionalMatchProcessor.cs | 44 ++++ .../Matchers/TreeMatcher.cs | 64 +++--- .../Matchers/TreeMatcherFactory.cs | 12 +- .../RouteValuesBasedEndpointFinder.cs | 44 +--- .../DispatcherSampleTest.cs | 68 +++++++ .../DefaultLinkGeneratorTest.cs | 9 +- .../DispatcherMiddlewareTest.cs | 2 - .../EndpointSelectorTests.cs | 6 +- .../DefaultMatchProcessorFactoryTest.cs | 190 ++++++++++++++++++ .../Matchers/MatcherConformanceTest.cs | 2 + .../Matchers/TreeMatcherTests.cs | 16 +- 25 files changed, 886 insertions(+), 110 deletions(-) create mode 100644 samples/DispatcherSample.Web/EndsWithStringMatchProcessor.cs create mode 100644 src/Microsoft.AspNetCore.Routing/Internal/NullRouter.cs create mode 100644 src/Microsoft.AspNetCore.Routing/Matchers/DefaultMatchProcessorFactory.cs create mode 100644 src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessor.cs create mode 100644 src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessorBase.cs create mode 100644 src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessorFactory.cs create mode 100644 src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessorReference.cs create mode 100644 src/Microsoft.AspNetCore.Routing/Matchers/OptionalMatchProcessor.cs create mode 100644 test/Microsoft.AspNetCore.Routing.Tests/Matchers/DefaultMatchProcessorFactoryTest.cs diff --git a/benchmarkapps/Benchmarks/StartupUsingDispatcher.cs b/benchmarkapps/Benchmarks/StartupUsingDispatcher.cs index 153b6e42cc..1351a20689 100644 --- a/benchmarkapps/Benchmarks/StartupUsingDispatcher.cs +++ b/benchmarkapps/Benchmarks/StartupUsingDispatcher.cs @@ -34,6 +34,7 @@ namespace Benchmarks template: "/plaintext", defaults: new RouteValueDictionary(), requiredValues: new RouteValueDictionary(), + nonInlineMatchProcessorReferences: null, order: 0, metadata: EndpointMetadataCollection.Empty, displayName: "Plaintext"), diff --git a/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matchers/MatcherBenchmarkBase.cs b/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matchers/MatcherBenchmarkBase.cs index 1ab67c9235..2e10ed2397 100644 --- a/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matchers/MatcherBenchmarkBase.cs +++ b/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matchers/MatcherBenchmarkBase.cs @@ -34,13 +34,14 @@ namespace Microsoft.AspNetCore.Routing.Matchers } return new MatcherEndpoint( - (next) => (context) => Task.CompletedTask, - template, - new RouteValueDictionary(), - new RouteValueDictionary(), - 0, - new EndpointMetadataCollection(metadata), - template); + (next) => (context) => Task.CompletedTask, + template, + new RouteValueDictionary(), + new RouteValueDictionary(), + new List(), + 0, + EndpointMetadataCollection.Empty, + template); } internal static int[] SampleRequests(int endpointCount, int count) diff --git a/samples/DispatcherSample.Web/EndsWithStringMatchProcessor.cs b/samples/DispatcherSample.Web/EndsWithStringMatchProcessor.cs new file mode 100644 index 0000000000..7284dc6d66 --- /dev/null +++ b/samples/DispatcherSample.Web/EndsWithStringMatchProcessor.cs @@ -0,0 +1,40 @@ +// 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.Globalization; +using Microsoft.AspNetCore.Routing.Matchers; +using Microsoft.Extensions.Logging; + +namespace DispatcherSample.Web +{ + internal class EndsWithStringMatchProcessor : MatchProcessorBase + { + private readonly ILogger _logger; + + public EndsWithStringMatchProcessor(ILogger logger) + { + _logger = logger; + } + + public override bool Process(object value) + { + if (value == null) + { + return false; + } + + var valueString = Convert.ToString(value, CultureInfo.InvariantCulture); + + var endsWith = valueString.EndsWith(ConstraintArgument, StringComparison.OrdinalIgnoreCase); + + if (!endsWith) + { + _logger.LogDebug( + $"Parameter '{ParameterName}' with value '{valueString}' does not end with '{ConstraintArgument}'."); + } + + return endsWith; + } + } +} diff --git a/samples/DispatcherSample.Web/Startup.cs b/samples/DispatcherSample.Web/Startup.cs index eca7ab2235..9f8747310f 100644 --- a/samples/DispatcherSample.Web/Startup.cs +++ b/samples/DispatcherSample.Web/Startup.cs @@ -2,8 +2,10 @@ // 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.Text; using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.Matchers; using Microsoft.Extensions.DependencyInjection; @@ -17,7 +19,13 @@ namespace DispatcherSample.Web public void ConfigureServices(IServiceCollection services) { - services.AddRouting(); + services.AddTransient(); + + services.AddRouting(options => + { + options.ConstraintMap.Add("endsWith", typeof(EndsWithStringMatchProcessor)); + }); + services.AddDispatcher(options => { options.DataSources.Add(new DefaultEndpointDataSource(new[] @@ -31,7 +39,13 @@ namespace DispatcherSample.Web response.ContentLength = payloadLength; return response.Body.WriteAsync(_homePayload, 0, payloadLength); }, - "/", new RouteValueDictionary(), new RouteValueDictionary(), 0, EndpointMetadataCollection.Empty, "Home"), + "/", + new RouteValueDictionary(), + new RouteValueDictionary(), + new List(), + 0, + EndpointMetadataCollection.Empty, + "Home"), new MatcherEndpoint((next) => (httpContext) => { var response = httpContext.Response; @@ -41,7 +55,41 @@ namespace DispatcherSample.Web response.ContentLength = payloadLength; return response.Body.WriteAsync(_helloWorldPayload, 0, payloadLength); }, - "/plaintext", new RouteValueDictionary(), new RouteValueDictionary(), 0, EndpointMetadataCollection.Empty, "Plaintext"), + "/plaintext", + new RouteValueDictionary(), + new RouteValueDictionary(), + new List(), + 0, + EndpointMetadataCollection.Empty, + "Plaintext"), + new MatcherEndpoint((next) => (httpContext) => + { + var response = httpContext.Response; + response.StatusCode = 200; + response.ContentType = "text/plain"; + return response.WriteAsync("WithConstraints"); + }, + "/withconstraints/{id:endsWith(_001)}", + new RouteValueDictionary(), + new RouteValueDictionary(), + new List(), + 0, + EndpointMetadataCollection.Empty, + "withconstraints"), + new MatcherEndpoint((next) => (httpContext) => + { + var response = httpContext.Response; + response.StatusCode = 200; + response.ContentType = "text/plain"; + return response.WriteAsync("withoptionalconstraints"); + }, + "/withoptionalconstraints/{id:endsWith(_001)?}", + new RouteValueDictionary(), + new RouteValueDictionary(), + new List(), + 0, + EndpointMetadataCollection.Empty, + "withoptionalconstraints"), })); }); } diff --git a/src/Microsoft.AspNetCore.Routing/DefaultInlineConstraintResolver.cs b/src/Microsoft.AspNetCore.Routing/DefaultInlineConstraintResolver.cs index 7426516db2..ed58e053ba 100644 --- a/src/Microsoft.AspNetCore.Routing/DefaultInlineConstraintResolver.cs +++ b/src/Microsoft.AspNetCore.Routing/DefaultInlineConstraintResolver.cs @@ -90,7 +90,7 @@ namespace Microsoft.AspNetCore.Routing } } - private static IRouteConstraint CreateConstraint(Type constraintType, string argumentString) + internal static IRouteConstraint CreateConstraint(Type constraintType, string argumentString) { // No arguments - call the default constructor if (argumentString == null) diff --git a/src/Microsoft.AspNetCore.Routing/DefaultLinkGenerator.cs b/src/Microsoft.AspNetCore.Routing/DefaultLinkGenerator.cs index 7efe2cedd8..e5b5f4ea93 100644 --- a/src/Microsoft.AspNetCore.Routing/DefaultLinkGenerator.cs +++ b/src/Microsoft.AspNetCore.Routing/DefaultLinkGenerator.cs @@ -13,15 +13,18 @@ using Microsoft.Extensions.ObjectPool; namespace Microsoft.AspNetCore.Routing { - public class DefaultLinkGenerator : ILinkGenerator + internal class DefaultLinkGenerator : ILinkGenerator { + private readonly MatchProcessorFactory _matchProcessorFactory; private readonly ObjectPool _uriBuildingContextPool; private readonly ILogger _logger; public DefaultLinkGenerator( + MatchProcessorFactory matchProcessorFactory, ObjectPool uriBuildingContextPool, ILogger logger) { + _matchProcessorFactory = matchProcessorFactory; _uriBuildingContextPool = uriBuildingContextPool; _logger = logger; } @@ -61,7 +64,7 @@ namespace Microsoft.AspNetCore.Routing foreach (var endpoint in matcherEndpoints) { - link = GetLink(endpoint.ParsedTemplate, endpoint.Defaults, explicitValues, ambientValues); + link = GetLink(endpoint, explicitValues, ambientValues); if (link != null) { return true; @@ -72,27 +75,55 @@ namespace Microsoft.AspNetCore.Routing } private string GetLink( - RouteTemplate template, - RouteValueDictionary defaults, + MatcherEndpoint endpoint, RouteValueDictionary explicitValues, RouteValueDictionary ambientValues) { var templateBinder = new TemplateBinder( UrlEncoder.Default, _uriBuildingContextPool, - template, - defaults); + endpoint.ParsedTemplate, + endpoint.Defaults); - var values = templateBinder.GetValues(ambientValues, explicitValues); - if (values == null) + var templateValuesResult = templateBinder.GetValues(ambientValues, explicitValues); + if (templateValuesResult == null) { // We're missing one of the required values for this route. return null; } - //TODO: route constraint matching here + if (!Match(endpoint, templateValuesResult.CombinedValues)) + { + return null; + } - return templateBinder.BindValues(values.AcceptedValues); + return templateBinder.BindValues(templateValuesResult.AcceptedValues); + } + + private bool Match(MatcherEndpoint endpoint, RouteValueDictionary routeValues) + { + if (routeValues == null) + { + throw new ArgumentNullException(nameof(routeValues)); + } + + for (var i = 0; i < endpoint.MatchProcessorReferences.Count; i++) + { + var matchProcessorReference = endpoint.MatchProcessorReferences[i]; + var parameter = endpoint.ParsedTemplate.GetParameter(matchProcessorReference.ParameterName); + if (parameter.IsOptional && !routeValues.ContainsKey(parameter.Name)) + { + continue; + } + + var matchProcessor = _matchProcessorFactory.Create(matchProcessorReference); + if (!matchProcessor.ProcessOutbound(httpContext: null, routeValues)) + { + return false; + } + } + + return true; } } } diff --git a/src/Microsoft.AspNetCore.Routing/DependencyInjection/RoutingServiceCollectionExtensions.cs b/src/Microsoft.AspNetCore.Routing/DependencyInjection/RoutingServiceCollectionExtensions.cs index d6883f6c95..18ededc423 100644 --- a/src/Microsoft.AspNetCore.Routing/DependencyInjection/RoutingServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNetCore.Routing/DependencyInjection/RoutingServiceCollectionExtensions.cs @@ -3,7 +3,9 @@ using System; using Microsoft.AspNetCore.Routing; +using Microsoft.AspNetCore.Routing.Constraints; using Microsoft.AspNetCore.Routing.Internal; +using Microsoft.AspNetCore.Routing.Matchers; using Microsoft.AspNetCore.Routing.Tree; using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Logging; @@ -28,6 +30,7 @@ namespace Microsoft.Extensions.DependencyInjection throw new ArgumentNullException(nameof(services)); } + services.TryAddSingleton(); services.TryAddTransient(); services.TryAddSingleton>(s => { diff --git a/src/Microsoft.AspNetCore.Routing/Internal/NullRouter.cs b/src/Microsoft.AspNetCore.Routing/Internal/NullRouter.cs new file mode 100644 index 0000000000..9fd1908942 --- /dev/null +++ b/src/Microsoft.AspNetCore.Routing/Internal/NullRouter.cs @@ -0,0 +1,26 @@ +// 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.Threading.Tasks; + +namespace Microsoft.AspNetCore.Routing +{ + internal class NullRouter : IRouter + { + public static readonly NullRouter Instance = new NullRouter(); + + private NullRouter() + { + } + + public VirtualPathData GetVirtualPath(VirtualPathContext context) + { + return null; + } + + public Task RouteAsync(RouteContext context) + { + return Task.CompletedTask; + } + } +} diff --git a/src/Microsoft.AspNetCore.Routing/Matchers/DefaultMatchProcessorFactory.cs b/src/Microsoft.AspNetCore.Routing/Matchers/DefaultMatchProcessorFactory.cs new file mode 100644 index 0000000000..5226683626 --- /dev/null +++ b/src/Microsoft.AspNetCore.Routing/Matchers/DefaultMatchProcessorFactory.cs @@ -0,0 +1,156 @@ +// 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 Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; + +namespace Microsoft.AspNetCore.Routing.Matchers +{ + internal class DefaultMatchProcessorFactory : MatchProcessorFactory + { + private readonly RouteOptions _options; + private readonly ILogger _logger; + private readonly IServiceProvider _serviceProvider; + + public DefaultMatchProcessorFactory( + IOptions options, + ILogger logger, + IServiceProvider serviceProvider) + { + _options = options.Value; + _logger = logger; + _serviceProvider = serviceProvider; + } + + public override MatchProcessor Create(MatchProcessorReference matchProcessorReference) + { + if (matchProcessorReference == null) + { + throw new ArgumentNullException(nameof(matchProcessorReference)); + } + + if (matchProcessorReference.MatchProcessor != null) + { + return matchProcessorReference.MatchProcessor; + } + + // Example: + // {productId:regex(\d+)} + // + // ParameterName: productId + // ConstraintText: regex(\d+) + // ConstraintName: regex + // ConstraintArgument: \d+ + + (var constraintName, var constraintArgument) = Parse(matchProcessorReference.ConstraintText); + + if (!_options.ConstraintMap.TryGetValue(constraintName, out var constraintType)) + { + throw new InvalidOperationException( + $"No constraint has been registered with name '{constraintName}'."); + } + + var processor = ResolveMatchProcessor( + matchProcessorReference.ParameterName, + matchProcessorReference.Optional, + constraintType, + constraintArgument); + + if (processor != null) + { + return processor; + } + + if (!typeof(IRouteConstraint).IsAssignableFrom(constraintType)) + { + throw new InvalidOperationException( + Resources.FormatDefaultInlineConstraintResolver_TypeNotConstraint( + constraintType, constraintName, typeof(IRouteConstraint).Name)); + } + + try + { + return CreateMatchProcessorFromRouteConstraint( + matchProcessorReference.ParameterName, + constraintType, + constraintArgument); + } + catch (RouteCreationException) + { + throw; + } + catch (Exception exception) + { + throw new InvalidOperationException( + $"An error occurred while trying to create an instance of route constraint '{constraintType.FullName}'.", + exception); + } + } + + private MatchProcessor CreateMatchProcessorFromRouteConstraint( + string parameterName, + Type constraintType, + string constraintArgument) + { + var routeConstraint = DefaultInlineConstraintResolver.CreateConstraint(constraintType, constraintArgument); + return (new MatchProcessorReference(parameterName, routeConstraint)).MatchProcessor; + } + + private MatchProcessor ResolveMatchProcessor( + string parameterName, + bool optional, + Type constraintType, + string constraintArgument) + { + if (constraintType == null) + { + throw new ArgumentNullException(nameof(constraintType)); + } + + if (!typeof(MatchProcessor).IsAssignableFrom(constraintType)) + { + // Since a constraint type could be of type IRouteConstraint, do not throw + return null; + } + + var registeredProcessor = _serviceProvider.GetRequiredService(constraintType); + if (registeredProcessor is MatchProcessor matchProcessor) + { + if (optional) + { + matchProcessor = new OptionalMatchProcessor(matchProcessor); + } + + matchProcessor.Initialize(parameterName, constraintArgument); + return matchProcessor; + } + else + { + throw new InvalidOperationException( + $"Registered constraint type '{constraintType}' is not of type '{typeof(MatchProcessor)}'."); + } + } + + private (string constraintName, string constraintArgument) Parse(string constraintText) + { + string constraintName; + string constraintArgument; + var indexOfFirstOpenParens = constraintText.IndexOf('('); + if (indexOfFirstOpenParens >= 0 && constraintText.EndsWith(")", StringComparison.Ordinal)) + { + constraintName = constraintText.Substring(0, indexOfFirstOpenParens); + constraintArgument = constraintText.Substring( + indexOfFirstOpenParens + 1, + constraintText.Length - indexOfFirstOpenParens - 2); + } + else + { + constraintName = constraintText; + constraintArgument = null; + } + return (constraintName, constraintArgument); + } + } +} diff --git a/src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessor.cs b/src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessor.cs new file mode 100644 index 0000000000..065ccd8e9b --- /dev/null +++ b/src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessor.cs @@ -0,0 +1,18 @@ +// 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 Microsoft.AspNetCore.Http; + +namespace Microsoft.AspNetCore.Routing.Matchers +{ + public abstract class MatchProcessor + { + public virtual void Initialize(string parameterName, string constraintArgument) + { + } + + public abstract bool ProcessInbound(HttpContext httpContext, RouteValueDictionary values); + + public abstract bool ProcessOutbound(HttpContext httpContext, RouteValueDictionary values); + } +} diff --git a/src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessorBase.cs b/src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessorBase.cs new file mode 100644 index 0000000000..d3f9db3811 --- /dev/null +++ b/src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessorBase.cs @@ -0,0 +1,42 @@ +// 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 Microsoft.AspNetCore.Http; + +namespace Microsoft.AspNetCore.Routing.Matchers +{ + public abstract class MatchProcessorBase : MatchProcessor + { + public string ParameterName { get; private set; } + + public string ConstraintArgument { get; private set; } + + public override void Initialize(string parameterName, string constraintArgument) + { + ParameterName = parameterName; + ConstraintArgument = constraintArgument; + } + + public abstract bool Process(object value); + + public override bool ProcessInbound(HttpContext httpContext, RouteValueDictionary values) + { + return Process(values); + } + + public override bool ProcessOutbound(HttpContext httpContext, RouteValueDictionary values) + { + return Process(values); + } + + private bool Process(RouteValueDictionary values) + { + if (!values.TryGetValue(ParameterName, out var value) || value == null) + { + return false; + } + + return Process(value); + } + } +} diff --git a/src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessorFactory.cs b/src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessorFactory.cs new file mode 100644 index 0000000000..95624ea420 --- /dev/null +++ b/src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessorFactory.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 Microsoft.AspNetCore.Routing.Matchers +{ + internal abstract class MatchProcessorFactory + { + public abstract MatchProcessor Create(MatchProcessorReference matchProcessorReference); + } +} diff --git a/src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessorReference.cs b/src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessorReference.cs new file mode 100644 index 0000000000..6266bb22e1 --- /dev/null +++ b/src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessorReference.cs @@ -0,0 +1,86 @@ +// 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 Microsoft.AspNetCore.Http; + +namespace Microsoft.AspNetCore.Routing.Matchers +{ + public sealed class MatchProcessorReference + { + // Example: + // api/products/{productId:regex(\d+)} + // + // ParameterName = productId + // ConstraintText = regex(\d+) + // ConstraintArgument = \d+ + + public MatchProcessorReference(string parameterName, string constraintText) + { + ParameterName = parameterName; + ConstraintText = constraintText; + } + + public MatchProcessorReference(string parameterName, bool optional, string constraintText) + { + ParameterName = parameterName; + Optional = optional; + ConstraintText = constraintText; + } + + public MatchProcessorReference(string parameterName, MatchProcessor matchProcessor) + { + ParameterName = parameterName; + MatchProcessor = matchProcessor; + } + + internal MatchProcessor MatchProcessor { get; private set; } + + internal string ConstraintText { get; private set; } + + internal string ParameterName { get; private set; } + + internal bool Optional { get; private set; } + + public MatchProcessorReference(string parameterName, IRouteConstraint routeConstraint) + : this(parameterName, new RouteConstraintMatchProcessorAdapter(parameterName, routeConstraint)) + { + } + + private class RouteConstraintMatchProcessorAdapter : MatchProcessor + { + public string ParameterName { get; private set; } + + public IRouteConstraint RouteConstraint { get; } + + public RouteConstraintMatchProcessorAdapter(string parameterName, IRouteConstraint routeConstraint) + { + ParameterName = parameterName; + RouteConstraint = routeConstraint; + } + + public override void Initialize(string parameterName, string constraintArgument) + { + } + + public override bool ProcessInbound(HttpContext httpContext, RouteValueDictionary routeValues) + { + return RouteConstraint.Match( + httpContext, + NullRouter.Instance, + ParameterName, + routeValues, + RouteDirection.IncomingRequest); + } + + public override bool ProcessOutbound(HttpContext httpContext, RouteValueDictionary values) + { + return RouteConstraint.Match( + httpContext, + NullRouter.Instance, + ParameterName, + values, + RouteDirection.UrlGeneration); + } + } + } +} diff --git a/src/Microsoft.AspNetCore.Routing/Matchers/MatcherEndpoint.cs b/src/Microsoft.AspNetCore.Routing/Matchers/MatcherEndpoint.cs index 41b629ef6c..4d7fa50e0e 100644 --- a/src/Microsoft.AspNetCore.Routing/Matchers/MatcherEndpoint.cs +++ b/src/Microsoft.AspNetCore.Routing/Matchers/MatcherEndpoint.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing.Template; @@ -20,6 +21,7 @@ namespace Microsoft.AspNetCore.Routing.Matchers string template, RouteValueDictionary defaults, RouteValueDictionary requiredValues, + List nonInlineMatchProcessorReferences, int order, EndpointMetadataCollection metadata, string displayName) @@ -44,6 +46,9 @@ namespace Microsoft.AspNetCore.Routing.Matchers RequiredValues = requiredValues; var mergedDefaults = GetDefaults(ParsedTemplate, defaults); Defaults = mergedDefaults; + + var mergedReferences = MergeMatchProcessorReferences(ParsedTemplate, nonInlineMatchProcessorReferences); + MatchProcessorReferences = mergedReferences.AsReadOnly(); } public int Order { get; } @@ -57,6 +62,8 @@ namespace Microsoft.AspNetCore.Routing.Matchers // Todo: needs review public RouteTemplate ParsedTemplate { get; } + public IReadOnlyList MatchProcessorReferences { get; } + // Merge inline and non inline defaults into one private RouteValueDictionary GetDefaults(RouteTemplate parsedTemplate, RouteValueDictionary nonInlineDefaults) { @@ -81,5 +88,33 @@ namespace Microsoft.AspNetCore.Routing.Matchers return result; } + + private List MergeMatchProcessorReferences( + RouteTemplate parsedTemplate, + List nonInlineReferences) + { + var matchProcessorReferences = new List(); + + if (nonInlineReferences != null) + { + matchProcessorReferences.AddRange(nonInlineReferences); + } + + foreach (var parameter in parsedTemplate.Parameters) + { + if (parameter.InlineConstraints != null) + { + foreach (var constraint in parameter.InlineConstraints) + { + matchProcessorReferences.Add( + new MatchProcessorReference( + parameter.Name, + optional: parameter.IsOptional, + constraintText: constraint.Constraint)); + } + } + } + return matchProcessorReferences; + } } } diff --git a/src/Microsoft.AspNetCore.Routing/Matchers/OptionalMatchProcessor.cs b/src/Microsoft.AspNetCore.Routing/Matchers/OptionalMatchProcessor.cs new file mode 100644 index 0000000000..9d445e1a44 --- /dev/null +++ b/src/Microsoft.AspNetCore.Routing/Matchers/OptionalMatchProcessor.cs @@ -0,0 +1,44 @@ +// 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 Microsoft.AspNetCore.Http; + +namespace Microsoft.AspNetCore.Routing.Matchers +{ + internal class OptionalMatchProcessor : MatchProcessor + { + private readonly MatchProcessor _innerMatchProcessor; + + public OptionalMatchProcessor(MatchProcessor innerMatchProcessor) + { + _innerMatchProcessor = innerMatchProcessor; + } + + public string ParameterName { get; private set; } + + public override void Initialize(string parameterName, string constraintArgument) + { + ParameterName = parameterName; + _innerMatchProcessor.Initialize(parameterName, constraintArgument); + } + + public override bool ProcessInbound(HttpContext httpContext, RouteValueDictionary values) + { + return Process(httpContext, values); + } + + public override bool ProcessOutbound(HttpContext httpContext, RouteValueDictionary values) + { + return Process(httpContext, values); + } + + private bool Process(HttpContext httpContext, RouteValueDictionary values) + { + if (values.TryGetValue(ParameterName, out var value)) + { + return _innerMatchProcessor.ProcessInbound(httpContext, values); + } + return true; + } + } +} diff --git a/src/Microsoft.AspNetCore.Routing/Matchers/TreeMatcher.cs b/src/Microsoft.AspNetCore.Routing/Matchers/TreeMatcher.cs index 8b1795b0f5..72fc5c9f6c 100644 --- a/src/Microsoft.AspNetCore.Routing/Matchers/TreeMatcher.cs +++ b/src/Microsoft.AspNetCore.Routing/Matchers/TreeMatcher.cs @@ -17,20 +17,20 @@ namespace Microsoft.AspNetCore.Routing.Matchers { internal class TreeMatcher : Matcher { - private readonly IInlineConstraintResolver _constraintFactory; + private readonly MatchProcessorFactory _matchProcessorFactory; private readonly ILogger _logger; private readonly EndpointSelector _endpointSelector; private readonly DataSourceDependantCache _cache; public TreeMatcher( - IInlineConstraintResolver constraintFactory, + MatchProcessorFactory matchProcessorFactory, ILogger logger, EndpointDataSource dataSource, EndpointSelector endpointSelector) { - if (constraintFactory == null) + if (matchProcessorFactory == null) { - throw new ArgumentNullException(nameof(constraintFactory)); + throw new ArgumentNullException(nameof(matchProcessorFactory)); } if (logger == null) @@ -43,7 +43,7 @@ namespace Microsoft.AspNetCore.Routing.Matchers throw new ArgumentNullException(nameof(dataSource)); } - _constraintFactory = constraintFactory; + _matchProcessorFactory = matchProcessorFactory; _logger = logger; _endpointSelector = endpointSelector; _cache = new DataSourceDependantCache(dataSource, CreateTrees); @@ -79,6 +79,7 @@ namespace Microsoft.AspNetCore.Routing.Matchers foreach (var item in node.Matches) { var entry = item.Entry; + var tagData = (InboundEntryTagData)entry.Tag; var matcher = item.TemplateMatcher; values.Clear(); @@ -89,12 +90,12 @@ namespace Microsoft.AspNetCore.Routing.Matchers Log.MatchedTemplate(_logger, httpContext, entry.RouteTemplate); - if (!MatchConstraints(httpContext, values, entry.Constraints)) + if (!MatchConstraints(httpContext, values, tagData.MatchProcessors)) { continue; } - SelectEndpoint(httpContext, feature, (MatcherEndpoint[])entry.Tag); + SelectEndpoint(httpContext, feature, tagData.Endpoints); if (feature.Endpoint != null) { @@ -123,18 +124,14 @@ namespace Microsoft.AspNetCore.Routing.Matchers private bool MatchConstraints( HttpContext httpContext, RouteValueDictionary values, - IDictionary constraints) + IList matchProcessors) { - if (constraints != null) + if (matchProcessors != null) { - foreach (var kvp in constraints) + foreach (var processor in matchProcessors) { - var constraint = kvp.Value; - if (!constraint.Match(httpContext, new DummyRouter(), kvp.Key, values, RouteDirection.IncomingRequest)) + if (!processor.ProcessInbound(httpContext, values)) { - values.TryGetValue(kvp.Key, out var value); - - Log.ConstraintFailed(_logger, value, kvp.Key, kvp.Value); return false; } } @@ -223,7 +220,7 @@ namespace Microsoft.AspNetCore.Routing.Matchers return trees.ToArray(); } - private InboundRouteEntry MapInbound(RouteTemplate template, Endpoint[] endpoints, int order) + private InboundRouteEntry MapInbound(RouteTemplate template, MatcherEndpoint[] endpoints, int order) { if (template == null) { @@ -235,27 +232,24 @@ namespace Microsoft.AspNetCore.Routing.Matchers Precedence = RoutePrecedence.ComputeInbound(template), RouteTemplate = template, Order = order, - Tag = endpoints, }; - var constraintBuilder = new RouteConstraintBuilder(_constraintFactory, template.TemplateText); - foreach (var parameter in template.Parameters) - { - if (parameter.InlineConstraints != null) - { - if (parameter.IsOptional) - { - constraintBuilder.SetOptional(parameter.Name); - } + // Since all endpoints within a group are expected to have same template and same constraints, + // get the first endpoint which has the processor references + var endpoint = endpoints[0]; - foreach (var constraint in parameter.InlineConstraints) - { - constraintBuilder.AddResolvedConstraint(parameter.Name, constraint.Constraint); - } - } + var matchProcessors = new List(); + foreach (var matchProcessorReference in endpoint.MatchProcessorReferences) + { + var matchProcessor = _matchProcessorFactory.Create(matchProcessorReference); + matchProcessors.Add(matchProcessor); } - entry.Constraints = constraintBuilder.Build(); + entry.Tag = new InboundEntryTagData() + { + Endpoints = endpoints, + MatchProcessors = matchProcessors, + }; entry.Defaults = new RouteValueDictionary(); foreach (var parameter in entry.RouteTemplate.Parameters) @@ -350,5 +344,11 @@ namespace Microsoft.AspNetCore.Routing.Matchers _matchedTemplate(logger, template.TemplateText, httpContext.Request.Path, null); } } + + private class InboundEntryTagData + { + public MatcherEndpoint[] Endpoints { get; set; } + public List MatchProcessors { get; set; } + } } } diff --git a/src/Microsoft.AspNetCore.Routing/Matchers/TreeMatcherFactory.cs b/src/Microsoft.AspNetCore.Routing/Matchers/TreeMatcherFactory.cs index 2d4ef3d864..39171909a2 100644 --- a/src/Microsoft.AspNetCore.Routing/Matchers/TreeMatcherFactory.cs +++ b/src/Microsoft.AspNetCore.Routing/Matchers/TreeMatcherFactory.cs @@ -9,18 +9,18 @@ namespace Microsoft.AspNetCore.Routing.Matchers { internal class TreeMatcherFactory : MatcherFactory { - private readonly IInlineConstraintResolver _constraintFactory; + private readonly MatchProcessorFactory _matchProcessorFactory; private readonly ILogger _logger; private readonly EndpointSelector _endpointSelector; public TreeMatcherFactory( - IInlineConstraintResolver constraintFactory, + MatchProcessorFactory matchProcessorFactory, ILogger logger, EndpointSelector endpointSelector) { - if (constraintFactory == null) + if (matchProcessorFactory == null) { - throw new ArgumentNullException(nameof(constraintFactory)); + throw new ArgumentNullException(nameof(matchProcessorFactory)); } if (logger == null) @@ -33,7 +33,7 @@ namespace Microsoft.AspNetCore.Routing.Matchers throw new ArgumentNullException(nameof(endpointSelector)); } - _constraintFactory = constraintFactory; + _matchProcessorFactory = matchProcessorFactory; _logger = logger; _endpointSelector = endpointSelector; } @@ -45,7 +45,7 @@ namespace Microsoft.AspNetCore.Routing.Matchers throw new ArgumentNullException(nameof(dataSource)); } - return new TreeMatcher(_constraintFactory, _logger, dataSource, _endpointSelector); + return new TreeMatcher(_matchProcessorFactory, _logger, dataSource, _endpointSelector); } } } diff --git a/src/Microsoft.AspNetCore.Routing/RouteValuesBasedEndpointFinder.cs b/src/Microsoft.AspNetCore.Routing/RouteValuesBasedEndpointFinder.cs index a926d51cf7..0b0fee9edd 100644 --- a/src/Microsoft.AspNetCore.Routing/RouteValuesBasedEndpointFinder.cs +++ b/src/Microsoft.AspNetCore.Routing/RouteValuesBasedEndpointFinder.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Threading.Tasks; using Microsoft.AspNetCore.Routing.EndpointFinders; using Microsoft.AspNetCore.Routing.Internal; using Microsoft.AspNetCore.Routing.Matchers; @@ -17,19 +16,16 @@ namespace Microsoft.AspNetCore.Routing internal class RouteValuesBasedEndpointFinder : IEndpointFinder { private readonly CompositeEndpointDataSource _endpointDataSource; - private readonly IInlineConstraintResolver _inlineConstraintResolver; private readonly ObjectPool _objectPool; private LinkGenerationDecisionTree _allMatchesLinkGenerationTree; private IDictionary _namedMatches; public RouteValuesBasedEndpointFinder( CompositeEndpointDataSource endpointDataSource, - ObjectPool objectPool, - IInlineConstraintResolver inlineConstraintResolver) + ObjectPool objectPool) { _endpointDataSource = endpointDataSource; _objectPool = objectPool; - _inlineConstraintResolver = inlineConstraintResolver; BuildOutboundMatches(); } @@ -110,28 +106,6 @@ namespace Microsoft.AspNetCore.Routing Data = endpoint, RouteName = routeNameMetadata?.Name, }; - - // TODO: review. These route constriants should be constructed when the endpoint - // is built. This way they can be checked for validity on app startup too - var constraintBuilder = new RouteConstraintBuilder( - _inlineConstraintResolver, - endpoint.ParsedTemplate.TemplateText); - foreach (var parameter in endpoint.ParsedTemplate.Parameters) - { - if (parameter.InlineConstraints != null) - { - if (parameter.IsOptional) - { - constraintBuilder.SetOptional(parameter.Name); - } - - foreach (var constraint in parameter.InlineConstraints) - { - constraintBuilder.AddResolvedConstraint(parameter.Name, constraint.Constraint); - } - } - } - entry.Constraints = constraintBuilder.Build(); entry.Defaults = endpoint.Defaults; return entry; } @@ -146,21 +120,5 @@ namespace Microsoft.AspNetCore.Routing } return result; } - - // Used only to hook up link generation, and it doesn't need to do anything. - private class NullRouter : IRouter - { - public static readonly NullRouter Instance = new NullRouter(); - - public VirtualPathData GetVirtualPath(VirtualPathContext context) - { - return null; - } - - public Task RouteAsync(RouteContext context) - { - throw new NotImplementedException(); - } - } } } diff --git a/test/Microsoft.AspNetCore.Routing.FunctionalTests/DispatcherSampleTest.cs b/test/Microsoft.AspNetCore.Routing.FunctionalTests/DispatcherSampleTest.cs index ff26a0e250..79925e8bf0 100644 --- a/test/Microsoft.AspNetCore.Routing.FunctionalTests/DispatcherSampleTest.cs +++ b/test/Microsoft.AspNetCore.Routing.FunctionalTests/DispatcherSampleTest.cs @@ -61,6 +61,74 @@ namespace Microsoft.AspNetCore.Routing.FunctionalTests Assert.Equal(expectedContent, actualContent); } + [Fact] + public async Task MatchesEndpoint_WithSuccessfulConstraintMatch() + { + // Arrange + var expectedContent = "WithConstraints"; + + // Act + var response = await _client.GetAsync("/withconstraints/555_001"); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.NotNull(response.Content); + var actualContent = await response.Content.ReadAsStringAsync(); + Assert.Equal(expectedContent, actualContent); + } + + [Fact] + public async Task DoesNotMatchEndpoint_IfConstraintMatchFails() + { + // Arrange & Act + var response = await _client.GetAsync("/withconstraints/555"); + + // Assert + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } + + [Fact] + public async Task MatchesEndpoint_WithSuccessful_OptionalConstraintMatch() + { + // Arrange + var expectedContent = "withoptionalconstraints"; + + // Act + var response = await _client.GetAsync("/withoptionalconstraints/555_001"); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.NotNull(response.Content); + var actualContent = await response.Content.ReadAsStringAsync(); + Assert.Equal(expectedContent, actualContent); + } + + [Fact] + public async Task MatchesEndpoint_WithSuccessful_OptionalConstraintMatch_NoValueForParameter() + { + // Arrange + var expectedContent = "withoptionalconstraints"; + + // Act + var response = await _client.GetAsync("/withoptionalconstraints"); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.NotNull(response.Content); + var actualContent = await response.Content.ReadAsStringAsync(); + Assert.Equal(expectedContent, actualContent); + } + + [Fact] + public async Task DoesNotMatchEndpoint_IfOptionalConstraintMatchFails() + { + // Arrange & Act + var response = await _client.GetAsync("/withoptionalconstraints/555"); + + // Assert + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } + public void Dispose() { _testServer.Dispose(); diff --git a/test/Microsoft.AspNetCore.Routing.Tests/DefaultLinkGeneratorTest.cs b/test/Microsoft.AspNetCore.Routing.Tests/DefaultLinkGeneratorTest.cs index ffb78a6ea1..db93c930c5 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/DefaultLinkGeneratorTest.cs +++ b/test/Microsoft.AspNetCore.Routing.Tests/DefaultLinkGeneratorTest.cs @@ -8,7 +8,9 @@ using Microsoft.AspNetCore.Routing.EndpointFinders; using Microsoft.AspNetCore.Routing.Internal; using Microsoft.AspNetCore.Routing.Matchers; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.ObjectPool; +using Microsoft.Extensions.Options; using Moq; using Xunit; @@ -788,6 +790,7 @@ namespace Microsoft.AspNetCore.Routing template, defaults, new RouteValueDictionary(), + new List(), 0, EndpointMetadataCollection.Empty, null); @@ -796,8 +799,12 @@ namespace Microsoft.AspNetCore.Routing private ILinkGenerator CreateLinkGenerator() { return new DefaultLinkGenerator( + new DefaultMatchProcessorFactory( + Options.Create(new RouteOptions()), + NullLogger.Instance, + Mock.Of()), new DefaultObjectPool(new UriBuilderContextPooledObjectPolicy()), - Mock.Of>()); + NullLogger.Instance); } } } diff --git a/test/Microsoft.AspNetCore.Routing.Tests/DispatcherMiddlewareTest.cs b/test/Microsoft.AspNetCore.Routing.Tests/DispatcherMiddlewareTest.cs index ad0855f4c8..4f5b72de8d 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/DispatcherMiddlewareTest.cs +++ b/test/Microsoft.AspNetCore.Routing.Tests/DispatcherMiddlewareTest.cs @@ -4,9 +4,7 @@ using System; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Routing.Matchers; using Microsoft.AspNetCore.Routing.TestObjects; -using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Logging.Testing; diff --git a/test/Microsoft.AspNetCore.Routing.Tests/EndpointConstraints/EndpointSelectorTests.cs b/test/Microsoft.AspNetCore.Routing.Tests/EndpointConstraints/EndpointSelectorTests.cs index 796909c728..008464113e 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/EndpointConstraints/EndpointSelectorTests.cs +++ b/test/Microsoft.AspNetCore.Routing.Tests/EndpointConstraints/EndpointSelectorTests.cs @@ -1,6 +1,9 @@ // 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 Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing.Matchers; using Microsoft.AspNetCore.Routing.TestObjects; @@ -9,9 +12,6 @@ using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Logging.Testing; using Moq; -using System; -using System.Collections.Generic; -using System.Linq; using Xunit; namespace Microsoft.AspNetCore.Routing.EndpointConstraints diff --git a/test/Microsoft.AspNetCore.Routing.Tests/Matchers/DefaultMatchProcessorFactoryTest.cs b/test/Microsoft.AspNetCore.Routing.Tests/Matchers/DefaultMatchProcessorFactoryTest.cs new file mode 100644 index 0000000000..1f89aa1928 --- /dev/null +++ b/test/Microsoft.AspNetCore.Routing.Tests/Matchers/DefaultMatchProcessorFactoryTest.cs @@ -0,0 +1,190 @@ +// 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.Globalization; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; +using Moq; +using Xunit; + +namespace Microsoft.AspNetCore.Routing.Matchers +{ + public class DefaultMatchProcessorFactoryTest + { + [Fact] + public void Create_ThrowsException_IfNoConstraintOrMatchProcessor_FoundInMap() + { + // Arrange + var factory = GetMatchProcessorFactory(); + var matchProcessorReference = new MatchProcessorReference("id", @"notpresent(\d+)"); + + // Act & Assert + var exception = Assert.Throws( + () => factory.Create(matchProcessorReference)); + } + + [Fact] + public void Create_CreatesMatchProcessor_FromConstraintText_AndRouteConstraint() + { + // Arrange + var factory = GetMatchProcessorFactory(); + var matchProcessorReference = new MatchProcessorReference("id", "int"); + + // Act 1 + var processor = factory.Create(matchProcessorReference); + + // Assert 1 + Assert.NotNull(processor); + + // Act 2 + var isMatch = processor.ProcessInbound( + new DefaultHttpContext(), + new RouteValueDictionary(new { id = 10 })); + + // Assert 2 + Assert.True(isMatch); + + // Act 2 + isMatch = processor.ProcessInbound( + new DefaultHttpContext(), + new RouteValueDictionary(new { id = "foo" })); + + // Assert 2 + Assert.False(isMatch); + } + + [Fact] + public void Create_CreatesMatchProcessor_FromConstraintText_AndCustomMatchProcessor() + { + // Arrange + var options = new RouteOptions(); + options.ConstraintMap.Add("endsWith", typeof(EndsWithStringMatchProcessor)); + var services = new ServiceCollection(); + services.AddTransient(); + var factory = GetMatchProcessorFactory(options, services); + var matchProcessorReference = new MatchProcessorReference("id", "endsWith(_001)"); + + // Act 1 + var processor = factory.Create(matchProcessorReference); + + // Assert 1 + Assert.NotNull(processor); + + // Act 2 + var isMatch = processor.ProcessInbound( + new DefaultHttpContext(), + new RouteValueDictionary(new { id = "555_001" })); + + // Assert 2 + Assert.True(isMatch); + + // Act 2 + isMatch = processor.ProcessInbound( + new DefaultHttpContext(), + new RouteValueDictionary(new { id = "444" })); + + // Assert 2 + Assert.False(isMatch); + } + + [Fact] + public void Create_ReturnsMatchProcessor_IfAvailable() + { + // Arrange + var factory = GetMatchProcessorFactory(); + var matchProcessorReference = new MatchProcessorReference("id", Mock.Of()); + var expected = matchProcessorReference.MatchProcessor; + + // Act + var processor = factory.Create(matchProcessorReference); + + // Assert + Assert.Same(expected, processor); + } + + [Fact] + public void Create_ReturnsMatchProcessor_WithSuppliedRouteConstraint() + { + // Arrange + var factory = GetMatchProcessorFactory(); + var constraint = TestRouteConstraint.Create(); + var matchProcessorReference = new MatchProcessorReference("id", constraint); + var processor = factory.Create(matchProcessorReference); + var expectedHttpContext = new DefaultHttpContext(); + var expectedValues = new RouteValueDictionary(); + + // Act + processor.ProcessInbound(expectedHttpContext, expectedValues); + + // Assert + Assert.Same(expectedHttpContext, constraint.HttpContext); + Assert.Same(expectedValues, constraint.Values); + Assert.Equal("id", constraint.RouteKey); + Assert.Equal(RouteDirection.IncomingRequest, constraint.RouteDirection); + Assert.Same(NullRouter.Instance, constraint.Route); + } + + private DefaultMatchProcessorFactory GetMatchProcessorFactory( + RouteOptions options = null, + ServiceCollection services = null) + { + if (options == null) + { + options = new RouteOptions(); + } + + if (services == null) + { + services = new ServiceCollection(); + } + + return new DefaultMatchProcessorFactory( + Options.Create(options), + NullLogger.Instance, + services.BuildServiceProvider()); + } + + private class TestRouteConstraint : IRouteConstraint + { + private TestRouteConstraint() { } + + public HttpContext HttpContext { get; private set; } + public IRouter Route { get; private set; } + public string RouteKey { get; private set; } + public RouteValueDictionary Values { get; private set; } + public RouteDirection RouteDirection { get; private set; } + + public static TestRouteConstraint Create() + { + return new TestRouteConstraint(); + } + + public bool Match( + HttpContext httpContext, + IRouter route, + string routeKey, + RouteValueDictionary values, + RouteDirection routeDirection) + { + HttpContext = httpContext; + Route = route; + RouteKey = routeKey; + Values = values; + RouteDirection = routeDirection; + return false; + } + } + + private class EndsWithStringMatchProcessor : MatchProcessorBase + { + public override bool Process(object value) + { + var valueString = Convert.ToString(value, CultureInfo.InvariantCulture); + return valueString.EndsWith(ConstraintArgument); + } + } + } +} diff --git a/test/Microsoft.AspNetCore.Routing.Tests/Matchers/MatcherConformanceTest.cs b/test/Microsoft.AspNetCore.Routing.Tests/Matchers/MatcherConformanceTest.cs index bbb08d5f9a..abef76e438 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/Matchers/MatcherConformanceTest.cs +++ b/test/Microsoft.AspNetCore.Routing.Tests/Matchers/MatcherConformanceTest.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; @@ -43,6 +44,7 @@ namespace Microsoft.AspNetCore.Routing.Matchers template, defaults, new RouteValueDictionary(), + new List(), order ?? 0, EndpointMetadataCollection.Empty, "endpoint: " + template); diff --git a/test/Microsoft.AspNetCore.Routing.Tests/Matchers/TreeMatcherTests.cs b/test/Microsoft.AspNetCore.Routing.Tests/Matchers/TreeMatcherTests.cs index cc865fed75..e32d4aed76 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/Matchers/TreeMatcherTests.cs +++ b/test/Microsoft.AspNetCore.Routing.Tests/Matchers/TreeMatcherTests.cs @@ -1,12 +1,14 @@ // 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.Http; using Microsoft.AspNetCore.Routing.EndpointConstraints; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; +using Moq; using Xunit; namespace Microsoft.AspNetCore.Routing.Matchers @@ -16,13 +18,23 @@ namespace Microsoft.AspNetCore.Routing.Matchers private MatcherEndpoint CreateEndpoint(string template, int order, object defaultValues = null, EndpointMetadataCollection metadata = null) { var defaults = defaultValues == null ? new RouteValueDictionary() : new RouteValueDictionary(defaultValues); - return new MatcherEndpoint((next) => null, template, defaults, new RouteValueDictionary(), order, metadata ?? EndpointMetadataCollection.Empty, template); + return new MatcherEndpoint( + (next) => null, + template, defaults, + new RouteValueDictionary(), + new List(), + order, + metadata ?? EndpointMetadataCollection.Empty, + template); } private TreeMatcher CreateTreeMatcher(EndpointDataSource endpointDataSource) { var compositeDataSource = new CompositeEndpointDataSource(new[] { endpointDataSource }); - var defaultInlineConstraintResolver = new DefaultInlineConstraintResolver(Options.Create(new RouteOptions())); + var defaultInlineConstraintResolver = new DefaultMatchProcessorFactory( + Options.Create(new RouteOptions()), + NullLogger.Instance, + Mock.Of()); var endpointSelector = new EndpointSelector( compositeDataSource, new EndpointConstraintCache(compositeDataSource, new IEndpointConstraintProvider[] { new DefaultEndpointConstraintProvider() }), From 3a022107dcba43fb6441a5fad741537f17a5fd17 Mon Sep 17 00:00:00 2001 From: Kiran Challa Date: Thu, 12 Jul 2018 06:24:37 -0700 Subject: [PATCH 2/3] Revert "Added support for route constraints in Dispatcher world" This reverts commit 85e92ab3cc4ec28a5386a15bf2a8df5c228d30cc. --- .../Benchmarks/StartupUsingDispatcher.cs | 1 - .../Matchers/MatcherBenchmarkBase.cs | 15 +- .../EndsWithStringMatchProcessor.cs | 40 ---- samples/DispatcherSample.Web/Startup.cs | 54 +---- .../DefaultInlineConstraintResolver.cs | 2 +- .../DefaultLinkGenerator.cs | 51 +---- .../RoutingServiceCollectionExtensions.cs | 3 - .../Internal/NullRouter.cs | 26 --- .../Matchers/DefaultMatchProcessorFactory.cs | 156 -------------- .../Matchers/MatchProcessor.cs | 18 -- .../Matchers/MatchProcessorBase.cs | 42 ---- .../Matchers/MatchProcessorFactory.cs | 10 - .../Matchers/MatchProcessorReference.cs | 86 -------- .../Matchers/MatcherEndpoint.cs | 35 ---- .../Matchers/OptionalMatchProcessor.cs | 44 ---- .../Matchers/TreeMatcher.cs | 64 +++--- .../Matchers/TreeMatcherFactory.cs | 12 +- .../RouteValuesBasedEndpointFinder.cs | 44 +++- .../DispatcherSampleTest.cs | 68 ------- .../DefaultLinkGeneratorTest.cs | 9 +- .../DispatcherMiddlewareTest.cs | 2 + .../EndpointSelectorTests.cs | 6 +- .../DefaultMatchProcessorFactoryTest.cs | 190 ------------------ .../Matchers/MatcherConformanceTest.cs | 2 - .../Matchers/TreeMatcherTests.cs | 16 +- 25 files changed, 110 insertions(+), 886 deletions(-) delete mode 100644 samples/DispatcherSample.Web/EndsWithStringMatchProcessor.cs delete mode 100644 src/Microsoft.AspNetCore.Routing/Internal/NullRouter.cs delete mode 100644 src/Microsoft.AspNetCore.Routing/Matchers/DefaultMatchProcessorFactory.cs delete mode 100644 src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessor.cs delete mode 100644 src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessorBase.cs delete mode 100644 src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessorFactory.cs delete mode 100644 src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessorReference.cs delete mode 100644 src/Microsoft.AspNetCore.Routing/Matchers/OptionalMatchProcessor.cs delete mode 100644 test/Microsoft.AspNetCore.Routing.Tests/Matchers/DefaultMatchProcessorFactoryTest.cs diff --git a/benchmarkapps/Benchmarks/StartupUsingDispatcher.cs b/benchmarkapps/Benchmarks/StartupUsingDispatcher.cs index 1351a20689..153b6e42cc 100644 --- a/benchmarkapps/Benchmarks/StartupUsingDispatcher.cs +++ b/benchmarkapps/Benchmarks/StartupUsingDispatcher.cs @@ -34,7 +34,6 @@ namespace Benchmarks template: "/plaintext", defaults: new RouteValueDictionary(), requiredValues: new RouteValueDictionary(), - nonInlineMatchProcessorReferences: null, order: 0, metadata: EndpointMetadataCollection.Empty, displayName: "Plaintext"), diff --git a/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matchers/MatcherBenchmarkBase.cs b/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matchers/MatcherBenchmarkBase.cs index 2e10ed2397..1ab67c9235 100644 --- a/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matchers/MatcherBenchmarkBase.cs +++ b/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matchers/MatcherBenchmarkBase.cs @@ -34,14 +34,13 @@ namespace Microsoft.AspNetCore.Routing.Matchers } return new MatcherEndpoint( - (next) => (context) => Task.CompletedTask, - template, - new RouteValueDictionary(), - new RouteValueDictionary(), - new List(), - 0, - EndpointMetadataCollection.Empty, - template); + (next) => (context) => Task.CompletedTask, + template, + new RouteValueDictionary(), + new RouteValueDictionary(), + 0, + new EndpointMetadataCollection(metadata), + template); } internal static int[] SampleRequests(int endpointCount, int count) diff --git a/samples/DispatcherSample.Web/EndsWithStringMatchProcessor.cs b/samples/DispatcherSample.Web/EndsWithStringMatchProcessor.cs deleted file mode 100644 index 7284dc6d66..0000000000 --- a/samples/DispatcherSample.Web/EndsWithStringMatchProcessor.cs +++ /dev/null @@ -1,40 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System; -using System.Globalization; -using Microsoft.AspNetCore.Routing.Matchers; -using Microsoft.Extensions.Logging; - -namespace DispatcherSample.Web -{ - internal class EndsWithStringMatchProcessor : MatchProcessorBase - { - private readonly ILogger _logger; - - public EndsWithStringMatchProcessor(ILogger logger) - { - _logger = logger; - } - - public override bool Process(object value) - { - if (value == null) - { - return false; - } - - var valueString = Convert.ToString(value, CultureInfo.InvariantCulture); - - var endsWith = valueString.EndsWith(ConstraintArgument, StringComparison.OrdinalIgnoreCase); - - if (!endsWith) - { - _logger.LogDebug( - $"Parameter '{ParameterName}' with value '{valueString}' does not end with '{ConstraintArgument}'."); - } - - return endsWith; - } - } -} diff --git a/samples/DispatcherSample.Web/Startup.cs b/samples/DispatcherSample.Web/Startup.cs index 9f8747310f..eca7ab2235 100644 --- a/samples/DispatcherSample.Web/Startup.cs +++ b/samples/DispatcherSample.Web/Startup.cs @@ -2,10 +2,8 @@ // 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.Text; using Microsoft.AspNetCore.Builder; -using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.Matchers; using Microsoft.Extensions.DependencyInjection; @@ -19,13 +17,7 @@ namespace DispatcherSample.Web public void ConfigureServices(IServiceCollection services) { - services.AddTransient(); - - services.AddRouting(options => - { - options.ConstraintMap.Add("endsWith", typeof(EndsWithStringMatchProcessor)); - }); - + services.AddRouting(); services.AddDispatcher(options => { options.DataSources.Add(new DefaultEndpointDataSource(new[] @@ -39,13 +31,7 @@ namespace DispatcherSample.Web response.ContentLength = payloadLength; return response.Body.WriteAsync(_homePayload, 0, payloadLength); }, - "/", - new RouteValueDictionary(), - new RouteValueDictionary(), - new List(), - 0, - EndpointMetadataCollection.Empty, - "Home"), + "/", new RouteValueDictionary(), new RouteValueDictionary(), 0, EndpointMetadataCollection.Empty, "Home"), new MatcherEndpoint((next) => (httpContext) => { var response = httpContext.Response; @@ -55,41 +41,7 @@ namespace DispatcherSample.Web response.ContentLength = payloadLength; return response.Body.WriteAsync(_helloWorldPayload, 0, payloadLength); }, - "/plaintext", - new RouteValueDictionary(), - new RouteValueDictionary(), - new List(), - 0, - EndpointMetadataCollection.Empty, - "Plaintext"), - new MatcherEndpoint((next) => (httpContext) => - { - var response = httpContext.Response; - response.StatusCode = 200; - response.ContentType = "text/plain"; - return response.WriteAsync("WithConstraints"); - }, - "/withconstraints/{id:endsWith(_001)}", - new RouteValueDictionary(), - new RouteValueDictionary(), - new List(), - 0, - EndpointMetadataCollection.Empty, - "withconstraints"), - new MatcherEndpoint((next) => (httpContext) => - { - var response = httpContext.Response; - response.StatusCode = 200; - response.ContentType = "text/plain"; - return response.WriteAsync("withoptionalconstraints"); - }, - "/withoptionalconstraints/{id:endsWith(_001)?}", - new RouteValueDictionary(), - new RouteValueDictionary(), - new List(), - 0, - EndpointMetadataCollection.Empty, - "withoptionalconstraints"), + "/plaintext", new RouteValueDictionary(), new RouteValueDictionary(), 0, EndpointMetadataCollection.Empty, "Plaintext"), })); }); } diff --git a/src/Microsoft.AspNetCore.Routing/DefaultInlineConstraintResolver.cs b/src/Microsoft.AspNetCore.Routing/DefaultInlineConstraintResolver.cs index ed58e053ba..7426516db2 100644 --- a/src/Microsoft.AspNetCore.Routing/DefaultInlineConstraintResolver.cs +++ b/src/Microsoft.AspNetCore.Routing/DefaultInlineConstraintResolver.cs @@ -90,7 +90,7 @@ namespace Microsoft.AspNetCore.Routing } } - internal static IRouteConstraint CreateConstraint(Type constraintType, string argumentString) + private static IRouteConstraint CreateConstraint(Type constraintType, string argumentString) { // No arguments - call the default constructor if (argumentString == null) diff --git a/src/Microsoft.AspNetCore.Routing/DefaultLinkGenerator.cs b/src/Microsoft.AspNetCore.Routing/DefaultLinkGenerator.cs index e5b5f4ea93..7efe2cedd8 100644 --- a/src/Microsoft.AspNetCore.Routing/DefaultLinkGenerator.cs +++ b/src/Microsoft.AspNetCore.Routing/DefaultLinkGenerator.cs @@ -13,18 +13,15 @@ using Microsoft.Extensions.ObjectPool; namespace Microsoft.AspNetCore.Routing { - internal class DefaultLinkGenerator : ILinkGenerator + public class DefaultLinkGenerator : ILinkGenerator { - private readonly MatchProcessorFactory _matchProcessorFactory; private readonly ObjectPool _uriBuildingContextPool; private readonly ILogger _logger; public DefaultLinkGenerator( - MatchProcessorFactory matchProcessorFactory, ObjectPool uriBuildingContextPool, ILogger logger) { - _matchProcessorFactory = matchProcessorFactory; _uriBuildingContextPool = uriBuildingContextPool; _logger = logger; } @@ -64,7 +61,7 @@ namespace Microsoft.AspNetCore.Routing foreach (var endpoint in matcherEndpoints) { - link = GetLink(endpoint, explicitValues, ambientValues); + link = GetLink(endpoint.ParsedTemplate, endpoint.Defaults, explicitValues, ambientValues); if (link != null) { return true; @@ -75,55 +72,27 @@ namespace Microsoft.AspNetCore.Routing } private string GetLink( - MatcherEndpoint endpoint, + RouteTemplate template, + RouteValueDictionary defaults, RouteValueDictionary explicitValues, RouteValueDictionary ambientValues) { var templateBinder = new TemplateBinder( UrlEncoder.Default, _uriBuildingContextPool, - endpoint.ParsedTemplate, - endpoint.Defaults); + template, + defaults); - var templateValuesResult = templateBinder.GetValues(ambientValues, explicitValues); - if (templateValuesResult == null) + var values = templateBinder.GetValues(ambientValues, explicitValues); + if (values == null) { // We're missing one of the required values for this route. return null; } - if (!Match(endpoint, templateValuesResult.CombinedValues)) - { - return null; - } + //TODO: route constraint matching here - return templateBinder.BindValues(templateValuesResult.AcceptedValues); - } - - private bool Match(MatcherEndpoint endpoint, RouteValueDictionary routeValues) - { - if (routeValues == null) - { - throw new ArgumentNullException(nameof(routeValues)); - } - - for (var i = 0; i < endpoint.MatchProcessorReferences.Count; i++) - { - var matchProcessorReference = endpoint.MatchProcessorReferences[i]; - var parameter = endpoint.ParsedTemplate.GetParameter(matchProcessorReference.ParameterName); - if (parameter.IsOptional && !routeValues.ContainsKey(parameter.Name)) - { - continue; - } - - var matchProcessor = _matchProcessorFactory.Create(matchProcessorReference); - if (!matchProcessor.ProcessOutbound(httpContext: null, routeValues)) - { - return false; - } - } - - return true; + return templateBinder.BindValues(values.AcceptedValues); } } } diff --git a/src/Microsoft.AspNetCore.Routing/DependencyInjection/RoutingServiceCollectionExtensions.cs b/src/Microsoft.AspNetCore.Routing/DependencyInjection/RoutingServiceCollectionExtensions.cs index 18ededc423..d6883f6c95 100644 --- a/src/Microsoft.AspNetCore.Routing/DependencyInjection/RoutingServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNetCore.Routing/DependencyInjection/RoutingServiceCollectionExtensions.cs @@ -3,9 +3,7 @@ using System; using Microsoft.AspNetCore.Routing; -using Microsoft.AspNetCore.Routing.Constraints; using Microsoft.AspNetCore.Routing.Internal; -using Microsoft.AspNetCore.Routing.Matchers; using Microsoft.AspNetCore.Routing.Tree; using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Logging; @@ -30,7 +28,6 @@ namespace Microsoft.Extensions.DependencyInjection throw new ArgumentNullException(nameof(services)); } - services.TryAddSingleton(); services.TryAddTransient(); services.TryAddSingleton>(s => { diff --git a/src/Microsoft.AspNetCore.Routing/Internal/NullRouter.cs b/src/Microsoft.AspNetCore.Routing/Internal/NullRouter.cs deleted file mode 100644 index 9fd1908942..0000000000 --- a/src/Microsoft.AspNetCore.Routing/Internal/NullRouter.cs +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System.Threading.Tasks; - -namespace Microsoft.AspNetCore.Routing -{ - internal class NullRouter : IRouter - { - public static readonly NullRouter Instance = new NullRouter(); - - private NullRouter() - { - } - - public VirtualPathData GetVirtualPath(VirtualPathContext context) - { - return null; - } - - public Task RouteAsync(RouteContext context) - { - return Task.CompletedTask; - } - } -} diff --git a/src/Microsoft.AspNetCore.Routing/Matchers/DefaultMatchProcessorFactory.cs b/src/Microsoft.AspNetCore.Routing/Matchers/DefaultMatchProcessorFactory.cs deleted file mode 100644 index 5226683626..0000000000 --- a/src/Microsoft.AspNetCore.Routing/Matchers/DefaultMatchProcessorFactory.cs +++ /dev/null @@ -1,156 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Options; - -namespace Microsoft.AspNetCore.Routing.Matchers -{ - internal class DefaultMatchProcessorFactory : MatchProcessorFactory - { - private readonly RouteOptions _options; - private readonly ILogger _logger; - private readonly IServiceProvider _serviceProvider; - - public DefaultMatchProcessorFactory( - IOptions options, - ILogger logger, - IServiceProvider serviceProvider) - { - _options = options.Value; - _logger = logger; - _serviceProvider = serviceProvider; - } - - public override MatchProcessor Create(MatchProcessorReference matchProcessorReference) - { - if (matchProcessorReference == null) - { - throw new ArgumentNullException(nameof(matchProcessorReference)); - } - - if (matchProcessorReference.MatchProcessor != null) - { - return matchProcessorReference.MatchProcessor; - } - - // Example: - // {productId:regex(\d+)} - // - // ParameterName: productId - // ConstraintText: regex(\d+) - // ConstraintName: regex - // ConstraintArgument: \d+ - - (var constraintName, var constraintArgument) = Parse(matchProcessorReference.ConstraintText); - - if (!_options.ConstraintMap.TryGetValue(constraintName, out var constraintType)) - { - throw new InvalidOperationException( - $"No constraint has been registered with name '{constraintName}'."); - } - - var processor = ResolveMatchProcessor( - matchProcessorReference.ParameterName, - matchProcessorReference.Optional, - constraintType, - constraintArgument); - - if (processor != null) - { - return processor; - } - - if (!typeof(IRouteConstraint).IsAssignableFrom(constraintType)) - { - throw new InvalidOperationException( - Resources.FormatDefaultInlineConstraintResolver_TypeNotConstraint( - constraintType, constraintName, typeof(IRouteConstraint).Name)); - } - - try - { - return CreateMatchProcessorFromRouteConstraint( - matchProcessorReference.ParameterName, - constraintType, - constraintArgument); - } - catch (RouteCreationException) - { - throw; - } - catch (Exception exception) - { - throw new InvalidOperationException( - $"An error occurred while trying to create an instance of route constraint '{constraintType.FullName}'.", - exception); - } - } - - private MatchProcessor CreateMatchProcessorFromRouteConstraint( - string parameterName, - Type constraintType, - string constraintArgument) - { - var routeConstraint = DefaultInlineConstraintResolver.CreateConstraint(constraintType, constraintArgument); - return (new MatchProcessorReference(parameterName, routeConstraint)).MatchProcessor; - } - - private MatchProcessor ResolveMatchProcessor( - string parameterName, - bool optional, - Type constraintType, - string constraintArgument) - { - if (constraintType == null) - { - throw new ArgumentNullException(nameof(constraintType)); - } - - if (!typeof(MatchProcessor).IsAssignableFrom(constraintType)) - { - // Since a constraint type could be of type IRouteConstraint, do not throw - return null; - } - - var registeredProcessor = _serviceProvider.GetRequiredService(constraintType); - if (registeredProcessor is MatchProcessor matchProcessor) - { - if (optional) - { - matchProcessor = new OptionalMatchProcessor(matchProcessor); - } - - matchProcessor.Initialize(parameterName, constraintArgument); - return matchProcessor; - } - else - { - throw new InvalidOperationException( - $"Registered constraint type '{constraintType}' is not of type '{typeof(MatchProcessor)}'."); - } - } - - private (string constraintName, string constraintArgument) Parse(string constraintText) - { - string constraintName; - string constraintArgument; - var indexOfFirstOpenParens = constraintText.IndexOf('('); - if (indexOfFirstOpenParens >= 0 && constraintText.EndsWith(")", StringComparison.Ordinal)) - { - constraintName = constraintText.Substring(0, indexOfFirstOpenParens); - constraintArgument = constraintText.Substring( - indexOfFirstOpenParens + 1, - constraintText.Length - indexOfFirstOpenParens - 2); - } - else - { - constraintName = constraintText; - constraintArgument = null; - } - return (constraintName, constraintArgument); - } - } -} diff --git a/src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessor.cs b/src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessor.cs deleted file mode 100644 index 065ccd8e9b..0000000000 --- a/src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessor.cs +++ /dev/null @@ -1,18 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using Microsoft.AspNetCore.Http; - -namespace Microsoft.AspNetCore.Routing.Matchers -{ - public abstract class MatchProcessor - { - public virtual void Initialize(string parameterName, string constraintArgument) - { - } - - public abstract bool ProcessInbound(HttpContext httpContext, RouteValueDictionary values); - - public abstract bool ProcessOutbound(HttpContext httpContext, RouteValueDictionary values); - } -} diff --git a/src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessorBase.cs b/src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessorBase.cs deleted file mode 100644 index d3f9db3811..0000000000 --- a/src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessorBase.cs +++ /dev/null @@ -1,42 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using Microsoft.AspNetCore.Http; - -namespace Microsoft.AspNetCore.Routing.Matchers -{ - public abstract class MatchProcessorBase : MatchProcessor - { - public string ParameterName { get; private set; } - - public string ConstraintArgument { get; private set; } - - public override void Initialize(string parameterName, string constraintArgument) - { - ParameterName = parameterName; - ConstraintArgument = constraintArgument; - } - - public abstract bool Process(object value); - - public override bool ProcessInbound(HttpContext httpContext, RouteValueDictionary values) - { - return Process(values); - } - - public override bool ProcessOutbound(HttpContext httpContext, RouteValueDictionary values) - { - return Process(values); - } - - private bool Process(RouteValueDictionary values) - { - if (!values.TryGetValue(ParameterName, out var value) || value == null) - { - return false; - } - - return Process(value); - } - } -} diff --git a/src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessorFactory.cs b/src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessorFactory.cs deleted file mode 100644 index 95624ea420..0000000000 --- a/src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessorFactory.cs +++ /dev/null @@ -1,10 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -namespace Microsoft.AspNetCore.Routing.Matchers -{ - internal abstract class MatchProcessorFactory - { - public abstract MatchProcessor Create(MatchProcessorReference matchProcessorReference); - } -} diff --git a/src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessorReference.cs b/src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessorReference.cs deleted file mode 100644 index 6266bb22e1..0000000000 --- a/src/Microsoft.AspNetCore.Routing/Matchers/MatchProcessorReference.cs +++ /dev/null @@ -1,86 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using Microsoft.AspNetCore.Http; - -namespace Microsoft.AspNetCore.Routing.Matchers -{ - public sealed class MatchProcessorReference - { - // Example: - // api/products/{productId:regex(\d+)} - // - // ParameterName = productId - // ConstraintText = regex(\d+) - // ConstraintArgument = \d+ - - public MatchProcessorReference(string parameterName, string constraintText) - { - ParameterName = parameterName; - ConstraintText = constraintText; - } - - public MatchProcessorReference(string parameterName, bool optional, string constraintText) - { - ParameterName = parameterName; - Optional = optional; - ConstraintText = constraintText; - } - - public MatchProcessorReference(string parameterName, MatchProcessor matchProcessor) - { - ParameterName = parameterName; - MatchProcessor = matchProcessor; - } - - internal MatchProcessor MatchProcessor { get; private set; } - - internal string ConstraintText { get; private set; } - - internal string ParameterName { get; private set; } - - internal bool Optional { get; private set; } - - public MatchProcessorReference(string parameterName, IRouteConstraint routeConstraint) - : this(parameterName, new RouteConstraintMatchProcessorAdapter(parameterName, routeConstraint)) - { - } - - private class RouteConstraintMatchProcessorAdapter : MatchProcessor - { - public string ParameterName { get; private set; } - - public IRouteConstraint RouteConstraint { get; } - - public RouteConstraintMatchProcessorAdapter(string parameterName, IRouteConstraint routeConstraint) - { - ParameterName = parameterName; - RouteConstraint = routeConstraint; - } - - public override void Initialize(string parameterName, string constraintArgument) - { - } - - public override bool ProcessInbound(HttpContext httpContext, RouteValueDictionary routeValues) - { - return RouteConstraint.Match( - httpContext, - NullRouter.Instance, - ParameterName, - routeValues, - RouteDirection.IncomingRequest); - } - - public override bool ProcessOutbound(HttpContext httpContext, RouteValueDictionary values) - { - return RouteConstraint.Match( - httpContext, - NullRouter.Instance, - ParameterName, - values, - RouteDirection.UrlGeneration); - } - } - } -} diff --git a/src/Microsoft.AspNetCore.Routing/Matchers/MatcherEndpoint.cs b/src/Microsoft.AspNetCore.Routing/Matchers/MatcherEndpoint.cs index 4d7fa50e0e..41b629ef6c 100644 --- a/src/Microsoft.AspNetCore.Routing/Matchers/MatcherEndpoint.cs +++ b/src/Microsoft.AspNetCore.Routing/Matchers/MatcherEndpoint.cs @@ -2,7 +2,6 @@ // 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.Http; using Microsoft.AspNetCore.Routing.Template; @@ -21,7 +20,6 @@ namespace Microsoft.AspNetCore.Routing.Matchers string template, RouteValueDictionary defaults, RouteValueDictionary requiredValues, - List nonInlineMatchProcessorReferences, int order, EndpointMetadataCollection metadata, string displayName) @@ -46,9 +44,6 @@ namespace Microsoft.AspNetCore.Routing.Matchers RequiredValues = requiredValues; var mergedDefaults = GetDefaults(ParsedTemplate, defaults); Defaults = mergedDefaults; - - var mergedReferences = MergeMatchProcessorReferences(ParsedTemplate, nonInlineMatchProcessorReferences); - MatchProcessorReferences = mergedReferences.AsReadOnly(); } public int Order { get; } @@ -62,8 +57,6 @@ namespace Microsoft.AspNetCore.Routing.Matchers // Todo: needs review public RouteTemplate ParsedTemplate { get; } - public IReadOnlyList MatchProcessorReferences { get; } - // Merge inline and non inline defaults into one private RouteValueDictionary GetDefaults(RouteTemplate parsedTemplate, RouteValueDictionary nonInlineDefaults) { @@ -88,33 +81,5 @@ namespace Microsoft.AspNetCore.Routing.Matchers return result; } - - private List MergeMatchProcessorReferences( - RouteTemplate parsedTemplate, - List nonInlineReferences) - { - var matchProcessorReferences = new List(); - - if (nonInlineReferences != null) - { - matchProcessorReferences.AddRange(nonInlineReferences); - } - - foreach (var parameter in parsedTemplate.Parameters) - { - if (parameter.InlineConstraints != null) - { - foreach (var constraint in parameter.InlineConstraints) - { - matchProcessorReferences.Add( - new MatchProcessorReference( - parameter.Name, - optional: parameter.IsOptional, - constraintText: constraint.Constraint)); - } - } - } - return matchProcessorReferences; - } } } diff --git a/src/Microsoft.AspNetCore.Routing/Matchers/OptionalMatchProcessor.cs b/src/Microsoft.AspNetCore.Routing/Matchers/OptionalMatchProcessor.cs deleted file mode 100644 index 9d445e1a44..0000000000 --- a/src/Microsoft.AspNetCore.Routing/Matchers/OptionalMatchProcessor.cs +++ /dev/null @@ -1,44 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using Microsoft.AspNetCore.Http; - -namespace Microsoft.AspNetCore.Routing.Matchers -{ - internal class OptionalMatchProcessor : MatchProcessor - { - private readonly MatchProcessor _innerMatchProcessor; - - public OptionalMatchProcessor(MatchProcessor innerMatchProcessor) - { - _innerMatchProcessor = innerMatchProcessor; - } - - public string ParameterName { get; private set; } - - public override void Initialize(string parameterName, string constraintArgument) - { - ParameterName = parameterName; - _innerMatchProcessor.Initialize(parameterName, constraintArgument); - } - - public override bool ProcessInbound(HttpContext httpContext, RouteValueDictionary values) - { - return Process(httpContext, values); - } - - public override bool ProcessOutbound(HttpContext httpContext, RouteValueDictionary values) - { - return Process(httpContext, values); - } - - private bool Process(HttpContext httpContext, RouteValueDictionary values) - { - if (values.TryGetValue(ParameterName, out var value)) - { - return _innerMatchProcessor.ProcessInbound(httpContext, values); - } - return true; - } - } -} diff --git a/src/Microsoft.AspNetCore.Routing/Matchers/TreeMatcher.cs b/src/Microsoft.AspNetCore.Routing/Matchers/TreeMatcher.cs index 72fc5c9f6c..8b1795b0f5 100644 --- a/src/Microsoft.AspNetCore.Routing/Matchers/TreeMatcher.cs +++ b/src/Microsoft.AspNetCore.Routing/Matchers/TreeMatcher.cs @@ -17,20 +17,20 @@ namespace Microsoft.AspNetCore.Routing.Matchers { internal class TreeMatcher : Matcher { - private readonly MatchProcessorFactory _matchProcessorFactory; + private readonly IInlineConstraintResolver _constraintFactory; private readonly ILogger _logger; private readonly EndpointSelector _endpointSelector; private readonly DataSourceDependantCache _cache; public TreeMatcher( - MatchProcessorFactory matchProcessorFactory, + IInlineConstraintResolver constraintFactory, ILogger logger, EndpointDataSource dataSource, EndpointSelector endpointSelector) { - if (matchProcessorFactory == null) + if (constraintFactory == null) { - throw new ArgumentNullException(nameof(matchProcessorFactory)); + throw new ArgumentNullException(nameof(constraintFactory)); } if (logger == null) @@ -43,7 +43,7 @@ namespace Microsoft.AspNetCore.Routing.Matchers throw new ArgumentNullException(nameof(dataSource)); } - _matchProcessorFactory = matchProcessorFactory; + _constraintFactory = constraintFactory; _logger = logger; _endpointSelector = endpointSelector; _cache = new DataSourceDependantCache(dataSource, CreateTrees); @@ -79,7 +79,6 @@ namespace Microsoft.AspNetCore.Routing.Matchers foreach (var item in node.Matches) { var entry = item.Entry; - var tagData = (InboundEntryTagData)entry.Tag; var matcher = item.TemplateMatcher; values.Clear(); @@ -90,12 +89,12 @@ namespace Microsoft.AspNetCore.Routing.Matchers Log.MatchedTemplate(_logger, httpContext, entry.RouteTemplate); - if (!MatchConstraints(httpContext, values, tagData.MatchProcessors)) + if (!MatchConstraints(httpContext, values, entry.Constraints)) { continue; } - SelectEndpoint(httpContext, feature, tagData.Endpoints); + SelectEndpoint(httpContext, feature, (MatcherEndpoint[])entry.Tag); if (feature.Endpoint != null) { @@ -124,14 +123,18 @@ namespace Microsoft.AspNetCore.Routing.Matchers private bool MatchConstraints( HttpContext httpContext, RouteValueDictionary values, - IList matchProcessors) + IDictionary constraints) { - if (matchProcessors != null) + if (constraints != null) { - foreach (var processor in matchProcessors) + foreach (var kvp in constraints) { - if (!processor.ProcessInbound(httpContext, values)) + var constraint = kvp.Value; + if (!constraint.Match(httpContext, new DummyRouter(), kvp.Key, values, RouteDirection.IncomingRequest)) { + values.TryGetValue(kvp.Key, out var value); + + Log.ConstraintFailed(_logger, value, kvp.Key, kvp.Value); return false; } } @@ -220,7 +223,7 @@ namespace Microsoft.AspNetCore.Routing.Matchers return trees.ToArray(); } - private InboundRouteEntry MapInbound(RouteTemplate template, MatcherEndpoint[] endpoints, int order) + private InboundRouteEntry MapInbound(RouteTemplate template, Endpoint[] endpoints, int order) { if (template == null) { @@ -232,24 +235,27 @@ namespace Microsoft.AspNetCore.Routing.Matchers Precedence = RoutePrecedence.ComputeInbound(template), RouteTemplate = template, Order = order, + Tag = endpoints, }; - // Since all endpoints within a group are expected to have same template and same constraints, - // get the first endpoint which has the processor references - var endpoint = endpoints[0]; - - var matchProcessors = new List(); - foreach (var matchProcessorReference in endpoint.MatchProcessorReferences) + var constraintBuilder = new RouteConstraintBuilder(_constraintFactory, template.TemplateText); + foreach (var parameter in template.Parameters) { - var matchProcessor = _matchProcessorFactory.Create(matchProcessorReference); - matchProcessors.Add(matchProcessor); + if (parameter.InlineConstraints != null) + { + if (parameter.IsOptional) + { + constraintBuilder.SetOptional(parameter.Name); + } + + foreach (var constraint in parameter.InlineConstraints) + { + constraintBuilder.AddResolvedConstraint(parameter.Name, constraint.Constraint); + } + } } - entry.Tag = new InboundEntryTagData() - { - Endpoints = endpoints, - MatchProcessors = matchProcessors, - }; + entry.Constraints = constraintBuilder.Build(); entry.Defaults = new RouteValueDictionary(); foreach (var parameter in entry.RouteTemplate.Parameters) @@ -344,11 +350,5 @@ namespace Microsoft.AspNetCore.Routing.Matchers _matchedTemplate(logger, template.TemplateText, httpContext.Request.Path, null); } } - - private class InboundEntryTagData - { - public MatcherEndpoint[] Endpoints { get; set; } - public List MatchProcessors { get; set; } - } } } diff --git a/src/Microsoft.AspNetCore.Routing/Matchers/TreeMatcherFactory.cs b/src/Microsoft.AspNetCore.Routing/Matchers/TreeMatcherFactory.cs index 39171909a2..2d4ef3d864 100644 --- a/src/Microsoft.AspNetCore.Routing/Matchers/TreeMatcherFactory.cs +++ b/src/Microsoft.AspNetCore.Routing/Matchers/TreeMatcherFactory.cs @@ -9,18 +9,18 @@ namespace Microsoft.AspNetCore.Routing.Matchers { internal class TreeMatcherFactory : MatcherFactory { - private readonly MatchProcessorFactory _matchProcessorFactory; + private readonly IInlineConstraintResolver _constraintFactory; private readonly ILogger _logger; private readonly EndpointSelector _endpointSelector; public TreeMatcherFactory( - MatchProcessorFactory matchProcessorFactory, + IInlineConstraintResolver constraintFactory, ILogger logger, EndpointSelector endpointSelector) { - if (matchProcessorFactory == null) + if (constraintFactory == null) { - throw new ArgumentNullException(nameof(matchProcessorFactory)); + throw new ArgumentNullException(nameof(constraintFactory)); } if (logger == null) @@ -33,7 +33,7 @@ namespace Microsoft.AspNetCore.Routing.Matchers throw new ArgumentNullException(nameof(endpointSelector)); } - _matchProcessorFactory = matchProcessorFactory; + _constraintFactory = constraintFactory; _logger = logger; _endpointSelector = endpointSelector; } @@ -45,7 +45,7 @@ namespace Microsoft.AspNetCore.Routing.Matchers throw new ArgumentNullException(nameof(dataSource)); } - return new TreeMatcher(_matchProcessorFactory, _logger, dataSource, _endpointSelector); + return new TreeMatcher(_constraintFactory, _logger, dataSource, _endpointSelector); } } } diff --git a/src/Microsoft.AspNetCore.Routing/RouteValuesBasedEndpointFinder.cs b/src/Microsoft.AspNetCore.Routing/RouteValuesBasedEndpointFinder.cs index 0b0fee9edd..a926d51cf7 100644 --- a/src/Microsoft.AspNetCore.Routing/RouteValuesBasedEndpointFinder.cs +++ b/src/Microsoft.AspNetCore.Routing/RouteValuesBasedEndpointFinder.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading.Tasks; using Microsoft.AspNetCore.Routing.EndpointFinders; using Microsoft.AspNetCore.Routing.Internal; using Microsoft.AspNetCore.Routing.Matchers; @@ -16,16 +17,19 @@ namespace Microsoft.AspNetCore.Routing internal class RouteValuesBasedEndpointFinder : IEndpointFinder { private readonly CompositeEndpointDataSource _endpointDataSource; + private readonly IInlineConstraintResolver _inlineConstraintResolver; private readonly ObjectPool _objectPool; private LinkGenerationDecisionTree _allMatchesLinkGenerationTree; private IDictionary _namedMatches; public RouteValuesBasedEndpointFinder( CompositeEndpointDataSource endpointDataSource, - ObjectPool objectPool) + ObjectPool objectPool, + IInlineConstraintResolver inlineConstraintResolver) { _endpointDataSource = endpointDataSource; _objectPool = objectPool; + _inlineConstraintResolver = inlineConstraintResolver; BuildOutboundMatches(); } @@ -106,6 +110,28 @@ namespace Microsoft.AspNetCore.Routing Data = endpoint, RouteName = routeNameMetadata?.Name, }; + + // TODO: review. These route constriants should be constructed when the endpoint + // is built. This way they can be checked for validity on app startup too + var constraintBuilder = new RouteConstraintBuilder( + _inlineConstraintResolver, + endpoint.ParsedTemplate.TemplateText); + foreach (var parameter in endpoint.ParsedTemplate.Parameters) + { + if (parameter.InlineConstraints != null) + { + if (parameter.IsOptional) + { + constraintBuilder.SetOptional(parameter.Name); + } + + foreach (var constraint in parameter.InlineConstraints) + { + constraintBuilder.AddResolvedConstraint(parameter.Name, constraint.Constraint); + } + } + } + entry.Constraints = constraintBuilder.Build(); entry.Defaults = endpoint.Defaults; return entry; } @@ -120,5 +146,21 @@ namespace Microsoft.AspNetCore.Routing } return result; } + + // Used only to hook up link generation, and it doesn't need to do anything. + private class NullRouter : IRouter + { + public static readonly NullRouter Instance = new NullRouter(); + + public VirtualPathData GetVirtualPath(VirtualPathContext context) + { + return null; + } + + public Task RouteAsync(RouteContext context) + { + throw new NotImplementedException(); + } + } } } diff --git a/test/Microsoft.AspNetCore.Routing.FunctionalTests/DispatcherSampleTest.cs b/test/Microsoft.AspNetCore.Routing.FunctionalTests/DispatcherSampleTest.cs index 79925e8bf0..ff26a0e250 100644 --- a/test/Microsoft.AspNetCore.Routing.FunctionalTests/DispatcherSampleTest.cs +++ b/test/Microsoft.AspNetCore.Routing.FunctionalTests/DispatcherSampleTest.cs @@ -61,74 +61,6 @@ namespace Microsoft.AspNetCore.Routing.FunctionalTests Assert.Equal(expectedContent, actualContent); } - [Fact] - public async Task MatchesEndpoint_WithSuccessfulConstraintMatch() - { - // Arrange - var expectedContent = "WithConstraints"; - - // Act - var response = await _client.GetAsync("/withconstraints/555_001"); - - // Assert - Assert.Equal(HttpStatusCode.OK, response.StatusCode); - Assert.NotNull(response.Content); - var actualContent = await response.Content.ReadAsStringAsync(); - Assert.Equal(expectedContent, actualContent); - } - - [Fact] - public async Task DoesNotMatchEndpoint_IfConstraintMatchFails() - { - // Arrange & Act - var response = await _client.GetAsync("/withconstraints/555"); - - // Assert - Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); - } - - [Fact] - public async Task MatchesEndpoint_WithSuccessful_OptionalConstraintMatch() - { - // Arrange - var expectedContent = "withoptionalconstraints"; - - // Act - var response = await _client.GetAsync("/withoptionalconstraints/555_001"); - - // Assert - Assert.Equal(HttpStatusCode.OK, response.StatusCode); - Assert.NotNull(response.Content); - var actualContent = await response.Content.ReadAsStringAsync(); - Assert.Equal(expectedContent, actualContent); - } - - [Fact] - public async Task MatchesEndpoint_WithSuccessful_OptionalConstraintMatch_NoValueForParameter() - { - // Arrange - var expectedContent = "withoptionalconstraints"; - - // Act - var response = await _client.GetAsync("/withoptionalconstraints"); - - // Assert - Assert.Equal(HttpStatusCode.OK, response.StatusCode); - Assert.NotNull(response.Content); - var actualContent = await response.Content.ReadAsStringAsync(); - Assert.Equal(expectedContent, actualContent); - } - - [Fact] - public async Task DoesNotMatchEndpoint_IfOptionalConstraintMatchFails() - { - // Arrange & Act - var response = await _client.GetAsync("/withoptionalconstraints/555"); - - // Assert - Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); - } - public void Dispose() { _testServer.Dispose(); diff --git a/test/Microsoft.AspNetCore.Routing.Tests/DefaultLinkGeneratorTest.cs b/test/Microsoft.AspNetCore.Routing.Tests/DefaultLinkGeneratorTest.cs index db93c930c5..ffb78a6ea1 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/DefaultLinkGeneratorTest.cs +++ b/test/Microsoft.AspNetCore.Routing.Tests/DefaultLinkGeneratorTest.cs @@ -8,9 +8,7 @@ using Microsoft.AspNetCore.Routing.EndpointFinders; using Microsoft.AspNetCore.Routing.Internal; using Microsoft.AspNetCore.Routing.Matchers; using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.ObjectPool; -using Microsoft.Extensions.Options; using Moq; using Xunit; @@ -790,7 +788,6 @@ namespace Microsoft.AspNetCore.Routing template, defaults, new RouteValueDictionary(), - new List(), 0, EndpointMetadataCollection.Empty, null); @@ -799,12 +796,8 @@ namespace Microsoft.AspNetCore.Routing private ILinkGenerator CreateLinkGenerator() { return new DefaultLinkGenerator( - new DefaultMatchProcessorFactory( - Options.Create(new RouteOptions()), - NullLogger.Instance, - Mock.Of()), new DefaultObjectPool(new UriBuilderContextPooledObjectPolicy()), - NullLogger.Instance); + Mock.Of>()); } } } diff --git a/test/Microsoft.AspNetCore.Routing.Tests/DispatcherMiddlewareTest.cs b/test/Microsoft.AspNetCore.Routing.Tests/DispatcherMiddlewareTest.cs index 4f5b72de8d..ad0855f4c8 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/DispatcherMiddlewareTest.cs +++ b/test/Microsoft.AspNetCore.Routing.Tests/DispatcherMiddlewareTest.cs @@ -4,7 +4,9 @@ using System; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Routing.Matchers; using Microsoft.AspNetCore.Routing.TestObjects; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Logging.Testing; diff --git a/test/Microsoft.AspNetCore.Routing.Tests/EndpointConstraints/EndpointSelectorTests.cs b/test/Microsoft.AspNetCore.Routing.Tests/EndpointConstraints/EndpointSelectorTests.cs index 008464113e..796909c728 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/EndpointConstraints/EndpointSelectorTests.cs +++ b/test/Microsoft.AspNetCore.Routing.Tests/EndpointConstraints/EndpointSelectorTests.cs @@ -1,9 +1,6 @@ // 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 Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing.Matchers; using Microsoft.AspNetCore.Routing.TestObjects; @@ -12,6 +9,9 @@ using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Logging.Testing; using Moq; +using System; +using System.Collections.Generic; +using System.Linq; using Xunit; namespace Microsoft.AspNetCore.Routing.EndpointConstraints diff --git a/test/Microsoft.AspNetCore.Routing.Tests/Matchers/DefaultMatchProcessorFactoryTest.cs b/test/Microsoft.AspNetCore.Routing.Tests/Matchers/DefaultMatchProcessorFactoryTest.cs deleted file mode 100644 index 1f89aa1928..0000000000 --- a/test/Microsoft.AspNetCore.Routing.Tests/Matchers/DefaultMatchProcessorFactoryTest.cs +++ /dev/null @@ -1,190 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System; -using System.Globalization; -using Microsoft.AspNetCore.Http; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging.Abstractions; -using Microsoft.Extensions.Options; -using Moq; -using Xunit; - -namespace Microsoft.AspNetCore.Routing.Matchers -{ - public class DefaultMatchProcessorFactoryTest - { - [Fact] - public void Create_ThrowsException_IfNoConstraintOrMatchProcessor_FoundInMap() - { - // Arrange - var factory = GetMatchProcessorFactory(); - var matchProcessorReference = new MatchProcessorReference("id", @"notpresent(\d+)"); - - // Act & Assert - var exception = Assert.Throws( - () => factory.Create(matchProcessorReference)); - } - - [Fact] - public void Create_CreatesMatchProcessor_FromConstraintText_AndRouteConstraint() - { - // Arrange - var factory = GetMatchProcessorFactory(); - var matchProcessorReference = new MatchProcessorReference("id", "int"); - - // Act 1 - var processor = factory.Create(matchProcessorReference); - - // Assert 1 - Assert.NotNull(processor); - - // Act 2 - var isMatch = processor.ProcessInbound( - new DefaultHttpContext(), - new RouteValueDictionary(new { id = 10 })); - - // Assert 2 - Assert.True(isMatch); - - // Act 2 - isMatch = processor.ProcessInbound( - new DefaultHttpContext(), - new RouteValueDictionary(new { id = "foo" })); - - // Assert 2 - Assert.False(isMatch); - } - - [Fact] - public void Create_CreatesMatchProcessor_FromConstraintText_AndCustomMatchProcessor() - { - // Arrange - var options = new RouteOptions(); - options.ConstraintMap.Add("endsWith", typeof(EndsWithStringMatchProcessor)); - var services = new ServiceCollection(); - services.AddTransient(); - var factory = GetMatchProcessorFactory(options, services); - var matchProcessorReference = new MatchProcessorReference("id", "endsWith(_001)"); - - // Act 1 - var processor = factory.Create(matchProcessorReference); - - // Assert 1 - Assert.NotNull(processor); - - // Act 2 - var isMatch = processor.ProcessInbound( - new DefaultHttpContext(), - new RouteValueDictionary(new { id = "555_001" })); - - // Assert 2 - Assert.True(isMatch); - - // Act 2 - isMatch = processor.ProcessInbound( - new DefaultHttpContext(), - new RouteValueDictionary(new { id = "444" })); - - // Assert 2 - Assert.False(isMatch); - } - - [Fact] - public void Create_ReturnsMatchProcessor_IfAvailable() - { - // Arrange - var factory = GetMatchProcessorFactory(); - var matchProcessorReference = new MatchProcessorReference("id", Mock.Of()); - var expected = matchProcessorReference.MatchProcessor; - - // Act - var processor = factory.Create(matchProcessorReference); - - // Assert - Assert.Same(expected, processor); - } - - [Fact] - public void Create_ReturnsMatchProcessor_WithSuppliedRouteConstraint() - { - // Arrange - var factory = GetMatchProcessorFactory(); - var constraint = TestRouteConstraint.Create(); - var matchProcessorReference = new MatchProcessorReference("id", constraint); - var processor = factory.Create(matchProcessorReference); - var expectedHttpContext = new DefaultHttpContext(); - var expectedValues = new RouteValueDictionary(); - - // Act - processor.ProcessInbound(expectedHttpContext, expectedValues); - - // Assert - Assert.Same(expectedHttpContext, constraint.HttpContext); - Assert.Same(expectedValues, constraint.Values); - Assert.Equal("id", constraint.RouteKey); - Assert.Equal(RouteDirection.IncomingRequest, constraint.RouteDirection); - Assert.Same(NullRouter.Instance, constraint.Route); - } - - private DefaultMatchProcessorFactory GetMatchProcessorFactory( - RouteOptions options = null, - ServiceCollection services = null) - { - if (options == null) - { - options = new RouteOptions(); - } - - if (services == null) - { - services = new ServiceCollection(); - } - - return new DefaultMatchProcessorFactory( - Options.Create(options), - NullLogger.Instance, - services.BuildServiceProvider()); - } - - private class TestRouteConstraint : IRouteConstraint - { - private TestRouteConstraint() { } - - public HttpContext HttpContext { get; private set; } - public IRouter Route { get; private set; } - public string RouteKey { get; private set; } - public RouteValueDictionary Values { get; private set; } - public RouteDirection RouteDirection { get; private set; } - - public static TestRouteConstraint Create() - { - return new TestRouteConstraint(); - } - - public bool Match( - HttpContext httpContext, - IRouter route, - string routeKey, - RouteValueDictionary values, - RouteDirection routeDirection) - { - HttpContext = httpContext; - Route = route; - RouteKey = routeKey; - Values = values; - RouteDirection = routeDirection; - return false; - } - } - - private class EndsWithStringMatchProcessor : MatchProcessorBase - { - public override bool Process(object value) - { - var valueString = Convert.ToString(value, CultureInfo.InvariantCulture); - return valueString.EndsWith(ConstraintArgument); - } - } - } -} diff --git a/test/Microsoft.AspNetCore.Routing.Tests/Matchers/MatcherConformanceTest.cs b/test/Microsoft.AspNetCore.Routing.Tests/Matchers/MatcherConformanceTest.cs index abef76e438..bbb08d5f9a 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/Matchers/MatcherConformanceTest.cs +++ b/test/Microsoft.AspNetCore.Routing.Tests/Matchers/MatcherConformanceTest.cs @@ -2,7 +2,6 @@ // 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 Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; @@ -44,7 +43,6 @@ namespace Microsoft.AspNetCore.Routing.Matchers template, defaults, new RouteValueDictionary(), - new List(), order ?? 0, EndpointMetadataCollection.Empty, "endpoint: " + template); diff --git a/test/Microsoft.AspNetCore.Routing.Tests/Matchers/TreeMatcherTests.cs b/test/Microsoft.AspNetCore.Routing.Tests/Matchers/TreeMatcherTests.cs index e32d4aed76..cc865fed75 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/Matchers/TreeMatcherTests.cs +++ b/test/Microsoft.AspNetCore.Routing.Tests/Matchers/TreeMatcherTests.cs @@ -1,14 +1,12 @@ // 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.Http; using Microsoft.AspNetCore.Routing.EndpointConstraints; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; -using Moq; using Xunit; namespace Microsoft.AspNetCore.Routing.Matchers @@ -18,23 +16,13 @@ namespace Microsoft.AspNetCore.Routing.Matchers private MatcherEndpoint CreateEndpoint(string template, int order, object defaultValues = null, EndpointMetadataCollection metadata = null) { var defaults = defaultValues == null ? new RouteValueDictionary() : new RouteValueDictionary(defaultValues); - return new MatcherEndpoint( - (next) => null, - template, defaults, - new RouteValueDictionary(), - new List(), - order, - metadata ?? EndpointMetadataCollection.Empty, - template); + return new MatcherEndpoint((next) => null, template, defaults, new RouteValueDictionary(), order, metadata ?? EndpointMetadataCollection.Empty, template); } private TreeMatcher CreateTreeMatcher(EndpointDataSource endpointDataSource) { var compositeDataSource = new CompositeEndpointDataSource(new[] { endpointDataSource }); - var defaultInlineConstraintResolver = new DefaultMatchProcessorFactory( - Options.Create(new RouteOptions()), - NullLogger.Instance, - Mock.Of()); + var defaultInlineConstraintResolver = new DefaultInlineConstraintResolver(Options.Create(new RouteOptions())); var endpointSelector = new EndpointSelector( compositeDataSource, new EndpointConstraintCache(compositeDataSource, new IEndpointConstraintProvider[] { new DefaultEndpointConstraintProvider() }), From 0cf972cc432e10e90fdf4f3aa27fc9ece09a3032 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Fri, 13 Jul 2018 10:16:21 +1200 Subject: [PATCH 3/3] Error message on no dispatcher middleware in endpoint middleware (#600) --- .../EndpointMiddleware.cs | 5 ++ .../EndpointMiddlewareTest.cs | 48 +++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 test/Microsoft.AspNetCore.Routing.Tests/EndpointMiddlewareTest.cs diff --git a/src/Microsoft.AspNetCore.Routing/EndpointMiddleware.cs b/src/Microsoft.AspNetCore.Routing/EndpointMiddleware.cs index 5fbb596d79..d7c90728dc 100644 --- a/src/Microsoft.AspNetCore.Routing/EndpointMiddleware.cs +++ b/src/Microsoft.AspNetCore.Routing/EndpointMiddleware.cs @@ -32,6 +32,11 @@ namespace Microsoft.AspNetCore.Routing public async Task Invoke(HttpContext httpContext) { var feature = httpContext.Features.Get(); + if (feature == null) + { + throw new InvalidOperationException("Unable to execute an endpoint because the dispatcher was not run. Ensure dispatcher middleware is registered."); + } + if (feature.Invoker != null) { Log.ExecutingEndpoint(_logger, feature.Endpoint); diff --git a/test/Microsoft.AspNetCore.Routing.Tests/EndpointMiddlewareTest.cs b/test/Microsoft.AspNetCore.Routing.Tests/EndpointMiddlewareTest.cs new file mode 100644 index 0000000000..ff968fb7be --- /dev/null +++ b/test/Microsoft.AspNetCore.Routing.Tests/EndpointMiddlewareTest.cs @@ -0,0 +1,48 @@ +// 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.Threading.Tasks; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Logging.Testing; +using Xunit; + +namespace Microsoft.AspNetCore.Routing +{ + public class EndpointMiddlewareTest + { + [Fact] + public async Task Invoke_NoFeature_ThrowFriendlyErrorMessage() + { + // Arrange + var httpContext = new DefaultHttpContext(); + httpContext.RequestServices = new ServiceProvider(); + + RequestDelegate next = (c) => + { + return Task.FromResult(null); + }; + + var middleware = new EndpointMiddleware(NullLogger.Instance, next); + + // Act + var invokeTask = middleware.Invoke(httpContext); + + // Assert + var ex = await Assert.ThrowsAsync(async () => await invokeTask); + + Assert.Equal("Unable to execute an endpoint because the dispatcher was not run. Ensure dispatcher middleware is registered.", ex.Message); + } + + private class ServiceProvider : IServiceProvider + { + public object GetService(Type serviceType) + { + throw new NotImplementedException(); + } + } + } +}