From 61436fb7d128e016090c0572b9b4a42156f71e95 Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Thu, 31 Jul 2014 15:01:03 -0700 Subject: [PATCH] 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 0e826e69e69a90f96444b912a188c2bf234da35d. --- .../RouteValueDictionary.cs | 190 +++++++++++++++++- .../Template/TemplateBinder.cs | 15 +- .../Template/TemplateMatcher.cs | 6 +- .../Template/TemplateRoute.cs | 2 +- .../Template/TemplateRouteTests.cs | 24 ++- 5 files changed, 215 insertions(+), 22 deletions(-) diff --git a/src/Microsoft.AspNet.Routing/RouteValueDictionary.cs b/src/Microsoft.AspNet.Routing/RouteValueDictionary.cs index 6c9ee06fdf..13a67420b3 100644 --- a/src/Microsoft.AspNet.Routing/RouteValueDictionary.cs +++ b/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 + /// + /// An type for route values. + /// + public class RouteValueDictionary : IDictionary { + private readonly Dictionary _dictionary; + + /// + /// Creates an empty RouteValueDictionary. + /// public RouteValueDictionary() - : base(StringComparer.OrdinalIgnoreCase) { + _dictionary = new Dictionary(StringComparer.OrdinalIgnoreCase); } + /// + /// Creates a RouteValueDictionary initialized with the provided input values. + /// + /// Input values to copy into the dictionary. + public RouteValueDictionary([NotNull] IDictionary values) + { + _dictionary = new Dictionary(values, StringComparer.OrdinalIgnoreCase); + } + + /// + /// Creates a RouteValueDictionary initialized with the provided input values. + /// + /// Input values to copy into the dictionary. + /// + /// 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. + /// public RouteValueDictionary(object obj) - : base(StringComparer.OrdinalIgnoreCase) + : this() { if (obj != null) { @@ -46,9 +73,162 @@ namespace Microsoft.AspNet.Routing } } - public RouteValueDictionary(IDictionary other) - : base(other, StringComparer.OrdinalIgnoreCase) + /// + public object this[[NotNull] string key] { + get + { + object value; + _dictionary.TryGetValue(key, out value); + return value; + } + + set + { + _dictionary[key] = value; + } + } + + /// + /// Gets the comparer for this dictionary. + /// + /// + /// This will always be a reference to + /// + public IEqualityComparer Comparer + { + get + { + return _dictionary.Comparer; + } + } + + /// + public int Count + { + get + { + return _dictionary.Count; + } + } + + /// + bool ICollection>.IsReadOnly + { + get + { + return ((ICollection>)_dictionary).IsReadOnly; + } + } + + /// + public Dictionary.KeyCollection Keys + { + get + { + return _dictionary.Keys; + } + } + + /// + ICollection IDictionary.Keys + { + get + { + return _dictionary.Keys; + } + } + + /// + public Dictionary.ValueCollection Values + { + get + { + return _dictionary.Values; + } + } + + /// + ICollection IDictionary.Values + { + get + { + return _dictionary.Values; + } + } + + /// + void ICollection>.Add(KeyValuePair item) + { + ((ICollection>)_dictionary).Add(item); + } + + /// + public void Add([NotNull] string key, object value) + { + _dictionary.Add(key, value); + } + + /// + public void Clear() + { + _dictionary.Clear(); + } + + /// + bool ICollection>.Contains(KeyValuePair item) + { + return ((ICollection>)_dictionary).Contains(item); + } + + /// + public bool ContainsKey([NotNull] string key) + { + return _dictionary.ContainsKey(key); + } + + /// + void ICollection>.CopyTo( + [NotNull] KeyValuePair[] array, + int arrayIndex) + { + ((ICollection>)_dictionary).CopyTo(array, arrayIndex); + } + + /// + public Dictionary.Enumerator GetEnumerator() + { + return _dictionary.GetEnumerator(); + } + + /// + IEnumerator> IEnumerable>.GetEnumerator() + { + return _dictionary.GetEnumerator(); + } + + /// + IEnumerator IEnumerable.GetEnumerator() + { + return _dictionary.GetEnumerator(); + } + + /// + bool ICollection>.Remove(KeyValuePair item) + { + return ((ICollection>)_dictionary).Remove(item); + } + + /// + public bool Remove([NotNull] string key) + { + return _dictionary.Remove(key); + } + + /// + public bool TryGetValue([NotNull] string key, out object value) + { + return _dictionary.TryGetValue(key, out value); } } } diff --git a/src/Microsoft.AspNet.Routing/Template/TemplateBinder.cs b/src/Microsoft.AspNet.Routing/Template/TemplateBinder.cs index 17f259a193..cc7bea4008 100644 --- a/src/Microsoft.AspNet.Routing/Template/TemplateBinder.cs +++ b/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(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; } - /// /// Compares two objects for equality as parts of a case-insensitive path. /// @@ -342,8 +341,8 @@ namespace Microsoft.AspNet.Routing.Template { private readonly IDictionary _defaults; - private readonly Dictionary _acceptedValues; - private readonly Dictionary _filters; + private readonly RouteValueDictionary _acceptedValues; + private readonly RouteValueDictionary _filters; public TemplateBindingContext(IDictionary defaults, IDictionary values) { @@ -354,20 +353,20 @@ namespace Microsoft.AspNet.Routing.Template _defaults = defaults; - _acceptedValues = new Dictionary(StringComparer.OrdinalIgnoreCase); + _acceptedValues = new RouteValueDictionary(); if (_defaults != null) { - _filters = new Dictionary(defaults, StringComparer.OrdinalIgnoreCase); + _filters = new RouteValueDictionary(defaults); } } - public Dictionary AcceptedValues + public RouteValueDictionary AcceptedValues { get { return _acceptedValues; } } - public Dictionary Filters + public RouteValueDictionary Filters { get { return _filters; } } diff --git a/src/Microsoft.AspNet.Routing/Template/TemplateMatcher.cs b/src/Microsoft.AspNet.Routing/Template/TemplateMatcher.cs index ed6ff5e9db..bdef28549f 100644 --- a/src/Microsoft.AspNet.Routing/Template/TemplateMatcher.cs +++ b/src/Microsoft.AspNet.Routing/Template/TemplateMatcher.cs @@ -32,10 +32,10 @@ namespace Microsoft.AspNet.Routing.Template if (defaults == null) { - defaults = new Dictionary(StringComparer.OrdinalIgnoreCase); + defaults = new RouteValueDictionary(); } - var values = new Dictionary(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 defaults, - Dictionary values) + RouteValueDictionary values) { Contract.Assert(routeSegment != null); Contract.Assert(routeSegment.Parts.Count > 1); diff --git a/src/Microsoft.AspNet.Routing/Template/TemplateRoute.cs b/src/Microsoft.AspNet.Routing/Template/TemplateRoute.cs index 382fc01d57..e6c774789d 100644 --- a/src/Microsoft.AspNet.Routing/Template/TemplateRoute.cs +++ b/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(StringComparer.OrdinalIgnoreCase); + _defaults = defaults ?? new RouteValueDictionary(); _constraints = RouteConstraintBuilder.BuildConstraints(constraints, _routeTemplate) ?? new Dictionary(); diff --git a/test/Microsoft.AspNet.Routing.Tests/Template/TemplateRouteTests.cs b/test/Microsoft.AspNet.Routing.Tests/Template/TemplateRouteTests.cs index df90bece69..d2d29f23c7 100644 --- a/test/Microsoft.AspNet.Routing.Tests/Template/TemplateRouteTests.cs +++ b/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(MockBehavior.Strict);