diff --git a/src/Microsoft.AspNetCore.Routing.Abstractions/RouteValueDictionary.cs b/src/Microsoft.AspNetCore.Routing.Abstractions/RouteValueDictionary.cs index ca132a78..55b2438e 100644 --- a/src/Microsoft.AspNetCore.Routing.Abstractions/RouteValueDictionary.cs +++ b/src/Microsoft.AspNetCore.Routing.Abstractions/RouteValueDictionary.cs @@ -388,7 +388,9 @@ public bool Remove(string key) return false; } - EnsureCapacity(Count); + // Ensure property storage is converted to array storage as we'll be + // applying the lookup and removal on the array + EnsureCapacity(_count); var index = FindIndex(key); if (index >= 0) @@ -404,6 +406,48 @@ public bool Remove(string key) return false; } + /// + /// Attempts to remove and return the value that has the specified key from the . + /// + /// The key of the element to remove and return. + /// When this method returns, contains the object removed from the , or null if key does not exist. + /// + /// true if the object was removed successfully; otherwise, false. + /// + public bool Remove(string key, out object value) + { + if (key == null) + { + ThrowArgumentNullExceptionForKey(); + } + + if (_count == 0) + { + value = default; + return false; + } + + // Ensure property storage is converted to array storage as we'll be + // applying the lookup and removal on the array + EnsureCapacity(_count); + + var index = FindIndex(key); + if (index >= 0) + { + _count--; + var array = _arrayStorage; + value = array[index].Value; + Array.Copy(array, index + 1, array, index, _count - index); + array[_count] = default; + + return true; + } + + value = default; + return false; + } + + /// /// Attempts to the add the provided and to the dictionary. /// diff --git a/src/Microsoft.AspNetCore.Routing/Template/TemplateBinder.cs b/src/Microsoft.AspNetCore.Routing/Template/TemplateBinder.cs index 40755676..4236c3c0 100644 --- a/src/Microsoft.AspNetCore.Routing/Template/TemplateBinder.cs +++ b/src/Microsoft.AspNetCore.Routing/Template/TemplateBinder.cs @@ -471,11 +471,7 @@ private bool TryBindValuesCore(UriBuildingContext context, RouteValueDictionary else if (part is RoutePatternParameterPart parameterPart) { // If it's a parameter, get its value - var hasValue = acceptedValues.TryGetValue(parameterPart.Name, out var value); - if (hasValue) - { - acceptedValues.Remove(parameterPart.Name); - } + acceptedValues.Remove(parameterPart.Name, out var value); var isSameAsDefault = false; if (_defaults != null && diff --git a/test/Microsoft.AspNetCore.Mvc.Routing.Abstractions.Tests/RouteValueDictionaryTests.cs b/test/Microsoft.AspNetCore.Mvc.Routing.Abstractions.Tests/RouteValueDictionaryTests.cs index 1d4ec848..35b8031f 100644 --- a/test/Microsoft.AspNetCore.Mvc.Routing.Abstractions.Tests/RouteValueDictionaryTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.Routing.Abstractions.Tests/RouteValueDictionaryTests.cs @@ -1393,6 +1393,235 @@ public void Remove_ListStorage_True_CaseInsensitive() Assert.IsType[]>(dict._arrayStorage); } + + [Fact] + public void Remove_KeyAndOutValue_EmptyStorage() + { + // Arrange + var dict = new RouteValueDictionary(); + + // Act + var result = dict.Remove("key", out var removedValue); + + // Assert + Assert.False(result); + Assert.Null(removedValue); + } + + [Fact] + public void Remove_KeyAndOutValue_EmptyStringIsAllowed() + { + // Arrange + var dict = new RouteValueDictionary(); + + // Act + var result = dict.Remove("", out var removedValue); + + // Assert + Assert.False(result); + Assert.Null(removedValue); + } + + [Fact] + public void Remove_KeyAndOutValue_PropertyStorage_Empty() + { + // Arrange + var dict = new RouteValueDictionary(new { }); + + // Act + var result = dict.Remove("other", out var removedValue); + + // Assert + Assert.False(result); + Assert.Null(removedValue); + Assert.Empty(dict); + Assert.NotNull(dict._propertyStorage); + } + + [Fact] + public void Remove_KeyAndOutValue_PropertyStorage_False() + { + // Arrange + var dict = new RouteValueDictionary(new { key = "value" }); + + // Act + var result = dict.Remove("other", out var removedValue); + + // Assert + Assert.False(result); + Assert.Null(removedValue); + Assert.Collection(dict, kvp => { Assert.Equal("key", kvp.Key); Assert.Equal("value", kvp.Value); }); + Assert.IsType[]>(dict._arrayStorage); + } + + [Fact] + public void Remove_KeyAndOutValue_PropertyStorage_True() + { + // Arrange + object value = "value"; + var dict = new RouteValueDictionary(new { key = value }); + + // Act + var result = dict.Remove("key", out var removedValue); + + // Assert + Assert.True(result); + Assert.Same(value, removedValue); + Assert.Empty(dict); + Assert.IsType[]>(dict._arrayStorage); + } + + [Fact] + public void Remove_KeyAndOutValue_PropertyStorage_True_CaseInsensitive() + { + // Arrange + object value = "value"; + var dict = new RouteValueDictionary(new { key = value }); + + // Act + var result = dict.Remove("kEy", out var removedValue); + + // Assert + Assert.True(result); + Assert.Same(value, removedValue); + Assert.Empty(dict); + Assert.IsType[]>(dict._arrayStorage); + } + + [Fact] + public void Remove_KeyAndOutValue_ListStorage_False() + { + // Arrange + var dict = new RouteValueDictionary() + { + { "key", "value" }, + }; + + // Act + var result = dict.Remove("other", out var removedValue); + + // Assert + Assert.False(result); + Assert.Null(removedValue); + Assert.Collection(dict, kvp => { Assert.Equal("key", kvp.Key); Assert.Equal("value", kvp.Value); }); + Assert.IsType[]>(dict._arrayStorage); + } + + [Fact] + public void Remove_KeyAndOutValue_ListStorage_True() + { + // Arrange + object value = "value"; + var dict = new RouteValueDictionary() + { + { "key", value } + }; + + // Act + var result = dict.Remove("key", out var removedValue); + + // Assert + Assert.True(result); + Assert.Same(value, removedValue); + Assert.Empty(dict); + Assert.IsType[]>(dict._arrayStorage); + } + + [Fact] + public void Remove_KeyAndOutValue_ListStorage_True_CaseInsensitive() + { + // Arrange + object value = "value"; + var dict = new RouteValueDictionary() + { + { "key", value } + }; + + // Act + var result = dict.Remove("kEy", out var removedValue); + + // Assert + Assert.True(result); + Assert.Same(value, removedValue); + Assert.Empty(dict); + Assert.IsType[]>(dict._arrayStorage); + } + + [Fact] + public void Remove_KeyAndOutValue_ListStorage_KeyExists_First() + { + // Arrange + object value = "value"; + var dict = new RouteValueDictionary() + { + { "key", value }, + { "other", 5 }, + { "dotnet", "rocks" } + }; + + // Act + var result = dict.Remove("key", out var removedValue); + + // Assert + Assert.True(result); + Assert.Same(value, removedValue); + Assert.Equal(2, dict.Count); + Assert.False(dict.ContainsKey("key")); + Assert.True(dict.ContainsKey("other")); + Assert.True(dict.ContainsKey("dotnet")); + Assert.IsType[]>(dict._arrayStorage); + } + + [Fact] + public void Remove_KeyAndOutValue_ListStorage_KeyExists_Middle() + { + // Arrange + object value = "value"; + var dict = new RouteValueDictionary() + { + { "other", 5 }, + { "key", value }, + { "dotnet", "rocks" } + }; + + // Act + var result = dict.Remove("key", out var removedValue); + + // Assert + Assert.True(result); + Assert.Same(value, removedValue); + Assert.Equal(2, dict.Count); + Assert.False(dict.ContainsKey("key")); + Assert.True(dict.ContainsKey("other")); + Assert.True(dict.ContainsKey("dotnet")); + Assert.IsType[]>(dict._arrayStorage); + } + + [Fact] + public void Remove_KeyAndOutValue_ListStorage_KeyExists_Last() + { + // Arrange + object value = "value"; + var dict = new RouteValueDictionary() + { + { "other", 5 }, + { "dotnet", "rocks" }, + { "key", value } + }; + + // Act + var result = dict.Remove("key", out var removedValue); + + // Assert + Assert.True(result); + Assert.Same(value, removedValue); + Assert.Equal(2, dict.Count); + Assert.False(dict.ContainsKey("key")); + Assert.True(dict.ContainsKey("other")); + Assert.True(dict.ContainsKey("dotnet")); + Assert.IsType[]>(dict._arrayStorage); + } + [Fact] public void TryAdd_EmptyStringIsAllowed() {