[Fixes #3068] TempData fails silently without sessions middleware

src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/SessionStateTempDataProvider.cs

@@ -59,17 +59,8 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures
                 throw new ArgumentNullException(nameof(context));
             }
 
-            if (!IsSessionEnabled(context))
+            // Accessing Session property will throw if the session middleware is not enabled.
-            {
-                // Session middleware is not enabled. No-op
-                return null;
-            }
-
             var session = context.Session;
-            if (session == null)
-            {
-                return null;
-            }
 
             var tempDataDictionary = new Dictionary<string, object>(StringComparer.OrdinalIgnoreCase);
             byte[] value;
@@ -162,6 +153,9 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures
                 throw new ArgumentNullException(nameof(context));
             }
 
+            // Accessing Session property will throw if the session middleware is not enabled.
+            var session = context.Session;
+
             var hasValues = (values != null && values.Count > 0);
             if (hasValues)
             {
@@ -171,9 +165,6 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures
                     EnsureObjectCanBeSerialized(item);
                 }
 
-                // Accessing Session property will throw if the session middleware is not enabled.
-                var session = context.Session;
-
                 using (var memoryStream = new MemoryStream())
                 {
                     using (var writer = new BsonWriter(memoryStream))
@@ -183,18 +174,12 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures
                     }
                 }
             }
-            else if (IsSessionEnabled(context))
+            else
             {
-                var session = context.Session;
                 session.Remove(TempDataSessionStateKey);
             }
         }
 
-        private static bool IsSessionEnabled(HttpContext context)
-        {
-            return context.Features.Get<ISessionFeature>() != null;
-        }
-
         internal void EnsureObjectCanBeSerialized(object item)
         {
             var itemType = item.GetType();

src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/TempDataDictionary.cs

@@ -101,7 +101,13 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures
         /// <inheritdoc />
         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);
         }

test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ViewFeatures/SessionStateTempDataProviderTest.cs

@@ -16,56 +16,7 @@ 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<ISession>()));
-
-            // Assert
-            Assert.Empty(tempDataDictionary);
-        }
-
-        [Fact]
-        public void Save_NullSession_NullDictionary_DoesNotThrow()
-        {
-            // Arrange
-            var testProvider = new SessionStateTempDataProvider();
-
-            // Act & Assert (does not throw)
-            testProvider.SaveTempData(GetHttpContext(session: null, sessionEnabled: false), null);
-        }
-
-        [Fact]
-        public void Save_NullSession_EmptyDictionary_DoesNotThrow()
-        {
-            // Arrange
-            var testProvider = new SessionStateTempDataProvider();
-
-            // Act & Assert (does not throw)
-            testProvider.SaveTempData(
-                GetHttpContext(session: null, sessionEnabled: false), new Dictionary<string, object>());
-        }
-
-        [Fact]
-        public void Save_NullSession_NonEmptyDictionary_Throws()
         {
             // Arrange
             var testProvider = new SessionStateTempDataProvider();
@@ -73,13 +24,38 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures
             // Act & Assert
             Assert.Throws<InvalidOperationException>(() =>
             {
-                testProvider.SaveTempData(
+                testProvider.LoadTempData(GetHttpContext(sessionEnabled: false));
-                    GetHttpContext(session: null, sessionEnabled: false),
-                    new Dictionary<string, object> { { "foo", "bar" } }
-                );
             });
         }
 
+        [Fact]
+        public void Save_ThrowsException_WhenSessionIsNotEnabled()
+        {
+            // Arrange
+            var testProvider = new SessionStateTempDataProvider();
+            var values = new Dictionary<string, object>();
+            values.Add("key1", "value1");
+
+            // Act & Assert
+            Assert.Throws<InvalidOperationException>(() =>
+            {
+                testProvider.SaveTempData(GetHttpContext(sessionEnabled: false), values);
+            });
+        }
+
+        [Fact]
+        public void Load_ReturnsEmptyDictionary_WhenNoSessionDataIsAvailable()
+        {
+            // Arrange
+            var testProvider = new SessionStateTempDataProvider();
+
+            // Act
+            var tempDataDictionary = testProvider.LoadTempData(GetHttpContext());
+
+            // Assert
+            Assert.Empty(tempDataDictionary);
+        }
+
         public static TheoryData<object, Type> InvalidTypes
         {
             get
@@ -97,7 +73,7 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures
 
         [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 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures
             {
                 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 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures
 
         [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 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures
             {
                 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 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures
 
         [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 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures
             {
                 { "string", "value" }
             };
-            var context = GetHttpContext(new TestSession(), true);
+            var context = GetHttpContext();
 
             // Act
             testProvider.SaveTempData(context, input);
@@ -201,7 +179,7 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures
             {
                 { "int", 10 }
             };
-            var context = GetHttpContext(new TestSession(), true);
+            var context = GetHttpContext();
 
             // Act
             testProvider.SaveTempData(context, input);
@@ -223,7 +201,7 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures
             {
                 { "bool", value }
             };
-            var context = GetHttpContext(new TestSession(), true);
+            var context = GetHttpContext();
 
             // Act
             testProvider.SaveTempData(context, input);
@@ -244,7 +222,7 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures
             {
                 { "DateTime", inputDatetime }
             };
-            var context = GetHttpContext(new TestSession(), true);
+            var context = GetHttpContext();
 
             // Act
             testProvider.SaveTempData(context, input);
@@ -265,7 +243,7 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures
             {
                 { "Guid", inputGuid }
             };
-            var context = GetHttpContext(new TestSession(), true);
+            var context = GetHttpContext();
 
             // Act
             testProvider.SaveTempData(context, input);
@@ -285,7 +263,7 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures
             {
                 { "List`string", new List<string> { "one", "two" } }
             };
-            var context = GetHttpContext(new TestSession(), true);
+            var context = GetHttpContext();
 
             // Act
             testProvider.SaveTempData(context, input);
@@ -311,7 +289,7 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures
             {
                 { "Dictionary", inputDictionary }
             };
-            var context = GetHttpContext(new TestSession(), true);
+            var context = GetHttpContext();
 
             // Act
             testProvider.SaveTempData(context, input);
@@ -331,7 +309,7 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures
             {
                 { "EmptyDictionary", new Dictionary<string, int>() }
             };
-            var context = GetHttpContext(new TestSession(), true);
+            var context = GetHttpContext();
 
             // Act
             testProvider.SaveTempData(context, input);
@@ -347,12 +325,12 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures
             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<ISessionFeature>(new SessionFeature() { Session = session });
+                httpContext.Features.Set<ISessionFeature>(new SessionFeature() { Session = new TestSession() });
             }
             return httpContext;
         }

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<InvalidOperationException>(() =>
+            {
+                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 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures
 
         private static IHttpContextAccessor GetHttpContextAccessor()
         {
-            var httpContext = new Mock<HttpContext>();
+            return new HttpContextAccessor() { HttpContext = new DefaultHttpContext() };
-            var httpContextAccessor = new Mock<IHttpContextAccessor>();
-            httpContextAccessor.Setup(h => h.HttpContext).Returns(httpContext.Object);
-            return httpContextAccessor.Object;
         }
     }
 }