Skip to content

Commit 1395fa8

Browse files
authored
Fix AdaptiveCapacityDictionary assert with 11 elements #34307 (#34313)
1 parent 2d241ff commit 1395fa8

File tree

5 files changed

+29
-97
lines changed

5 files changed

+29
-97
lines changed

src/Http/Http/perf/Microbenchmarks/AdaptiveCapacityDictionaryBenchmark.cs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,11 @@ public void Setup()
4040

4141
_smallCapDict = new AdaptiveCapacityDictionary<string, string>(capacity: 1, StringComparer.OrdinalIgnoreCase);
4242
_smallCapDictTen = new AdaptiveCapacityDictionary<string, string>(capacity: 10, StringComparer.OrdinalIgnoreCase);
43-
_filledSmallDictionary = new AdaptiveCapacityDictionary<string, string>(_tenValues, capacity: 10, StringComparer.OrdinalIgnoreCase);
43+
_filledSmallDictionary = new AdaptiveCapacityDictionary<string, string>(capacity: 10, StringComparer.OrdinalIgnoreCase);
44+
foreach (var a in _tenValues)
45+
{
46+
_filledSmallDictionary[a.Key] = a.Value;
47+
}
4448

4549
_dict = new Dictionary<string, string>(1, StringComparer.OrdinalIgnoreCase);
4650
_dictTen = new Dictionary<string, string>(10, StringComparer.OrdinalIgnoreCase);

src/Http/Http/src/Internal/RequestCookieCollection.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,9 @@ internal static RequestCookieCollection ParseInternal(StringValues values, bool
7070
{
7171
return Empty;
7272
}
73-
var collection = new RequestCookieCollection(values.Count);
73+
74+
// Do not set the collection capacity based on StringValues.Count, the Cookie header is supposed to be a single combined value.
75+
var collection = new RequestCookieCollection();
7476
var store = collection.Store!;
7577

7678
if (CookieHeaderParserShared.TryParseValues(values, store, enableCookieNameEncoding, supportsMultipleValues: true))

src/Http/Http/test/RequestCookiesCollectionTests.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,5 +38,13 @@ public void AppContextSwitchUnEscapesKeysAndValues(string input, string expected
3838
Assert.Equal(expectedKey, cookies.Keys.Single());
3939
Assert.Equal(expectedValue, cookies[expectedKey]);
4040
}
41+
42+
[Fact]
43+
public void ParseManyCookies()
44+
{
45+
var cookies = RequestCookieCollection.Parse(new StringValues(new[] { "a=a", "b=b", "c=c", "d=d", "e=e", "f=f", "g=g", "h=h", "i=i", "j=j", "k=k", "l=l" }));
46+
47+
Assert.Equal(12, cookies.Count);
48+
}
4149
}
4250
}

src/Shared/Dictionary/AdaptiveCapacityDictionary.cs

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -77,29 +77,6 @@ public AdaptiveCapacityDictionary(int capacity, IEqualityComparer<TKey> comparer
7777
else
7878
{
7979
_dictionaryStorage = new Dictionary<TKey, TValue>(capacity);
80-
_arrayStorage = Array.Empty<KeyValuePair<TKey, TValue>>();
81-
}
82-
}
83-
84-
/// <summary>
85-
/// Creates a <see cref="AdaptiveCapacityDictionary{TKey, TValue}"/> initialized with the specified <paramref name="values"/>.
86-
/// </summary>
87-
/// <param name="values">An object to initialize the dictionary. The value can be of type
88-
/// <see cref="IDictionary{TKey, TValue}"/> or <see cref="IReadOnlyDictionary{TKey, TValue}"/>
89-
/// or an object with public properties as key-value pairs.
90-
/// </param>
91-
/// <remarks>This constructor is unoptimized and primarily used for tests.</remarks>
92-
/// <param name="comparer">Equality comparison.</param>
93-
/// <param name="capacity">Initial capacity.</param>
94-
internal AdaptiveCapacityDictionary(IEnumerable<KeyValuePair<TKey, TValue>> values, int capacity, IEqualityComparer<TKey> comparer)
95-
{
96-
_comparer = comparer ?? EqualityComparer<TKey>.Default;
97-
98-
_arrayStorage = new KeyValuePair<TKey, TValue>[capacity];
99-
100-
foreach (var kvp in values)
101-
{
102-
Add(kvp.Key, kvp.Value);
10380
}
10481
}
10582

src/Shared/test/Shared.Tests/AdaptiveCapacityDictionaryTests.cs

Lines changed: 13 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.Collections.Generic;
6+
using System.Globalization;
67
using System.Linq;
78
using Microsoft.AspNetCore.Internal;
89
using Microsoft.AspNetCore.Testing;
@@ -38,66 +39,23 @@ public void CreateFromNull()
3839
Assert.Null(dict._dictionaryStorage);
3940
}
4041

