From 06e388e357cbf4b989094bcc3eb8f09bd8fd4a1e Mon Sep 17 00:00:00 2001 From: Kiran Challa Date: Mon, 28 Sep 2015 14:47:56 -0700 Subject: [PATCH] [Fixes #3068] TempData fails silently without sessions middleware --- .../SessionStateTempDataProvider.cs | 25 +---- .../ViewFeatures/TempDataDictionary.cs | 8 +- .../SessionStateTempDataProviderTest.cs | 96 +++++++------------ .../ViewFeatures/TempDataDictionaryTest.cs | 30 +++++- 4 files changed, 75 insertions(+), 84 deletions(-) diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/SessionStateTempDataProvider.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/SessionStateTempDataProvider.cs index aec95be4da..afe0874dac 100644 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/SessionStateTempDataProvider.cs +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/SessionStateTempDataProvider.cs @@ -59,17 +59,8 @@ public virtual IDictionary LoadTempData(HttpContext context) throw new ArgumentNullException(nameof(context)); } - if (!IsSessionEnabled(context)) - { - // Session middleware is not enabled. No-op - return null; - } - + // Accessing Session property will throw if the session middleware is not enabled. var session = context.Session; - if (session == null) - { - return null; - } var tempDataDictionary = new Dictionary(StringComparer.OrdinalIgnoreCase); byte[] value; @@ -162,6 +153,9 @@ public virtual void SaveTempData(HttpContext context, IDictionary 0); if (hasValues) { @@ -171,9 +165,6 @@ public virtual void SaveTempData(HttpContext context, IDictionary() != null; - } - internal void EnsureObjectCanBeSerialized(object item) { var itemType = item.GetType(); diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/TempDataDictionary.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/TempDataDictionary.cs index 694e90f976..1b383c0400 100644 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/TempDataDictionary.cs +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/TempDataDictionary.cs @@ -101,7 +101,13 @@ public object this[string key] /// public void Keep() { - Load(); + // if the data is not loaded, we can assume none of it has been read + // and so silently return. + if (!_loaded) + { + return; + } + _retainedKeys.Clear(); _retainedKeys.UnionWith(_data.Keys); } diff --git a/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ViewFeatures/SessionStateTempDataProviderTest.cs b/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ViewFeatures/SessionStateTempDataProviderTest.cs index 7cae2e63f4..3f2010a252 100644 --- a/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ViewFeatures/SessionStateTempDataProviderTest.cs +++ b/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ViewFeatures/SessionStateTempDataProviderTest.cs @@ -16,68 +16,44 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures public class SessionStateTempDataProviderTest { [Fact] - public void Load_NullSession_ReturnsEmptyDictionary() + public void Load_ThrowsException_WhenSessionIsNotEnabled() { // Arrange var testProvider = new SessionStateTempDataProvider(); - // Act - var tempDataDictionary = testProvider.LoadTempData( - GetHttpContext(session: null, sessionEnabled: true)); - - // Assert - Assert.Null(tempDataDictionary); - } - - [Fact] - public void Load_NonNullSession_NoSessionData_ReturnsEmptyDictionary() - { - // Arrange - var testProvider = new SessionStateTempDataProvider(); - - // Act - var tempDataDictionary = testProvider.LoadTempData( - GetHttpContext(Mock.Of())); - - // Assert - Assert.Empty(tempDataDictionary); + // Act & Assert + Assert.Throws(() => + { + testProvider.LoadTempData(GetHttpContext(sessionEnabled: false)); + }); } [Fact] - public void Save_NullSession_NullDictionary_DoesNotThrow() + public void Save_ThrowsException_WhenSessionIsNotEnabled() { // Arrange var testProvider = new SessionStateTempDataProvider(); + var values = new Dictionary(); + values.Add("key1", "value1"); - // Act & Assert (does not throw) - testProvider.SaveTempData(GetHttpContext(session: null, sessionEnabled: false), null); + // Act & Assert + Assert.Throws(() => + { + testProvider.SaveTempData(GetHttpContext(sessionEnabled: false), values); + }); } [Fact] - public void Save_NullSession_EmptyDictionary_DoesNotThrow() + public void Load_ReturnsEmptyDictionary_WhenNoSessionDataIsAvailable() { // Arrange var testProvider = new SessionStateTempDataProvider(); - // Act & Assert (does not throw) - testProvider.SaveTempData( - GetHttpContext(session: null, sessionEnabled: false), new Dictionary()); - } - - [Fact] - public void Save_NullSession_NonEmptyDictionary_Throws() - { - // Arrange - var testProvider = new SessionStateTempDataProvider(); + // Act + var tempDataDictionary = testProvider.LoadTempData(GetHttpContext()); - // Act & Assert - Assert.Throws(() => - { - testProvider.SaveTempData( - GetHttpContext(session: null, sessionEnabled: false), - new Dictionary { { "foo", "bar" } } - ); - }); + // Assert + Assert.Empty(tempDataDictionary); } public static TheoryData InvalidTypes @@ -97,7 +73,7 @@ public static TheoryData InvalidTypes [Theory] [MemberData(nameof(InvalidTypes))] - public void EnsureObjectCanBeSerialized_InvalidType_Throws(object value, Type type) + public void EnsureObjectCanBeSerialized_ThrowsException_OnInvalidType(object value, Type type) { // Arrange var testProvider = new SessionStateTempDataProvider(); @@ -107,7 +83,8 @@ public void EnsureObjectCanBeSerialized_InvalidType_Throws(object value, Type ty { testProvider.EnsureObjectCanBeSerialized(value); }); - Assert.Equal($"The '{typeof(SessionStateTempDataProvider).FullName}' cannot serialize an object of type '{type}' to session state.", + Assert.Equal($"The '{typeof(SessionStateTempDataProvider).FullName}' cannot serialize " + + $"an object of type '{type}' to session state.", exception.Message); } @@ -127,7 +104,7 @@ public static TheoryData InvalidDictionaryTypes [Theory] [MemberData(nameof(InvalidDictionaryTypes))] - public void EnsureObjectCanBeSerialized_InvalidDictionaryType_Throws(object value, Type type) + public void EnsureObjectCanBeSerialized_ThrowsException_OnInvalidDictionaryType(object value, Type type) { // Arrange var testProvider = new SessionStateTempDataProvider(); @@ -137,7 +114,8 @@ public void EnsureObjectCanBeSerialized_InvalidDictionaryType_Throws(object valu { testProvider.EnsureObjectCanBeSerialized(value); }); - Assert.Equal($"The '{typeof(SessionStateTempDataProvider).FullName}' cannot serialize a dictionary with a key of type '{type}' to session state.", + Assert.Equal($"The '{typeof(SessionStateTempDataProvider).FullName}' cannot serialize a dictionary " + + $"with a key of type '{type}' to session state.", exception.Message); } @@ -163,7 +141,7 @@ public static TheoryData ValidTypes [Theory] [MemberData(nameof(ValidTypes))] - public void EnsureObjectCanBeSerialized_ValidType_DoesNotThrow(object value) + public void EnsureObjectCanBeSerialized_DoesNotThrow_OnValidType(object value) { // Arrange var testProvider = new SessionStateTempDataProvider(); @@ -181,7 +159,7 @@ public void SaveAndLoad_StringCanBeStoredAndLoaded() { { "string", "value" } }; - var context = GetHttpContext(new TestSession(), true); + var context = GetHttpContext(); // Act testProvider.SaveTempData(context, input); @@ -201,7 +179,7 @@ public void SaveAndLoad_IntCanBeStoredAndLoaded() { { "int", 10 } }; - var context = GetHttpContext(new TestSession(), true); + var context = GetHttpContext(); // Act testProvider.SaveTempData(context, input); @@ -223,7 +201,7 @@ public void SaveAndLoad_BoolCanBeStoredAndLoaded(bool value) { { "bool", value } }; - var context = GetHttpContext(new TestSession(), true); + var context = GetHttpContext(); // Act testProvider.SaveTempData(context, input); @@ -244,7 +222,7 @@ public void SaveAndLoad_DateTimeCanBeStoredAndLoaded() { { "DateTime", inputDatetime } }; - var context = GetHttpContext(new TestSession(), true); + var context = GetHttpContext(); // Act testProvider.SaveTempData(context, input); @@ -265,7 +243,7 @@ public void SaveAndLoad_GuidCanBeStoredAndLoaded() { { "Guid", inputGuid } }; - var context = GetHttpContext(new TestSession(), true); + var context = GetHttpContext(); // Act testProvider.SaveTempData(context, input); @@ -285,7 +263,7 @@ public void SaveAndLoad_ListCanBeStoredAndLoaded() { { "List`string", new List { "one", "two" } } }; - var context = GetHttpContext(new TestSession(), true); + var context = GetHttpContext(); // Act testProvider.SaveTempData(context, input); @@ -311,7 +289,7 @@ public void SaveAndLoad_DictionaryCanBeStoredAndLoaded() { { "Dictionary", inputDictionary } }; - var context = GetHttpContext(new TestSession(), true); + var context = GetHttpContext(); // Act testProvider.SaveTempData(context, input); @@ -331,7 +309,7 @@ public void SaveAndLoad_EmptyDictionary_RoundTripsAsNull() { { "EmptyDictionary", new Dictionary() } }; - var context = GetHttpContext(new TestSession(), true); + var context = GetHttpContext(); // Act testProvider.SaveTempData(context, input); @@ -347,12 +325,12 @@ private class TestItem public int DummyInt { get; set; } } - private HttpContext GetHttpContext(ISession session, bool sessionEnabled = true) + private HttpContext GetHttpContext(bool sessionEnabled = true) { var httpContext = new DefaultHttpContext(); - if(sessionEnabled) + if (sessionEnabled) { - httpContext.Features.Set(new SessionFeature() { Session = session }); + httpContext.Features.Set(new SessionFeature() { Session = new TestSession() }); } return httpContext; } diff --git a/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ViewFeatures/TempDataDictionaryTest.cs b/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ViewFeatures/TempDataDictionaryTest.cs index 460a40a6c0..cbcbb1bfcc 100644 --- a/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ViewFeatures/TempDataDictionaryTest.cs +++ b/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ViewFeatures/TempDataDictionaryTest.cs @@ -1,8 +1,10 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.Collections.Generic; using Microsoft.AspNet.Http; +using Microsoft.AspNet.Http.Internal; using Moq; using Xunit; @@ -10,6 +12,29 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures { public class TempDataDictionaryTest { + [Fact] + public void ThrowscdException_OnSettingValue_AndWhenSessionIsNotEnabled() + { + // Arrange + var tempData = new TempDataDictionary(GetHttpContextAccessor(), new SessionStateTempDataProvider()); + + // Act & Assert + Assert.Throws(() => + { + tempData["key1"] = "value1"; + }); + } + + [Fact] + public void Keep_DoesNotThrowException_WhenDataIsNotLoaded() + { + // Arrange + var tempData = new TempDataDictionary(GetHttpContextAccessor(), new SessionStateTempDataProvider()); + + // Act & Assert + tempData.Keep(); + } + [Fact] public void TempData_Load_CreatesEmptyDictionaryIfProviderReturnsNull() { @@ -214,10 +239,7 @@ public void SaveTempData(HttpContext context, IDictionary values private static IHttpContextAccessor GetHttpContextAccessor() { - var httpContext = new Mock(); - var httpContextAccessor = new Mock(); - httpContextAccessor.Setup(h => h.HttpContext).Returns(httpContext.Object); - return httpContextAccessor.Object; + return new HttpContextAccessor() { HttpContext = new DefaultHttpContext() }; } } } \ No newline at end of file