Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Commit

Permalink
Respect ApiBehaviorOptions.SuppressInferBindingSourcesForParameters w…
Browse files Browse the repository at this point in the history
…hen specified

Fixes #8657
  • Loading branch information
pranavkm committed Oct 29, 2018
1 parent 35d2ab3 commit 22dc79c
Show file tree
Hide file tree
Showing 8 changed files with 305 additions and 17 deletions.
46 changes: 46 additions & 0 deletions src/Microsoft.AspNetCore.Mvc.Core/ApiBehaviorOptions.cs
Expand Up @@ -17,6 +17,7 @@ public class ApiBehaviorOptions : IEnumerable<ICompatibilitySwitch>
{
private readonly CompatibilitySwitch<bool> _suppressMapClientErrors;
private readonly CompatibilitySwitch<bool> _suppressUseValidationProblemDetailsForInvalidModelStateResponses;
private readonly CompatibilitySwitch<bool> _allowInferringBinderModelNameWhenBindingInfoIsSpecified;
private readonly ICompatibilitySwitch[] _switches;

private Func<ActionContext, IActionResult> _invalidModelStateResponseFactory;
Expand All @@ -28,10 +29,12 @@ public ApiBehaviorOptions()
{
_suppressMapClientErrors = new CompatibilitySwitch<bool>(nameof(SuppressMapClientErrors));
_suppressUseValidationProblemDetailsForInvalidModelStateResponses = new CompatibilitySwitch<bool>(nameof(SuppressUseValidationProblemDetailsForInvalidModelStateResponses));
_allowInferringBinderModelNameWhenBindingInfoIsSpecified = new CompatibilitySwitch<bool>(nameof(AllowInferringBinderModelNameWhenBindingInfoIsSpecified));
_switches = new[]
{
_suppressMapClientErrors,
_suppressUseValidationProblemDetailsForInvalidModelStateResponses,
_allowInferringBinderModelNameWhenBindingInfoIsSpecified
};
}

Expand Down Expand Up @@ -151,6 +154,49 @@ public bool SuppressUseValidationProblemDetailsForInvalidModelStateResponses
set => _suppressUseValidationProblemDetailsForInvalidModelStateResponses.Value = value;
}

/// <summary>
/// Gets or sets a value that determines if <see cref="BindingInfo.BinderModelName"/> is inferred when the parameter or property
/// specifies any binding-specific value.
/// <para>
/// <see cref="BindingInfo.BinderModelName"/> is inferred for for bound properties and parameters on controllers
/// with <see cref="ApiControllerAttribute"/> for complex (<see cref="ModelMetadata.IsComplexType"/>) types with greedy
/// (<see cref="BindingSource.IsGreedy"/>) binding sources.
/// <para></para>
/// When <see langword="false"/> the <see cref="BindingInfo.BinderModelName"/> is not inferred,
/// if the parameter or property is annotated with any model binding specific attribute
/// such as <see cref="FromQueryAttribute"/>, or <see cref="ModelBinderAttribute"/> etc., or the associated
/// <see cref="ModelMetadata"/> specifies a value for the <see cref="BindingInfo"/>.
/// </para>
/// </summary>
/// <value>
/// The default value is <see langword="false"/> if the version is
/// <see cref="CompatibilityVersion.Version_2_2"/> or later; <see langword="true"/> otherwise.
/// </value>
/// <remarks>
/// <para>
/// This property is associated with a compatibility switch and can provide a different behavior depending on
/// the configured compatibility version for the application. See <see cref="CompatibilityVersion"/> for
/// guidance and examples of setting the application's compatibility version.
/// </para>
/// <para>
/// Configuring the desired value of the compatibility switch by calling this property's setter will take
/// precedence over the value implied by the application's <see cref="CompatibilityVersion"/>.
/// </para>
/// <para>
/// If the application's compatibility version is set to <see cref="CompatibilityVersion.Version_2_1"/> or
/// lower then this setting will have the value <see langword="true"/> unless explicitly configured.
/// </para>
/// <para>
/// If the application's compatibility version is set to <see cref="CompatibilityVersion.Version_2_2"/> or
/// higher then this setting will have the value <see langword="false"/> unless explicitly configured.
/// </para>
/// </remarks>
public bool AllowInferringBinderModelNameWhenBindingInfoIsSpecified
{
get => _allowInferringBinderModelNameWhenBindingInfoIsSpecified.Value;
set => _allowInferringBinderModelNameWhenBindingInfoIsSpecified.Value = value;
}

