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

This is reverting the revert. We're going to go ahead with this change and
work around it in MVC.

This reverts commit 0e826e69e6.

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);