From 1ff5bdca796d48293a42015d77528851684db669 Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Mon, 19 Mar 2018 22:22:08 -0700 Subject: [PATCH] Set model prefix for [ApiController] Infers the 'empty' model prefix for complex types that are read from the value providers. This gives us better defaults when using the parameter object pattern with respect to swagger/API explorer. --- .../ApiBehaviorApplicationModelProvider.cs | 74 ++++++++++++++++--- ...ApiBehaviorApplicationModelProviderTest.cs | 47 ++++++++++++ .../ApiBehaviorTest.cs | 42 +++++++++++ .../Controllers/ContactApiController.cs | 6 ++ 4 files changed, 159 insertions(+), 10 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs index d645da1546..3e44c3b9a8 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs @@ -64,6 +64,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal controllerModel.ApiExplorer.IsVisible = true; } + if (isApiController) + { + InferBoundPropertyModelPrefixes(controllerModel); + } + var controllerHasSelectorModel = controllerModel.Selectors.Any(s => s.AttributeRouteModel != null); foreach (var actionModel in controllerModel.Actions) @@ -79,6 +84,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal InferParameterBindingSources(actionModel); + InferParameterModelPrefixes(actionModel); + AddMultipartFormDataConsumesAttribute(actionModel); } } @@ -179,6 +186,50 @@ namespace Microsoft.AspNetCore.Mvc.Internal } } + // For any complex types that are bound from value providers, set the prefix + // to the empty prefix by default. This makes binding much more predictable + // and describable via ApiExplorer + + // internal for testing + internal void InferBoundPropertyModelPrefixes(ControllerModel controllerModel) + { + foreach (var property in controllerModel.ControllerProperties) + { + if (property.BindingInfo != null && + property.BindingInfo.BinderModelName == null && + property.BindingInfo.BindingSource != null && + !property.BindingInfo.BindingSource.IsGreedy) + { + var metadata = _modelMetadataProvider.GetMetadataForProperty( + controllerModel.ControllerType, + property.PropertyInfo.Name); + if (metadata.IsComplexType) + { + property.BindingInfo.BinderModelName = string.Empty; + } + } + } + } + + // internal for testing + internal void InferParameterModelPrefixes(ActionModel actionModel) + { + foreach (var parameter in actionModel.Parameters) + { + if (parameter.BindingInfo != null && + parameter.BindingInfo.BinderModelName == null && + parameter.BindingInfo.BindingSource != null && + !parameter.BindingInfo.BindingSource.IsGreedy) + { + var metadata = GetParameterMetadata(parameter); + if (metadata.IsComplexType) + { + parameter.BindingInfo.BinderModelName = string.Empty; + } + } + } + } + // Internal for unit testing. internal BindingSource InferBindingSourceForParameter(ParameterModel parameter) { @@ -189,16 +240,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal } else { - ModelMetadata parameterMetadata; - if (_modelMetadataProvider is ModelMetadataProvider modelMetadataProvider) - { - parameterMetadata = modelMetadataProvider.GetMetadataForParameter(parameter.ParameterInfo); - } - else - { - parameterMetadata = _modelMetadataProvider.GetMetadataForType(parameter.ParameterInfo.ParameterType); - } - + var parameterMetadata = GetParameterMetadata(parameter); if (parameterMetadata != null) { var bindingSource = parameterMetadata.IsComplexType ? @@ -234,5 +276,17 @@ namespace Microsoft.AspNetCore.Mvc.Internal return parameterExistsInSomeRoute; } + + private ModelMetadata GetParameterMetadata(ParameterModel parameter) + { + if (_modelMetadataProvider is ModelMetadataProvider modelMetadataProvider) + { + return modelMetadataProvider.GetMetadataForParameter(parameter.ParameterInfo); + } + else + { + return _modelMetadataProvider.GetMetadataForType(parameter.ParameterInfo.ParameterType); + } + } } } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorApplicationModelProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorApplicationModelProviderTest.cs index 88664b689a..8bdea7d70c 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorApplicationModelProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorApplicationModelProviderTest.cs @@ -398,6 +398,38 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.Same(BindingSource.Query, result); } + [Fact] + public void InferBoundPropertyModelPrefixes_SetsModelPrefix_ForComplexTypeFromValueProvider() + { + // Arrange + var controller = GetControllerModel(typeof(ControllerWithBoundProperty)); + + var provider = GetProvider(); + + // Act + provider.InferBoundPropertyModelPrefixes(controller); + + // Assert + var property = Assert.Single(controller.ControllerProperties); + Assert.Equal(string.Empty, property.BindingInfo.BinderModelName); + } + + [Fact] + public void InferParameterModelPrefixes_SetsModelPrefix_ForComplexTypeFromValueProvider() + { + // Arrange + var action = GetActionModel(typeof(ControllerWithBoundProperty), nameof(ControllerWithBoundProperty.SomeAction)); + + var provider = GetProvider(); + + // Act + provider.InferParameterModelPrefixes(action); + + // Assert + var parameter = Assert.Single(action.Parameters); + Assert.Equal(string.Empty, parameter.BindingInfo.BinderModelName); + } + [Fact] public void AddMultipartFormDataConsumesAttribute_NoOpsIfBehaviorIsDisabled() { @@ -483,6 +515,12 @@ namespace Microsoft.AspNetCore.Mvc.Internal return context; } + private static ControllerModel GetControllerModel(Type controllerType) + { + var context = GetContext(controllerType); + return Assert.Single(context.Result.Controllers); + } + private static ActionModel GetActionModel(Type controllerType, string actionName) { var context = GetContext(controllerType); @@ -622,5 +660,14 @@ namespace Microsoft.AspNetCore.Mvc.Internal public override bool CanConvertFrom(ITypeDescriptorContext context, Type sourceType) => sourceType == typeof(string); } + + [ApiController] + private class ControllerWithBoundProperty + { + [FromQuery] + public TestModel TestProperty { get; set; } + + public IActionResult SomeAction([FromQuery] TestModel test) => null; + } } } diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiBehaviorTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiBehaviorTest.cs index 3f7ff1a9cb..78e1d637c6 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiBehaviorTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiBehaviorTest.cs @@ -124,5 +124,47 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests Assert.Equal(name, result.Name); Assert.Equal(email, result.Email); } + + [Fact] + public async Task ActionsWithApiBehavior_InferEmptyPrefixForComplexValueProviderModel_Success() + { + // Arrange + var id = 31; + var name = "test_user"; + var email = "email@test.com"; + var url = $"/contact/ActionWithInferredEmptyPrefix?name={name}&contactid={id}&email={email}"; + + // Act + var response = await Client.GetAsync(url); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + var result = await response.Content.ReadAsAsync(); + Assert.Equal(id, result.ContactId); + Assert.Equal(name, result.Name); + Assert.Equal(email, result.Email); + } + + [Fact] + public async Task ActionsWithApiBehavior_InferEmptyPrefixForComplexValueProviderModel_Ignored() + { + // Arrange + var id = 31; + var name = "test_user"; + var email = "email@test.com"; + var url = $"/contact/ActionWithInferredEmptyPrefix?contact.name={name}&contact.contactid={id}&contact.email={email}"; + + // Act + var response = await Client.GetAsync(url); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + var result = await response.Content.ReadAsAsync(); + Assert.Equal(0, result.ContactId); + Assert.Null(result.Name); + Assert.Null(result.Email); + } } } diff --git a/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs b/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs index 38195a455d..d2141e6dd5 100644 --- a/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs +++ b/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs @@ -61,5 +61,11 @@ namespace BasicWebSite Email = email, }; } + + [HttpGet("[action]")] + public ActionResult ActionWithInferredEmptyPrefix([FromQuery] Contact contact) + { + return contact; + } } } \ No newline at end of file