From 2d63669695379d7da9c92653c24c8c626df02dad Mon Sep 17 00:00:00 2001 From: Kiran Challa Date: Tue, 22 May 2018 11:57:34 -0700 Subject: [PATCH] [Fixes #7609] ApiBehaviorApplicationModelProvider overwrites existing BindingInfo in entirety when inferring binding sources --- .../ApiBehaviorApplicationModelProvider.cs | 7 +- ...ApiBehaviorApplicationModelProviderTest.cs | 332 +++++++++++++++++- .../ApiBehaviorTest.cs | 30 ++ .../Controllers/ContactApiController.cs | 30 ++ 4 files changed, 391 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs index 57c65a6d32..9d86de29e5 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs @@ -155,10 +155,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal if (bindingSource == null) { bindingSource = InferBindingSourceForParameter(parameter); - parameter.BindingInfo = new BindingInfo - { - BindingSource = bindingSource, - }; + + parameter.BindingInfo = parameter.BindingInfo ?? new BindingInfo(); + parameter.BindingInfo.BindingSource = bindingSource; } } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorApplicationModelProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorApplicationModelProviderTest.cs index 5ce29079a1..a90dbc2943 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorApplicationModelProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorApplicationModelProviderTest.cs @@ -6,6 +6,7 @@ using System.ComponentModel; using System.Linq; using System.Reflection; using System.Threading; +using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.ApplicationModels; using Microsoft.AspNetCore.Mvc.Infrastructure; @@ -457,12 +458,13 @@ Environment.NewLine + "int b"; Assert.Equal(expected, ex.Message); } - [Fact(Skip = "https://github.com/aspnet/Mvc/issues/7609")] - public void OnProvidersExecuting_PreservesBindingInfo_WhenInferringBindingSource() + [Fact] + public void OnProvidersExecuting_PreservesBindingInfo_WhenInferringFor_ParameterWithModelBinder_AndExplicitName() { // Arrange - var actionName = nameof(ParameterBindingController.ModelBinderAttribute); - var context = GetContext(typeof(ParameterBindingController)); + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var actionName = nameof(ModelBinderOnParameterController.ModelBinderAttributeWithExplicitModelName); + var context = GetContext(typeof(ModelBinderOnParameterController), modelMetadataProvider); var provider = GetProvider(); // Act @@ -479,6 +481,263 @@ Environment.NewLine + "int b"; Assert.Equal("top", bindingInfo.BinderModelName); } + [Fact] + public void OnProvidersExecuting_PreservesBindingInfo_WhenInferringFor_ParameterWithModelBinderType() + { + // Arrange + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var actionName = nameof(ModelBinderOnParameterController.ModelBinderType); + var context = GetContext(typeof(ModelBinderOnParameterController), modelMetadataProvider); + var provider = GetProvider(); + + // Act + provider.OnProvidersExecuting(context); + + // Assert + var controller = Assert.Single(context.Result.Controllers); + var action = Assert.Single(controller.Actions, a => a.ActionName == actionName); + var parameter = Assert.Single(action.Parameters); + + var bindingInfo = parameter.BindingInfo; + Assert.NotNull(bindingInfo); + Assert.Same(BindingSource.Custom, bindingInfo.BindingSource); + Assert.Null(bindingInfo.BinderModelName); + } + + [Fact] + public void OnProvidersExecuting_PreservesBindingInfo_WhenInferringFor_ParameterWithModelBinderType_AndExplicitModelName() + { + // Arrange + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var actionName = nameof(ModelBinderOnParameterController.ModelBinderTypeWithExplicitModelName); + var context = GetContext(typeof(ModelBinderOnParameterController), modelMetadataProvider); + var provider = GetProvider(); + + // Act + provider.OnProvidersExecuting(context); + + // Assert + var controller = Assert.Single(context.Result.Controllers); + var action = Assert.Single(controller.Actions, a => a.ActionName == actionName); + var parameter = Assert.Single(action.Parameters); + + var bindingInfo = parameter.BindingInfo; + Assert.NotNull(bindingInfo); + Assert.Same(BindingSource.Custom, bindingInfo.BindingSource); + Assert.Equal("foo", bindingInfo.BinderModelName); + } + + [Fact] + public void PreservesBindingSourceInference_ForFromQueryParameter_WithDefaultName() + { + // Arrange + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var actionName = nameof(ParameterBindingController.FromQuery); + var context = GetContext(typeof(ParameterBindingController), modelMetadataProvider); + var provider = GetProvider(); + + // Act + provider.OnProvidersExecuting(context); + + // Assert + var controller = Assert.Single(context.Result.Controllers); + var action = Assert.Single(controller.Actions, a => a.ActionName == actionName); + var parameter = Assert.Single(action.Parameters); + + var bindingInfo = parameter.BindingInfo; + Assert.NotNull(bindingInfo); + Assert.Same(BindingSource.Query, bindingInfo.BindingSource); + Assert.Null(bindingInfo.BinderModelName); + } + + [Fact] + public void PreservesBindingSourceInference_ForFromQueryParameter_WithCustomName() + { + // Arrange + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var actionName = nameof(ParameterBindingController.FromQueryWithCustomName); + var context = GetContext(typeof(ParameterBindingController), modelMetadataProvider); + var provider = GetProvider(); + + // Act + provider.OnProvidersExecuting(context); + + // Assert + var controller = Assert.Single(context.Result.Controllers); + var action = Assert.Single(controller.Actions, a => a.ActionName == actionName); + var parameter = Assert.Single(action.Parameters); + + var bindingInfo = parameter.BindingInfo; + Assert.NotNull(bindingInfo); + Assert.Same(BindingSource.Query, bindingInfo.BindingSource); + Assert.Equal("top", bindingInfo.BinderModelName); + } + + [Fact] + public void PreservesBindingSourceInference_ForFromQueryParameterOnComplexType_WithDefaultName() + { + // Arrange + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var actionName = nameof(ParameterBindingController.FromQueryOnComplexType); + var context = GetContext(typeof(ParameterBindingController), modelMetadataProvider); + var provider = GetProvider(); + + // Act + provider.OnProvidersExecuting(context); + + // Assert + var controller = Assert.Single(context.Result.Controllers); + var action = Assert.Single(controller.Actions, a => a.ActionName == actionName); + var parameter = Assert.Single(action.Parameters); + + var bindingInfo = parameter.BindingInfo; + Assert.NotNull(bindingInfo); + Assert.Same(BindingSource.Query, bindingInfo.BindingSource); + Assert.Equal(string.Empty, bindingInfo.BinderModelName); + } + + [Fact] + public void PreservesBindingSourceInference_ForFromQueryParameterOnComplexType_WithCustomName() + { + // Arrange + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var actionName = nameof(ParameterBindingController.FromQueryOnComplexTypeWithCustomName); + var context = GetContext(typeof(ParameterBindingController), modelMetadataProvider); + var provider = GetProvider(); + + // Act + provider.OnProvidersExecuting(context); + + // Assert + var controller = Assert.Single(context.Result.Controllers); + var action = Assert.Single(controller.Actions, a => a.ActionName == actionName); + var parameter = Assert.Single(action.Parameters); + + var bindingInfo = parameter.BindingInfo; + Assert.NotNull(bindingInfo); + Assert.Same(BindingSource.Query, bindingInfo.BindingSource); + Assert.Equal("gps", bindingInfo.BinderModelName); + } + + [Fact] + public void PreservesBindingSourceInference_ForFromRouteParameter_WithDefaultName() + { + // Arrange + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var actionName = nameof(ParameterBindingController.FromRoute); + var context = GetContext(typeof(ParameterBindingController), modelMetadataProvider); + var provider = GetProvider(); + + // Act + provider.OnProvidersExecuting(context); + + // Assert + var controller = Assert.Single(context.Result.Controllers); + var action = Assert.Single(controller.Actions, a => a.ActionName == actionName); + var parameter = Assert.Single(action.Parameters); + + var bindingInfo = parameter.BindingInfo; + Assert.NotNull(bindingInfo); + Assert.Same(BindingSource.Path, bindingInfo.BindingSource); + Assert.Null(bindingInfo.BinderModelName); + } + + [Fact] + public void PreservesBindingSourceInference_ForFromRouteParameter_WithCustomName() + { + // Arrange + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var actionName = nameof(ParameterBindingController.FromRouteWithCustomName); + var context = GetContext(typeof(ParameterBindingController), modelMetadataProvider); + var provider = GetProvider(); + + // Act + provider.OnProvidersExecuting(context); + + // Assert + var controller = Assert.Single(context.Result.Controllers); + var action = Assert.Single(controller.Actions, a => a.ActionName == actionName); + var parameter = Assert.Single(action.Parameters); + + var bindingInfo = parameter.BindingInfo; + Assert.NotNull(bindingInfo); + Assert.Same(BindingSource.Path, bindingInfo.BindingSource); + Assert.Equal("top", bindingInfo.BinderModelName); + } + + [Fact] + public void PreservesBindingSourceInference_ForFromRouteParameterOnComplexType_WithDefaultName() + { + // Arrange + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var actionName = nameof(ParameterBindingController.FromRouteOnComplexType); + var context = GetContext(typeof(ParameterBindingController), modelMetadataProvider); + var provider = GetProvider(); + + // Act + provider.OnProvidersExecuting(context); + + // Assert + var controller = Assert.Single(context.Result.Controllers); + var action = Assert.Single(controller.Actions, a => a.ActionName == actionName); + var parameter = Assert.Single(action.Parameters); + + var bindingInfo = parameter.BindingInfo; + Assert.NotNull(bindingInfo); + Assert.Same(BindingSource.Path, bindingInfo.BindingSource); + Assert.Equal(string.Empty, bindingInfo.BinderModelName); + } + + [Fact] + public void PreservesBindingSourceInference_ForFromRouteParameterOnComplexType_WithCustomName() + { + // Arrange + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var actionName = nameof(ParameterBindingController.FromRouteOnComplexTypeWithCustomName); + var context = GetContext(typeof(ParameterBindingController), modelMetadataProvider); + var provider = GetProvider(); + + // Act + provider.OnProvidersExecuting(context); + + // Assert + var controller = Assert.Single(context.Result.Controllers); + var action = Assert.Single(controller.Actions, a => a.ActionName == actionName); + var parameter = Assert.Single(action.Parameters); + + var bindingInfo = parameter.BindingInfo; + Assert.NotNull(bindingInfo); + Assert.Same(BindingSource.Path, bindingInfo.BindingSource); + Assert.Equal("gps", bindingInfo.BinderModelName); + } + + [Fact] + public void PreservesBindingSourceInference_ForParameterWithRequestPredicateAndPropertyFilterProvider() + { + // Arrange + var expectedPredicate = CustomRequestPredicateAndPropertyFilterProviderAttribute.RequestPredicateStatic; + var expectedPropertyFilter = CustomRequestPredicateAndPropertyFilterProviderAttribute.PropertyFilterStatic; + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var actionName = nameof(ParameterBindingController.ParameterWithRequestPredicateProvider); + var context = GetContext(typeof(ParameterBindingController), modelMetadataProvider); + var provider = GetProvider(); + + // Act + provider.OnProvidersExecuting(context); + + // Assert + var controller = Assert.Single(context.Result.Controllers); + var action = Assert.Single(controller.Actions, a => a.ActionName == actionName); + var parameter = Assert.Single(action.Parameters); + + var bindingInfo = parameter.BindingInfo; + Assert.NotNull(bindingInfo); + Assert.Same(BindingSource.Query, bindingInfo.BindingSource); + Assert.Same(expectedPredicate, bindingInfo.RequestPredicate); + Assert.Same(expectedPropertyFilter, bindingInfo.PropertyFilterProvider.PropertyFilter); + Assert.Null(bindingInfo.BinderModelName); + } + [Fact] public void InferParameterBindingSources_SetsCorrectBindingSourceForComplexTypesWithCancellationToken() { @@ -749,6 +1008,57 @@ Environment.NewLine + "int b"; [HttpGet("parameter-with-model-binder-attribute")] public IActionResult ModelBinderAttribute([ModelBinder(Name = "top")] int value) => null; + + [HttpGet("parameter-with-fromquery")] + public IActionResult FromQuery([FromQuery] int value) => null; + + [HttpGet("parameter-with-fromquery-and-customname")] + public IActionResult FromQueryWithCustomName([FromQuery(Name = "top")] int value) => null; + + [HttpGet("parameter-with-fromquery-on-complextype")] + public IActionResult FromQueryOnComplexType([FromQuery] GpsCoordinates gpsCoordinates) => null; + + [HttpGet("parameter-with-fromquery-on-complextype-and-customname")] + public IActionResult FromQueryOnComplexTypeWithCustomName([FromQuery(Name = "gps")] GpsCoordinates gpsCoordinates) => null; + + [HttpGet("parameter-with-fromroute")] + public IActionResult FromRoute([FromRoute] int value) => null; + + [HttpGet("parameter-with-fromroute-and-customname")] + public IActionResult FromRouteWithCustomName([FromRoute(Name = "top")] int value) => null; + + [HttpGet("parameter-with-fromroute-on-complextype")] + public IActionResult FromRouteOnComplexType([FromRoute] GpsCoordinates gpsCoordinates) => null; + + [HttpGet("parameter-with-fromroute-on-complextype-and-customname")] + public IActionResult FromRouteOnComplexTypeWithCustomName([FromRoute(Name = "gps")] GpsCoordinates gpsCoordinates) => null; + + [HttpGet] + public IActionResult ParameterWithRequestPredicateProvider([CustomRequestPredicateAndPropertyFilterProvider] int value) => null; + } + + private class CustomRequestPredicateAndPropertyFilterProviderAttribute : Attribute, IRequestPredicateProvider, IPropertyFilterProvider + { + public static Func RequestPredicateStatic => (c) => true; + public static Func PropertyFilterStatic => (c) => true; + + public Func RequestPredicate => RequestPredicateStatic; + + public Func PropertyFilter => PropertyFilterStatic; + } + + [ApiController] + [Route("[controller]/[action]")] + private class ModelBinderOnParameterController + { + [HttpGet] + public IActionResult ModelBinderAttributeWithExplicitModelName([ModelBinder(Name = "top")] int value) => null; + + [HttpGet] + public IActionResult ModelBinderType([ModelBinder(typeof(TestModelBinder))] string name) => null; + + [HttpGet] + public IActionResult ModelBinderTypeWithExplicitModelName([ModelBinder(typeof(TestModelBinder), Name = "foo")] string name) => null; } [ApiController] @@ -837,5 +1147,19 @@ Environment.NewLine + "int b"; [HttpGet("test")] public IActionResult Action([ModelBinder(typeof(object))] Car car) => null; } + + private class GpsCoordinates + { + public long Latitude { get; set; } + public long Longitude { get; set; } + } + + private class TestModelBinder : IModelBinder + { + public Task BindModelAsync(ModelBindingContext bindingContext) + { + throw new NotImplementedException(); + } + } } } diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiBehaviorTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiBehaviorTest.cs index 3b5d399c2b..b27a549321 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiBehaviorTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiBehaviorTest.cs @@ -173,5 +173,35 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests Assert.Null(result.Name); Assert.Null(result.Email); } + + [Fact] + public async Task ActionsWithApiBehavior_InferModelBinderType() + { + // Arrange + var expected = "From TestModelBinder: Hello!"; + + // Act + var response = await Client.GetAsync("/contact/ActionWithInferredModelBinderType?foo=Hello!"); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var result = await response.Content.ReadAsStringAsync(); + Assert.Equal(expected, result); + } + + [Fact] + public async Task ActionsWithApiBehavior_InferModelBinderTypeWithExplicitModelName() + { + // Arrange + var expected = "From TestModelBinder: Hello!"; + + // Act + var response = await Client.GetAsync("/contact/ActionWithInferredModelBinderTypeWithExplicitModelName?bar=Hello!"); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var result = await response.Content.ReadAsStringAsync(); + Assert.Equal(expected, result); + } } } diff --git a/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs b/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs index e4c443954c..77a757f1ff 100644 --- a/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs +++ b/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs @@ -8,6 +8,7 @@ using System.Threading; using System.Threading.Tasks; using BasicWebSite.Models; using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.ModelBinding; namespace BasicWebSite { @@ -72,5 +73,34 @@ namespace BasicWebSite { return contact; } + + [HttpGet("[action]")] + public ActionResult ActionWithInferredModelBinderType( + [ModelBinder(typeof(TestModelBinder))] string foo) + { + return foo; + } + + [HttpGet("[action]")] + public ActionResult ActionWithInferredModelBinderTypeWithExplicitModelName( + [ModelBinder(typeof(TestModelBinder), Name = "bar")] string foo) + { + return foo; + } + + private class TestModelBinder : IModelBinder + { + public Task BindModelAsync(ModelBindingContext bindingContext) + { + var val = bindingContext.ValueProvider.GetValue(bindingContext.ModelName); + if (val == null) + { + return Task.CompletedTask; + } + + bindingContext.Result = ModelBindingResult.Success("From TestModelBinder: " + val); + return Task.CompletedTask; + } + } } }