Code refactor

ajaybhargavb · ajaybhargavb · commit 4feb51c902a4 · 2015-04-17T10:58:00.000-07:00
diff --git a/src/Microsoft.AspNet.Mvc.Core/SessionStateTempDataProvider.cs b/src/Microsoft.AspNet.Mvc.Core/SessionStateTempDataProvider.cs
@@ -35,7 +35,7 @@ public class SessionStateTempDataProvider : ITempDataProvider
 
         private static readonly ConcurrentDictionary<Type, Func<JArray, object>> _arrayConverters =
             new ConcurrentDictionary<Type, Func<JArray, object>>();
-        private static readonly ConcurrentDictionary<Type, Func<JObject, object>> _dictConverters =
+        private static readonly ConcurrentDictionary<Type, Func<JObject, object>> _dictionaryConverters =
             new ConcurrentDictionary<Type, Func<JObject, object>>();
 
         private static readonly Dictionary<JTokenType, Type> _tokenTypeLookup = new Dictionary<JTokenType, Type>
@@ -77,8 +77,9 @@ public virtual IDictionary<string, object> LoadTempData([NotNull] HttpContext co
                     var jArrayValue = item.Value as JArray;
                     if (jArrayValue != null && jArrayValue.Count > 0)
                     {
+                        var arrayType = jArrayValue[0].Type;
                         Type returnType;
-                        if (_tokenTypeLookup.TryGetValue(jArrayValue[0].Type, out returnType))
+                        if (_tokenTypeLookup.TryGetValue(arrayType, out returnType))
                         {
                             var arrayConverter = _arrayConverters.GetOrAdd(returnType, type =>
                             {
@@ -90,7 +91,7 @@ public virtual IDictionary<string, object> LoadTempData([NotNull] HttpContext co
                         }
                         else
                         {
-                            var message = Resources.FormatTempData_CannotDeserializeToken(jArrayValue[0].Type);
+                            var message = Resources.FormatTempData_CannotDeserializeToken(arrayType);
                             throw new InvalidOperationException(message);
                         }
                     }
@@ -106,7 +107,7 @@ public virtual IDictionary<string, object> LoadTempData([NotNull] HttpContext co
                         Type valueType;
                         if (_tokenTypeLookup.TryGetValue(jTokenType, out valueType))
                         {
-                            var dictConverter = _dictConverters.GetOrAdd(valueType, type =>
+                            var dictConverter = _dictionaryConverters.GetOrAdd(valueType, type =>
                             {
                                 return (Func<JObject, object>)_convertDictMethodInfo.MakeGenericMethod(type).CreateDelegate(typeof(Func<JObject, object>));
                             });
@@ -174,7 +175,7 @@ private static bool IsSessionEnabled(HttpContext context)
         internal void EnsureObjectCanBeSerialized(object item)
         {
             var itemType = item.GetType();
-            Type[] actualTypes = null;
+            Type actualType = null;
 
             if (itemType.IsArray)
             {
@@ -185,29 +186,35 @@ internal void EnsureObjectCanBeSerialized(object item)
                 if (itemType.ExtractGenericInterface(typeof(IList<>)) != null ||
                     itemType.ExtractGenericInterface(typeof(IDictionary<,>)) != null)
                 {
-                    actualTypes = itemType.GetGenericArguments();
+                    var genericTypeArguments = itemType.GetGenericArguments();
+                    if (genericTypeArguments.Length > 1)
+                    {
+                        // Throw if the key type of the dictionary is not string.
+                        if (genericTypeArguments[0] != typeof(string))
+                        {
+                            var message = Resources.FormatTempData_CannotSerializeDictionary(genericTypeArguments[0],
+                                typeof(SessionStateTempDataProvider).FullName);
+                            throw new InvalidOperationException(message);
+                        }
+                        else
+                        {
+                            actualType = genericTypeArguments[1];
+                        }
+                    }
+                    else
+                    {
+                        actualType = genericTypeArguments[0];
+                    }
                 }
             }
 
-            actualTypes = actualTypes ?? new Type[] { itemType };
-
-            // Throw if the key type of the dictionary is not string.
-            if (actualTypes.Length > 1 && actualTypes[0] != typeof(string))
-            {
-                var message = Resources.FormatTempData_CannotSerializeDictionary(actualTypes[0],
-                        typeof(SessionStateTempDataProvider).FullName);
-                throw new InvalidOperationException(message);
-            }
-
-            foreach (var actualType in actualTypes)
+            actualType = actualType ?? itemType;
+            if (!TypeHelper.IsSimpleType(actualType))
             {
                 var underlyingType = Nullable.GetUnderlyingType(actualType) ?? actualType;
-                if (!TypeHelper.IsSimpleType(actualType))
-                {
-                    var message = Resources.FormatTempData_CannotSerializeToSession(underlyingType,
-                        typeof(SessionStateTempDataProvider).FullName);
-                    throw new InvalidOperationException(message);
-                }
+                var message = Resources.FormatTempData_CannotSerializeToSession(underlyingType,
+                    typeof(SessionStateTempDataProvider).FullName);
+                throw new InvalidOperationException(message);
             }
         }
 
@@ -216,12 +223,12 @@ private static IList<TVal> ConvertArray<TVal>(JArray array)
             return array.Values<TVal>().ToArray();
         }
 
-        private static IDictionary<string, T> ConvertDict<T>(JObject jObject)
+        private static IDictionary<string, TVal> ConvertDict<TVal>(JObject jObject)
         {
-            var convertedDict = new Dictionary<string, T>();
+            var convertedDict = new Dictionary<string, TVal>(StringComparer.Ordinal);
             foreach (var item in jObject)
             {
-                convertedDict.Add(item.Key, jObject.Value<T>(item.Key));
+                convertedDict.Add(item.Key, jObject.Value<TVal>(item.Key));
             }
             return convertedDict;
         }
diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/SessionStateTempDataProviderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/SessionStateTempDataProviderTest.cs
@@ -170,12 +170,12 @@ public void EnsureObjectCanBeSerialized_ValidType_DoesNotThrow(object value)
         }
 
         [Fact]
-        public void SaveAndLoad_WorksAsExpected()
+        public void SaveAndLoad_SimpleTypesCanBeStoredAndLoaded()
         {
             // Arrange
             var testProvider = new SessionStateTempDataProvider();
             var inputGuid = Guid.NewGuid();
-            var inputDict = new Dictionary<string, string>
+            var inputDictionary = new Dictionary<string, string>
             {
                 { "Hello", "World" },
             };
@@ -187,27 +187,31 @@ public void SaveAndLoad_WorksAsExpected()
                 { "DateTime", new DateTime() },
                 { "Guid", inputGuid },
                 { "List`string", new List<string> { "one", "two" } },
-                { "Dictionary", inputDict },
+                { "Dictionary", inputDictionary },
             };
             var context = GetHttpContext(new TestSessionCollection(), true);
 
             // Act
-            //System.Diagnostics.Debugger.Launch();
             testProvider.SaveTempData(context, input);
             var TempData = testProvider.LoadTempData(context);
 
             // Assert
-            Assert.Equal("value", TempData["string"]);
-            Assert.Equal(10, Convert.ToInt32(TempData["int"]));
-            Assert.Equal(false, (bool)TempData["bool"]);
-            Assert.Equal(new DateTime().ToString(), ((DateTime)TempData["DateTime"]).ToString());
-            Assert.Equal(inputGuid.ToString(), ((Guid)TempData["Guid"]).ToString());
+            var stringVal = Assert.IsType<string>(TempData["string"]);
+            Assert.Equal("value", stringVal);
+            var intVal = Convert.ToInt32(TempData["int"]);
+            Assert.Equal(10, intVal);
+            var boolVal = Assert.IsType<bool>(TempData["bool"]);
+            Assert.Equal(false, boolVal);
+            var datetimeVal = Assert.IsType<DateTime>(TempData["DateTime"]);
+            Assert.Equal(new DateTime().ToString(), datetimeVal.ToString());
+            var guidVal = Assert.IsType<Guid>(TempData["Guid"]);
+            Assert.Equal(inputGuid.ToString(), guidVal.ToString());
             var list = (IList<string>)TempData["List`string"];
             Assert.Equal(2, list.Count);
             Assert.Equal("one", list[0]);
             Assert.Equal("two", list[1]);
-            var dict = (IDictionary<string, string>)TempData["Dictionary"];
-            Assert.Equal("World", dict["Hello"]);
+            var dictionary = Assert.IsType<Dictionary<string, string>>(TempData["Dictionary"]);
+            Assert.Equal("World", dictionary["Hello"]);
         }
 
         private class TestItem
@@ -239,49 +243,49 @@ private HttpContext GetHttpContext(ISessionCollection session, bool sessionEnabl
 
         private class TestSessionCollection : ISessionCollection
         {
-            private Dictionary<string, byte[]> _innerDict = new Dictionary<string, byte[]>();
+            private Dictionary<string, byte[]> _innerDictionary = new Dictionary<string, byte[]>();
 
             public byte[] this[string key]
             {
                 get
                 {
-                    return _innerDict[key];
+                    return _innerDictionary[key];
                 }
 
                 set
                 {
-                    _innerDict[key] = value;
+                    _innerDictionary[key] = value;
                 }
             }
 
             public void Clear()
             {
-                _innerDict.Clear();
+                _innerDictionary.Clear();
             }
 
             public IEnumerator<KeyValuePair<string, byte[]>> GetEnumerator()
             {
-                return _innerDict.GetEnumerator();
+                return _innerDictionary.GetEnumerator();
             }
 
             public void Remove(string key)
             {
-                _innerDict.Remove(key);
+                _innerDictionary.Remove(key);
             }
 
             public void Set(string key, ArraySegment<byte> value)
             {
-                _innerDict[key] = value.AsArray();
+                _innerDictionary[key] = value.AsArray();
             }
 
             public bool TryGetValue(string key, out byte[] value)
             {
-                return _innerDict.TryGetValue(key, out value);
+                return _innerDictionary.TryGetValue(key, out value);
             }
 
             IEnumerator IEnumerable.GetEnumerator()
             {
-                return _innerDict.GetEnumerator();
+                return _innerDictionary.GetEnumerator();
             }
         }
     }

Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,7 @@ public class SessionStateTempDataProvider : ITempDataProvider`
`35`	`35`
`36`	`36`	`private static readonly ConcurrentDictionary<Type, Func<JArray, object>> _arrayConverters =`
`37`	`37`	`new ConcurrentDictionary<Type, Func<JArray, object>>();`
`38`		`- private static readonly ConcurrentDictionary<Type, Func<JObject, object>> _dictConverters =`
	`38`	`+ private static readonly ConcurrentDictionary<Type, Func<JObject, object>> _dictionaryConverters =`
`39`	`39`	`new ConcurrentDictionary<Type, Func<JObject, object>>();`
`40`	`40`
`41`	`41`	`private static readonly Dictionary<JTokenType, Type> _tokenTypeLookup = new Dictionary<JTokenType, Type>`
`@@ -77,8 +77,9 @@ public virtual IDictionary<string, object> LoadTempData([NotNull] HttpContext co`
`77`	`77`	`var jArrayValue = item.Value as JArray;`
`78`	`78`	`if (jArrayValue != null && jArrayValue.Count > 0)`
`79`	`79`	`{`
	`80`	`+ var arrayType = jArrayValue[0].Type;`
`80`	`81`	`Type returnType;`
`81`		`- if (_tokenTypeLookup.TryGetValue(jArrayValue[0].Type, out returnType))`
	`82`	`+ if (_tokenTypeLookup.TryGetValue(arrayType, out returnType))`
`82`	`83`	`{`
`83`	`84`	`var arrayConverter = _arrayConverters.GetOrAdd(returnType, type =>`
`84`	`85`	`{`
`@@ -90,7 +91,7 @@ public virtual IDictionary<string, object> LoadTempData([NotNull] HttpContext co`
`90`	`91`	`}`
`91`	`92`	`else`
`92`	`93`	`{`
`93`		`- var message = Resources.FormatTempData_CannotDeserializeToken(jArrayValue[0].Type);`
	`94`	`+ var message = Resources.FormatTempData_CannotDeserializeToken(arrayType);`
`94`	`95`	`throw new InvalidOperationException(message);`
`95`	`96`	`}`
`96`	`97`	`}`
`@@ -106,7 +107,7 @@ public virtual IDictionary<string, object> LoadTempData([NotNull] HttpContext co`
`106`	`107`	`Type valueType;`
`107`	`108`	`if (_tokenTypeLookup.TryGetValue(jTokenType, out valueType))`
`108`	`109`	`{`
`109`		`- var dictConverter = _dictConverters.GetOrAdd(valueType, type =>`
	`110`	`+ var dictConverter = _dictionaryConverters.GetOrAdd(valueType, type =>`
`110`	`111`	`{`
`111`	`112`	`return (Func<JObject, object>)_convertDictMethodInfo.MakeGenericMethod(type).CreateDelegate(typeof(Func<JObject, object>));`
`112`	`113`	`});`
`@@ -174,7 +175,7 @@ private static bool IsSessionEnabled(HttpContext context)`
`174`	`175`	`internal void EnsureObjectCanBeSerialized(object item)`
`175`	`176`	`{`
`176`	`177`	`var itemType = item.GetType();`
`177`		`- Type[] actualTypes = null;`
	`178`	`+ Type actualType = null;`
`178`	`179`
`179`	`180`	`if (itemType.IsArray)`
`180`	`181`	`{`
`@@ -185,29 +186,35 @@ internal void EnsureObjectCanBeSerialized(object item)`
`185`	`186`	`if (itemType.ExtractGenericInterface(typeof(IList<>)) != null \|\|`
`186`	`187`	`itemType.ExtractGenericInterface(typeof(IDictionary<,>)) != null)`
`187`	`188`	`{`
`188`		`- actualTypes = itemType.GetGenericArguments();`
	`189`	`+ var genericTypeArguments = itemType.GetGenericArguments();`
	`190`	`+ if (genericTypeArguments.Length > 1)`
	`191`	`+ {`
	`192`	`+ // Throw if the key type of the dictionary is not string.`
	`193`	`+ if (genericTypeArguments[0] != typeof(string))`
	`194`	`+ {`
	`195`	`+ var message = Resources.FormatTempData_CannotSerializeDictionary(genericTypeArguments[0],`
	`196`	`+ typeof(SessionStateTempDataProvider).FullName);`
	`197`	`+ throw new InvalidOperationException(message);`
	`198`	`+ }`
	`199`	`+ else`
	`200`	`+ {`
	`201`	`+ actualType = genericTypeArguments[1];`
	`202`	`+ }`
	`203`	`+ }`
	`204`	`+ else`
	`205`	`+ {`
	`206`	`+ actualType = genericTypeArguments[0];`
	`207`	`+ }`
`189`	`208`	`}`
`190`	`209`	`}`
`191`	`210`
`192`		`- actualTypes = actualTypes ?? new Type[] { itemType };`
`193`		`-`
`194`		`- // Throw if the key type of the dictionary is not string.`
`195`		`- if (actualTypes.Length > 1 && actualTypes[0] != typeof(string))`
`196`		`- {`
`197`		`- var message = Resources.FormatTempData_CannotSerializeDictionary(actualTypes[0],`
`198`		`- typeof(SessionStateTempDataProvider).FullName);`
`199`		`- throw new InvalidOperationException(message);`
`200`		`- }`
`201`		`-`
`202`		`- foreach (var actualType in actualTypes)`
	`211`	`+ actualType = actualType ?? itemType;`
	`212`	`+ if (!TypeHelper.IsSimpleType(actualType))`
`203`	`213`	`{`
`204`	`214`	`var underlyingType = Nullable.GetUnderlyingType(actualType) ?? actualType;`
`205`		`- if (!TypeHelper.IsSimpleType(actualType))`
`206`		`- {`
`207`		`- var message = Resources.FormatTempData_CannotSerializeToSession(underlyingType,`
`208`		`- typeof(SessionStateTempDataProvider).FullName);`
`209`		`- throw new InvalidOperationException(message);`
`210`		`- }`
	`215`	`+ var message = Resources.FormatTempData_CannotSerializeToSession(underlyingType,`
	`216`	`+ typeof(SessionStateTempDataProvider).FullName);`
	`217`	`+ throw new InvalidOperationException(message);`
`211`	`218`	`}`
`212`	`219`	`}`
`213`	`220`
`@@ -216,12 +223,12 @@ private static IList<TVal> ConvertArray<TVal>(JArray array)`
`216`	`223`	`return array.Values<TVal>().ToArray();`
`217`	`224`	`}`
`218`	`225`
`219`		`- private static IDictionary<string, T> ConvertDict<T>(JObject jObject)`
	`226`	`+ private static IDictionary<string, TVal> ConvertDict<TVal>(JObject jObject)`
`220`	`227`	`{`
`221`		`- var convertedDict = new Dictionary<string, T>();`
	`228`	`+ var convertedDict = new Dictionary<string, TVal>(StringComparer.Ordinal);`
`222`	`229`	`foreach (var item in jObject)`
`223`	`230`	`{`
`224`		`- convertedDict.Add(item.Key, jObject.Value<T>(item.Key));`
	`231`	`+ convertedDict.Add(item.Key, jObject.Value<TVal>(item.Key));`
`225`	`232`	`}`
`226`	`233`	`return convertedDict;`
`227`	`234`	`}`
Original file line number	Diff line number	Diff line change
`@@ -170,12 +170,12 @@ public void EnsureObjectCanBeSerialized_ValidType_DoesNotThrow(object value)`
`170`	`170`	`}`
`171`	`171`
`172`	`172`	`[Fact]`
`173`		`- public void SaveAndLoad_WorksAsExpected()`
	`173`	`+ public void SaveAndLoad_SimpleTypesCanBeStoredAndLoaded()`
`174`	`174`	`{`
`175`	`175`	`// Arrange`
`176`	`176`	`var testProvider = new SessionStateTempDataProvider();`
`177`	`177`	`var inputGuid = Guid.NewGuid();`
`178`		`- var inputDict = new Dictionary<string, string>`
	`178`	`+ var inputDictionary = new Dictionary<string, string>`
`179`	`179`	`{`
`180`	`180`	`{ "Hello", "World" },`
`181`	`181`	`};`
`@@ -187,27 +187,31 @@ public void SaveAndLoad_WorksAsExpected()`
`187`	`187`	`{ "DateTime", new DateTime() },`
`188`	`188`	`{ "Guid", inputGuid },`
`189`	`189`	{ "List`string", new List<string> { "one", "two" } },
`190`		`- { "Dictionary", inputDict },`
	`190`	`+ { "Dictionary", inputDictionary },`
`191`	`191`	`};`
`192`	`192`	`var context = GetHttpContext(new TestSessionCollection(), true);`
`193`	`193`
`194`	`194`	`// Act`
`195`		`- //System.Diagnostics.Debugger.Launch();`
`196`	`195`	`testProvider.SaveTempData(context, input);`
`197`	`196`	`var TempData = testProvider.LoadTempData(context);`
`198`	`197`
`199`	`198`	`// Assert`
`200`		`- Assert.Equal("value", TempData["string"]);`
`201`		`- Assert.Equal(10, Convert.ToInt32(TempData["int"]));`
`202`		`- Assert.Equal(false, (bool)TempData["bool"]);`
`203`		`- Assert.Equal(new DateTime().ToString(), ((DateTime)TempData["DateTime"]).ToString());`
`204`		`- Assert.Equal(inputGuid.ToString(), ((Guid)TempData["Guid"]).ToString());`
	`199`	`+ var stringVal = Assert.IsType<string>(TempData["string"]);`
	`200`	`+ Assert.Equal("value", stringVal);`
	`201`	`+ var intVal = Convert.ToInt32(TempData["int"]);`
	`202`	`+ Assert.Equal(10, intVal);`
	`203`	`+ var boolVal = Assert.IsType<bool>(TempData["bool"]);`
	`204`	`+ Assert.Equal(false, boolVal);`
	`205`	`+ var datetimeVal = Assert.IsType<DateTime>(TempData["DateTime"]);`
	`206`	`+ Assert.Equal(new DateTime().ToString(), datetimeVal.ToString());`
	`207`	`+ var guidVal = Assert.IsType<Guid>(TempData["Guid"]);`
	`208`	`+ Assert.Equal(inputGuid.ToString(), guidVal.ToString());`
`205`	`209`	var list = (IList<string>)TempData["List`string"];
`206`	`210`	`Assert.Equal(2, list.Count);`
`207`	`211`	`Assert.Equal("one", list[0]);`
`208`	`212`	`Assert.Equal("two", list[1]);`
`209`		`- var dict = (IDictionary<string, string>)TempData["Dictionary"];`
`210`		`- Assert.Equal("World", dict["Hello"]);`
	`213`	`+ var dictionary = Assert.IsType<Dictionary<string, string>>(TempData["Dictionary"]);`
	`214`	`+ Assert.Equal("World", dictionary["Hello"]);`
`211`	`215`	`}`
`212`	`216`
`213`	`217`	`private class TestItem`
`@@ -239,49 +243,49 @@ private HttpContext GetHttpContext(ISessionCollection session, bool sessionEnabl`
`239`	`243`
`240`	`244`	`private class TestSessionCollection : ISessionCollection`
`241`	`245`	`{`
`242`		`- private Dictionary<string, byte[]> _innerDict = new Dictionary<string, byte[]>();`
	`246`	`+ private Dictionary<string, byte[]> _innerDictionary = new Dictionary<string, byte[]>();`
`243`	`247`
`244`	`248`	`public byte[] this[string key]`
`245`	`249`	`{`
`246`	`250`	`get`
`247`	`251`	`{`
`248`		`- return _innerDict[key];`
	`252`	`+ return _innerDictionary[key];`
`249`	`253`	`}`
`250`	`254`
`251`	`255`	`set`
`252`	`256`	`{`
`253`		`- _innerDict[key] = value;`
	`257`	`+ _innerDictionary[key] = value;`
`254`	`258`	`}`
`255`	`259`	`}`
`256`	`260`
`257`	`261`	`public void Clear()`
`258`	`262`	`{`
`259`		`- _innerDict.Clear();`
	`263`	`+ _innerDictionary.Clear();`
`260`	`264`	`}`
`261`	`265`
`262`	`266`	`public IEnumerator<KeyValuePair<string, byte[]>> GetEnumerator()`
`263`	`267`	`{`
`264`		`- return _innerDict.GetEnumerator();`
	`268`	`+ return _innerDictionary.GetEnumerator();`
`265`	`269`	`}`
`266`	`270`
`267`	`271`	`public void Remove(string key)`
`268`	`272`	`{`
`269`		`- _innerDict.Remove(key);`
	`273`	`+ _innerDictionary.Remove(key);`
`270`	`274`	`}`
`271`	`275`
`272`	`276`	`public void Set(string key, ArraySegment<byte> value)`
`273`	`277`	`{`
`274`		`- _innerDict[key] = value.AsArray();`
	`278`	`+ _innerDictionary[key] = value.AsArray();`
`275`	`279`	`}`
`276`	`280`
`277`	`281`	`public bool TryGetValue(string key, out byte[] value)`
`278`	`282`	`{`
`279`		`- return _innerDict.TryGetValue(key, out value);`
	`283`	`+ return _innerDictionary.TryGetValue(key, out value);`
`280`	`284`	`}`
`281`	`285`
`282`	`286`	`IEnumerator IEnumerable.GetEnumerator()`
`283`	`287`	`{`
`284`		`- return _innerDict.GetEnumerator();`
	`288`	`+ return _innerDictionary.GetEnumerator();`
`285`	`289`	`}`
`286`	`290`	`}`
`287`	`291`	`}`