Fix for issue 85 - Dictionary types should return null on key not found

This change makes RouteValueDictionary a full IDictionary implementation
instead of a subclass of Dictionary.

Followed the patterns used in the old implementation, namely preserving
the struct-returning behavior of Keys/Values/GetEnumerator.

src/Microsoft.AspNet.Routing/RouteValueDictionary.cs

@@ -2,21 +2,48 @@
 // 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.Linq;
 using System.Reflection;
 
 namespace Microsoft.AspNet.Routing
 {
-    public class RouteValueDictionary : Dictionary<string, object>
+    /// <summary>
+    /// An <see cref="IDictionary{string, object}"/> type for route values.
+    /// </summary>
+    public class RouteValueDictionary : IDictionary<string, object>
     {
+        private readonly Dictionary<string, object> _dictionary;
+
+        /// <summary>
+        /// Creates an empty RouteValueDictionary.
+        /// </summary>
         public RouteValueDictionary()
-            : base(StringComparer.OrdinalIgnoreCase)
         {
+            _dictionary = new Dictionary<string, object>(StringComparer.OrdinalIgnoreCase);
         }
 
+        /// <summary>
+        /// Creates a RouteValueDictionary initialized with the provided input values.
+        /// </summary>
+        /// <param name="values">Input values to copy into the dictionary.</param>
+        public RouteValueDictionary([NotNull] IDictionary<string, object> values)
+        {
+            _dictionary = new Dictionary<string, object>(values, StringComparer.OrdinalIgnoreCase);
+        }
+
+        /// <summary>
+        /// Creates a RouteValueDictionary initialized with the provided input values.
+        /// </summary>
+        /// <param name="values">Input values to copy into the dictionary.</param>
+        /// <remarks>
+        /// The input parameter is interpreted as a set of key-value-pairs where the property names
+        /// are keys, and property values are the values, and copied into the dictionary. Only public
+        /// instance non-index properties are considered.
+        /// </remarks>
         public RouteValueDictionary(object obj)
-            : base(StringComparer.OrdinalIgnoreCase)
+            : this()
         {
             if (obj != null)
             {
@@ -46,9 +73,162 @@ namespace Microsoft.AspNet.Routing
             }
         }
 
-        public RouteValueDictionary(IDictionary<string, object> other)
-            : base(other, StringComparer.OrdinalIgnoreCase)
+        /// <inheritdoc />
+        public object this[[NotNull] string key]
         {
+            get
+            {
+                object value;
+                _dictionary.TryGetValue(key, out value);
+                return value;
+            }
+
+            set
+            {
+                _dictionary[key] = value;
+            }
+        }
+
+        /// <summary>
+        /// Gets the comparer for this dictionary.
+        /// </summary>
+        /// <remarks>
+        /// This will always be a reference to <see cref="StringComparer.OrdinalIgnoreCase"/>
+        /// </remarks>
+        public IEqualityComparer<string> Comparer
+        {
+            get
+            {
+                return _dictionary.Comparer;
+            }
+        }
+
+        /// <inheritdoc />
+        public int Count
+        {
+            get
+            {
+                return _dictionary.Count;
+            }
+        }
+
+        /// <inheritdoc />
+        bool ICollection<KeyValuePair<string, object>>.IsReadOnly
+        {
+            get
+            {
+                return ((ICollection<KeyValuePair<string, object>>)_dictionary).IsReadOnly;
+            }
+        }
+
+        /// <inheritdoc />
+        public Dictionary<string, object>.KeyCollection Keys
+        {
+            get
+            {
+                return _dictionary.Keys;
+            }
+        }
+
+        /// <inheritdoc />
+        ICollection<string> IDictionary<string, object>.Keys
+        {
+            get
+            {
+                return _dictionary.Keys;
+            }
+        }
+
+        /// <inheritdoc />
+        public Dictionary<string, object>.ValueCollection Values
+        {
+            get
+            {
+                return _dictionary.Values;
+            }
+        }
+
+        /// <inheritdoc />
+        ICollection<object> IDictionary<string, object>.Values
+        {
+            get
+            {
+                return _dictionary.Values;
+            }
+        }
+
+        /// <inheritdoc />
+        void ICollection<KeyValuePair<string, object>>.Add(KeyValuePair<string, object> item)
+        {
+            ((ICollection<KeyValuePair<string, object>>)_dictionary).Add(item);
+        }
+
+        /// <inheritdoc />
+        public void Add([NotNull] string key, object value)
+        {
+            _dictionary.Add(key, value);
+        }
+
+        /// <inheritdoc />
+        public void Clear()
+        {
+            _dictionary.Clear();
+        }
+
+        /// <inheritdoc />
+        bool ICollection<KeyValuePair<string, object>>.Contains(KeyValuePair<string, object> item)
+        {
+            return ((ICollection<KeyValuePair<string, object>>)_dictionary).Contains(item);
+        }
+
+        /// <inheritdoc />
+        public bool ContainsKey([NotNull] string key)
+        {
+            return _dictionary.ContainsKey(key);
+        }
+
+        /// <inheritdoc />
+        void ICollection<KeyValuePair<string, object>>.CopyTo(
+            [NotNull] KeyValuePair<string, object>[] array, 
+            int arrayIndex)
+        {
+            ((ICollection<KeyValuePair<string, object>>)_dictionary).CopyTo(array, arrayIndex);
+        }
+
+        /// <inheritdoc />
+        public Dictionary<string, object>.Enumerator GetEnumerator()
+        {
+            return _dictionary.GetEnumerator();
+        }
+
+        /// <inheritdoc />
+        IEnumerator<KeyValuePair<string, object>> IEnumerable<KeyValuePair<string, object>>.GetEnumerator()
+        {
+            return _dictionary.GetEnumerator();
+        }
+
+        /// <inheritdoc />
+        IEnumerator IEnumerable.GetEnumerator()
+        {
+            return _dictionary.GetEnumerator();
+        }
+
+        /// <inheritdoc />
+        bool ICollection<KeyValuePair<string, object>>.Remove(KeyValuePair<string, object> item)
+        {
+            return ((ICollection<KeyValuePair<string, object>>)_dictionary).Remove(item);
+        }
+
+        /// <inheritdoc />
+        public bool Remove([NotNull] string key)
+        {
+            return _dictionary.Remove(key);
+        }
+
+        /// <inheritdoc />
+        public bool TryGetValue([NotNull] string key, out object value)
+        {
+            return _dictionary.TryGetValue(key, out value);
         }
     }
 }

