From d360886b788421a1375670c2aec433787524f716 Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Wed, 28 Mar 2018 20:21:25 -0700 Subject: [PATCH] Fix #7558 infer [FromRoute] with parameter in ANY route This changes the logic for when we infer [FromRoute] on an action parameter from *ALL* to *ANY*. This means that if a parameter occurs in any route on an ApiController, we will treat it as [FromRoute]. We think this is the best decision because it's less ambiguous. If a parameter appears in a route, it won't be eligible to be bound from query. I think that's good. If for some reason you want this kind of behavior (route or query) then we suggest breaking up the actions. This isn't very documentation friendly (swagger) so we don't suggest it. --- .../ApiBehaviorApplicationModelProvider.cs | 14 +++++--------- .../ApiBehaviorApplicationModelProviderTest.cs | 10 +++++----- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs index e91b46c995..7bc6c6892d 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs @@ -230,7 +230,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Internal for unit testing. internal BindingSource InferBindingSourceForParameter(ParameterModel parameter) { - if (ParameterExistsInAllRoutes(parameter.Action, parameter.ParameterName)) + if (ParameterExistsInAnyRoute(parameter.Action, parameter.ParameterName)) { return BindingSource.Path; } @@ -242,9 +242,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal return bindingSource; } - private bool ParameterExistsInAllRoutes(ActionModel actionModel, string parameterName) + private bool ParameterExistsInAnyRoute(ActionModel actionModel, string parameterName) { - var parameterExistsInSomeRoute = false; foreach (var (route, _, _) in ActionAttributeRouteModel.GetAttributeRoutes(actionModel)) { if (route == null) @@ -253,16 +252,13 @@ namespace Microsoft.AspNetCore.Mvc.Internal } var parsedTemplate = TemplateParser.Parse(route.Template); - if (parsedTemplate.GetParameter(parameterName) == null) + if (parsedTemplate.GetParameter(parameterName) != null) { - return false; + return true; } - - // Ensure at least one route exists. - parameterExistsInSomeRoute = true; } - return parameterExistsInSomeRoute; + return false; } private bool IsComplexTypeParameter(ParameterModel parameter) diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorApplicationModelProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorApplicationModelProviderTest.cs index 935e0094e9..a9a01f5aba 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorApplicationModelProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorApplicationModelProviderTest.cs @@ -219,7 +219,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal } [Fact] - public void InferBindingSourceForParameter_ReturnsPath_IfParameterAppearsInAllRoutes() + public void InferBindingSourceForParameter_ReturnsPath_IfParameterAppearsInAnyRoutes_MulitpleRoutes() { // Arrange var actionName = nameof(ParameterBindingController.ParameterInMultipleRoutes); @@ -234,7 +234,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal } [Fact] - public void InferBindingSourceForParameter_DoesNotReturnPath_IfParameterDoesNotAppearInAllRoutes() + public void InferBindingSourceForParameter_ReturnsPath_IfParameterAppearsInAnyRoute() { // Arrange var actionName = nameof(ParameterBindingController.ParameterNotInAllRoutes); @@ -245,7 +245,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var result = provider.InferBindingSourceForParameter(parameter); // Assert - Assert.Same(BindingSource.Query, result); + Assert.Same(BindingSource.Path, result); } [Fact] @@ -309,7 +309,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal } [Fact] - public void InferBindingSourceForParameter_DoesNotReturnPath_IfOneActionRouteOverridesControllerRoute() + public void InferBindingSourceForParameter_ReturnsPath_IfParameterPresentInNonOverriddenControllerRoute() { // Arrange var actionName = nameof(ParameterInController.MultipleRouteWithOverride); @@ -320,7 +320,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var result = provider.InferBindingSourceForParameter(parameter); // Assert - Assert.Same(BindingSource.Query, result); + Assert.Same(BindingSource.Path, result); } [Fact]