More Feedback

Author: JunTaoLuo

src/Microsoft.AspNetCore.Session/DistributedSession.cs

@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Collections;
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
@@ -26,8 +27,8 @@ public class DistributedSession : ISession
         private readonly string _sessionKey;
         private readonly TimeSpan _idleTimeout;
         private readonly Func<bool> _tryEstablishSession;
-        private readonly IDictionary<EncodedKey, byte[]> _store;
         private readonly ILogger _logger;
+        private IDictionary<EncodedKey, byte[]> _store;
         private bool _isModified;
         private bool _loaded;
         private bool _isAvailable;
@@ -86,27 +87,19 @@ public string Id
             get
             {
                 Load();
-                if (_isAvailable)
+                if (_sessionId == null)
                 {
-                    if (_sessionId == null)
-                    {
-                        _sessionId = new Guid(IdBytes).ToString();
-                    }
-                    return _sessionId;
-                }
-                else
-                {
-                    return string.Empty;
+                    _sessionId = new Guid(IdBytes).ToString();
                 }
+                return _sessionId;
             }
         }
 
         private byte[] IdBytes
         {
             get
             {
-                Load();
-                if (_isAvailable)
+                if (IsAvailable)
                 {
                     if (_sessionIdBytes == null)
                     {
@@ -127,29 +120,14 @@ public IEnumerable<string> Keys
             get
             {
                 Load();
-                if (_isAvailable)
-                {
-                    return _store.Keys.Select(key => key.KeyString);
-                }
-                else
-                {
-                    return Enumerable.Empty<string>();
-                }
+                return _store.Keys.Select(key => key.KeyString);
             }
         }
 
         public bool TryGetValue(string key, out byte[] value)
         {
             Load();
-            if (_isAvailable)
-            {
-                return _store.TryGetValue(new EncodedKey(key), out value);
-            }
-            else
-            {
-                value = default(byte[]);
-                return false;
-            }
+            return _store.TryGetValue(new EncodedKey(key), out value);
         }
 
         public void Set(string key, byte[] value)
@@ -159,16 +137,15 @@ public void Set(string key, byte[] value)
                 throw new ArgumentNullException(nameof(value));
             }
 
-            var encodedKey = new EncodedKey(key);
-            if (encodedKey.KeyBytes.Length > KeyLengthLimit)
+            if (IsAvailable)
             {
-                throw new ArgumentOutOfRangeException(nameof(key),
-                    Resources.FormatException_KeyLengthIsExceeded(KeyLengthLimit));
-            }
+                var encodedKey = new EncodedKey(key);
+                if (encodedKey.KeyBytes.Length > KeyLengthLimit)
+                {
+                    throw new ArgumentOutOfRangeException(nameof(key),
+                        Resources.FormatException_KeyLengthIsExceeded(KeyLengthLimit));
+                }
 
-            Load();
-            if (_isAvailable)
-            {
                 if (!_tryEstablishSession())
                 {
                     throw new InvalidOperationException(Resources.Exception_InvalidSessionEstablishment);
@@ -183,20 +160,14 @@ public void Set(string key, byte[] value)
         public void Remove(string key)
         {
             Load();
-            if (_isAvailable)
-            {
-                _isModified |= _store.Remove(new EncodedKey(key));
-            }
+            _isModified |= _store.Remove(new EncodedKey(key));
         }
 
         public void Clear()
         {
             Load();
-            if (_isAvailable)
-            {
-                _isModified |= _store.Count > 0;
-                _store.Clear();
-            }
+            _isModified |= _store.Count > 0;
+            _store.Clear();
         }
 
         private void Load()
@@ -220,6 +191,8 @@ private void Load()
                 {
                     _logger.SessionCacheReadException(_sessionKey, exception);
                     _isAvailable = false;
+                    _sessionId = string.Empty;
+                    _store = new EmptySessionStore();
                 }
                 finally
                 {
@@ -228,8 +201,7 @@ private void Load()
             }
         }
 
-        // TODO: This should throw if called directly, but most other places it should fail silently
-        // (e.g. TryGetValue should just return null).
+        // This will throw if called directly and a failure occurs. The user is expected to handle the failures.
         public async Task LoadAsync()
         {
             if (!_loaded)
@@ -268,7 +240,6 @@ public async Task CommitAsync()
                         return;
                     }
                 }
-                _isModified = false;
 
                 var stream = new MemoryStream();
                 Serialize(stream);
@@ -285,6 +256,8 @@ await _cache.SetAsync(
                     _logger.SessionCacheWriteException(_sessionKey, Id, exception);
                     return;
                 }
+
+                _isModified = false;
                 _logger.SessionStored(_sessionKey, Id, _store.Count);
             }
             else
@@ -329,7 +302,6 @@ private void Deserialize(Stream content)
         {
             if (content == null || content.ReadByte() != SerializationRevision)
             {
-                // TODO: Throw?
                 // Replace the un-readable format.
                 _isModified = true;
                 return;
@@ -489,5 +461,55 @@ public override string ToString()
                 return KeyString;
             }
         }
+
+        private class EmptySessionStore : IDictionary<EncodedKey, byte[]>
+        {
+            public byte[] this[EncodedKey key]
+            {
+                get
+                {
+                    return null;
+                }
+
+                set
+                {
+
+                }
+            }
+
+            public int Count { get; } = 0;
+
+            public bool IsReadOnly { get; } = false;
+
+            public ICollection<EncodedKey> Keys { get; } = new EncodedKey[0];
+
+            public ICollection<byte[]> Values { get; } = new byte[0][];
+
+            public void Add(KeyValuePair<EncodedKey, byte[]> item) { }
+
+            public void Add(EncodedKey key, byte[] value) { }
+
+            public void Clear() { }
+
+            public bool Contains(KeyValuePair<EncodedKey, byte[]> item) => false;
+
+            public bool ContainsKey(EncodedKey key) => false;
+
+            public void CopyTo(KeyValuePair<EncodedKey, byte[]>[] array, int arrayIndex) { }
+
+            public IEnumerator<KeyValuePair<EncodedKey, byte[]>> GetEnumerator() => Enumerable.Empty<KeyValuePair<EncodedKey, byte[]>>().GetEnumerator();
+
+            public bool Remove(KeyValuePair<EncodedKey, byte[]> item) => false;
+
+            public bool Remove(EncodedKey key) => false;
+
+            public bool TryGetValue(EncodedKey key, out byte[] value)
+            {
+                value = null;
+                return false;
+            }
+
+            IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
+        }
     }
 }