diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/SaveTempDataPropertyFilterBase.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/SaveTempDataPropertyFilterBase.cs index 0ec912bbca..85643a33e8 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/SaveTempDataPropertyFilterBase.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/SaveTempDataPropertyFilterBase.cs @@ -98,10 +98,14 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal var propertyType = Nullable.GetUnderlyingType(property.PropertyType) ?? property.PropertyType; - if (!(propertyType.GetTypeInfo().IsPrimitive || propertyType == typeof(string))) + if (!TempDataSerializer.CanSerializeType(propertyType, out var errorMessage)) { - throw new InvalidOperationException( - Resources.FormatTempDataProperties_PrimitiveTypeOrString(property.DeclaringType.FullName, property.Name, nameof(TempDataAttribute))); + var messageWithPropertyInfo = Resources.FormatTempDataProperties_InvalidType( + property.DeclaringType.FullName, + property.Name, + nameof(TempDataAttribute)); + + throw new InvalidOperationException($"{messageWithPropertyInfo} {errorMessage}"); } } diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/TempDataSerializer.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/TempDataSerializer.cs index 7915dcac3d..fd92d4d8ca 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/TempDataSerializer.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/TempDataSerializer.cs @@ -157,34 +157,50 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal public void EnsureObjectCanBeSerialized(object item) { - var itemType = item.GetType(); - Type actualType = null; + item = item ?? throw new ArgumentNullException(nameof(item)); - if (itemType.IsArray) + var itemType = item.GetType(); + + if (!CanSerializeType(itemType, out var errorMessage)) { - itemType = itemType.GetElementType(); + throw new InvalidOperationException(errorMessage); } - else if (itemType.GetTypeInfo().IsGenericType) + } + + public static bool CanSerializeType(Type typeToSerialize, out string errorMessage) + { + typeToSerialize = typeToSerialize ?? throw new ArgumentNullException(nameof(typeToSerialize)); + + errorMessage = null; + + Type actualType = null; + if (typeToSerialize.IsArray) { - if (ClosedGenericMatcher.ExtractGenericInterface(itemType, typeof(IList<>)) != null) + actualType = typeToSerialize.GetElementType(); + } + else if (typeToSerialize.GetTypeInfo().IsGenericType) + { + if (ClosedGenericMatcher.ExtractGenericInterface(typeToSerialize, typeof(IList<>)) != null) { - var genericTypeArguments = itemType.GenericTypeArguments; + var genericTypeArguments = typeToSerialize.GenericTypeArguments; Debug.Assert(genericTypeArguments.Length == 1, "IList has one generic argument"); actualType = genericTypeArguments[0]; } - else if (ClosedGenericMatcher.ExtractGenericInterface(itemType, typeof(IDictionary<,>)) != null) + else if (ClosedGenericMatcher.ExtractGenericInterface(typeToSerialize, typeof(IDictionary<,>)) != null) { - var genericTypeArguments = itemType.GenericTypeArguments; + var genericTypeArguments = typeToSerialize.GenericTypeArguments; Debug.Assert( genericTypeArguments.Length == 2, "IDictionary has two generic arguments"); - // Throw if the key type of the dictionary is not string. + // The key must be of type string. if (genericTypeArguments[0] != typeof(string)) { - var message = Resources.FormatTempData_CannotSerializeDictionary( - typeof(TempDataSerializer).FullName, genericTypeArguments[0]); - throw new InvalidOperationException(message); + errorMessage = Resources.FormatTempData_CannotSerializeDictionary( + typeof(TempDataSerializer).FullName, + genericTypeArguments[0], + typeof(string).FullName); + return false; } else { @@ -193,14 +209,16 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal } } - actualType = actualType ?? itemType; + actualType = actualType ?? typeToSerialize; if (!IsSimpleType(actualType)) { - var underlyingType = Nullable.GetUnderlyingType(actualType) ?? actualType; - var message = Resources.FormatTempData_CannotSerializeType( - typeof(TempDataSerializer).FullName, underlyingType); - throw new InvalidOperationException(message); + errorMessage = Resources.FormatTempData_CannotSerializeType( + typeof(TempDataSerializer).FullName, + actualType); + return false; } + + return true; } private static IList ConvertArray(JArray array) diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Properties/Resources.Designer.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Properties/Resources.Designer.cs index 7a828a777f..78f9ebcfb6 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Properties/Resources.Designer.cs @@ -683,7 +683,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures => string.Format(CultureInfo.CurrentCulture, GetString("TempData_CannotDeserializeToken"), p0, p1); /// - /// The '{0}' cannot serialize a dictionary with a key of type '{1}'. + /// The '{0}' cannot serialize a dictionary with a key of type '{1}'. The key must be of type '{2}'. /// internal static string TempData_CannotSerializeDictionary { @@ -691,10 +691,10 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures } /// - /// The '{0}' cannot serialize a dictionary with a key of type '{1}'. + /// The '{0}' cannot serialize a dictionary with a key of type '{1}'. The key must be of type '{2}'. /// - internal static string FormatTempData_CannotSerializeDictionary(object p0, object p1) - => string.Format(CultureInfo.CurrentCulture, GetString("TempData_CannotSerializeDictionary"), p0, p1); + internal static string FormatTempData_CannotSerializeDictionary(object p0, object p1, object p2) + => string.Format(CultureInfo.CurrentCulture, GetString("TempData_CannotSerializeDictionary"), p0, p1, p2); /// /// The '{0}' cannot serialize an object of type '{1}'. @@ -795,18 +795,18 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures => string.Format(CultureInfo.CurrentCulture, GetString("ViewEnginesAreRequired"), p0, p1, p2); /// - /// The '{0}.{1}' property with {2} is invalid. A property using {2} must be a primitive or string type. + /// The '{0}.{1}' property with {2} is invalid. /// - internal static string TempDataProperties_PrimitiveTypeOrString + internal static string TempDataProperties_InvalidType { - get => GetString("TempDataProperties_PrimitiveTypeOrString"); + get => GetString("TempDataProperties_InvalidType"); } /// - /// The '{0}.{1}' property with {2} is invalid. A property using {2} must be a primitive or string type. + /// The '{0}.{1}' property with {2} is invalid. /// - internal static string FormatTempDataProperties_PrimitiveTypeOrString(object p0, object p1, object p2) - => string.Format(CultureInfo.CurrentCulture, GetString("TempDataProperties_PrimitiveTypeOrString"), p0, p1, p2); + internal static string FormatTempDataProperties_InvalidType(object p0, object p1, object p2) + => string.Format(CultureInfo.CurrentCulture, GetString("TempDataProperties_InvalidType"), p0, p1, p2); /// /// The '{0}.{1}' property with {2} is invalid. A property using {2} must have a public getter and setter. diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Resources.resx b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Resources.resx index 4ef5d2597e..4bfed7a827 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Resources.resx +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Resources.resx @@ -263,7 +263,7 @@ Cannot deserialize {0} of type '{1}'. - The '{0}' cannot serialize a dictionary with a key of type '{1}'. + The '{0}' cannot serialize a dictionary with a key of type '{1}'. The key must be of type '{2}'. The '{0}' cannot serialize an object of type '{1}'. @@ -286,8 +286,8 @@ '{0}.{1}' must not be empty. At least one '{2}' is required to locate a view for rendering. - - The '{0}.{1}' property with {2} is invalid. A property using {2} must be a primitive or string type. + + The '{0}.{1}' property with {2} is invalid. The '{0}.{1}' property with {2} is invalid. A property using {2} must have a public getter and setter. diff --git a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/TempDataApplicationModelProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/TempDataApplicationModelProviderTest.cs index 3b0ad1e896..8faed68cf2 100644 --- a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/TempDataApplicationModelProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/TempDataApplicationModelProviderTest.cs @@ -16,8 +16,10 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal { [Theory] [InlineData(typeof(TestController_OneTempDataProperty))] + [InlineData(typeof(TestController_ListOfString))] [InlineData(typeof(TestController_OneNullableTempDataProperty))] [InlineData(typeof(TestController_TwoTempDataProperties))] + [InlineData(typeof(TestController_NullableNonPrimitiveTempDataProperty))] public void AddsTempDataPropertyFilter_ForTempDataAttributeProperties(Type type) { // Arrange @@ -58,23 +60,6 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal Assert.Same(expected, tempDataPropertyHelper.PropertyInfo); } - [Fact] - public void DoesNotInitializeFilterFactory_ThrowsInvalidOperationException_NonPrimitiveType() - { - // Arrange - var provider = new TempDataApplicationModelProvider(); - var defaultProvider = new DefaultApplicationModelProvider(Options.Create(new MvcOptions())); - - var context = new ApplicationModelProviderContext(new[] { typeof(TestController_OneValid_OneInvalidProperty).GetTypeInfo() }); - defaultProvider.OnProvidersExecuting(context); - - // Act - var exception = Assert.Throws(() => - provider.OnProvidersExecuting(context)); - - Assert.Equal($"The '{typeof(TestController_OneValid_OneInvalidProperty).FullName}.{nameof(TestController_OneValid_OneInvalidProperty.Test2)}' property with {nameof(TempDataAttribute)} is invalid. A property using {nameof(TempDataAttribute)} must be a primitive or string type.", exception.Message); - } - [Fact] public void ThrowsInvalidOperationException_PrivateSetter() { @@ -89,7 +74,11 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal var exception = Assert.Throws(() => provider.OnProvidersExecuting(context)); - Assert.Equal($"The '{typeof(TestController_PrivateSet).FullName}.{nameof(TestController_NonPrimitiveType.Test)}' property with {nameof(TempDataAttribute)} is invalid. A property using {nameof(TempDataAttribute)} must have a public getter and setter.", exception.Message); + Assert.Equal( + $"The '{typeof(TestController_PrivateSet).FullName}.{nameof(TestController_NonPrimitiveType.Test)}'" + + $" property with {nameof(TempDataAttribute)} is invalid. A property using {nameof(TempDataAttribute)}" + + " must have a public getter and setter.", + exception.Message); } [Fact] @@ -106,26 +95,34 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal var exception = Assert.Throws(() => provider.OnProvidersExecuting(context)); - Assert.Equal($"The '{typeof(TestController_NonPrimitiveType).FullName}.{nameof(TestController_NonPrimitiveType.Test)}' property with {nameof(TempDataAttribute)} is invalid. A property using {nameof(TempDataAttribute)} must be a primitive or string type.", exception.Message); + Assert.Equal( + $"The '{typeof(TestController_NonPrimitiveType).FullName}.{nameof(TestController_NonPrimitiveType.Test)}'" + + $" property with {nameof(TempDataAttribute)} is invalid. The '{typeof(TempDataSerializer).FullName}'" + + $" cannot serialize an object of type '{typeof(Object).FullName}'.", + exception.Message); } [Fact] - public void ThrowsInvalidOperationException_ForNullableNonPrimitiveType() + public void ThrowsInvalidOperationException_NonStringDictionaryKey() { // Arrange var provider = new TempDataApplicationModelProvider(); var defaultProvider = new DefaultApplicationModelProvider(Options.Create(new MvcOptions())); - var controllerType = typeof(TestController_NullableNonPrimitiveTempDataProperty); - var context = new ApplicationModelProviderContext(new[] { controllerType.GetTypeInfo() }); + + var context = new ApplicationModelProviderContext( + new[] { typeof(TestController_NonStringDictionaryKey).GetTypeInfo() }); defaultProvider.OnProvidersExecuting(context); // Act & Assert var exception = Assert.Throws(() => provider.OnProvidersExecuting(context)); - Assert.Equal($"The '{controllerType.FullName}.{nameof(TestController_NullableNonPrimitiveTempDataProperty.DateTime)}'" - + $" property with {nameof(TempDataAttribute)} is invalid. A property using {nameof(TempDataAttribute)} " - + $"must be a primitive or string type.", exception.Message); + Assert.Equal( + $"The '{typeof(TestController_NonStringDictionaryKey).FullName}.{nameof(TestController_NonStringDictionaryKey.Test)}'" + + $" property with {nameof(TempDataAttribute)} is invalid. The '{typeof(TempDataSerializer).FullName}'" + + $" cannot serialize a dictionary with a key of type '{typeof(Object)}'. The key must be of type" + + $" '{typeof(string).FullName}'.", + exception.Message); } public class TestController_NullableNonPrimitiveTempDataProperty @@ -159,13 +156,10 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal public int? Test2 { get; set; } } - public class TestController_OneValid_OneInvalidProperty + public class TestController_ListOfString { [TempData] - public int Test { get; set; } - - [TempData] - public IList Test2 { get; set; } + public IList Test { get; set; } } public class TestController_PrivateSet @@ -179,5 +173,11 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal [TempData] public object Test { get; set; } } + + public class TestController_NonStringDictionaryKey + { + [TempData] + public IDictionary Test { get; set; } + } } } diff --git a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/TempDataSerializerTest.cs b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/TempDataSerializerTest.cs index 9db91f81bb..2cc577b21d 100644 --- a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/TempDataSerializerTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/TempDataSerializerTest.cs @@ -68,7 +68,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal testProvider.EnsureObjectCanBeSerialized(value); }); Assert.Equal($"The '{typeof(TempDataSerializer).FullName}' cannot serialize a dictionary " + - $"with a key of type '{type}'.", + $"with a key of type '{type}'. The key must be of type 'System.String'.", exception.Message); }