diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/SessionStateTempDataProvider.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/SessionStateTempDataProvider.cs index aec95be4da..00b7fda01a 100644 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/SessionStateTempDataProvider.cs +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/SessionStateTempDataProvider.cs @@ -59,16 +59,10 @@ public virtual IDictionary LoadTempData(HttpContext context) throw new ArgumentNullException(nameof(context)); } - if (!IsSessionEnabled(context)) - { - // Session middleware is not enabled. No-op - return null; - } - var session = context.Session; if (session == null) { - return null; + throw new InvalidOperationException("Session cannot be null."); } var tempDataDictionary = new Dictionary(StringComparer.OrdinalIgnoreCase); @@ -162,6 +156,9 @@ public virtual void SaveTempData(HttpContext context, IDictionary 0); if (hasValues) { @@ -171,9 +168,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..d7e1c2809d 100644 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/TempDataDictionary.cs +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/TempDataDictionary.cs @@ -101,6 +101,13 @@ public object this[string key] /// public void Keep() { + // if the data is not loaded, we can assume none of it has been read + // and so silently return. + if (!_loaded) + { + return; + } + Load(); _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..c9ce3d9438 100644 --- a/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ViewFeatures/SessionStateTempDataProviderTest.cs +++ b/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ViewFeatures/SessionStateTempDataProviderTest.cs @@ -16,52 +16,54 @@ 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); + // Act & Assert + var exception = Assert.Throws(() => testProvider.LoadTempData( + GetHttpContext(session: null, sessionEnabled: false))); } [Fact] - public void Load_NonNullSession_NoSessionData_ReturnsEmptyDictionary() + public void Save_ThrowsException_WhenSessionIsNotEnabled() { // Arrange var testProvider = new SessionStateTempDataProvider(); + var values = new Dictionary(); + values.Add("key1", "value1"); - // Act - var tempDataDictionary = testProvider.LoadTempData( - GetHttpContext(Mock.Of())); - - // Assert - Assert.Empty(tempDataDictionary); + // Act & Assert + var exception = Assert.Throws(() => testProvider.SaveTempData( + GetHttpContext(session: null, sessionEnabled: false), values)); } [Fact] - public void Save_NullSession_NullDictionary_DoesNotThrow() + public void Load_NullSession_ReturnsEmptyDictionary() { // Arrange var testProvider = new SessionStateTempDataProvider(); - // Act & Assert (does not throw) - testProvider.SaveTempData(GetHttpContext(session: null, sessionEnabled: false), null); + // Act & Assert + var exception = Assert.Throws(() => testProvider.LoadTempData( + GetHttpContext(session: null, sessionEnabled: true))); + + Assert.Equal("Session cannot be null.", exception.Message); } [Fact] - public void Save_NullSession_EmptyDictionary_DoesNotThrow() + public void Load_NonNullSession_NoSessionData_ReturnsEmptyDictionary() { // Arrange var testProvider = new SessionStateTempDataProvider(); - // Act & Assert (does not throw) - testProvider.SaveTempData( - GetHttpContext(session: null, sessionEnabled: false), new Dictionary()); + // Act + var tempDataDictionary = testProvider.LoadTempData( + GetHttpContext(Mock.Of())); + + // Assert + Assert.Empty(tempDataDictionary); } [Fact] diff --git a/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ViewFeatures/TempDataDictionaryTest.cs b/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ViewFeatures/TempDataDictionaryTest.cs index 460a40a6c0..93d4396d0d 100644 --- a/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ViewFeatures/TempDataDictionaryTest.cs +++ b/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ViewFeatures/TempDataDictionaryTest.cs @@ -1,6 +1,7 @@ // 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 Moq; @@ -10,6 +11,26 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures { public class TempDataDictionaryTest { + [Fact] + public void ThrowsException_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() {