41-
public static KeyValuePair<string, object>[] IEnumerableKeyValuePairData
42-
{
43-
get
44-
{
45-
return new[]
46-
{
47-
new KeyValuePair<string, object?>("Name", "James"),
48-
new KeyValuePair<string, object?>("Age", 30),
49-
new KeyValuePair<string, object?>("Address", new Address() { City = "Redmond", State = "WA" })
50-
};
51-
}
52-
}
53-
54-
public static KeyValuePair<string, string>[] IEnumerableStringValuePairData
55-
{
56-
get
57-
{
58-
return new[]
59-
{
60-
new KeyValuePair<string, string>("First Name", "James"),
61-
new KeyValuePair<string, string>("Last Name", "Henrik"),
62-
new KeyValuePair<string, string>("Middle Name", "Bob")
63-
};
64-
}
65-
}
66-
6742
[Fact]
68-
public void CreateFromIEnumerableKeyValuePair_CopiesValues()
43+
public void CreateWithCapacityOverDefaultLimit()
6944
{
70-
// Arrange & Act
71-
var dict = new AdaptiveCapacityDictionary<string, object?>(IEnumerableKeyValuePairData, capacity: IEnumerableKeyValuePairData.Length, EqualityComparer<string>.Default);
45+
// The default threshold between array and dictionary is 10. If we created one over that limit it should go directly to a dictionary.
46+
var dict = new AdaptiveCapacityDictionary<string, string>(capacity: 12, StringComparer.OrdinalIgnoreCase);
7247

73-
// Assert
74-
Assert.IsType<KeyValuePair<string, object?>[]>(dict._arrayStorage);
75-
Assert.Collection(
76-
dict.OrderBy(kvp => kvp.Key),
77-
kvp =>
78-
{
79-
Assert.Equal("Address", kvp.Key);
80-
var address = Assert.IsType<Address>(kvp.Value);
81-
Assert.Equal("Redmond", address.City);
82-
Assert.Equal("WA", address.State);
83-
},
84-
kvp => { Assert.Equal("Age", kvp.Key); Assert.Equal(30, kvp.Value); },
85-
kvp => { Assert.Equal("Name", kvp.Key); Assert.Equal("James", kvp.Value); });
86-
}
48+
Assert.Null(dict._arrayStorage);
49+
Assert.NotNull(dict._dictionaryStorage);
8750

88-
[Fact]
89-
public void CreateFromIEnumerableStringValuePair_CopiesValues()
90-
{
91-
// Arrange & Act
92-
var dict = new AdaptiveCapacityDictionary<string, string>(IEnumerableStringValuePairData, capacity: 3, StringComparer.OrdinalIgnoreCase);
51+
for (var i = 0; i < 12; i++)
52+
{
53+
dict[i.ToString(CultureInfo.InvariantCulture)] = i.ToString(CultureInfo.InvariantCulture);
54+
}
9355

94-
// Assert
95-
Assert.IsType<KeyValuePair<string, string>[]>(dict._arrayStorage);
96-
Assert.Collection(
97-
dict.OrderBy(kvp => kvp.Key),
98-
kvp => { Assert.Equal("First Name", kvp.Key); Assert.Equal("James", kvp.Value); },
99-
kvp => { Assert.Equal("Last Name", kvp.Key); Assert.Equal("Henrik", kvp.Value); },
100-
kvp => { Assert.Equal("Middle Name", kvp.Key); Assert.Equal("Bob", kvp.Value); });
56+
Assert.Null(dict._arrayStorage);
57+
Assert.NotNull(dict._dictionaryStorage);
58+
Assert.Equal(12, dict.Count);
10159
}
10260

10361
[Fact]
@@ -114,23 +72,6 @@ public void CreateFromIEnumerableKeyValuePair_ThrowsExceptionForDuplicateKey()
11472
$"An element with the key 'Name' already exists in the {nameof(AdaptiveCapacityDictionary<string, object?>)}.");
11573
}
11674

117-
[Fact]
118-
public void CreateFromIEnumerableStringValuePair_ThrowsExceptionForDuplicateKey()
119-
{
120-
// Arrange
121-
var values = new List<KeyValuePair<string, string>>()
122-
{
123-
new KeyValuePair<string, string>("name", "Billy"),
124-
new KeyValuePair<string, string>("Name", "Joey"),
125-
};
126-
127-
// Act & Assert
128-
ExceptionAssert.ThrowsArgument(
129-
() => new AdaptiveCapacityDictionary<string, string>(values, capacity: 3, StringComparer.OrdinalIgnoreCase),
130-
"key",
131-
$"An element with the key 'Name' already exists in the {nameof(AdaptiveCapacityDictionary<string, object>)}.");
132-
}
133-
13475
[Fact]
13576
public void Comparer_IsOrdinalIgnoreCase()
13677
{

0 commit comments

Comments
 (0)