[Perf]:Cache TypeConverters in SimpleTypeModelBinder

Fixes #4361

src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/SimpleTypeModelBinder.cs

@@ -2,6 +2,8 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.ComponentModel;
+using System.Runtime.ExceptionServices;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Mvc.Internal;
 
@@ -12,6 +14,18 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
     /// </summary>
     public class SimpleTypeModelBinder : IModelBinder
     {
+        private readonly TypeConverter _typeConverter;
+
+        public SimpleTypeModelBinder(Type type)
+        {
+            if (type == null)
+            {
+                throw new ArgumentNullException(nameof(type));
+            }
+
+            _typeConverter = TypeDescriptor.GetConverter(type);
+        }
+
         /// <inheritdoc />
         public Task BindModelAsync(ModelBindingContext bindingContext)
         {
@@ -32,7 +46,16 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
 
             try
             {
-                var model = valueProviderResult.ConvertTo(bindingContext.ModelType);
+                var value = valueProviderResult.FirstValue;
+
+                object model = null;
+                if (!string.IsNullOrWhiteSpace(value))
+                {
+                    model = _typeConverter.ConvertFrom(
+                        context: null,
+                        culture: valueProviderResult.Culture,
+                        value: value);
+                }
 
                 if (bindingContext.ModelType == typeof(string))
                 {
@@ -65,6 +88,14 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
             }
             catch (Exception exception)
             {
+                var isFormatException = exception is FormatException;
+                if (!isFormatException && exception.InnerException != null)
+                {
+                    // TypeConverter throws System.Exception wrapping the FormatException,
+                    // so we capture the inner exception.
+                    exception = ExceptionDispatchInfo.Capture(exception.InnerException).SourceException;
+                }
+
                 bindingContext.ModelState.TryAddModelError(
                     bindingContext.ModelName,
                     exception,

src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/SimpleTypeModelBinderProvider.cs

@@ -20,7 +20,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
 
             if (!context.Metadata.IsComplexType)
             {
-                return new SimpleTypeModelBinder();
+                return new SimpleTypeModelBinder(context.Metadata.ModelType);
             }
 
             return null;

test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/ArrayModelBinderTest.cs

@@ -21,7 +21,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
             };
             var bindingContext = GetBindingContext(valueProvider);
             var modelState = bindingContext.ModelState;
-            var binder = new ArrayModelBinder<int>(new SimpleTypeModelBinder());
+            var binder = new ArrayModelBinder<int>(new SimpleTypeModelBinder(typeof(int)));
 
             // Act
             var result = await binder.BindModelResultAsync(bindingContext);
@@ -37,7 +37,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
         public async Task ArrayModelBinder_CreatesEmptyCollection_IfIsTopLevelObject()
         {
             // Arrange
-            var binder = new ArrayModelBinder<string>(new SimpleTypeModelBinder());
+            var binder = new ArrayModelBinder<string>(new SimpleTypeModelBinder(typeof(string)));
 
             var context = CreateContext();
             context.IsTopLevelObject = true;
@@ -67,7 +67,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
         public async Task ArrayModelBinder_DoesNotCreateCollection_IfNotIsTopLevelObject(string prefix)
         {
             // Arrange
-            var binder = new ArrayModelBinder<string>(new SimpleTypeModelBinder());
+            var binder = new ArrayModelBinder<string>(new SimpleTypeModelBinder(typeof(string)));
 
             var context = CreateContext();
             context.ModelName = ModelNames.CreatePropertyModelName(prefix, "ArrayProperty");
@@ -116,7 +116,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
             var modelState = bindingContext.ModelState;
             bindingContext.Model = model;
 
-            var binder = new ArrayModelBinder<int>(new SimpleTypeModelBinder());
+            var binder = new ArrayModelBinder<int>(new SimpleTypeModelBinder(typeof(int)));
 
             // Act
             var result = await binder.BindModelResultAsync(bindingContext);

test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/DictionaryModelBinderTest.cs

@@ -35,7 +35,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
             var bindingContext = GetModelBindingContext(isReadOnly, values);
             bindingContext.ValueProvider = CreateEnumerableValueProvider("{0}", values);
 
-            var binder = new DictionaryModelBinder<int, string>(new SimpleTypeModelBinder(), new SimpleTypeModelBinder());
+            var binder = new DictionaryModelBinder<int, string>(
+                new SimpleTypeModelBinder(typeof(int)), 
+                new SimpleTypeModelBinder(typeof(string)));
 
             // Act
             var result = await binder.BindModelResultAsync(bindingContext);
@@ -73,7 +75,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
             var dictionary = new Dictionary<int, string>();
             bindingContext.Model = dictionary;
 
-            var binder = new DictionaryModelBinder<int, string>(new SimpleTypeModelBinder(), new SimpleTypeModelBinder());
+            var binder = new DictionaryModelBinder<int, string>(
+                new SimpleTypeModelBinder(typeof(int)), 
+                new SimpleTypeModelBinder(typeof(string)));
 
             // Act
             var result = await binder.BindModelResultAsync(bindingContext);
@@ -127,7 +131,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
             IDictionary<string, string> dictionary)
         {
             // Arrange
-            var binder = new DictionaryModelBinder<string, string>(new SimpleTypeModelBinder(), new SimpleTypeModelBinder());
+            var binder = new DictionaryModelBinder<string, string>(
+                new SimpleTypeModelBinder(typeof(string)), 
+                new SimpleTypeModelBinder(typeof(string)));
 
             var context = CreateContext();
             context.ModelName = modelName;
@@ -163,7 +169,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
                 { "three", "three" },
             };
 
-            var binder = new DictionaryModelBinder<string, string>(new SimpleTypeModelBinder(), new SimpleTypeModelBinder());
+            var binder = new DictionaryModelBinder<string, string>(
+                new SimpleTypeModelBinder(typeof(string)), 
+                new SimpleTypeModelBinder(typeof(string)));
 
             var context = CreateContext();
             context.ModelName = "prefix";
@@ -214,7 +222,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
             // Arrange
             var stringDictionary = dictionary.ToDictionary(kvp => kvp.Key.ToString(), kvp => kvp.Value.ToString());
 
-            var binder = new DictionaryModelBinder<long, int>(new SimpleTypeModelBinder(), new SimpleTypeModelBinder());
+            var binder = new DictionaryModelBinder<long, int>(
+                new SimpleTypeModelBinder(typeof(long)), 
+                new SimpleTypeModelBinder(typeof(int)));
 
             var context = CreateContext();
             context.ModelName = "prefix";
@@ -271,11 +281,11 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
             var valueMetadata = metadataProvider.GetMetadataForType(typeof(ModelWithProperties));
 
             var binder = new DictionaryModelBinder<int, ModelWithProperties>(
-                new SimpleTypeModelBinder(),
+                new SimpleTypeModelBinder(typeof(int)),
                 new ComplexTypeModelBinder(new Dictionary<ModelMetadata, IModelBinder>()
                 {
-                    { valueMetadata.Properties["Id"], new SimpleTypeModelBinder() },
+                    { valueMetadata.Properties["Id"], new SimpleTypeModelBinder(typeof(int)) },
-                    { valueMetadata.Properties["Name"], new SimpleTypeModelBinder() },
+                    { valueMetadata.Properties["Name"], new SimpleTypeModelBinder(typeof(string)) },
                 }));
 
             // Act
@@ -311,7 +321,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
         {
             // Arrange
             var expectedDictionary = new SortedDictionary<string, string>(dictionary);
-            var binder = new DictionaryModelBinder<string, string>(new SimpleTypeModelBinder(), new SimpleTypeModelBinder());
+            var binder = new DictionaryModelBinder<string, string>(
+                new SimpleTypeModelBinder(typeof(string)), 
+                new SimpleTypeModelBinder(typeof(string)));
 
             var context = CreateContext();
             context.ModelName = modelName;
@@ -341,7 +353,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
         public async Task DictionaryModelBinder_CreatesEmptyCollection_IfIsTopLevelObject()
         {
             // Arrange
-            var binder = new DictionaryModelBinder<string, string>(new SimpleTypeModelBinder(), new SimpleTypeModelBinder());
+            var binder = new DictionaryModelBinder<string, string>(
+                new SimpleTypeModelBinder(typeof(string)), 
+                new SimpleTypeModelBinder(typeof(string)));
 
             var context = CreateContext();
             context.IsTopLevelObject = true;
@@ -371,7 +385,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
         public async Task DictionaryModelBinder_DoesNotCreateCollection_IfNotIsTopLevelObject(string prefix)
         {
             // Arrange
-            var binder = new DictionaryModelBinder<int, int>(new SimpleTypeModelBinder(), new SimpleTypeModelBinder());
+            var binder = new DictionaryModelBinder<int, int>(
+                new SimpleTypeModelBinder(typeof(int)), 
+                new SimpleTypeModelBinder(typeof(int)));
 
             var context = CreateContext();
             context.ModelName = ModelNames.CreatePropertyModelName(prefix, "ListProperty");
@@ -413,7 +429,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
         public void CanCreateInstance_ReturnsExpectedValue(Type modelType, bool expectedResult)
         {
             // Arrange
-            var binder = new DictionaryModelBinder<int, int>(new SimpleTypeModelBinder(), new SimpleTypeModelBinder());
+            var binder = new DictionaryModelBinder<int, int>(
+                new SimpleTypeModelBinder(typeof(int)), 
+                new SimpleTypeModelBinder(typeof(int)));
 
             // Act
             var result = binder.CanCreateInstance(modelType);

test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/KeyValuePairModelBinderTest.cs

@@ -136,7 +136,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
         public async Task KeyValuePairModelBinder_CreatesEmptyCollection_IfIsTopLevelObject()
         {
             // Arrange
-            var binder = new KeyValuePairModelBinder<string, string>(new SimpleTypeModelBinder(), new SimpleTypeModelBinder());
+            var binder = new KeyValuePairModelBinder<string, string>(
+                new SimpleTypeModelBinder(typeof(string)), 
+                new SimpleTypeModelBinder(typeof(string)));
 
             var context = CreateContext();
             context.IsTopLevelObject = true;
@@ -167,7 +169,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
         public async Task KeyValuePairModelBinder_DoesNotCreateCollection_IfNotIsTopLevelObject(string prefix)
         {
             // Arrange
-            var binder = new KeyValuePairModelBinder<string, string>(new SimpleTypeModelBinder(), new SimpleTypeModelBinder());
+            var binder = new KeyValuePairModelBinder<string, string>(
+                new SimpleTypeModelBinder(typeof(string)), 
+                new SimpleTypeModelBinder(typeof(string)));
 
             var context = CreateContext();
             context.ModelName = ModelNames.CreatePropertyModelName(prefix, "KeyValuePairProperty");

test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/SimpleTypeModelBinderTest.cs

@@ -47,7 +47,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
                 { "theModelName", "some-value" }
             };
 
-            var binder = new SimpleTypeModelBinder();
+            var binder = new SimpleTypeModelBinder(destinationType);
 
             // Act
             var result = await binder.BindModelResultAsync(bindingContext);
@@ -73,7 +73,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
             {
                 { "theModelName", string.Empty }
             };
-            var binder = new SimpleTypeModelBinder();
+            var binder = new SimpleTypeModelBinder(destinationType);
 
             // Act
             var result = await binder.BindModelResultAsync(bindingContext);
@@ -98,7 +98,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
                 { "theModelName", "not an integer" }
             };
 
-            var binder = new SimpleTypeModelBinder();
+            var binder = new SimpleTypeModelBinder(typeof(int));
 
             // Act
             var result = await binder.BindModelResultAsync(bindingContext);
@@ -116,7 +116,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
         {
             // Arrange
             var bindingContext = GetBindingContext(typeof(int));
-            var binder = new SimpleTypeModelBinder();
+            var binder = new SimpleTypeModelBinder(typeof(int));
 
             // Act
             var result = await binder.BindModelResultAsync(bindingContext);
@@ -136,7 +136,29 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
                 { "theModelName", string.Empty }
             };
 
-            var binder = new SimpleTypeModelBinder();
+            var binder = new SimpleTypeModelBinder(typeof(string));
+
+            // Act
+            var result = await binder.BindModelResultAsync(bindingContext);
+
+            // Assert
+            Assert.Null(result.Model);
+            Assert.True(bindingContext.ModelState.ContainsKey("theModelName"));
+        }
+
+        [Theory]
+        [InlineData("")]
+        [InlineData(" \t \r\n ")]
+        public async Task BindModel_ReturnsNull_IfTrimmedValueIsEmptyString(object value)
+        {
+            // Arrange
+            var bindingContext = GetBindingContext(typeof(string));
+            bindingContext.ValueProvider = new SimpleValueProvider
+            {
+                { "theModelName", value }
+            };
+
+            var binder = new SimpleTypeModelBinder(typeof(string));
 
             // Act
             var result = await binder.BindModelResultAsync(bindingContext);
@@ -146,17 +168,59 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
             Assert.True(bindingContext.ModelState.ContainsKey("theModelName"));
         }
 
+        [Fact]
+        public async Task BindModel_NullableIntegerValueProviderResult_ReturnsModel()
+        {
+            // Arrange
+            var bindingContext = GetBindingContext(typeof(int?));
+            bindingContext.ValueProvider = new SimpleValueProvider
+            {
+                { "theModelName", "12" }
+            };
+
+            var binder = new SimpleTypeModelBinder(typeof(int?));
+
+            // Act
+            var result = await binder.BindModelResultAsync(bindingContext);
+
+            // Assert
+            Assert.True(result.IsModelSet);
+            Assert.Equal(12, result.Model);
+            Assert.True(bindingContext.ModelState.ContainsKey("theModelName"));
+        }
+
+        [Fact]
+        public async Task BindModel_NullableDoubleValueProviderResult_ReturnsModel()
+        {
+            // Arrange
+            var bindingContext = GetBindingContext(typeof(double?));
+            bindingContext.ValueProvider = new SimpleValueProvider
+            {
+                { "theModelName", "12.5" }
+            };
+
+            var binder = new SimpleTypeModelBinder(typeof(double?));
+
+            // Act
+            var result = await binder.BindModelResultAsync(bindingContext);
+
+            // Assert
+            Assert.True(result.IsModelSet);
+            Assert.Equal(12.5, result.Model);
+            Assert.True(bindingContext.ModelState.ContainsKey("theModelName"));
+        }
+
         [Fact]
         public async Task BindModel_ValidValueProviderResult_ReturnsModel()
         {
             // Arrange
             var bindingContext = GetBindingContext(typeof(int));
             bindingContext.ValueProvider = new SimpleValueProvider
-            {
+            { 
                 { "theModelName", "42" }
             };
 
-            var binder = new SimpleTypeModelBinder();
+            var binder = new SimpleTypeModelBinder(typeof(int));
 
             // Act
             var result = await binder.BindModelResultAsync(bindingContext);
@@ -167,6 +231,93 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
             Assert.True(bindingContext.ModelState.ContainsKey("theModelName"));
         }
 
+        [Fact]
+        public async Task BindModel_ValidValueProviderResultWithProvidedCulture_ReturnsModel()
+        {
+            // Arrange
+            var bindingContext = GetBindingContext(typeof(decimal));
+            bindingContext.ValueProvider = new SimpleValueProvider(new CultureInfo("fr-FR"))
+            {
+                { "theModelName", "12,5" }
+            };
+
+            var binder = new SimpleTypeModelBinder(typeof(decimal));
+
+            // Act
+            var result = await binder.BindModelResultAsync(bindingContext);
+
+            // Assert
+            Assert.True(result.IsModelSet);
+            Assert.Equal(12.5M, result.Model);
+            Assert.True(bindingContext.ModelState.ContainsKey("theModelName"));
+        }
+
+        [Fact]
+        public async Task BindModel_CreatesErrorForFormatException_ValueProviderResultWithInvalidCulture()
+        {
+            // Arrange
+            var bindingContext = GetBindingContext(typeof(decimal));
+            bindingContext.ValueProvider = new SimpleValueProvider(new CultureInfo("en-GB"))
+            {
+                { "theModelName", "12,5" }
+            };
+
+            var binder = new SimpleTypeModelBinder(typeof(decimal));
+
+            // Act
+            var result = await binder.BindModelResultAsync(bindingContext);
+
+            // Assert
+            Assert.False(result.IsModelSet);
+            Assert.Null(result.Model);
+
+            var error = Assert.Single(bindingContext.ModelState["theModelName"].Errors);
+            Assert.Equal("The value '12,5' is not valid for Decimal.", error.ErrorMessage, StringComparer.Ordinal);
+            Assert.Null(error.Exception);
+        }
+
+        [Fact]
+        public async Task BindModel_BindsEnumModels_IfArrayElementIsStringKey()
+        {
+            // Arrange
+            var bindingContext = GetBindingContext(typeof(IntEnum));
+            bindingContext.ValueProvider = new SimpleValueProvider
+            {
+                { "theModelName", new object[] { "Value1" } }
+            };
+
+            var binder = new SimpleTypeModelBinder(typeof(IntEnum));
+
+            // Act
+            var result = await binder.BindModelResultAsync(bindingContext);
+
+            // Assert
+            Assert.True(result.IsModelSet);
+            var boundModel = Assert.IsType<IntEnum>(result.Model);
+            Assert.Equal(IntEnum.Value1, boundModel);
+        }
+
+        [Fact]
+        public async Task BindModel_BindsEnumModels_IfArrayElementIsStringValue()
+        {
+            // Arrange
+            var bindingContext = GetBindingContext(typeof(IntEnum));
+            bindingContext.ValueProvider = new SimpleValueProvider
+            {
+                { "theModelName", new object[] { "1" } }
+            };
+
+            var binder = new SimpleTypeModelBinder(typeof(IntEnum));
+
+            // Act
+            var result = await binder.BindModelResultAsync(bindingContext);
+
+            // Assert
+            Assert.True(result.IsModelSet);
+            var boundModel = Assert.IsType<IntEnum>(result.Model);
+            Assert.Equal(IntEnum.Value1, boundModel);
+        }
+
         [Theory]
         [InlineData("0", 0)]
         [InlineData("1", 1)]
@@ -182,7 +333,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
                 { "theModelName", flagsEnumValue }
             };
 
-            var binder = new SimpleTypeModelBinder();
+            var binder = new SimpleTypeModelBinder(typeof(FlagsEnum));
 
             // Act
             var result = await binder.BindModelResultAsync(bindingContext);
@@ -216,5 +367,12 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
             Value4 = 4,
             Value8 = 8
         }
+
+        private enum IntEnum
+        {
+            Value0 = 0,
+            Value1 = 1,
+            MaxValue = int.MaxValue
+        }
     }
 }