From e12d44b40adfbc063cdf92683388b7d3919743a9 Mon Sep 17 00:00:00 2001 From: Kiran Challa Date: Fri, 21 Aug 2015 13:56:18 -0700 Subject: [PATCH] [Fixes #2900] Get rid of manual body-read-state-tracking --- .../ModelBinding/BodyBindingState.cs | 29 ---- .../ModelBinding/OperationBindingContext.cs | 5 - .../ModelBinding/CompositeModelBinder.cs | 38 ------ .../ModelBindingTest.cs | 126 ------------------ .../ModelBindingTestHelper.cs | 1 - .../Controllers/FromAttributesController.cs | 17 --- .../FromBodyControllerPropertyController.cs | 5 - .../MultiplePropertiesFromBodyController.cs | 22 --- test/WebSites/ModelBindingWebSite/Startup.cs | 2 +- 9 files changed, 1 insertion(+), 244 deletions(-) delete mode 100644 src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/BodyBindingState.cs delete mode 100644 test/WebSites/ModelBindingWebSite/Controllers/MultiplePropertiesFromBodyController.cs diff --git a/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/BodyBindingState.cs b/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/BodyBindingState.cs deleted file mode 100644 index d3a19a700e..0000000000 --- a/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/BodyBindingState.cs +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -namespace Microsoft.AspNet.Mvc.ModelBinding -{ - /// - /// Represents state of models which are bound using body. - /// - public enum BodyBindingState - { - /// - /// Represents if there has been no metadata found which needs to read the body during the current - /// model binding process. - /// - NotBodyBased, - - /// - /// Represents if there is a that - /// has been found during the current model binding process. - /// - FormatterBased, - - /// - /// Represents if there is a that - /// has been found during the current model binding process. - /// - FormBased - } -} diff --git a/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/OperationBindingContext.cs b/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/OperationBindingContext.cs index 1aa2ff74ec..0928417905 100644 --- a/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/OperationBindingContext.cs +++ b/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/OperationBindingContext.cs @@ -13,11 +13,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding /// public class OperationBindingContext { - /// - /// Represents if there has been a body bound model found during the current model binding process. - /// - public BodyBindingState BodyBindingState { get; set; } = BodyBindingState.NotBodyBased; - /// /// Gets or sets the for the current request. /// diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CompositeModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CompositeModelBinder.cs index ee7f34f928..be58a547e9 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CompositeModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CompositeModelBinder.cs @@ -64,9 +64,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding return null; } - bindingContext.OperationBindingContext.BodyBindingState = - newBindingContext.OperationBindingContext.BodyBindingState; - var bindingKey = bindingContext.ModelName; if (modelBindingResult.IsModelSet) { @@ -159,8 +156,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding IsTopLevelObject = oldBindingContext.IsTopLevelObject, }; - newBindingContext.OperationBindingContext.BodyBindingState = GetBodyBindingState(oldBindingContext); - // If the property has a specified data binding sources, we need to filter the set of value providers // to just those that match. We can skip filtering when IsGreedy == true, because that can't use // value providers. @@ -197,38 +192,5 @@ namespace Microsoft.AspNet.Mvc.ModelBinding return newBindingContext; } - - private static BodyBindingState GetBodyBindingState(ModelBindingContext oldBindingContext) - { - var bindingSource = oldBindingContext.BindingSource; - - var willReadBodyWithFormatter = bindingSource == BindingSource.Body; - var willReadBodyAsFormData = bindingSource == BindingSource.Form; - - var currentModelNeedsToReadBody = willReadBodyWithFormatter || willReadBodyAsFormData; - var oldState = oldBindingContext.OperationBindingContext.BodyBindingState; - - // We need to throw if there are multiple models which can cause body to be read multiple times. - // Reading form data multiple times is ok since we cache form data. For the models marked to read using - // formatters, multiple reads are not allowed. - if (oldState == BodyBindingState.FormatterBased && currentModelNeedsToReadBody || - oldState == BodyBindingState.FormBased && willReadBodyWithFormatter) - { - throw new InvalidOperationException(Resources.MultipleBodyParametersOrPropertiesAreNotAllowed); - } - - var state = oldBindingContext.OperationBindingContext.BodyBindingState; - if (willReadBodyWithFormatter) - { - state = BodyBindingState.FormatterBased; - } - else if (willReadBodyAsFormData && oldState != BodyBindingState.FormatterBased) - { - // Only update the model binding state if we have not discovered formatter based state already. - state = BodyBindingState.FormBased; - } - - return state; - } } } diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs index 0fbe0999c9..eaa7ede274 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs @@ -157,24 +157,6 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests Assert.Equal("50", await response.Content.ReadAsStringAsync()); } - [Fact] - public async Task MultipleParametersMarkedWithFromBody_Throws() - { - // Arrange - var server = TestHelper.CreateServer(_app, SiteName, _configureServices); - var client = server.CreateClient(); - - // Act - var response = await client.GetAsync("http://localhost/FromAttributes/FromBodyParametersThrows"); - - // Assert - var exception = response.GetServerException(); - Assert.Equal(typeof(InvalidOperationException).FullName, exception.ExceptionType); - Assert.Equal( - "More than one parameter and/or property is bound to the HTTP request's content.", - exception.ExceptionMessage); - } - [Fact] public async Task ControllerPropertyAndAnActionWithoutFromBody_InvokesWithoutErrors() { @@ -189,114 +171,6 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests Assert.Equal(HttpStatusCode.NoContent, response.StatusCode); } - [Fact] - public async Task ControllerPropertyAndAnActionParameterWithFromBody_Throws() - { - // Arrange - var server = TestHelper.CreateServer(_app, SiteName, _configureServices); - var client = server.CreateClient(); - - // Act - var response = await client.GetAsync("http://localhost/FromBodyControllerProperty/AddUser"); - - // Assert - var exception = response.GetServerException(); - Assert.Equal(typeof(InvalidOperationException).FullName, exception.ExceptionType); - Assert.Equal( - "More than one parameter and/or property is bound to the HTTP request's content.", - exception.ExceptionMessage); - } - - [Fact] - public async Task ControllerPropertyAndAModelPropertyWithFromBody_Throws() - { - // Arrange - var server = TestHelper.CreateServer(_app, SiteName, _configureServices); - var client = server.CreateClient(); - - // Act - var response = await client.GetAsync("http://localhost/FromBodyControllerProperty/AddUser"); - - // Assert - var exception = response.GetServerException(); - Assert.Equal(typeof(InvalidOperationException).FullName, exception.ExceptionType); - Assert.Equal( - "More than one parameter and/or property is bound to the HTTP request's content.", - exception.ExceptionMessage); - } - - [Fact] - public async Task MultipleControllerPropertiesMarkedWithFromBody_Throws() - { - // Arrange - var server = TestHelper.CreateServer(_app, SiteName, _configureServices); - var client = server.CreateClient(); - - // Act - var response = await client.GetAsync("http://localhost/MultiplePropertiesFromBody/GetUser"); - - // Assert - var exception = response.GetServerException(); - Assert.Equal(typeof(InvalidOperationException).FullName, exception.ExceptionType); - Assert.Equal( - "More than one parameter and/or property is bound to the HTTP request's content.", - exception.ExceptionMessage); - } - - [Fact] - public async Task MultipleParameterAndPropertiesMarkedWithFromBody_Throws() - { - // Arrange - var server = TestHelper.CreateServer(_app, SiteName, _configureServices); - var client = server.CreateClient(); - - // Act - var response = await client.GetAsync("http://localhost/FromAttributes/FromBodyParameterAndPropertyThrows"); - - // Assert - var exception = response.GetServerException(); - Assert.Equal(typeof(InvalidOperationException).FullName, exception.ExceptionType); - Assert.Equal( - "More than one parameter and/or property is bound to the HTTP request's content.", - exception.ExceptionMessage); - } - - [Fact] - public async Task MultipleParametersMarkedWith_FromFormAndFromBody_Throws() - { - // Arrange - var server = TestHelper.CreateServer(_app, SiteName, _configureServices); - var client = server.CreateClient(); - - // Act - var response = await client.GetAsync("http://localhost/FromAttributes/FormAndBody_AsParameters_Throws"); - - // Assert - var exception = response.GetServerException(); - Assert.Equal(typeof(InvalidOperationException).FullName, exception.ExceptionType); - Assert.Equal( - "More than one parameter and/or property is bound to the HTTP request's content.", - exception.ExceptionMessage); - } - - [Fact] - public async Task MultipleParameterAndPropertiesMarkedWith_FromFormAndFromBody_Throws() - { - // Arrange - var server = TestHelper.CreateServer(_app, SiteName, _configureServices); - var client = server.CreateClient(); - - // Act - var response = await client.GetAsync("http://localhost/FromAttributes/FormAndBody_Throws"); - - // Assert - var exception = response.GetServerException(); - Assert.Equal(typeof(InvalidOperationException).FullName, exception.ExceptionType); - Assert.Equal( - "More than one parameter and/or property is bound to the HTTP request's content.", - exception.ExceptionMessage); - } - [Fact] public async Task CanBind_MultipleParameters_UsingFromForm() { diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/ModelBindingTestHelper.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/ModelBindingTestHelper.cs index be6c7432a7..0c55f372ae 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/ModelBindingTestHelper.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/ModelBindingTestHelper.cs @@ -39,7 +39,6 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests return new OperationBindingContext() { - BodyBindingState = BodyBindingState.NotBodyBased, HttpContext = httpContext, InputFormatters = actionBindingContext.InputFormatters, MetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(), diff --git a/test/WebSites/ModelBindingWebSite/Controllers/FromAttributesController.cs b/test/WebSites/ModelBindingWebSite/Controllers/FromAttributesController.cs index df068e5242..badf3b39b0 100644 --- a/test/WebSites/ModelBindingWebSite/Controllers/FromAttributesController.cs +++ b/test/WebSites/ModelBindingWebSite/Controllers/FromAttributesController.cs @@ -50,22 +50,5 @@ namespace ModelBindingWebSite.Controllers user.HomeAddress = defaultAddress; return user; } - - public void FromBodyParametersThrows([FromBody] int id, [FromBody] string emp) - { - } - - // Customer has a FromBody Property. - public void FromBodyParameterAndPropertyThrows([FromBody] Person p, Customer customer) - { - } - - public void FormAndBody_Throws([FromForm] Person p, Customer customer) - { - } - - public void FormAndBody_AsParameters_Throws([FromBody] int id, [FromForm] string emp) - { - } } } \ No newline at end of file diff --git a/test/WebSites/ModelBindingWebSite/Controllers/FromBodyControllerPropertyController.cs b/test/WebSites/ModelBindingWebSite/Controllers/FromBodyControllerPropertyController.cs index a215eb8532..19bafdddc8 100644 --- a/test/WebSites/ModelBindingWebSite/Controllers/FromBodyControllerPropertyController.cs +++ b/test/WebSites/ModelBindingWebSite/Controllers/FromBodyControllerPropertyController.cs @@ -21,10 +21,5 @@ namespace ModelBindingWebSite.Controllers { return customer; } - - // Will throw as a controller property and a parameter name are being read from body. - public void AddUser([FromBody] User user) - { - } } } \ No newline at end of file diff --git a/test/WebSites/ModelBindingWebSite/Controllers/MultiplePropertiesFromBodyController.cs b/test/WebSites/ModelBindingWebSite/Controllers/MultiplePropertiesFromBodyController.cs deleted file mode 100644 index bf334561f8..0000000000 --- a/test/WebSites/ModelBindingWebSite/Controllers/MultiplePropertiesFromBodyController.cs +++ /dev/null @@ -1,22 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using Microsoft.AspNet.Mvc; -using ModelBindingWebSite.Models; - -namespace ModelBindingWebSite.Controllers -{ - public class MultiplePropertiesFromBodyController : Controller - { - [FromBody] - public User SiteUser { get; set; } - - [FromBody] - public Country Country { get; set; } - - public User GetUser() - { - return SiteUser; - } - } -} \ No newline at end of file diff --git a/test/WebSites/ModelBindingWebSite/Startup.cs b/test/WebSites/ModelBindingWebSite/Startup.cs index 617ea60d42..84aaf5424c 100644 --- a/test/WebSites/ModelBindingWebSite/Startup.cs +++ b/test/WebSites/ModelBindingWebSite/Startup.cs @@ -20,7 +20,7 @@ namespace ModelBindingWebSite { m.MaxModelValidationErrors = 8; m.ModelBinders.Insert(0, new TestBindingSourceModelBinder()); - + m.ValidationExcludeFilters.Add(typeof(Address)); // ModelMetadataController relies on additional values AdditionalValuesMetadataProvider provides.