From e09ea405517a5a330afaa8b1e5e1b67cd0ea723a Mon Sep 17 00:00:00 2001 From: Kiran Challa Date: Fri, 1 Dec 2017 15:47:45 -0800 Subject: [PATCH] [Fixes #6591] TempData should support nullable types --- .../SaveTempDataPropertyFilterBase.cs | 4 +- .../Properties/Resources.Designer.cs | 4 +- .../Resources.resx | 2 +- .../TempDataPropertyTest.cs | 22 +++++------ .../TempDataApplicationModelProviderTest.cs | 38 ++++++++++++++++++- .../Controllers/TempDataPropertyController.cs | 13 +++++-- .../Views/TempDataProperty/DetailsView.cshtml | 2 +- 7 files changed, 64 insertions(+), 21 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/SaveTempDataPropertyFilterBase.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/SaveTempDataPropertyFilterBase.cs index 6f194ce986..0ec912bbca 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/SaveTempDataPropertyFilterBase.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/SaveTempDataPropertyFilterBase.cs @@ -96,7 +96,9 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal Resources.FormatTempDataProperties_PublicGetterSetter(property.DeclaringType.FullName, property.Name, nameof(TempDataAttribute))); } - if (!(property.PropertyType.GetTypeInfo().IsPrimitive || property.PropertyType == typeof(string))) + var propertyType = Nullable.GetUnderlyingType(property.PropertyType) ?? property.PropertyType; + + if (!(propertyType.GetTypeInfo().IsPrimitive || propertyType == typeof(string))) { throw new InvalidOperationException( Resources.FormatTempDataProperties_PrimitiveTypeOrString(property.DeclaringType.FullName, property.Name, nameof(TempDataAttribute))); diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Properties/Resources.Designer.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Properties/Resources.Designer.cs index 976283c6c0..7a828a777f 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Properties/Resources.Designer.cs @@ -795,7 +795,7 @@ 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 of primitive or string type. + /// The '{0}.{1}' property with {2} is invalid. A property using {2} must be a primitive or string type. /// internal static string TempDataProperties_PrimitiveTypeOrString { @@ -803,7 +803,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures } /// - /// The '{0}.{1}' property with {2} is invalid. A property using {2} must be of primitive or string type. + /// The '{0}.{1}' property with {2} is invalid. A property using {2} must be a primitive or string type. /// internal static string FormatTempDataProperties_PrimitiveTypeOrString(object p0, object p1, object p2) => string.Format(CultureInfo.CurrentCulture, GetString("TempDataProperties_PrimitiveTypeOrString"), p0, p1, p2); diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Resources.resx b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Resources.resx index baa6c7a29b..4ef5d2597e 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Resources.resx +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Resources.resx @@ -287,7 +287,7 @@ '{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 of primitive or string type. + 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. A property using {2} must have a public getter and setter. diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/TempDataPropertyTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/TempDataPropertyTest.cs index c7cece01b7..a6268cfa8d 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/TempDataPropertyTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/TempDataPropertyTest.cs @@ -25,13 +25,13 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests public async Task TempDataPropertyAttribute_RetainsTempDataWithView() { // Arrange - var message = "Success (from Temp Data)"; + var tempDataContent = "Success (from Temp Data)100"; var nameValueCollection = new List> { new KeyValuePair("FullName", "Bob"), new KeyValuePair("id", "1"), }; - var expected = $"{message} for person {nameValueCollection[0].Value} with id {nameValueCollection[1].Value}."; + var expected = $"{tempDataContent} for person {nameValueCollection[0].Value} with id {nameValueCollection[1].Value}."; var content = new FormUrlEncodedContent(nameValueCollection); // Act 1 @@ -53,13 +53,13 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests public async Task TempDataPropertyAttribute_RetainsTempDataWithoutView() { // Arrange - var message = "Success (from Temp Data)"; + var tempDataContent = "Success (from Temp Data)100"; var nameValueCollection = new List> { new KeyValuePair("FullName", "Bob"), new KeyValuePair("id", "1"), }; - var expected = $"{message} for person {nameValueCollection[0].Value} with id {nameValueCollection[1].Value}."; + var expected = $"{tempDataContent} for person {nameValueCollection[0].Value} with id {nameValueCollection[1].Value}."; var content = new FormUrlEncodedContent(nameValueCollection); // Act 1 @@ -81,14 +81,14 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests public async Task TempDataPropertyAttribute_TempDataKept() { // Arrange - var message = "Success (from Temp Data)"; + var tempDataContent = "Success (from Temp Data)100"; var nameValueCollection = new List> { new KeyValuePair("FullName", "Bob"), new KeyValuePair("id", "1"), }; - var expected = $"{message} for person {nameValueCollection[0].Value} with id {nameValueCollection[1].Value}."; + var expected = $"{tempDataContent} for person {nameValueCollection[0].Value} with id {nameValueCollection[1].Value}."; var content = new FormUrlEncodedContent(nameValueCollection); // Act 1 @@ -103,7 +103,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests // Assert 2 Assert.Equal(HttpStatusCode.OK, response.StatusCode); var body = await response.Content.ReadAsStringAsync(); - Assert.Equal(message, body); + Assert.Equal(tempDataContent, body); // Act 3 response = await Client.SendAsync(GetRequest("TempDataProperty/ReadTempData", response)); @@ -111,21 +111,21 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests // Assert 3 Assert.Equal(HttpStatusCode.OK, response.StatusCode); body = await response.Content.ReadAsStringAsync(); - Assert.Equal(message, body); + Assert.Equal(tempDataContent, body); } [Fact] public async Task TempDataPropertyAttribute_TempDataNotKept() { // Arrange - var message = "Success (from Temp Data)"; + var tempDataContent = "Success (from Temp Data)100"; var nameValueCollection = new List> { new KeyValuePair("FullName", "Bob"), new KeyValuePair("id", "1"), }; - var expected = $"{message} for person {nameValueCollection[0].Value} with id {nameValueCollection[1].Value}."; + var expected = $"{tempDataContent} for person {nameValueCollection[0].Value} with id {nameValueCollection[1].Value}."; var content = new FormUrlEncodedContent(nameValueCollection); // Act 1 @@ -140,7 +140,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests // Assert 2 Assert.Equal(HttpStatusCode.OK, response.StatusCode); var body = await response.Content.ReadAsStringAsync(); - Assert.Equal(message, body); + Assert.Equal(tempDataContent, body); // Act 3 response = await Client.SendAsync(GetRequest("TempDataProperty/ReadTempData", response)); diff --git a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/TempDataApplicationModelProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/TempDataApplicationModelProviderTest.cs index f061e3371d..3b0ad1e896 100644 --- a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/TempDataApplicationModelProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/TempDataApplicationModelProviderTest.cs @@ -16,6 +16,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal { [Theory] [InlineData(typeof(TestController_OneTempDataProperty))] + [InlineData(typeof(TestController_OneNullableTempDataProperty))] [InlineData(typeof(TestController_TwoTempDataProperties))] public void AddsTempDataPropertyFilter_ForTempDataAttributeProperties(Type type) { @@ -71,7 +72,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal 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 of primitive or string type.", exception.Message); + 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] @@ -105,7 +106,32 @@ 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 of primitive or string type.", exception.Message); + 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); + } + + [Fact] + public void ThrowsInvalidOperationException_ForNullableNonPrimitiveType() + { + // 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() }); + 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); + } + + public class TestController_NullableNonPrimitiveTempDataProperty + { + [TempData] + public DateTime? DateTime { get; set; } } public class TestController_OneTempDataProperty @@ -125,6 +151,14 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal public int Test2 { get; set; } } + public class TestController_OneNullableTempDataProperty + { + public string Test { get; set; } + + [TempData] + public int? Test2 { get; set; } + } + public class TestController_OneValid_OneInvalidProperty { [TempData] diff --git a/test/WebSites/BasicWebSite/Controllers/TempDataPropertyController.cs b/test/WebSites/BasicWebSite/Controllers/TempDataPropertyController.cs index bc99885881..75994f0b1a 100644 --- a/test/WebSites/BasicWebSite/Controllers/TempDataPropertyController.cs +++ b/test/WebSites/BasicWebSite/Controllers/TempDataPropertyController.cs @@ -13,10 +13,14 @@ namespace BasicWebSite.Controllers [TempData] public string Message { get; set; } + [TempData] + public int? NullableInt { get; set; } + [HttpPost] public IActionResult CreateForView(Person person) { Message = "Success (from Temp Data)"; + NullableInt = 100; return RedirectToAction("DetailsView", person); } @@ -24,35 +28,38 @@ namespace BasicWebSite.Controllers public IActionResult Create(Person person) { Message = "Success (from Temp Data)"; + NullableInt = 100; return RedirectToAction("Details", person); } public IActionResult DetailsView(Person person) { ViewData["Message"] = Message; + ViewData["NullableInt"] = NullableInt; return View(person); } public string Details(Person person) { - return $"{Message} for person {person.FullName} with id {person.id}."; + return $"{Message}{NullableInt} for person {person.FullName} with id {person.id}."; } public StatusCodeResult CreateNoRedirect(Person person) { Message = "Success (from Temp Data)"; + NullableInt = 100; return new OkResult(); } public string TempDataKept() { TempData.Keep(); - return Message; + return Message + NullableInt; } public string ReadTempData() { - return Message; + return Message + NullableInt; } } } diff --git a/test/WebSites/BasicWebSite/Views/TempDataProperty/DetailsView.cshtml b/test/WebSites/BasicWebSite/Views/TempDataProperty/DetailsView.cshtml index cc23b5c6cb..f108e3a7fe 100644 --- a/test/WebSites/BasicWebSite/Views/TempDataProperty/DetailsView.cshtml +++ b/test/WebSites/BasicWebSite/Views/TempDataProperty/DetailsView.cshtml @@ -1,2 +1,2 @@ @model BasicWebSite.Models.Person -@ViewData["Message"] for person @Model.FullName with id @Model.id. +@ViewData["Message"]@ViewData["NullableInt"] for person @Model.FullName with id @Model.id.