/// <summary>
/// Gets a map of HTTP status codes to <see cref="ClientErrorData"/>. Configured values
/// are used to transform <see cref="IClientErrorActionResult"/> to an <see cref="ObjectResult"/>
Expand Down
Expand Up @@ -50,7 +50,8 @@ internal class ApiBehaviorApplicationModelProvider : IApplicationModelProvider

var inferParameterBindingInfoConvention = new InferParameterBindingInfoConvention(modelMetadataProvider)
{
SuppressInferBindingSourcesForParameters = options.SuppressInferBindingSourcesForParameters
SuppressInferBindingSourcesForParameters = options.SuppressInferBindingSourcesForParameters,
AllowInferringBinderModelNameWhenBindingInfoIsSpecified = options.AllowInferringBinderModelNameWhenBindingInfoIsSpecified,
};
ControllerModelConventions = new List<IControllerModelConvention>
{
Expand Down
Expand Up @@ -34,6 +34,8 @@ public class InferParameterBindingInfoConvention : IControllerModelConvention
/// </summary>
public bool SuppressInferBindingSourcesForParameters { get; set; }

internal bool AllowInferringBinderModelNameWhenBindingInfoIsSpecified { get; set; }

protected virtual bool ShouldApply(ControllerModel controller) => true;

public void Apply(ControllerModel controller)
Expand All @@ -49,7 +51,6 @@ public void Apply(ControllerModel controller)
}

InferBoundPropertyModelPrefixes(controller);

