From c34830f9dea700ccca47a310756fa08a4630e0ac Mon Sep 17 00:00:00 2001 From: Doug Bunting Date: Thu, 5 Jul 2018 12:03:04 -0700 Subject: [PATCH 1/3] Use OSS package versions consistent with aspnet/benchmarks and Microsoft.AspNetcore.All 2.1.2 - update our own NuGet packages to align lower-level dependencies - add metadata to BasicApi controllers - avoids analyzer failures when building with Microsoft.AspNetCore.All e.g. in benchmark runs - especially important in `PetController` because it's associated with `[ApiContoller]` - use pooled `DbContext`s for MySql too --- .../BasicApi/Controllers/PetController.cs | 17 +++++++++++++++++ .../BasicApi/Controllers/TokenController.cs | 5 ++++- benchmarkapps/BasicApi/Startup.cs | 2 +- benchmarkapps/BasicViews/Startup.cs | 2 +- build/dependencies.props | 14 +++++++------- 5 files changed, 30 insertions(+), 10 deletions(-) diff --git a/benchmarkapps/BasicApi/Controllers/PetController.cs b/benchmarkapps/BasicApi/Controllers/PetController.cs index 828b445899..8a30fb18c2 100644 --- a/benchmarkapps/BasicApi/Controllers/PetController.cs +++ b/benchmarkapps/BasicApi/Controllers/PetController.cs @@ -25,6 +25,9 @@ namespace BasicApi.Controllers public BasicApiContext DbContext { get; } [HttpGet("{id}", Name = "FindPetById")] + [ProducesResponseType(typeof(Pet), StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status401Unauthorized)] + [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task> FindById(int id) { var pet = await DbContext.Pets @@ -42,6 +45,8 @@ namespace BasicApi.Controllers [AllowAnonymous] [HttpGet("anonymous/{id}")] + [ProducesResponseType(typeof(Pet), StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task> FindByIdWithoutToken(int id) { var pet = await DbContext.Pets @@ -58,6 +63,9 @@ namespace BasicApi.Controllers } [HttpGet("findByCategory/{categoryId}")] + [ProducesResponseType(typeof(Pet), StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status401Unauthorized)] + [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task> FindByCategory(int categoryId) { var pet = await DbContext.Pets @@ -74,6 +82,9 @@ namespace BasicApi.Controllers } [HttpGet("findByStatus")] + [ProducesResponseType(typeof(Pet), StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status401Unauthorized)] + [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task> FindByStatus(string status) { var pet = await DbContext.Pets @@ -90,6 +101,9 @@ namespace BasicApi.Controllers } [HttpGet("findByTags")] + [ProducesResponseType(typeof(Pet), StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status401Unauthorized)] + [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task> FindByTags(string[] tags) { var pet = await DbContext.Pets @@ -107,6 +121,9 @@ namespace BasicApi.Controllers [Authorize("pet-store-writer")] [HttpPost] + [ProducesResponseType(StatusCodes.Status201Created)] + [ProducesResponseType(StatusCodes.Status401Unauthorized)] + [ProducesResponseType(StatusCodes.Status403Forbidden)] public async Task AddPet([FromBody] Pet pet) { DbContext.Pets.Add(pet); diff --git a/benchmarkapps/BasicApi/Controllers/TokenController.cs b/benchmarkapps/BasicApi/Controllers/TokenController.cs index 624288ddd7..803a1d208e 100644 --- a/benchmarkapps/BasicApi/Controllers/TokenController.cs +++ b/benchmarkapps/BasicApi/Controllers/TokenController.cs @@ -7,6 +7,7 @@ using System.IdentityModel.Tokens.Jwt; using System.Linq; using System.Security.Claims; using Microsoft.AspNetCore.Authentication.JwtBearer; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Options; using Microsoft.IdentityModel.Tokens; @@ -45,11 +46,13 @@ namespace BasicApi.Controllers } [HttpGet("/token")] + [ProducesResponseType(StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status403Forbidden)] public IActionResult GetToken(string username) { if (username == null || !_identities.TryGetValue(username, out var identity)) { - return new StatusCodeResult(403); + return new StatusCodeResult(StatusCodes.Status403Forbidden); } var handler = _options.SecurityTokenValidators.OfType().First(); diff --git a/benchmarkapps/BasicApi/Startup.cs b/benchmarkapps/BasicApi/Startup.cs index 8fcc4787ae..52fa10105a 100644 --- a/benchmarkapps/BasicApi/Startup.cs +++ b/benchmarkapps/BasicApi/Startup.cs @@ -68,7 +68,7 @@ namespace BasicApi case "MYSQL": services .AddEntityFrameworkMySql() - .AddDbContext(options => options.UseMySql(connectionString)); + .AddDbContextPool(options => options.UseMySql(connectionString)); break; #endif diff --git a/benchmarkapps/BasicViews/Startup.cs b/benchmarkapps/BasicViews/Startup.cs index 30ebb341bd..8d24470a28 100644 --- a/benchmarkapps/BasicViews/Startup.cs +++ b/benchmarkapps/BasicViews/Startup.cs @@ -49,7 +49,7 @@ namespace BasicViews case "MYSQL": services .AddEntityFrameworkMySql() - .AddDbContext(options => options.UseMySql(connectionString)); + .AddDbContextPool(options => options.UseMySql(connectionString)); break; #endif diff --git a/build/dependencies.props b/build/dependencies.props index 78e6a161ca..d443b2338e 100644 --- a/build/dependencies.props +++ b/build/dependencies.props @@ -7,15 +7,15 @@ is not otherwise referenced. They avoid unnecessary changes to the Universe build graph or to product dependencies. Do not use these properties elsewhere. --> - + 0.9.9 0.10.13 - 2.1.0 - 2.1.0 - 2.1.0 - 0.42.1 - 2.1.0 - 2.1.0-rc1-final + 2.1.1 + 2.1.1 + 2.1.1 + 0.43.0 + 2.1.1.1 + 2.1.1 2.2.0-preview1-34823 2.2.0-preview1-17102 2.2.0-preview1-34823 From 44f5b54f5fdabf0696708654ef64a3a318767594 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 2 Aug 2018 13:37:43 +1200 Subject: [PATCH 2/3] React to routing API review (#8194) --- build/dependencies.props | 4 ++-- .../Internal/MvcEndpointDataSource.cs | 2 +- .../Routing/ActionConstraintMatcherPolicy.cs | 2 +- .../Routing/EndpointRoutingUrlHelper.cs | 9 ++++----- .../Routing/UrlHelperFactory.cs | 3 +-- .../Internal/MvcEndpointDataSourceTests.cs | 2 +- .../Routing/ActionConstraintMatcherPolicyTest.cs | 2 +- .../Routing/EndpointRoutingUrlHelperTest.cs | 2 +- 8 files changed, 12 insertions(+), 14 deletions(-) diff --git a/build/dependencies.props b/build/dependencies.props index d443b2338e..9ab9fc484e 100644 --- a/build/dependencies.props +++ b/build/dependencies.props @@ -48,8 +48,8 @@ 2.2.0-preview1-34823 2.2.0-preview1-34823 2.2.0-preview1-34823 - 2.2.0-preview1-34823 - 2.2.0-preview1-34823 + 2.2.0-a-preview1-routing-api-review-p1-16821 + 2.2.0-a-preview1-routing-api-review-p1-16821 2.2.0-preview1-34823 2.2.0-preview1-34823 2.2.0-preview1-34823 diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcEndpointDataSource.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcEndpointDataSource.cs index b16b9ed018..867d5dd5ed 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcEndpointDataSource.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcEndpointDataSource.cs @@ -11,7 +11,7 @@ using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.ActionConstraints; using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Routing; -using Microsoft.AspNetCore.Routing.Matchers; +using Microsoft.AspNetCore.Routing.Matching; using Microsoft.AspNetCore.Routing.Metadata; using Microsoft.AspNetCore.Routing.Patterns; using Microsoft.AspNetCore.Routing.Template; diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Routing/ActionConstraintMatcherPolicy.cs b/src/Microsoft.AspNetCore.Mvc.Core/Routing/ActionConstraintMatcherPolicy.cs index 60a1abe6b1..090ade0cf1 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Routing/ActionConstraintMatcherPolicy.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Routing/ActionConstraintMatcherPolicy.cs @@ -10,7 +10,7 @@ using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.ActionConstraints; using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.AspNetCore.Routing; -using Microsoft.AspNetCore.Routing.Matchers; +using Microsoft.AspNetCore.Routing.Matching; using Microsoft.Extensions.Logging; namespace Microsoft.AspNetCore.Mvc.Routing diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Routing/EndpointRoutingUrlHelper.cs b/src/Microsoft.AspNetCore.Mvc.Core/Routing/EndpointRoutingUrlHelper.cs index 27590902e9..6f24248950 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Routing/EndpointRoutingUrlHelper.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Routing/EndpointRoutingUrlHelper.cs @@ -3,7 +3,6 @@ using System; using Microsoft.AspNetCore.Routing; -using Microsoft.AspNetCore.Routing.EndpointFinders; using Microsoft.Extensions.Logging; namespace Microsoft.AspNetCore.Mvc.Routing @@ -16,7 +15,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing { private readonly ILogger _logger; private readonly LinkGenerator _linkGenerator; - private readonly IEndpointFinder _routeValuesBasedEndpointFinder; + private readonly IEndpointFinder _routeValuesBasedEndpointFinder; /// /// Initializes a new instance of the class using the specified @@ -30,7 +29,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing /// The . public EndpointRoutingUrlHelper( ActionContext actionContext, - IEndpointFinder routeValuesBasedEndpointFinder, + IEndpointFinder routeValuesBasedEndpointFinder, LinkGenerator linkGenerator, ILogger logger) : base(actionContext) @@ -87,7 +86,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing } var endpoints = _routeValuesBasedEndpointFinder.FindEndpoints( - new RouteValuesBasedEndpointFinderContext() + new RouteValuesAddress() { ExplicitValues = valuesDictionary, AmbientValues = AmbientValues @@ -124,7 +123,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing var valuesDictionary = routeContext.Values as RouteValueDictionary ?? GetValuesDictionary(routeContext.Values); var endpoints = _routeValuesBasedEndpointFinder.FindEndpoints( - new RouteValuesBasedEndpointFinderContext() + new RouteValuesAddress() { RouteName = routeContext.RouteName, ExplicitValues = valuesDictionary, diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Routing/UrlHelperFactory.cs b/src/Microsoft.AspNetCore.Mvc.Core/Routing/UrlHelperFactory.cs index b6c5acfdf4..fb6bbb0ae5 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Routing/UrlHelperFactory.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Routing/UrlHelperFactory.cs @@ -5,7 +5,6 @@ using System; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Core; using Microsoft.AspNetCore.Routing; -using Microsoft.AspNetCore.Routing.EndpointFinders; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; @@ -52,7 +51,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing { var services = httpContext.RequestServices; var linkGenerator = services.GetRequiredService(); - var routeValuesBasedEndpointFinder = services.GetRequiredService>(); + var routeValuesBasedEndpointFinder = services.GetRequiredService>(); var logger = services.GetRequiredService>(); urlHelper = new EndpointRoutingUrlHelper( diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MvcEndpointDataSourceTests.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MvcEndpointDataSourceTests.cs index c49d9f0243..6304099fc0 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MvcEndpointDataSourceTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MvcEndpointDataSourceTests.cs @@ -14,7 +14,7 @@ using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.Routing; using Microsoft.AspNetCore.Routing; -using Microsoft.AspNetCore.Routing.Matchers; +using Microsoft.AspNetCore.Routing.Matching; using Microsoft.Extensions.Options; using Microsoft.Extensions.Primitives; using Moq; diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ActionConstraintMatcherPolicyTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ActionConstraintMatcherPolicyTest.cs index 51a00d6ddd..77288f75d3 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ActionConstraintMatcherPolicyTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ActionConstraintMatcherPolicyTest.cs @@ -13,7 +13,7 @@ using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.AspNetCore.Routing; -using Microsoft.AspNetCore.Routing.Matchers; +using Microsoft.AspNetCore.Routing.Matching; using Microsoft.AspNetCore.Routing.Patterns; using Moq; using Xunit; diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/EndpointRoutingUrlHelperTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/EndpointRoutingUrlHelperTest.cs index faa0eebf0c..867832e808 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/EndpointRoutingUrlHelperTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/EndpointRoutingUrlHelperTest.cs @@ -6,7 +6,7 @@ using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; using Microsoft.AspNetCore.Routing; -using Microsoft.AspNetCore.Routing.Matchers; +using Microsoft.AspNetCore.Routing.Matching; using Microsoft.AspNetCore.Routing.Patterns; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; From 2b289d2f2c353c2d87ceff3255d366adfc332e5f Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Tue, 31 Jul 2018 20:58:26 -0700 Subject: [PATCH 3/3] Use MatcherPolicy for Consumes --- .../MvcCoreServiceCollectionExtensions.cs | 1 + .../Internal/MvcEndpointDataSource.cs | 6 + .../Routing/ConsumesMatcherPolicy.cs | 244 ++++++++++++++++++ .../Routing/ConsumesMetadata.cs | 23 ++ .../MvcCoreServiceCollectionExtensionsTest.cs | 1 + .../Routing/ConsumesMatcherPolicyTest.cs | 237 +++++++++++++++++ .../ConsumesAttributeEndpointRoutingTests.cs | 18 ++ .../ConsumesAttributeTestsBase.cs | 2 +- .../RequestServicesEndpointRoutingTest.cs | 1 + 9 files changed, 532 insertions(+), 1 deletion(-) create mode 100644 src/Microsoft.AspNetCore.Mvc.Core/Routing/ConsumesMatcherPolicy.cs create mode 100644 src/Microsoft.AspNetCore.Mvc.Core/Routing/ConsumesMetadata.cs create mode 100644 test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ConsumesMatcherPolicyTest.cs diff --git a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs index 477a67ac0a..3ebdc371fa 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs @@ -175,6 +175,7 @@ namespace Microsoft.Extensions.DependencyInjection services.TryAddEnumerable(ServiceDescriptor.Transient()); // Policies for Endpoints + services.TryAddEnumerable(ServiceDescriptor.Singleton()); services.TryAddEnumerable(ServiceDescriptor.Singleton()); // diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcEndpointDataSource.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcEndpointDataSource.cs index 867d5dd5ed..180b34011f 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcEndpointDataSource.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcEndpointDataSource.cs @@ -10,6 +10,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.ActionConstraints; using Microsoft.AspNetCore.Mvc.Infrastructure; +using Microsoft.AspNetCore.Mvc.Routing; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.Matching; using Microsoft.AspNetCore.Routing.Metadata; @@ -348,6 +349,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal { metadata.Add(new HttpMethodMetadata(httpMethodActionConstraint.HttpMethods)); } + else if (actionConstraint is ConsumesAttribute consumesAttribute && + !metadata.OfType().Any()) + { + metadata.Add(new ConsumesMetadata(consumesAttribute.ContentTypes.ToArray())); + } else if (!metadata.Contains(actionConstraint)) { // The constraint might have been added earlier, e.g. it is also a filter descriptor diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Routing/ConsumesMatcherPolicy.cs b/src/Microsoft.AspNetCore.Mvc.Core/Routing/ConsumesMatcherPolicy.cs new file mode 100644 index 0000000000..dcaeaca936 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/Routing/ConsumesMatcherPolicy.cs @@ -0,0 +1,244 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.Formatters; +using Microsoft.AspNetCore.Routing; +using Microsoft.AspNetCore.Routing.Matching; +using Microsoft.AspNetCore.Routing.Patterns; + +namespace Microsoft.AspNetCore.Mvc.Routing +{ + internal class ConsumesMatcherPolicy : MatcherPolicy, IEndpointComparerPolicy, INodeBuilderPolicy + { + internal const string Http415EndpointDisplayName = "415 HTTP Unsupported Media Type"; + internal const string AnyContentType = "*/*"; + + // Run after HTTP methods, but before 'default'. + public override int Order { get; } = -100; + + public IComparer Comparer { get; } = new ConsumesMetadataEndpointComparer(); + + public bool AppliesToNode(IReadOnlyList endpoints) + { + if (endpoints == null) + { + throw new ArgumentNullException(nameof(endpoints)); + } + + return endpoints.Any(e => e.Metadata.GetMetadata()?.ContentTypes.Count > 0); + } + + public IReadOnlyList GetEdges(IReadOnlyList endpoints) + { + if (endpoints == null) + { + throw new ArgumentNullException(nameof(endpoints)); + } + + // The algorithm here is designed to be preserve the order of the endpoints + // while also being relatively simple. Preserving order is important. + + // First, build a dictionary of all of the content-type patterns that are included + // at this node. + // + // For now we're just building up the set of keys. We don't add any endpoints + // to lists now because we don't want ordering problems. + var edges = new Dictionary>(StringComparer.OrdinalIgnoreCase); + for (var i = 0; i < endpoints.Count; i++) + { + var endpoint = endpoints[i]; + var contentTypes = endpoint.Metadata.GetMetadata()?.ContentTypes; + if (contentTypes == null || contentTypes.Count == 0) + { + contentTypes = new string[] { AnyContentType, }; + } + + for (var j = 0; j < contentTypes.Count; j++) + { + var contentType = contentTypes[j]; + + if (!edges.ContainsKey(contentType)) + { + edges.Add(contentType, new List()); + } + } + } + + // Now in a second loop, add endpoints to these lists. We've enumerated all of + // the states, so we want to see which states this endpoint matches. + for (var i = 0; i < endpoints.Count; i++) + { + var endpoint = endpoints[i]; + var contentTypes = endpoint.Metadata.GetMetadata()?.ContentTypes ?? Array.Empty(); + if (contentTypes.Count == 0) + { + // OK this means that this endpoint matches *all* content methods. + // So, loop and add it to all states. + foreach (var kvp in edges) + { + kvp.Value.Add(endpoint); + } + } + else + { + // OK this endpoint matches specific content types -- we have to loop through edges here + // because content types could either be exact (like 'application/json') or they + // could have wildcards (like 'text/*'). We don't expect wildcards to be especially common + // with consumes, but we need to support it. + foreach (var kvp in edges) + { + // The edgeKey maps to a possible request header value + var edgeKey = new MediaType(kvp.Key); + + for (var j = 0; j < contentTypes.Count; j++) + { + var contentType = contentTypes[j]; + + var mediaType = new MediaType(contentType); + + // Example: 'application/json' is subset of 'application/*' + // + // This means that when the request has content-type 'application/json' an endpoint + // what consumes 'application/*' should match. + if (edgeKey.IsSubsetOf(mediaType)) + { + kvp.Value.Add(endpoint); + + // It's possible that a ConsumesMetadata defines overlapping wildcards. Don't add an endpoint + // to any edge twice + break; + } + } + } + } + } + + // If after we're done there isn't any endpoint that accepts */*, then we'll synthesize an + // endpoint that always returns a 415. + if (!edges.ContainsKey(AnyContentType)) + { + edges.Add(AnyContentType, new List() + { + CreateRejectionEndpoint(), + }); + } + + return edges + .Select(kvp => new PolicyNodeEdge(kvp.Key, kvp.Value)) + .ToArray(); + } + + private Endpoint CreateRejectionEndpoint() + { + return new MatcherEndpoint( + (next) => (context) => + { + context.Response.StatusCode = StatusCodes.Status415UnsupportedMediaType; + return Task.CompletedTask; + }, + RoutePatternFactory.Parse("/"), + new RouteValueDictionary(), + 0, + EndpointMetadataCollection.Empty, + Http415EndpointDisplayName); + } + + public PolicyJumpTable BuildJumpTable(int exitDestination, IReadOnlyList edges) + { + if (edges == null) + { + throw new ArgumentNullException(nameof(edges)); + } + + // Since our 'edges' can have wildcards, we do a sort based on how wildcard-ey they + // are then then execute them in linear order. + var ordered = edges + .Select(e => (mediaType: new MediaType((string)e.State), destination: e.Destination)) + .OrderBy(e => GetScore(e.mediaType)) + .ToArray(); + + // If any edge matches all content types, then treat that as the 'exit'. This will + // always happen because we insert a 415 endpoint. + for (var i = 0; i < ordered.Length; i++) + { + if (ordered[i].mediaType.MatchesAllTypes) + { + exitDestination = ordered[i].destination; + break; + } + } + + return new ConsumesPolicyJumpTable(exitDestination, ordered); + } + + private int GetScore(MediaType mediaType) + { + // Higher score == lower priority - see comments on MediaType. + if (mediaType.MatchesAllTypes) + { + return 4; + } + else if (mediaType.MatchesAllSubTypes) + { + return 3; + } + else if (mediaType.MatchesAllSubTypesWithoutSuffix) + { + return 2; + } + else + { + return 1; + } + } + + private class ConsumesMetadataEndpointComparer : EndpointMetadataComparer + { + protected override int CompareMetadata(IConsumesMetadata x, IConsumesMetadata y) + { + // Ignore the metadata if it has an empty list of content types. + return base.CompareMetadata( + x?.ContentTypes.Count > 0 ? x : null, + y?.ContentTypes.Count > 0 ? y : null); + } + } + + private class ConsumesPolicyJumpTable : PolicyJumpTable + { + private (MediaType mediaType, int destination)[] _destinations; + private int _exitDestination; + + public ConsumesPolicyJumpTable(int exitDestination, (MediaType mediaType, int destination)[] destinations) + { + _exitDestination = exitDestination; + _destinations = destinations; + } + + public override int GetDestination(HttpContext httpContext) + { + var contentType = httpContext.Request.ContentType; + if (string.IsNullOrEmpty(contentType)) + { + return _exitDestination; + } + + var requestMediaType = new MediaType(contentType); + var destinations = _destinations; + for (var i = 0; i < destinations.Length; i++) + { + if (requestMediaType.IsSubsetOf(destinations[i].mediaType)) + { + return destinations[i].destination; + } + } + + return _exitDestination; + } + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Routing/ConsumesMetadata.cs b/src/Microsoft.AspNetCore.Mvc.Core/Routing/ConsumesMetadata.cs new file mode 100644 index 0000000000..40c7c86f1d --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/Routing/ConsumesMetadata.cs @@ -0,0 +1,23 @@ +// 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; + +namespace Microsoft.AspNetCore.Mvc.Routing +{ + internal class ConsumesMetadata : IConsumesMetadata + { + public ConsumesMetadata(string[] contentTypes) + { + if (contentTypes == null) + { + throw new ArgumentNullException(nameof(contentTypes)); + } + + ContentTypes = contentTypes; + } + + public IReadOnlyList ContentTypes { get; } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/DependencyInjection/MvcCoreServiceCollectionExtensionsTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/DependencyInjection/MvcCoreServiceCollectionExtensionsTest.cs index 9660812ec5..f7223852de 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/DependencyInjection/MvcCoreServiceCollectionExtensionsTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/DependencyInjection/MvcCoreServiceCollectionExtensionsTest.cs @@ -325,6 +325,7 @@ namespace Microsoft.AspNetCore.Mvc typeof(MatcherPolicy), new Type[] { + typeof(ConsumesMatcherPolicy), typeof(ActionConstraintMatcherPolicy), } }, diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ConsumesMatcherPolicyTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ConsumesMatcherPolicyTest.cs new file mode 100644 index 0000000000..d52ddebf74 --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ConsumesMatcherPolicyTest.cs @@ -0,0 +1,237 @@ +// 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; +using Microsoft.AspNetCore.Routing.Matching; +using Microsoft.AspNetCore.Routing.Patterns; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.Routing +{ + public class ConsumesMatcherPolicyTest + { + [Fact] + public void AppliesToNode_EndpointWithoutMetadata_ReturnsFalse() + { + // Arrange + var endpoints = new[] { CreateEndpoint("/", null), }; + + var policy = CreatePolicy(); + + // Act + var result = policy.AppliesToNode(endpoints); + + // Assert + Assert.False(result); + } + + [Fact] + public void AppliesToNode_EndpointWithoutContentTypes_ReturnsFalse() + { + // Arrange + var endpoints = new[] + { + CreateEndpoint("/", new ConsumesMetadata(Array.Empty())), + }; + + var policy = CreatePolicy(); + + // Act + var result = policy.AppliesToNode(endpoints); + + // Assert + Assert.False(result); + } + + [Fact] + public void AppliesToNode_EndpointHasContentTypes_ReturnsTrue() + { + // Arrange + var endpoints = new[] + { + CreateEndpoint("/", new ConsumesMetadata(Array.Empty())), + CreateEndpoint("/", new ConsumesMetadata(new[] { "application/json", })), + }; + + var policy = CreatePolicy(); + + // Act + var result = policy.AppliesToNode(endpoints); + + // Assert + Assert.True(result); + } + + [Fact] + public void GetEdges_GroupsByContentType() + { + // Arrange + var endpoints = new[] + { + // These are arrange in an order that we won't actually see in a product scenario. It's done + // this way so we can verify that ordering is preserved by GetEdges. + CreateEndpoint("/", new ConsumesMetadata(new[] { "application/json", "application/*+json", })), + CreateEndpoint("/", new ConsumesMetadata(Array.Empty())), + CreateEndpoint("/", new ConsumesMetadata(new[] { "application/xml", "application/*+xml", })), + CreateEndpoint("/", new ConsumesMetadata(new[] { "application/*", })), + CreateEndpoint("/", new ConsumesMetadata(new[]{ "*/*", })), + }; + + var policy = CreatePolicy(); + + // Act + var edges = policy.GetEdges(endpoints); + + // Assert + Assert.Collection( + edges.OrderBy(e => e.State), + e => + { + Assert.Equal("*/*", e.State); + Assert.Equal(new[] { endpoints[1], endpoints[4], }, e.Endpoints.ToArray()); + }, + e => + { + Assert.Equal("application/*", e.State); + Assert.Equal(new[] { endpoints[1], endpoints[3], endpoints[4], }, e.Endpoints.ToArray()); + }, + e => + { + Assert.Equal("application/*+json", e.State); + Assert.Equal(new[] { endpoints[0], endpoints[1], endpoints[3], endpoints[4], }, e.Endpoints.ToArray()); + }, + e => + { + Assert.Equal("application/*+xml", e.State); + Assert.Equal(new[] { endpoints[1], endpoints[2], endpoints[3], endpoints[4], }, e.Endpoints.ToArray()); + }, + e => + { + Assert.Equal("application/json", e.State); + Assert.Equal(new[] { endpoints[0], endpoints[1], endpoints[3], endpoints[4], }, e.Endpoints.ToArray()); + }, + e => + { + Assert.Equal("application/xml", e.State); + Assert.Equal(new[] { endpoints[1], endpoints[2], endpoints[3], endpoints[4], }, e.Endpoints.ToArray()); + }); + } + + [Fact] // See explanation in GetEdges for how this case is different + public void GetEdges_GroupsByContentType_CreatesHttp405Endpoint() + { + // Arrange + var endpoints = new[] + { + // These are arrange in an order that we won't actually see in a product scenario. It's done + // this way so we can verify that ordering is preserved by GetEdges. + CreateEndpoint("/", new ConsumesMetadata(new[] { "application/json", "application/*+json", })), + CreateEndpoint("/", new ConsumesMetadata(new[] { "application/xml", "application/*+xml", })), + CreateEndpoint("/", new ConsumesMetadata(new[] { "application/*", })), + }; + + var policy = CreatePolicy(); + + // Act + var edges = policy.GetEdges(endpoints); + + // Assert + Assert.Collection( + edges.OrderBy(e => e.State), + e => + { + Assert.Equal("*/*", e.State); + Assert.Equal(ConsumesMatcherPolicy.Http415EndpointDisplayName, Assert.Single(e.Endpoints).DisplayName); + }, + e => + { + Assert.Equal("application/*", e.State); + Assert.Equal(new[] { endpoints[2], }, e.Endpoints.ToArray()); + }, + e => + { + Assert.Equal("application/*+json", e.State); + Assert.Equal(new[] { endpoints[0], endpoints[2], }, e.Endpoints.ToArray()); + }, + e => + { + Assert.Equal("application/*+xml", e.State); + Assert.Equal(new[] { endpoints[1], endpoints[2], }, e.Endpoints.ToArray()); + }, + e => + { + Assert.Equal("application/json", e.State); + Assert.Equal(new[] { endpoints[0], endpoints[2], }, e.Endpoints.ToArray()); + }, + e => + { + Assert.Equal("application/xml", e.State); + Assert.Equal(new[] { endpoints[1], endpoints[2], }, e.Endpoints.ToArray()); + }); + + } + + [Theory] + [InlineData("image/png", 1)] + [InlineData("application/foo", 2)] + [InlineData("text/xml", 3)] + [InlineData("application/product+json", 6)] // application/json will match this + [InlineData("application/product+xml", 7)] // application/xml will match this + [InlineData("application/json", 6)] + [InlineData("application/xml", 7)] + public void BuildJumpTable_SortsEdgesByPriority(string contentType, int expected) + { + // Arrange + var edges = new PolicyJumpTableEdge[] + { + // In reverse order of how they should be processed + new PolicyJumpTableEdge("*/*", 1), + new PolicyJumpTableEdge("application/*", 2), + new PolicyJumpTableEdge("text/*", 3), + new PolicyJumpTableEdge("application/*+xml", 4), + new PolicyJumpTableEdge("application/*+json", 5), + new PolicyJumpTableEdge("application/json", 6), + new PolicyJumpTableEdge("application/xml", 7), + }; + + var policy = CreatePolicy(); + + var jumpTable = policy.BuildJumpTable(-1, edges); + + var httpContext = new DefaultHttpContext(); + httpContext.Request.ContentType = contentType; + + // Act + var actual = jumpTable.GetDestination(httpContext); + + // Assert + Assert.Equal(expected, actual); + } + + private static MatcherEndpoint CreateEndpoint(string template, ConsumesMetadata consumesMetadata) + { + var metadata = new List(); + if (consumesMetadata != null) + { + metadata.Add(consumesMetadata); + } + + return new MatcherEndpoint( + (next) => null, + RoutePatternFactory.Parse(template), + new RouteValueDictionary(), + 0, + new EndpointMetadataCollection(metadata), + $"test: {template} - {string.Join(", ", consumesMetadata?.ContentTypes ?? Array.Empty())}"); + } + + private static ConsumesMatcherPolicy CreatePolicy() + { + return new ConsumesMatcherPolicy(); + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ConsumesAttributeEndpointRoutingTests.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ConsumesAttributeEndpointRoutingTests.cs index 7377442ac6..9aebbcb2d4 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ConsumesAttributeEndpointRoutingTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ConsumesAttributeEndpointRoutingTests.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.Net; +using System.Net.Http; using System.Threading.Tasks; using Newtonsoft.Json; using Xunit; @@ -29,5 +30,22 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests Assert.True(result); } + + // The endpoint routing version of this feature has fixed https://github.com/aspnet/Mvc/issues/8174 + [Fact] + public override async Task NoRequestContentType_Selects_IfASingleActionWithConstraintIsPresent() + { + // Arrange + var request = new HttpRequestMessage( + HttpMethod.Post, + "http://localhost/ConsumesAttribute_PassThrough/CreateProduct"); + + // Act + var response = await Client.SendAsync(request); + var body = await response.Content.ReadAsStringAsync(); + + // Assert + Assert.Equal(HttpStatusCode.UnsupportedMediaType, response.StatusCode); + } } } \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ConsumesAttributeTestsBase.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ConsumesAttributeTestsBase.cs index acc68febbc..302d50fc9d 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ConsumesAttributeTestsBase.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ConsumesAttributeTestsBase.cs @@ -49,7 +49,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests } [Fact] - public async Task NoRequestContentType_Selects_IfASingleActionWithConstraintIsPresent() + public virtual async Task NoRequestContentType_Selects_IfASingleActionWithConstraintIsPresent() { // Arrange var request = new HttpRequestMessage( diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RequestServicesEndpointRoutingTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RequestServicesEndpointRoutingTest.cs index 91cba5aeb5..113cfde4dc 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RequestServicesEndpointRoutingTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RequestServicesEndpointRoutingTest.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.Net; +using System.Net.Http; using System.Threading.Tasks; using Newtonsoft.Json; using Xunit;