From 950db6587c3422b42b3bc47c953f1bbb89486f8f Mon Sep 17 00:00:00 2001 From: Pranav K Date: Fri, 22 Sep 2017 17:35:49 -0700 Subject: [PATCH] Require attribute routing with [ApiController] Fixes #6870 --- .../ApiControllerApplicationModelProvider.cs | 25 +++++++------ .../Properties/Resources.Designer.cs | 14 ++++++++ .../Resources.resx | 3 ++ ...iControllerApplicationModelProviderTest.cs | 35 ++++++++++++++++--- 4 files changed, 60 insertions(+), 17 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiControllerApplicationModelProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiControllerApplicationModelProvider.cs index 27d83200b2..a8ddde6528 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiControllerApplicationModelProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiControllerApplicationModelProvider.cs @@ -33,9 +33,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal } /// - /// Order is set to execute after the . + /// Order is set to execute after the and allow any other user + /// that configure routing to execute. /// - public int Order => -1000 + 10; + public int Order => -1000 + 100; public void OnProvidersExecuted(ApplicationModelProviderContext context) { @@ -45,21 +46,19 @@ namespace Microsoft.AspNetCore.Mvc.Internal { foreach (var controllerModel in context.Result.Controllers) { - if (controllerModel.Attributes.OfType().Any()) - { - if (_apiBehaviorOptions.EnableModelStateInvalidFilter) - { - Debug.Assert(_apiBehaviorOptions.InvalidModelStateResponseFactory != null); - controllerModel.Filters.Add(_modelStateInvalidFilter); - } - - continue; - } + var isApiController = controllerModel.Attributes.OfType().Any(); + var controllerHasSelectorModel = controllerModel.Selectors.Any(s => s.AttributeRouteModel != null); foreach (var actionModel in controllerModel.Actions) { - if (actionModel.Attributes.OfType().Any()) + if (isApiController || actionModel.Attributes.OfType().Any()) { + if (!controllerHasSelectorModel && !actionModel.Selectors.Any(s => s.AttributeRouteModel != null)) + { + // Require attribute routing with controllers annotated with ApiControllerAttribute + throw new InvalidOperationException(Resources.FormatApiController_AttributeRouteRequired(nameof(ApiControllerAttribute))); + } + if (_apiBehaviorOptions.EnableModelStateInvalidFilter) { Debug.Assert(_apiBehaviorOptions.InvalidModelStateResponseFactory != null); diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Properties/Resources.Designer.cs b/src/Microsoft.AspNetCore.Mvc.Core/Properties/Resources.Designer.cs index f424455c65..772a8977d3 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Properties/Resources.Designer.cs @@ -1340,6 +1340,20 @@ namespace Microsoft.AspNetCore.Mvc.Core internal static string FormatValidationProblemDescription_Title() => GetString("ValidationProblemDescription_Title"); + /// + /// Action methods on controllers annotated with {0} must have an attribute route. + /// + internal static string ApiController_AttributeRouteRequired + { + get => GetString("ApiController_AttributeRouteRequired"); + } + + /// + /// Action methods on controllers annotated with {0} must have an attribute route. + /// + internal static string FormatApiController_AttributeRouteRequired(object p0) + => string.Format(CultureInfo.CurrentCulture, GetString("ApiController_AttributeRouteRequired"), p0); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Resources.resx b/src/Microsoft.AspNetCore.Mvc.Core/Resources.resx index eceaf33576..dce45c2f75 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Resources.resx +++ b/src/Microsoft.AspNetCore.Mvc.Core/Resources.resx @@ -415,4 +415,7 @@ One or more validation errors occured. + + Action methods on controllers annotated with {0} must have an attribute route. + \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiControllerApplicationModelProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiControllerApplicationModelProviderTest.cs index dc78ee56d7..15a00e5445 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiControllerApplicationModelProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiControllerApplicationModelProviderTest.cs @@ -22,15 +22,15 @@ namespace Microsoft.AspNetCore.Mvc.Internal { InvalidModelStateResponseFactory = _ => null, }); - + var provider = new ApiControllerApplicationModelProvider(options, NullLoggerFactory.Instance); // Act provider.OnProvidersExecuting(context); // Assert - var controllerModel = Assert.Single(context.Result.Controllers); - Assert.IsType(controllerModel.Filters.Last()); + var actionModel = Assert.Single(Assert.Single(context.Result.Controllers).Actions); + Assert.IsType(actionModel.Filters.Last()); } [Fact] @@ -109,6 +109,25 @@ namespace Microsoft.AspNetCore.Mvc.Internal }); } + [Fact] + public void OnProvidersExecuting_ThrowsIfControllerWithAttribute_HasActionsWithoutAttributeRouting() + { + // Arrange + var context = GetContext(typeof(ActionsWithoutAttributeRouting)); + var options = new TestOptionsManager(new ApiBehaviorOptions + { + InvalidModelStateResponseFactory = _ => null, + }); + + var provider = new ApiControllerApplicationModelProvider(options, NullLoggerFactory.Instance); + + // Act & Assert + var ex = Assert.Throws(() => provider.OnProvidersExecuting(context)); + Assert.Equal( + "Action methods on controllers annotated with ApiControllerAttribute must have an attribute route.", + ex.Message); + } + private static ApplicationModelProviderContext GetContext(Type type) { var context = new ApplicationModelProviderContext(new[] { type.GetTypeInfo() }); @@ -117,20 +136,28 @@ namespace Microsoft.AspNetCore.Mvc.Internal } [ApiController] + [Route("TestApi")] private class TestApiController : Controller { + [HttpGet] public IActionResult TestAction() => null; } - private class SimpleController : Controller { public IActionResult ActionWithoutFilter() => null; [TestApiBehavior] + [HttpGet("/Simple/ActionWithFilter")] public IActionResult ActionWithFilter() => null; } + [ApiController] + private class ActionsWithoutAttributeRouting + { + public IActionResult Index() => null; + } + [AttributeUsage(AttributeTargets.Method)] private class TestApiBehavior : Attribute, IApiBehaviorMetadata {