foreach (var action in controller.Actions)
{
InferParameterBindingSources(action);
Expand All @@ -59,18 +60,56 @@ public void Apply(ControllerModel controller)

internal void InferParameterBindingSources(ActionModel action)
{
if (SuppressInferBindingSourcesForParameters)
{
// Do nothing.
return;
}

var inferredBindingSources = new BindingSource[action.Parameters.Count];

for (var i = 0; i < action.Parameters.Count; i++)
{
var parameter = action.Parameters[i];
var bindingSource = parameter.BindingInfo?.BindingSource;

var userSpecifiedBindingInfo = parameter.BindingInfo;
if (userSpecifiedBindingInfo?.BindingSource != null)
{
continue;
}

var bindingSource = InferBindingSourceForParameter(parameter);

if (bindingSource == null)
{
bindingSource = InferBindingSourceForParameter(parameter);
continue;
}

parameter.BindingInfo = parameter.BindingInfo ?? new BindingInfo();
parameter.BindingInfo.BindingSource = bindingSource;
parameter.BindingInfo = parameter.BindingInfo ?? new BindingInfo();
parameter.BindingInfo.BindingSource = bindingSource;

if (AllowInferringBinderModelNameWhenBindingInfoIsSpecified)
{
// Using legacy compat behavior. BinderModelName will be inferred eventually.
return;
}

// For any complex types that are bound from value providers, set the BinderModelName
// to an empty string. This makes binding much more predictable
// and documentable via ApiExplorer.
// Only do this if the user hasn't explicitly specified any binding information for the parameter.
if (userSpecifiedBindingInfo != null)
{
continue;
}

var bindingInfo = parameter.BindingInfo;
if (bindingInfo.BinderModelName == null &&
bindingSource.IsGreedy &&
!IsFormFile(parameter.ParameterType) &&
IsComplexTypeParameter(parameter))
{
parameter.BindingInfo.BinderModelName = string.Empty;
}
}

Expand Down Expand Up @@ -109,6 +148,11 @@ internal BindingSource InferBindingSourceForParameter(ParameterModel parameter)
// and describable via ApiExplorer
internal void InferBoundPropertyModelPrefixes(ControllerModel controllerModel)
{
if (!AllowInferringBinderModelNameWhenBindingInfoIsSpecified)
{
return;
}

foreach (var property in controllerModel.ControllerProperties)
{
if (property.BindingInfo != null &&
Expand All @@ -130,6 +174,11 @@ internal void InferBoundPropertyModelPrefixes(ControllerModel controllerModel)

internal void InferParameterModelPrefixes(ActionModel action)
{
if (!AllowInferringBinderModelNameWhenBindingInfoIsSpecified)
{
return;
}

foreach (var parameter in action.Parameters)
{
var bindingInfo = parameter.BindingInfo;
Expand Down
Expand Up @@ -35,6 +35,7 @@ public class ApiBehaviorOptionsSetup :
{
dictionary[nameof(ApiBehaviorOptions.SuppressMapClientErrors)] = true;
dictionary[nameof(ApiBehaviorOptions.SuppressUseValidationProblemDetailsForInvalidModelStateResponses)] = true;
dictionary[nameof(ApiBehaviorOptions.AllowInferringBinderModelNameWhenBindingInfoIsSpecified)] = true;
}

return dictionary;
Expand Down
Expand Up @@ -156,6 +156,150 @@ public void OnProvidersExecuting_PreservesBindingInfo_WhenInferringFor_Parameter
Assert.Equal("foo", bindingInfo.BinderModelName);
}

[Fact]
public void Apply_InfersBindingSourceAndBinderModelName()
{
// Arrange
var actionName = nameof(TestController.PutModel);
var convention = GetConvention();
var controller = GetControllerModel(typeof(TestController));

// Act
convention.Apply(controller);

// Assert
var action = Assert.Single(controller.Actions, a => a.ActionName == actionName);

Assert.Collection(
action.Parameters,
parameter =>
{
Assert.Equal("id", parameter.Name);
var bindingInfo = parameter.BindingInfo;
Assert.NotNull(bindingInfo);
Assert.Same(BindingSource.Path, bindingInfo.BindingSource);
Assert.Null(bindingInfo.BinderModelName);
},
parameter =>
{
Assert.Equal("model", parameter.Name);
var bindingInfo = parameter.BindingInfo;
Assert.NotNull(bindingInfo);
Assert.Same(BindingSource.Body, bindingInfo.BindingSource);
Assert.Empty(bindingInfo.BinderModelName);
});
}

[Fact]
public void Apply_DoesNotInferBindingSource_IfSuppressInferBindingSourcesForParametersIsSet()
{
// Arrange
var actionName = nameof(TestController.PutModel);
var convention = GetConvention();
convention.SuppressInferBindingSourcesForParameters = true;
var controller = GetControllerModel(typeof(TestController));

// Act
convention.Apply(controller);

// Assert
var action = Assert.Single(controller.Actions, a => a.ActionName == actionName);

Assert.Collection(
action.Parameters,
parameter =>
{
Assert.Equal("id", parameter.Name);
var bindingInfo = parameter.BindingInfo;
Assert.Null(bindingInfo);
},
parameter =>
{
Assert.Equal("model", parameter.Name);
var bindingInfo = parameter.BindingInfo;
Assert.Null(bindingInfo);
});
}

[Fact]
public void Apply_DoesNotInferParameterPrefix_IfSuppressInferBindingSourcesForParametersIsSet()
{
// Arrange
var actionName = nameof(TestController.GetByCoordinates);
var convention = GetConvention();
convention.SuppressInferBindingSourcesForParameters = true;
var controller = GetControllerModel(typeof(TestController));

// Act
convention.Apply(controller);

// Assert
var action = Assert.Single(controller.Actions, a => a.ActionName == actionName);

var parameter = Assert.Single(action.Parameters);
var bindingInfo = parameter.BindingInfo;
Assert.NotNull(bindingInfo);
Assert.Equal(BindingSource.Query, bindingInfo.BindingSource);
Assert.Null(bindingInfo.BinderModelName);
}

[Fact]
public void Apply_InferParameterPrefix_IfAllowInferringBinderModelNameWhenBindingInfoIsSpecifiedIsSet()
{
// Arrange
var actionName = nameof(TestController.GetByCoordinates);
var convention = GetConvention();
convention.SuppressInferBindingSourcesForParameters = true;
convention.AllowInferringBinderModelNameWhenBindingInfoIsSpecified = true;
var controller = GetControllerModel(typeof(TestController));

// Act
convention.Apply(controller);

// Assert
var action = Assert.Single(controller.Actions, a => a.ActionName == actionName);

var parameter = Assert.Single(action.Parameters);
var bindingInfo = parameter.BindingInfo;
Assert.NotNull(bindingInfo);
Assert.Equal(BindingSource.Query, bindingInfo.BindingSource);
Assert.Empty(bindingInfo.BinderModelName);
}

[Fact]
public void Apply_InfersPropertyPrefix_IfAllowInferringBinderModelNameWhenBindingInfoIsSpecifiedIsSet()
{
// Arrange
var convention = GetConvention();
convention.AllowInferringBinderModelNameWhenBindingInfoIsSpecified = true;
var controller = GetControllerModel(typeof(ControllerWithBoundProperty));

// Act
convention.Apply(controller);

// Assert
Assert.Collection(
controller.ControllerProperties.OrderBy(p => p.Name),
property =>
{
Assert.Equal(nameof(ControllerWithBoundProperty.Files), property.Name);
var bindingInfo = property.BindingInfo;
Assert.NotNull(bindingInfo);
Assert.Equal(BindingSource.Form, bindingInfo.BindingSource);
Assert.Null(bindingInfo.BinderModelName); // BindingSource is not greedy, BinderModelName should not be inferred.
},
property =>
{
Assert.Equal(nameof(ControllerWithBoundProperty.TestProperty), property.Name);
var bindingInfo = property.BindingInfo;
Assert.NotNull(bindingInfo);
Assert.Equal(BindingSource.Query, bindingInfo.BindingSource);
Assert.Empty(bindingInfo.BinderModelName); // BindingSource is greedy, BinderModelName should be inferred.
});
}

[Fact]
public void InferBindingSourceForParameter_ReturnsPath_IfParameterNameExistsInRouteAsSimpleToken()
{
Expand Down Expand Up @@ -681,6 +825,7 @@ public void InferBoundPropertyModelPrefixes_SetsModelPrefix_ForComplexTypeFromVa
// Arrange
var controller = GetControllerModel(typeof(ControllerWithBoundProperty));
var convention = GetConvention();
convention.AllowInferringBinderModelNameWhenBindingInfoIsSpecified = true;

// Act
convention.InferBoundPropertyModelPrefixes(controller);
Expand Down Expand Up @@ -711,6 +856,7 @@ public void InferParameterModelPrefixes_SetsModelPrefix_ForComplexTypeFromValueP
// Arrange
var action = GetActionModel(typeof(ControllerWithBoundProperty), nameof(ControllerWithBoundProperty.SomeAction));
var convention = GetConvention();
convention.AllowInferringBinderModelNameWhenBindingInfoIsSpecified = true;

// Act
convention.InferParameterModelPrefixes(action);
Expand All @@ -725,7 +871,7 @@ public void InferParameterModelPrefixes_DoesNotSetModelPrefix_ForFormFileParamet
{
// Arrange
var action = GetActionModel(
typeof(ParameterBindingController),
typeof(ParameterBindingController),
nameof(ParameterBindingController.FromFormFormFileParameters),
TestModelMetadataProvider.CreateDefaultProvider());
var convention = GetConvention();
Expand Down Expand Up @@ -758,8 +904,8 @@ public void InferParameterModelPrefixes_DoesNotSetModelPrefix_ForFormFileParamet
{
// Arrange
var action = GetActionModel(
typeof(ParameterBindingController),
nameof(ParameterBindingController.FormFileParameters),
typeof(ParameterBindingController),
nameof(ParameterBindingController.FormFileParameters),
TestModelMetadataProvider.CreateDefaultProvider());
var convention = GetConvention();

Expand Down Expand Up @@ -1070,5 +1216,14 @@ private class ParameterWithBindingInfo
[HttpGet("test")]
public IActionResult Action([ModelBinder(typeof(object))] Car car) => null;
}

private class TestController
{
[HttpPut("{id}")]
public IActionResult PutModel(int id, TestModel model) => null;

[HttpPut("{id}")]
public IActionResult GetByCoordinates([FromQuery] GpsCoordinates coordinates) => null;
}
}
}

0 comments on commit 22dc79c

Please sign in to comment.