src/Microsoft.AspNet.Routing/Template/TemplateBinder.cs

@@ -147,7 +147,7 @@ namespace Microsoft.AspNet.Routing.Template
 
             // Add any ambient values that don't match parameters - they need to be visible to constraints
             // but they will ignored by link generation.
-            var combinedValues = new Dictionary<string, object>(context.AcceptedValues, StringComparer.OrdinalIgnoreCase);
+            var combinedValues = new RouteValueDictionary(context.AcceptedValues);
             if (ambientValues != null)
             {
                 foreach (var kvp in ambientValues)
@@ -292,7 +292,6 @@ namespace Microsoft.AspNet.Routing.Template
             return null;
         }
 
-
         /// <summary>
         /// Compares two objects for equality as parts of a case-insensitive path.
         /// </summary>
@@ -342,8 +341,8 @@ namespace Microsoft.AspNet.Routing.Template
         {
             private readonly IDictionary<string, object> _defaults;
 
-            private readonly Dictionary<string, object> _acceptedValues;
-            private readonly Dictionary<string, object> _filters;
+            private readonly RouteValueDictionary _acceptedValues;
+            private readonly RouteValueDictionary _filters;
 
             public TemplateBindingContext(IDictionary<string, object> defaults, IDictionary<string, object> values)
             {
@@ -354,20 +353,20 @@ namespace Microsoft.AspNet.Routing.Template
 
                 _defaults = defaults;
 
-                _acceptedValues = new Dictionary<string, object>(StringComparer.OrdinalIgnoreCase);
+                _acceptedValues = new RouteValueDictionary();
 
                 if (_defaults != null)
                 {
-                    _filters = new Dictionary<string, object>(defaults, StringComparer.OrdinalIgnoreCase);
+                    _filters = new RouteValueDictionary(defaults);
                 }
             }
 
-            public Dictionary<string, object> AcceptedValues
+            public RouteValueDictionary AcceptedValues
             {
                 get { return _acceptedValues; }
             }
 
-            public Dictionary<string, object> Filters
+            public RouteValueDictionary Filters
             {
                 get { return _filters; }
             }

src/Microsoft.AspNet.Routing/Template/TemplateMatcher.cs

@@ -32,10 +32,10 @@ namespace Microsoft.AspNet.Routing.Template
 
             if (defaults == null)
             {
-                defaults = new Dictionary<string, object>(StringComparer.OrdinalIgnoreCase);
+                defaults = new RouteValueDictionary();
             }
 
-            var values = new Dictionary<string, object>(StringComparer.OrdinalIgnoreCase);
+            var values = new RouteValueDictionary();
 
             for (var i = 0; i < requestSegments.Length; i++)
             {
@@ -175,7 +175,7 @@ namespace Microsoft.AspNet.Routing.Template
         private bool MatchComplexSegment(TemplateSegment routeSegment,
                                          string requestSegment,
                                          IDictionary<string, object> defaults,
-                                         Dictionary<string, object> values)
+                                         RouteValueDictionary values)
         {
             Contract.Assert(routeSegment != null);
             Contract.Assert(routeSegment.Parts.Count > 1);

src/Microsoft.AspNet.Routing/Template/TemplateRoute.cs

@@ -42,7 +42,7 @@ namespace Microsoft.AspNet.Routing.Template
             _target = target;
             _routeTemplate = routeTemplate ?? string.Empty;
             Name = routeName;
-            _defaults = defaults ?? new Dictionary<string, object>(StringComparer.OrdinalIgnoreCase);
+            _defaults = defaults ?? new RouteValueDictionary();
             _constraints = RouteConstraintBuilder.BuildConstraints(constraints, _routeTemplate) ??
                                                             new Dictionary<string, IRouteConstraint>();
 

test/Microsoft.AspNet.Routing.Tests/Template/TemplateRouteTests.cs

@@ -23,7 +23,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests
 
         // PathString in HttpAbstractions guarantees a leading slash - so no value in testing other cases.
         [Fact]
-        public async void Match_Success_LeadingSlash()
+        public async Task Match_Success_LeadingSlash()
         {
             // Arrange
             var route = CreateRoute("{controller}/{action}");
@@ -40,7 +40,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests
         }
 
         [Fact]
-        public async void Match_Success_RootUrl()
+        public async Task Match_Success_RootUrl()
         {
             // Arrange
             var route = CreateRoute("");
@@ -55,7 +55,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests
         }
 
         [Fact]
-        public async void Match_Success_Defaults()
+        public async Task Match_Success_Defaults()
         {
             // Arrange
             var route = CreateRoute("{controller}/{action}", new { action = "Index" });
@@ -72,7 +72,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests
         }
 
         [Fact]
-        public async void Match_Fails()
+        public async Task Match_Fails()
         {
             // Arrange
             var route = CreateRoute("{controller}/{action}");
@@ -86,7 +86,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests
         }
 
         [Fact]
-        public async void Match_RejectedByHandler()
+        public async Task Match_RejectedByHandler()
         {
             // Arrange
             var route = CreateRoute("{controller}", accept: false);
@@ -102,6 +102,20 @@ namespace Microsoft.AspNet.Routing.Template.Tests
             Assert.NotNull(context.RouteData.Values);
         }
 
+        [Fact]
+        public async Task Match_RouteValuesDoesntThrowOnKeyNotFound()
+        {
+            // Arrange
+            var route = CreateRoute("{controller}/{action}");
+            var context = CreateRouteContext("/Home/Index");
+
+            // Act
+            await route.RouteAsync(context);
+
+            // Assert
+            Assert.Null(context.RouteData.Values["1controller"]);
+        }
+
         private static RouteContext CreateRouteContext(string requestPath)
         {
             var request = new Mock<HttpRequest>(MockBehavior.Strict);