From cc00d8cff7ddf1d207e017174cafa4369a5ba411 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Mon, 21 Apr 2014 18:44:05 -0700 Subject: [PATCH] Modify TypeConverterModelBinder to use ValueProviderResult.CanConvertFromString to determine if it can convert a value * Adding support for extra type conversions --- src/Common/TypeExtensions.cs | 16 -- .../Binders/MutableObjectModelBinder.cs | 2 +- .../Binders/TypeConverterModelBinder.cs | 12 +- .../Metadata/ModelMetadata.cs | 2 +- .../ValueProviders/ValueProviderResult.cs | 193 ++++++++++++------ .../Binders/TypeConverterModelBinderTest.cs | 53 ++++- .../ValueProviders/ValueProviderResultTest.cs | 43 ++++ 7 files changed, 230 insertions(+), 91 deletions(-) diff --git a/src/Common/TypeExtensions.cs b/src/Common/TypeExtensions.cs index ad9c327374..deb6a34d6a 100644 --- a/src/Common/TypeExtensions.cs +++ b/src/Common/TypeExtensions.cs @@ -100,22 +100,6 @@ namespace Microsoft.AspNet.Mvc return (!type.GetTypeInfo().IsValueType || IsNullableValueType(type)); } - public static bool HasStringConverter([NotNull] this Type type) - { - var typeInfo = type.GetTypeInfo(); - if (typeInfo.IsPrimitive || type == typeof(string)) - { - return true; - } - if (IsNullableValueType(type) && HasStringConverter(type.GenericTypeArguments[0])) - { - // Nullable where T is a primitive type or has a type converter - return true; - } - - return false; - } - public static Type[] GetTypeArgumentsIfMatch([NotNull] Type closedType, Type matchingOpenType) { var closedTypeInfo = closedType.GetTypeInfo(); diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/MutableObjectModelBinder.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/MutableObjectModelBinder.cs index 4f28764854..452e897f14 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/MutableObjectModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/MutableObjectModelBinder.cs @@ -39,7 +39,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding private static bool CanBindType(Type modelType) { // Simple types cannot use this binder - var isComplexType = !modelType.HasStringConverter(); + var isComplexType = !ValueProviderResult.CanConvertFromString(modelType); if (!isComplexType) { return false; diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/TypeConverterModelBinder.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/TypeConverterModelBinder.cs index 46dd0c37b3..db7311338c 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/TypeConverterModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/TypeConverterModelBinder.cs @@ -13,26 +13,25 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { ModelBindingHelper.ValidateBindingContext(bindingContext); - if (!bindingContext.ModelType.HasStringConverter()) + if (!ValueProviderResult.CanConvertFromString(bindingContext.ModelType)) { // this type cannot be converted return false; } - ValueProviderResult valueProviderResult = bindingContext.ValueProvider.GetValue(bindingContext.ModelName); + var valueProviderResult = bindingContext.ValueProvider.GetValue(bindingContext.ModelName); if (valueProviderResult == null) { return false; // no entry } - // Provider should have verified this before creating - Contract.Assert(bindingContext.ModelType.HasStringConverter()); - object newModel; bindingContext.ModelState.SetModelValue(bindingContext.ModelName, valueProviderResult); try { newModel = valueProviderResult.ConvertTo(bindingContext.ModelType); + ModelBindingHelper.ReplaceEmptyStringWithNull(bindingContext.ModelMetadata, ref newModel); + bindingContext.Model = newModel; } catch (Exception ex) { @@ -45,11 +44,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { bindingContext.ModelState.AddModelError(bindingContext.ModelName, ex); } - return false; } - ModelBindingHelper.ReplaceEmptyStringWithNull(bindingContext.ModelMetadata, ref newModel); - bindingContext.Model = newModel; return true; } diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/ModelMetadata.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/ModelMetadata.cs index 3d1f853f79..87cad826d5 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/ModelMetadata.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/ModelMetadata.cs @@ -56,7 +56,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding public virtual bool IsComplexType { - get { return !ModelType.HasStringConverter(); } + get { return !ValueProviderResult.CanConvertFromString(ModelType); } } public bool IsNullableValueType diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/ValueProviders/ValueProviderResult.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/ValueProviders/ValueProviderResult.cs index e1e797d6ed..fe95031c54 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/ValueProviders/ValueProviderResult.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/ValueProviders/ValueProviderResult.cs @@ -2,7 +2,6 @@ using System.Collections; using System.Globalization; using System.Reflection; -using Microsoft.AspNet.Mvc.ModelBinding.Internal; namespace Microsoft.AspNet.Mvc.ModelBinding { @@ -64,7 +63,12 @@ namespace Microsoft.AspNet.Mvc.ModelBinding return UnwrapPossibleArrayType(cultureToUse, value, type); } - private static object ConvertSimpleType(CultureInfo culture, object value, Type destinationType) + public static bool CanConvertFromString(Type destinationType) + { + return GetConverterDelegate(destinationType) != null; + } + + private object ConvertSimpleType(CultureInfo culture, object value, Type destinationType) { if (value == null || value.GetType().IsAssignableFrom(destinationType)) { @@ -72,80 +76,26 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } // In case of a Nullable object, we try again with its underlying type. - var underlyingType = Nullable.GetUnderlyingType(destinationType); - if (underlyingType != null) - { - destinationType = underlyingType; - } + destinationType = UnwrapNullableType(destinationType); // if this is a user-input value but the user didn't type anything, return no value var valueAsString = value as string; - if (valueAsString != null && string.IsNullOrWhiteSpace(valueAsString)) { return null; } - if (destinationType == typeof(string)) + var converter = GetConverterDelegate(destinationType); + if (converter == null) { - return Convert.ToString(value, culture); + var message = Resources.FormatValueProviderResult_NoConverterExists(value.GetType(), destinationType); + throw new InvalidOperationException(message); } - if (destinationType == typeof(int)) - { - return Convert.ToInt32(value, culture); - } - - if (destinationType == typeof(long)) - { - return Convert.ToInt64(value, culture); - } - - if (destinationType == typeof(float)) - { - return Convert.ToSingle(value, culture); - } - - if (destinationType == typeof(double)) - { - return Convert.ToDouble(value, culture); - } - - if (destinationType == typeof(decimal)) - { - return Convert.ToDecimal(value, culture); - } - - if (destinationType == typeof(bool)) - { - return Convert.ToBoolean(value, culture); - } - - if (destinationType.GetTypeInfo().IsEnum) - { - // EnumConverter cannot convert integer, so we verify manually - if ((value is int)) - { - if (Enum.IsDefined(destinationType, value)) - { - return Enum.ToObject(destinationType, (int)value); - } - - throw new FormatException( - Resources.FormatValueProviderResult_CannotConvertEnum(value, - destinationType)); - } - else - { - return Enum.Parse(destinationType, valueAsString); - } - } - - var message = Resources.FormatValueProviderResult_NoConverterExists(value.GetType(), destinationType); - throw new InvalidOperationException(message); + return converter(value, culture); } - private static object UnwrapPossibleArrayType(CultureInfo culture, object value, Type destinationType) + private object UnwrapPossibleArrayType(CultureInfo culture, object value, Type destinationType) { // array conversion results in four cases, as below var valueAsArray = value as Array; @@ -189,5 +139,122 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // case 4: both destination + source type are single elements, so convert return ConvertSimpleType(culture, value, destinationType); } + + private static Func GetConverterDelegate(Type destinationType) + { + destinationType = UnwrapNullableType(destinationType); + + if (destinationType == typeof(string)) + { + return (value, culture) => Convert.ToString(value, culture); + } + + if (destinationType == typeof(int)) + { + return (value, culture) => Convert.ToInt32(value, culture); + } + + if (destinationType == typeof(long)) + { + return (value, culture) => Convert.ToInt64(value, culture); + } + + if (destinationType == typeof(float)) + { + return (value, culture) => Convert.ToSingle(value, culture); + } + + if (destinationType == typeof(double)) + { + return (value, culture) => Convert.ToDouble(value, culture); + } + + if (destinationType == typeof(decimal)) + { + return (value, culture) => Convert.ToDecimal(value, culture); + } + + if (destinationType == typeof(bool)) + { + return (value, culture) => Convert.ToBoolean(value, culture); + } + + if (destinationType == typeof(DateTime)) + { + return (value, culture) => + { + ThrowIfNotStringType(value, destinationType); + return DateTime.Parse((string)value, culture); + }; + } + + if (destinationType == typeof(DateTimeOffset)) + { + return (value, culture) => + { + ThrowIfNotStringType(value, destinationType); + return DateTimeOffset.Parse((string)value, culture); + }; + } + + if (destinationType == typeof(TimeSpan)) + { + return (value, culture) => + { + ThrowIfNotStringType(value, destinationType); + return TimeSpan.Parse((string)value, culture); + }; + } + + if (destinationType == typeof(Guid)) + { + return (value, culture) => + { + ThrowIfNotStringType(value, destinationType); + return Guid.Parse((string)value); + }; + } + + if (destinationType.GetTypeInfo().IsEnum) + { + return (value, culture) => + { + // EnumConverter cannot convert integer, so we verify manually + if ((value is int)) + { + if (Enum.IsDefined(destinationType, value)) + { + return Enum.ToObject(destinationType, (int)value); + } + + throw new FormatException( + Resources.FormatValueProviderResult_CannotConvertEnum(value, + destinationType)); + } + else + { + ThrowIfNotStringType(value, destinationType); + return Enum.Parse(destinationType, (string)value); + } + }; + } + + return null; + } + + private static Type UnwrapNullableType(Type destinationType) + { + return Nullable.GetUnderlyingType(destinationType) ?? destinationType; + } + + private static void ThrowIfNotStringType(object value, Type destinationType) + { + var type = value.GetType(); + if (type != typeof(string)) + { + string message = Resources.FormatValueProviderResult_NoConverterExists(type, destinationType); + throw new InvalidOperationException(message); + } + } } } diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/TypeConverterModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/TypeConverterModelBinderTest.cs index a62a03ab5b..f44d0bccd5 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/TypeConverterModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/TypeConverterModelBinderTest.cs @@ -7,7 +7,52 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test { public class TypeConverterModelBinderTest { - // private static readonly ModelBinderErrorMessageProvider = (modelMetadata, incomingValue) => null; + [Theory] + [InlineData(typeof(object))] + [InlineData(typeof(Calendar))] + [InlineData(typeof(TestClass))] + public void BindModel_ReturnsFalse_IfTypeCannotBeConverted(Type destinationType) + { + // Arrange + var bindingContext = GetBindingContext(destinationType); + bindingContext.ValueProvider = new SimpleHttpValueProvider + { + { "theModelName", "some-value" } + }; + + var binder = new TypeConverterModelBinder(); + + // Act + var retVal = binder.BindModel(bindingContext); + + // Assert + Assert.False(retVal); + } + + [Theory] + [InlineData(typeof(int))] + [InlineData(typeof(long))] + [InlineData(typeof(Guid))] + [InlineData(typeof(DateTimeOffset))] + [InlineData(typeof(double))] + [InlineData(typeof(DayOfWeek))] + public void BindModel_ReturnsTrue_IfTypeCanBeConverted(Type destinationType) + { + // Arrange + var bindingContext = GetBindingContext(destinationType); + bindingContext.ValueProvider = new SimpleHttpValueProvider + { + { "theModelName", "some-value" } + }; + + var binder = new TypeConverterModelBinder(); + + // Act + var retVal = binder.BindModel(bindingContext); + + // Assert + Assert.True(retVal); + } [Fact] public void BindModel_Error_FormatExceptionsTurnedIntoStringsInModelState() @@ -25,7 +70,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test bool retVal = binder.BindModel(bindingContext); // Assert - Assert.False(retVal); + Assert.True(retVal); Assert.Null(bindingContext.Model); Assert.Equal(false, bindingContext.ModelState.IsValid); Assert.Equal("Input string was not in a correct format.", bindingContext.ModelState["theModelName"].Errors[0].ErrorMessage); @@ -98,5 +143,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test ValueProvider = new SimpleHttpValueProvider() // empty }; } + + private sealed class TestClass + { + } } } diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/ValueProviders/ValueProviderResultTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/ValueProviders/ValueProviderResultTest.cs index a173ee2467..a94f2b081f 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/ValueProviders/ValueProviderResultTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/ValueProviders/ValueProviderResultTest.cs @@ -390,9 +390,52 @@ namespace Microsoft.AspNet.Mvc.ModelBinding yield return new object[] { 42L, 42 }; yield return new object[] { (float)42.0, 42 }; yield return new object[] { (double)42.0, 42 }; + yield return new object[] { "2008-01-01", new DateTime(2008, 01, 01) }; + yield return new object[] { "00:00:20", TimeSpan.FromSeconds(20) }; + yield return new object[] { "c6687d3a-51f9-4159-8771-a66d2b7d7038", + Guid.Parse("c6687d3a-51f9-4159-8771-a66d2b7d7038") }; } } + [Theory] + [InlineData(typeof(TimeSpan))] + [InlineData(typeof(DateTime))] + [InlineData(typeof(DateTimeOffset))] + [InlineData(typeof(Guid))] + [InlineData(typeof(MyEnum))] + public void ConvertTo_Throws_IfValueIsNotStringData(Type destinationType) + { + // Arrange + var result = new ValueProviderResult(new MyClassWithoutConverter(), "", CultureInfo.InvariantCulture); + + // Act + var ex = Assert.Throws(() => result.ConvertTo(destinationType)); + + // Assert + var expectedMessage = string.Format("The parameter conversion from type '{0}' to type '{1}' " + + "failed because no type converter can convert between these types.", + typeof(MyClassWithoutConverter), destinationType); + Assert.Equal(expectedMessage, ex.Message); + } + + [Fact] + public void ConvertTo_Throws_IfDestinationTypeIsNotConvertible() + { + // Arrange + var value = "Hello world"; + var destinationType = typeof(MyClassWithoutConverter); + var result = new ValueProviderResult(value, "", CultureInfo.InvariantCulture); + + // Act + var ex = Assert.Throws(() => result.ConvertTo(destinationType)); + + // Assert + var expectedMessage = string.Format("The parameter conversion from type '{0}' to type '{1}' " + + "failed because no type converter can convert between these types.", + value.GetType(), typeof(MyClassWithoutConverter)); + Assert.Equal(expectedMessage, ex.Message); + } + private class MyClassWithoutConverter { }