diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultApplicationModelProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultApplicationModelProvider.cs index 9b9c345df0..ccfead7293 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultApplicationModelProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultApplicationModelProvider.cs @@ -293,7 +293,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal { actionModel.RouteValues.Add(routeValueProvider.RouteKey, routeValueProvider.RouteValue); } - + //TODO: modify comment // Now we need to determine the action selection info (cross-section of routes and constraints) // @@ -468,6 +468,16 @@ namespace Microsoft.AspNetCore.Mvc.Internal // 1. [HttpPost("Api/Things")] // 2. [HttpGet], [AcceptVerbs("POST", "PUT")] // + // Another example of this situation is: + // + // [Route("api/Products")] + // [AcceptVerbs("GET", "HEAD")] + // [HttpPost("api/Products/new")] + // + // This will generate 2 selectors: + // 1. [AcceptVerbs("GET", "HEAD")] + // 2. [HttpPost] + // // Note that having a route attribute that doesn't define a route template _might_ be an error. We // don't have enough context to really know at this point so we just pass it on. var routeProviders = new List(); @@ -530,18 +540,25 @@ namespace Microsoft.AspNetCore.Mvc.Internal var filteredAttributes = new List(); foreach (var attribute in attributes) { - if (attribute == routeProvider) + if (ReferenceEquals(attribute, routeProvider)) { filteredAttributes.Add(attribute); } - else if (routeProviders.Contains(attribute)) + else if (InRouteProviders(routeProviders, attribute)) { // Exclude other route template providers + // Example: + // [HttpGet("template")] + // [Route("template/{id}")] } else if ( routeProvider is IActionHttpMethodProvider && attribute is IActionHttpMethodProvider) { + // Example: + // [HttpGet("template")] + // [AcceptVerbs("GET", "POST")] + // // Exclude other http method providers if this route is an // http method provider. } @@ -559,7 +576,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var filteredAttributes = new List(); foreach (var attribute in attributes) { - if (!routeProviders.Contains(attribute)) + if (!InRouteProviders(routeProviders, attribute)) { filteredAttributes.Add(attribute); } @@ -572,6 +589,19 @@ namespace Microsoft.AspNetCore.Mvc.Internal return selectorModels; } + private static bool InRouteProviders(List routeProviders, object attribute) + { + foreach (var rp in routeProviders) + { + if (ReferenceEquals(rp, attribute)) + { + return true; + } + } + + return false; + } + private static SelectorModel CreateSelectorModel(IRouteTemplateProvider route, IList attributes) { var selectorModel = new SelectorModel(); diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/DefaultApplicationModelProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/DefaultApplicationModelProviderTest.cs index 5931328b24..bfe0be0051 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/DefaultApplicationModelProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/DefaultApplicationModelProviderTest.cs @@ -792,6 +792,29 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.Equal(new string[] { "POST" }, methodConstraint.HttpMethods); } + [Fact] + public void CreateActionModel_MixedHttpVerbsAndRoutes_WithRouteOnController() + { + // Arrange + var builder = new TestApplicationModelProvider(); + var typeInfo = typeof(RouteAttributeOnController).GetTypeInfo(); + var actionName = nameof(RouteAttributeOnController.Get); + + // Act + var action = builder.CreateActionModel(typeInfo, typeInfo.AsType().GetMethod(actionName)); + + // Assert + Assert.Equal(2, action.Selectors.Count); + + var selectorModel = Assert.Single(action.Selectors, s => s.AttributeRouteModel == null); + var methodConstraint = Assert.Single(selectorModel.ActionConstraints.OfType()); + Assert.Equal(new string[] { "GET" }, methodConstraint.HttpMethods); + + selectorModel = Assert.Single(action.Selectors, s => s.AttributeRouteModel?.Template == "id/{id?}"); + methodConstraint = Assert.Single(selectorModel.ActionConstraints.OfType()); + Assert.Equal(new string[] { "GET" }, methodConstraint.HttpMethods); + } + [Fact] public void CreateActionModel_MixedHttpVerbsAndRoutes_MultipleEmptyAndNonEmptyVerbs() { @@ -1124,6 +1147,23 @@ namespace Microsoft.AspNetCore.Mvc.Internal public void Invalid() { } } + [Route("api/[controller]")] + private class RouteAttributeOnController : Controller + { + [HttpGet] + [HttpGet("id/{id?}")] + public object Get(short? id) + { + return null; + } + + [HttpDelete("{id}")] + public object Delete(int id) + { + return null; + } + } + // Here the constraints on the methods are acting as an IActionHttpMethodProvider and // not as an IRouteTemplateProvider given that there is no RouteAttribute // on the controller and the template for all the constraints on a method is null. diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RoutingTests.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RoutingTests.cs index 055824a4b5..c3ee4d0757 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RoutingTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RoutingTests.cs @@ -157,6 +157,66 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests result.RouteValues); } + [Theory] + [InlineData("Get", "/Friends")] + [InlineData("Get", "/Friends/Peter")] + [InlineData("Delete", "/Friends")] + public async Task AttributeRoutedAction_AcceptRequestsWithValidMethods_InRoutesWithoutExtraTemplateSegmentsOnTheAction( + string method, + string url) + { + // Arrange + var request = new HttpRequestMessage(new HttpMethod(method), $"http://localhost{url}"); + + // Assert + var response = await Client.SendAsync(request); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + var body = await response.Content.ReadAsStringAsync(); + var result = JsonConvert.DeserializeObject(body); + + Assert.Contains(url, result.ExpectedUrls); + Assert.Equal("Friends", result.Controller); + Assert.Equal(method, result.Action); + + Assert.Contains( + new KeyValuePair("controller", "Friends"), + result.RouteValues); + + Assert.Contains( + new KeyValuePair("action", method), + result.RouteValues); + + if (result.RouteValues.ContainsKey("id")) + { + Assert.Contains( + new KeyValuePair("id", "Peter"), + result.RouteValues); + } + } + + [Theory] + [InlineData("Post", "/Friends")] + [InlineData("Put", "/Friends")] + [InlineData("Patch", "/Friends")] + [InlineData("Options", "/Friends")] + [InlineData("Head", "/Friends")] + public async Task AttributeRoutedAction_RejectsRequestsWithWrongMethods_InRoutesWithoutExtraTemplateSegmentsOnTheAction( + string method, + string url) + { + // Arrange + var request = new HttpRequestMessage(new HttpMethod(method), $"http://localhost{url}"); + + // Assert + var response = await Client.SendAsync(request); + + // Assert + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } + [Theory] [InlineData("http://localhost/api/v1/Maps")] [InlineData("http://localhost/api/v2/Maps")] @@ -1190,7 +1250,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests return Url + "?" + string.Join("&", Values.Select(kvp => kvp.Key + "=" + kvp.Value)); } - public static implicit operator string (LinkBuilder builder) + public static implicit operator string(LinkBuilder builder) { return builder.ToString(); } diff --git a/test/WebSites/RoutingWebSite/Controllers/FriendsController.cs b/test/WebSites/RoutingWebSite/Controllers/FriendsController.cs new file mode 100644 index 0000000000..b6f3779e2d --- /dev/null +++ b/test/WebSites/RoutingWebSite/Controllers/FriendsController.cs @@ -0,0 +1,31 @@ +// 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.Mvc; + +namespace RoutingWebSite +{ + [Route("Friends")] + public class FriendsController : Controller + { + private readonly TestResponseGenerator _generator; + + public FriendsController(TestResponseGenerator generator) + { + _generator = generator; + } + + [HttpGet] + [HttpGet("{id}")] + public IActionResult Get([FromRoute]string id) + { + return _generator.Generate(id == null ? "/Friends" : $"/Friends/{id}"); + } + + [HttpDelete] + public IActionResult Delete() + { + return _generator.Generate("/Friends"); + } + } +}