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.
This commit is contained in:
Ryan Nowak 2018-03-28 20:21:25 -07:00
parent fc3a815e57
commit d360886b78
2 changed files with 10 additions and 14 deletions

View File

@ -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)

View File

@ -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]