Removing IInputFormatterSelector

Follow up work item to #2269

src/Microsoft.AspNet.Mvc.Core/Formatters/DefaultInputFormatterSelector.cs

@@ -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<IInputFormatter> inputFormatters, 
-            InputFormatterContext context)
-        {
-            foreach (var formatter in inputFormatters)
-            {
-                if (formatter.CanRead(context))
-                {
-                    return formatter;
-                }
-            }
-
-            return null;
-        }
-    }
-}

src/Microsoft.AspNet.Mvc.Core/Formatters/IInputFormatterSelector.cs

@@ -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<IInputFormatter> inputFormatters,
-            [NotNull] InputFormatterContext context);
-    }
-}

src/Microsoft.AspNet.Mvc.Core/ModelBinders/BodyModelBinder.cs

@@ -30,12 +30,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
         {
             var requestServices = bindingContext.OperationBindingContext.HttpContext.RequestServices;
 
-            var formatterSelector = requestServices.GetRequiredService<IInputFormatterSelector>();
             var actionContext = requestServices.GetRequiredService<IScopedInstance<ActionContext>>().Value;
             var formatters = requestServices.GetRequiredService<IScopedInstance<ActionBindingContext>>().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 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                 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;
         }
     }
 }

src/Microsoft.AspNet.Mvc/MvcServices.cs

@@ -92,7 +92,6 @@ namespace Microsoft.AspNet.Mvc
                 return new DefaultCompositeMetadataDetailsProvider(options.ModelMetadataDetailsProviders);
             });
 
-            services.AddTransient<IInputFormatterSelector, DefaultInputFormatterSelector>();
             services.AddInstance(new JsonOutputFormatter());
 
             // Razor, Views and runtime compilation

test/Microsoft.AspNet.Mvc.Core.Test/DefaultInputFormatterSelectorTests.cs

@@ -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<IInputFormatter>()
-            {
-                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<TestInputFormatter>(selectedFormatter);
-            Assert.Equal(2, testFormatter.Index);
-        }
-
-        private static ActionContext GetActionContext()
-        {
-            return new ActionContext(Mock.Of<HttpContext>(), 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<object> ReadAsync(InputFormatterContext context)
-            {
-                return Task.FromResult<object>(Index);
-            }
-        }
-    }
-}

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 @@ namespace Microsoft.AspNet.Mvc
         {
             // Arrange
             var mockInputFormatter = new Mock<IInputFormatter>();
+            mockInputFormatter.Setup(f => f.CanRead(It.IsAny<InputFormatterContext>()))
+                .Returns(true)
+                .Verifiable();
             mockInputFormatter.Setup(o => o.ReadAsync(It.IsAny<InputFormatterContext>()))
                               .Returns(Task.FromResult<object>(new Person()))
                               .Verifiable();
@@ -32,7 +35,10 @@ namespace Microsoft.AspNet.Mvc
             var provider = new TestModelMetadataProvider();
             provider.ForType<Person>().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 @@ namespace Microsoft.AspNet.Mvc
             var binderResult = await binder.BindModelAsync(bindingContext);
 
             // Assert
+            mockInputFormatter.Verify(v => v.CanRead(It.IsAny<InputFormatterContext>()), Times.Once);
             mockInputFormatter.Verify(v => v.ReadAsync(It.IsAny<InputFormatterContext>()), Times.Once);
             Assert.NotNull(binderResult);
             Assert.True(binderResult.IsModelSet);
@@ -138,7 +145,7 @@ namespace Microsoft.AspNet.Mvc
 
             var bindingContext = GetBindingContext(
                 typeof(Person),
-                inputFormatter: new XyzFormatter(),
+                inputFormatters: new[] { new XyzFormatter() },
                 httpContext: httpContext,
                 metadataProvider: provider);
 
@@ -170,7 +177,7 @@ namespace Microsoft.AspNet.Mvc
 
             var bindingContext = GetBindingContext(
                 typeof(Person),
-                inputFormatter: null,
+                inputFormatters: null,
                 httpContext: httpContext,
                 metadataProvider: provider);
 
@@ -190,9 +197,35 @@ namespace Microsoft.AspNet.Mvc
             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<IInputFormatter>()
+            {
+                new TestInputFormatter(canRead: false),
+                new TestInputFormatter(canRead: false),
+                canReadFormatter1,
+                canReadFormatter2
+            };
+            var provider = new TestModelMetadataProvider();
+            provider.ForType<Person>().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<IInputFormatter> inputFormatters = null,
             HttpContext httpContext = null,
             IModelMetadataProvider metadataProvider = null)
         {
@@ -200,7 +233,8 @@ namespace Microsoft.AspNet.Mvc
             {
                 httpContext = new DefaultHttpContext();
             }
-            UpdateServiceProvider(httpContext, inputFormatter);
+
+            UpdateServiceProvider(httpContext, inputFormatters ?? Enumerable.Empty<IInputFormatter>());
 
             if (metadataProvider == null)
             {
@@ -227,21 +261,14 @@ namespace Microsoft.AspNet.Mvc
             return bindingContext;
         }
 
-        private static void UpdateServiceProvider(HttpContext httpContext, IInputFormatter inputFormatter)
+        private static void UpdateServiceProvider(
+            HttpContext httpContext,
+            IEnumerable<IInputFormatter> inputFormatters)
         {
             var serviceProvider = new ServiceCollection();
-            var inputFormatterSelector = new Mock<IInputFormatterSelector>();
-            inputFormatterSelector
-                .Setup(o => o.SelectFormatter(
-                    It.IsAny<IReadOnlyList<IInputFormatter>>(),
-                    It.IsAny<InputFormatterContext>()))
-                .Returns(inputFormatter);
-
-            serviceProvider.AddInstance(inputFormatterSelector.Object);
-
             var bindingContext = new ActionBindingContext()
             {
-                InputFormatters = new List<IInputFormatter>(),
+                InputFormatters = inputFormatters.ToArray(),
             };
 
             var bindingContextAccessor = new MockScopedInstance<ActionBindingContext>()
@@ -296,5 +323,25 @@ namespace Microsoft.AspNet.Mvc
                 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<object> ReadAsync(InputFormatterContext context)
+            {
+                return Task.FromResult<object>(this);
+            }
+        }
     }
 }

test/Microsoft.AspNet.Mvc.FunctionalTests/InputFormatterTests.cs

@@ -126,6 +126,44 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
             Assert.Equal(expectedSampleIntValue.ToString(), responseBody);
         }
 
+        [Theory]
+        [InlineData("\"I'm a JSON string!\"")]
+        [InlineData("true")]
+        [InlineData("\"\"")] // Empty string
+        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("{}")]

test/WebSites/FormatterWebSite/Controllers/JsonFormatterController.cs

@@ -38,5 +38,16 @@ namespace FormatterWebSite.Controllers
             }
             return Content(dummyObject.SampleInt.ToString());
         }
+
+        [HttpPost]
+        public IActionResult ValueTypeAsBody([FromBody] int value)
+        {
+            if (!ModelState.IsValid)
+            {
+                Response.StatusCode = StatusCodes.Status400BadRequest;
+            }
+
+            return Content(value.ToString());
+        }
     }
 }