diff --git a/src/Microsoft.AspNet.Mvc.Core/Formatters/DefaultInputFormatterSelector.cs b/src/Microsoft.AspNet.Mvc.Core/Formatters/DefaultInputFormatterSelector.cs deleted file mode 100644 index 800a35203e..0000000000 --- a/src/Microsoft.AspNet.Mvc.Core/Formatters/DefaultInputFormatterSelector.cs +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System.Collections.Generic; - -namespace Microsoft.AspNet.Mvc -{ - public class DefaultInputFormatterSelector : IInputFormatterSelector - { - - public IInputFormatter SelectFormatter( - IReadOnlyList inputFormatters, - InputFormatterContext context) - { - foreach (var formatter in inputFormatters) - { - if (formatter.CanRead(context)) - { - return formatter; - } - } - - return null; - } - } -} diff --git a/src/Microsoft.AspNet.Mvc.Core/Formatters/IInputFormatterSelector.cs b/src/Microsoft.AspNet.Mvc.Core/Formatters/IInputFormatterSelector.cs deleted file mode 100644 index f7149ef48a..0000000000 --- a/src/Microsoft.AspNet.Mvc.Core/Formatters/IInputFormatterSelector.cs +++ /dev/null @@ -1,15 +0,0 @@ -// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System.Collections.Generic; -using Microsoft.Framework.Internal; - -namespace Microsoft.AspNet.Mvc -{ - public interface IInputFormatterSelector - { - IInputFormatter SelectFormatter( - [NotNull] IReadOnlyList inputFormatters, - [NotNull] InputFormatterContext context); - } -} diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinders/BodyModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinders/BodyModelBinder.cs index fdb1cf6c78..b233f20aee 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinders/BodyModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinders/BodyModelBinder.cs @@ -30,12 +30,11 @@ protected async override Task BindModelCoreAsync([NotNull] M { var requestServices = bindingContext.OperationBindingContext.HttpContext.RequestServices; - var formatterSelector = requestServices.GetRequiredService(); var actionContext = requestServices.GetRequiredService>().Value; var formatters = requestServices.GetRequiredService>().Value.InputFormatters; var formatterContext = new InputFormatterContext(actionContext, bindingContext.ModelType); - var formatter = formatterSelector.SelectFormatter(formatters.ToList(), formatterContext); + var formatter = formatters.FirstOrDefault(f => f.CanRead(formatterContext)); if (formatter == null) { @@ -48,34 +47,21 @@ protected async override Task BindModelCoreAsync([NotNull] M return new ModelBindingResult(model: null, key: bindingContext.ModelName, isModelSet: false); } - object model = null; try { - model = await formatter.ReadAsync(formatterContext); + var model = await formatter.ReadAsync(formatterContext); + + // key is empty to ensure that the model name is not used as a prefix for validation. + return new ModelBindingResult(model, key: string.Empty, isModelSet: true); } catch (Exception ex) { - model = GetDefaultValueForType(bindingContext.ModelType); bindingContext.ModelState.AddModelError(bindingContext.ModelName, ex); // This model binder is the only handler for the Body binding source. // Always tell the model binding system to skip other model binders i.e. return non-null. return new ModelBindingResult(model: null, key: bindingContext.ModelName, isModelSet: false); } - - // Success - // key is empty to ensure that the model name is not used as a prefix for validation. - return new ModelBindingResult(model, key: string.Empty, isModelSet: true); - } - - private object GetDefaultValueForType(Type modelType) - { - if (modelType.GetTypeInfo().IsValueType) - { - return Activator.CreateInstance(modelType); - } - - return null; } } } diff --git a/src/Microsoft.AspNet.Mvc/MvcServices.cs b/src/Microsoft.AspNet.Mvc/MvcServices.cs index 23002b7c7a..381f81bb97 100644 --- a/src/Microsoft.AspNet.Mvc/MvcServices.cs +++ b/src/Microsoft.AspNet.Mvc/MvcServices.cs @@ -92,7 +92,6 @@ public static IServiceCollection GetDefaultServices() return new DefaultCompositeMetadataDetailsProvider(options.ModelMetadataDetailsProviders); }); - services.AddTransient(); services.AddInstance(new JsonOutputFormatter()); // Razor, Views and runtime compilation diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/DefaultInputFormatterSelectorTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/DefaultInputFormatterSelectorTests.cs deleted file mode 100644 index 4bfd03e724..0000000000 --- a/test/Microsoft.AspNet.Mvc.Core.Test/DefaultInputFormatterSelectorTests.cs +++ /dev/null @@ -1,67 +0,0 @@ -// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System.Collections.Generic; -using System.Threading.Tasks; -using Microsoft.AspNet.Http; -using Microsoft.AspNet.Routing; -using Moq; -using Xunit; - -namespace Microsoft.AspNet.Mvc.Core.Test -{ - public class DefaultInputFormatterSelectorTests - { - [Fact] - public void DefaultInputFormatterSelectorTests_ReturnsFirstFormatterWhichReturnsTrue() - { - // Arrange - var actionContext = GetActionContext(); - var inputFormatters = new List() - { - new TestInputFormatter(false, 0), - new TestInputFormatter(false, 1), - new TestInputFormatter(true, 2), - new TestInputFormatter(true, 3) - }; - - var context = new InputFormatterContext(actionContext, typeof(int)); - var selector = new DefaultInputFormatterSelector(); - - // Act - var selectedFormatter = selector.SelectFormatter(inputFormatters, context); - - // Assert - var testFormatter = Assert.IsType(selectedFormatter); - Assert.Equal(2, testFormatter.Index); - } - - private static ActionContext GetActionContext() - { - return new ActionContext(Mock.Of(), new RouteData(), new ActionDescriptor()); - - } - - private class TestInputFormatter : IInputFormatter - { - private bool _canRead = false; - - public TestInputFormatter(bool canRead, int index) - { - _canRead = canRead; - Index = index; - } - public int Index { get; set; } - - public bool CanRead(InputFormatterContext context) - { - return _canRead; - } - - public Task ReadAsync(InputFormatterContext context) - { - return Task.FromResult(Index); - } - } - } -} diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/BodyModelBinderTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BodyModelBinderTests.cs similarity index 80% rename from test/Microsoft.AspNet.Mvc.Core.Test/BodyModelBinderTests.cs rename to test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BodyModelBinderTests.cs index e7d9ffb909..b49217db20 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/BodyModelBinderTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BodyModelBinderTests.cs @@ -4,18 +4,18 @@ using System; using System.Collections.Generic; using System.IO; +using System.Linq; using System.Text; using System.Threading.Tasks; using Microsoft.AspNet.Http; using Microsoft.AspNet.Http.Core; -using Microsoft.AspNet.Mvc.ModelBinding; using Microsoft.AspNet.Routing; using Microsoft.Framework.DependencyInjection; using Microsoft.Net.Http.Headers; using Moq; using Xunit; -namespace Microsoft.AspNet.Mvc +namespace Microsoft.AspNet.Mvc.ModelBinding { public class BodyModelBinderTests { @@ -24,6 +24,9 @@ public async Task BindModel_CallsSelectedInputFormatterOnce() { // Arrange var mockInputFormatter = new Mock(); + mockInputFormatter.Setup(f => f.CanRead(It.IsAny())) + .Returns(true) + .Verifiable(); mockInputFormatter.Setup(o => o.ReadAsync(It.IsAny())) .Returns(Task.FromResult(new Person())) .Verifiable(); @@ -32,7 +35,10 @@ public async Task BindModel_CallsSelectedInputFormatterOnce() var provider = new TestModelMetadataProvider(); provider.ForType().BindingDetails(d => d.BindingSource = BindingSource.Body); - var bindingContext = GetBindingContext(typeof(Person), inputFormatter, metadataProvider: provider); + var bindingContext = GetBindingContext( + typeof(Person), + new[] { inputFormatter }, + metadataProvider: provider); var binder = new BodyModelBinder(); @@ -40,6 +46,7 @@ public async Task BindModel_CallsSelectedInputFormatterOnce() var binderResult = await binder.BindModelAsync(bindingContext); // Assert + mockInputFormatter.Verify(v => v.CanRead(It.IsAny()), Times.Once); mockInputFormatter.Verify(v => v.ReadAsync(It.IsAny()), Times.Once); Assert.NotNull(binderResult); Assert.True(binderResult.IsModelSet); @@ -138,7 +145,7 @@ public async Task CustomFormatterDeserializationException_AddedToModelState() var bindingContext = GetBindingContext( typeof(Person), - inputFormatter: new XyzFormatter(), + inputFormatters: new[] { new XyzFormatter() }, httpContext: httpContext, metadataProvider: provider); @@ -170,7 +177,7 @@ public async Task NullFormatterError_AddedToModelState() var bindingContext = GetBindingContext( typeof(Person), - inputFormatter: null, + inputFormatters: null, httpContext: httpContext, metadataProvider: provider); @@ -190,9 +197,35 @@ public async Task NullFormatterError_AddedToModelState() Assert.Equal("Unsupported content type 'text/xyz'.", errorMessage); } + [Fact] + public async Task BindModelCoreAsync_UsesFirstFormatterWhichCanRead() + { + // Arrange + var canReadFormatter1 = new TestInputFormatter(canRead: true); + var canReadFormatter2 = new TestInputFormatter(canRead: true); + var inputFormatters = new List() + { + new TestInputFormatter(canRead: false), + new TestInputFormatter(canRead: false), + canReadFormatter1, + canReadFormatter2 + }; + var provider = new TestModelMetadataProvider(); + provider.ForType().BindingDetails(d => d.BindingSource = BindingSource.Body); + var bindingContext = GetBindingContext(typeof(Person), inputFormatters, metadataProvider: provider); + var binder = bindingContext.OperationBindingContext.ModelBinder; + + // Act + var binderResult = await binder.BindModelAsync(bindingContext); + + // Assert + Assert.True(binderResult.IsModelSet); + Assert.Same(canReadFormatter1, binderResult.Model); + } + private static ModelBindingContext GetBindingContext( Type modelType, - IInputFormatter inputFormatter = null, + IEnumerable inputFormatters = null, HttpContext httpContext = null, IModelMetadataProvider metadataProvider = null) { @@ -200,7 +233,8 @@ public async Task NullFormatterError_AddedToModelState() { httpContext = new DefaultHttpContext(); } - UpdateServiceProvider(httpContext, inputFormatter); + + UpdateServiceProvider(httpContext, inputFormatters ?? Enumerable.Empty()); if (metadataProvider == null) { @@ -227,21 +261,14 @@ public async Task NullFormatterError_AddedToModelState() return bindingContext; } - private static void UpdateServiceProvider(HttpContext httpContext, IInputFormatter inputFormatter) + private static void UpdateServiceProvider( + HttpContext httpContext, + IEnumerable inputFormatters) { var serviceProvider = new ServiceCollection(); - var inputFormatterSelector = new Mock(); - inputFormatterSelector - .Setup(o => o.SelectFormatter( - It.IsAny>(), - It.IsAny())) - .Returns(inputFormatter); - - serviceProvider.AddInstance(inputFormatterSelector.Object); - var bindingContext = new ActionBindingContext() { - InputFormatters = new List(), + InputFormatters = inputFormatters.ToArray(), }; var bindingContextAccessor = new MockScopedInstance() @@ -296,5 +323,25 @@ public override Task ReadRequestBodyAsync(InputFormatterContext context) throw new InvalidOperationException("Your input is bad!"); } } + + private class TestInputFormatter : IInputFormatter + { + private readonly bool _canRead; + + public TestInputFormatter(bool canRead) + { + _canRead = canRead; + } + + public bool CanRead(InputFormatterContext context) + { + return _canRead; + } + + public Task ReadAsync(InputFormatterContext context) + { + return Task.FromResult(this); + } + } } } \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/InputFormatterTests.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/InputFormatterTests.cs index 055c2962b9..f3c27fb8bb 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/InputFormatterTests.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/InputFormatterTests.cs @@ -126,6 +126,43 @@ public async Task JsonInputFormatter_IsModelStateValid_ForValidContentType(strin Assert.Equal(expectedSampleIntValue.ToString(), responseBody); } + [Theory] + [InlineData("\"I'm a JSON string!\"")] + [InlineData("true")] + public async Task JsonInputFormatter_ReturnsDefaultValue_ForValueTypes(string input) + { + // Arrange + var server = TestHelper.CreateServer(_app, SiteName, _configureServices); + var client = server.CreateClient(); + var content = new StringContent(input, Encoding.UTF8, "application/json"); + + // Act + var response = await client.PostAsync("http://localhost/JsonFormatter/ValueTypeAsBody/", content); + var responseBody = await response.Content.ReadAsStringAsync(); + + //Assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + Assert.Equal("0", responseBody); + } + + [Fact] + public async Task JsonInputFormatter_ReadsPrimitiveTypes() + { + // Arrange + var expected = "1773"; + var server = TestHelper.CreateServer(_app, SiteName, _configureServices); + var client = server.CreateClient(); + var content = new StringContent(expected, Encoding.UTF8, "application/json"); + + // Act + var response = await client.PostAsync("http://localhost/JsonFormatter/ValueTypeAsBody/", content); + var responseBody = await response.Content.ReadAsStringAsync(); + + //Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal(expected, responseBody); + } + [Theory] [InlineData("{\"SampleInt\":10}")] [InlineData("{}")] diff --git a/test/WebSites/FormatterWebSite/Controllers/JsonFormatterController.cs b/test/WebSites/FormatterWebSite/Controllers/JsonFormatterController.cs index 936c23b8bc..e4730ba58e 100644 --- a/test/WebSites/FormatterWebSite/Controllers/JsonFormatterController.cs +++ b/test/WebSites/FormatterWebSite/Controllers/JsonFormatterController.cs @@ -38,5 +38,16 @@ public IActionResult ReturnInput([FromBody]DummyClass dummyObject) } return Content(dummyObject.SampleInt.ToString()); } + + [HttpPost] + public IActionResult ValueTypeAsBody([FromBody] int value) + { + if (!ModelState.IsValid) + { + Response.StatusCode = StatusCodes.Status400BadRequest; + } + + return Content(value.ToString()); + } } } \ No newline at end of file