Skip to content

Commit 41a4dd7

Browse files
committed
PR feedback
1 parent b9f61a0 commit 41a4dd7

File tree

4 files changed

+32
-152
lines changed

4 files changed

+32
-152
lines changed

src/Http/Http/src/Features/QueryFeature.cs

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -132,16 +132,7 @@ public IQueryCollection Query
132132

133133
if (equalIndex >= 0)
134134
{
135-
var i = 0;
136-
for (; i < querySegment.Length; ++i)
137-
{
138-
if (!char.IsWhiteSpace(querySegment[i]))
139-
{
140-
break;
141-
}
142-
}
143-
144-
var name = SpanHelper.ReplacePlusWithSpace(querySegment[i..equalIndex]);
135+
var name = SpanHelper.ReplacePlusWithSpace(querySegment.Slice(0, equalIndex));
145136
var value = SpanHelper.ReplacePlusWithSpace(querySegment.Slice(equalIndex + 1));
146137

147138
accumulator.Append(
@@ -210,11 +201,12 @@ public void Append(string key, string value)
210201

211202
if (_expandingAccumulator is null)
212203
{
213-
_expandingAccumulator = new AdaptiveCapacityDictionary<string, List<string>>(5, StringComparer.OrdinalIgnoreCase);
204+
_expandingAccumulator = new AdaptiveCapacityDictionary<string, List<string>>(capacity: 5, StringComparer.OrdinalIgnoreCase);
214205
}
215206

216-
// Already 5 entries so use starting allocated as 10; then use List's expansion mechanism for more
217-
var list = new List<string>(10);
207+
// Already 3 (2 existing + the new one) entries so use starting allocated as 6; then use List's expansion
208+
// mechanism for more
209+
var list = new List<string>(capacity: 6);
218210

219211
list.AddRange(values);
220212
list.Add(value);

src/Http/Http/src/QueryCollectionInternal.cs

Lines changed: 11 additions & 137 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4-
using System;
54
using System.Collections;
65
using System.Collections.Generic;
76
using Microsoft.AspNetCore.Internal;
@@ -14,25 +13,7 @@ namespace Microsoft.AspNetCore.Http
1413
/// </summary>
1514
internal class QueryCollectionInternal : IQueryCollection
1615
{
17-
/// <summary>
18-
/// Gets an empty <see cref="QueryCollectionInternal"/>.
19-
/// </summary>
20-
public static readonly QueryCollectionInternal Empty = new QueryCollectionInternal();
21-
private static readonly string[] EmptyKeys = Array.Empty<string>();
22-
private static readonly StringValues[] EmptyValues = Array.Empty<StringValues>();
23-
private static readonly Enumerator EmptyEnumerator = new Enumerator();
24-
// Pre-box
25-
private static readonly IEnumerator<KeyValuePair<string, StringValues>> EmptyIEnumeratorType = EmptyEnumerator;
26-
private static readonly IEnumerator EmptyIEnumerator = EmptyEnumerator;
27-
28-
private AdaptiveCapacityDictionary<string, StringValues>? Store { get; }
29-
30-
/// <summary>
31-
/// Initializes a new instance of <see cref="QueryCollectionInternal"/>.
32-
/// </summary>
33-
public QueryCollectionInternal()
34-
{
35-
}
16+
private AdaptiveCapacityDictionary<string, StringValues> Store { get; }
3617

3718
/// <summary>
3819
/// Initializes a new instance of <see cref="QueryCollection"/>.
@@ -43,148 +24,57 @@ internal QueryCollectionInternal(AdaptiveCapacityDictionary<string, StringValues
4324
Store = store;
4425
}
4526

46-
/// <summary>
47-
/// Creates a shallow copy of the specified <paramref name="store"/>.
48-
/// </summary>
49-
/// <param name="store">The <see cref="QueryCollection"/> to clone.</param>
50-
public QueryCollectionInternal(QueryCollectionInternal store)
51-
{
52-
Store = store.Store;
53-
}
54-
55-
/// <summary>
56-
/// Initializes a new instance of <see cref="QueryCollection"/>.
57-
/// </summary>
58-
/// <param name="capacity">The initial number of query items that this instance can contain.</param>
59-
public QueryCollectionInternal(int capacity)
60-
{
61-
Store = new AdaptiveCapacityDictionary<string, StringValues>(capacity, StringComparer.OrdinalIgnoreCase);
62-
}
63-
6427
/// <summary>
6528
/// Gets the associated set of values from the collection.
6629
/// </summary>
6730
/// <param name="key">The key name.</param>
6831
/// <returns>the associated value from the collection as a StringValues or StringValues.Empty if the key is not present.</returns>
69-
public StringValues this[string key]
70-
{
71-
get
72-
{
73-
if (Store == null)
74-
{
75-
return StringValues.Empty;
76-
}
77-
78-
if (TryGetValue(key, out var value))
79-
{
80-
return value;
81-
}
82-
return StringValues.Empty;
83-
}
84-
}
32+
public StringValues this[string key] => TryGetValue(key, out var value) ? value : StringValues.Empty;
8533

8634
/// <summary>
8735
/// Gets the number of elements contained in the <see cref="QueryCollection" />;.
8836
/// </summary>
8937
/// <returns>The number of elements contained in the <see cref="QueryCollection" />.</returns>
90-
public int Count
91-
{
92-
get
93-
{
94-
if (Store == null)
95-
{
96-
return 0;
97-
}
98-
return Store.Count;
99-
}
100-
}
38+
public int Count => Store.Count;
10139

10240
/// <summary>
10341
/// Gets the collection of query names in this instance.
10442
/// </summary>
105-
public ICollection<string> Keys
106-
{
107-
get
108-
{
109-
if (Store == null)
110-
{
111-
return EmptyKeys;
112-
}
113-
return Store.Keys;
114-
}
115-
}
43+
public ICollection<string> Keys => Store.Keys;
11644

11745
/// <summary>
11846
/// Determines whether the <see cref="QueryCollection" /> contains a specific key.
11947
/// </summary>
12048
/// <param name="key">The key.</param>
12149
/// <returns>true if the <see cref="QueryCollection" /> contains a specific key; otherwise, false.</returns>
122-
public bool ContainsKey(string key)
123-
{
124-
if (Store == null)
125-
{
126-
return false;
127-
}
128-
return Store.ContainsKey(key);
129-
}
50+
public bool ContainsKey(string key) => Store.ContainsKey(key);
13051

13152
/// <summary>
13253
/// Retrieves a value from the collection.
13354
/// </summary>
13455
/// <param name="key">The key.</param>
13556
/// <param name="value">The value.</param>
13657
/// <returns>true if the <see cref="QueryCollection" /> contains the key; otherwise, false.</returns>
137-
public bool TryGetValue(string key, out StringValues value)
138-
{
139-
if (Store == null)
140-
{
141-
value = default(StringValues);
142-
return false;
143-
}
144-
return Store.TryGetValue(key, out value);
145-
}
58+
public bool TryGetValue(string key, out StringValues value) => Store.TryGetValue(key, out value);
14659

14760
/// <summary>
14861
/// Returns an enumerator that iterates through a collection.
14962
/// </summary>
15063
/// <returns>An <see cref="Enumerator" /> object that can be used to iterate through the collection.</returns>
151-
public Enumerator GetEnumerator()
152-
{
153-
if (Store == null || Store.Count == 0)
154-
{
155-
// Non-boxed Enumerator
156-
return EmptyEnumerator;
157-
}
158-
return new Enumerator(Store.GetEnumerator());
159-
}
64+
public Enumerator GetEnumerator() => new Enumerator(Store.GetEnumerator());
16065

16166
/// <summary>
16267
/// Returns an enumerator that iterates through a collection.
16368
/// </summary>
16469
/// <returns>An <see cref="IEnumerator{T}" /> object that can be used to iterate through the collection.</returns>
16570
IEnumerator<KeyValuePair<string, StringValues>> IEnumerable<KeyValuePair<string, StringValues>>.GetEnumerator()
166-
{
167-
if (Store == null || Store.Count == 0)
168-
{
169-
// Non-boxed Enumerator
170-
return EmptyIEnumeratorType;
171-
}
172-
return Store.GetEnumerator();
173-
}
71+
=> Store.GetEnumerator();
17472

17573
/// <summary>
17674
/// Returns an enumerator that iterates through a collection.
17775
/// </summary>
17876
/// <returns>An <see cref="IEnumerator" /> object that can be used to iterate through the collection.</returns>
179-
IEnumerator IEnumerable.GetEnumerator()
180-
{
181-
if (Store == null || Store.Count == 0)
182-
{
183-
// Non-boxed Enumerator
184-
return EmptyIEnumerator;
185-
}
186-
return Store.GetEnumerator();
187-
}
77+
IEnumerator IEnumerable.GetEnumerator() => Store.GetEnumerator();
18878

18979
/// <summary>
19080
/// Enumerates a <see cref="QueryCollection"/>.
@@ -218,30 +108,14 @@ public bool MoveNext()
218108
/// <summary>
219109
/// Gets the element at the current position of the enumerator.
220110
/// </summary>
221-
public KeyValuePair<string, StringValues> Current
222-
{
223-
get
224-
{
225-
if (_notEmpty)
226-
{
227-
return _dictionaryEnumerator.Current;
228-
}
229-
return default(KeyValuePair<string, StringValues>);
230-
}
231-
}
111+
public KeyValuePair<string, StringValues> Current => _notEmpty ? _dictionaryEnumerator.Current : default;
232112

233113
/// <inheritdoc />
234114
public void Dispose()
235115
{
236116
}
237117

238-
object IEnumerator.Current
239-
{
240-
get
241-
{
242-
return Current;
243-
}
244-
}
118+
object IEnumerator.Current => Current;
245119

246120
void IEnumerator.Reset()
247121
{

src/Http/Http/test/Features/QueryFeatureTests.cs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,21 @@ public void ParseQueryWithDuplicateKeysGroups()
105105
Assert.Equal("valueB", queryCollection["key2"].FirstOrDefault());
106106
}
107107

108+
[Fact]
109+
public void ParseQueryWithThreefoldKeysGroups()
110+
{
111+
var features = new FeatureCollection();
112+
features[typeof(IHttpRequestFeature)] = new HttpRequestFeature { QueryString = "?key1=valueA&key2=valueB&key1=valueC&key1=valueD" };
113+
114+
var provider = new QueryFeature(features);
115+
116+
var queryCollection = provider.Query;
117+
118+
Assert.Equal(2, queryCollection.Count);
119+
Assert.Equal(new[] { "valueA", "valueC", "valueD" }, queryCollection["key1"]);
120+
Assert.Equal("valueB", queryCollection["key2"].FirstOrDefault());
121+
}
122+
108123
[Fact]
109124
public void ParseQueryWithEmptyValuesWorks()
110125
{

src/Shared/Dictionary/AdaptiveCapacityDictionary.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ internal class AdaptiveCapacityDictionary<TKey, TValue> : IDictionary<TKey, TVal
2222
internal KeyValuePair<TKey, TValue>[]? _arrayStorage;
2323
private int _count;
2424
internal Dictionary<TKey, TValue>? _dictionaryStorage;
25-
private IEqualityComparer<TKey> _comparer;
25+
private readonly IEqualityComparer<TKey> _comparer;
2626

2727
/// <summary>
2828
/// Creates an empty <see cref="AdaptiveCapacityDictionary{TKey, TValue}"/>.
@@ -621,7 +621,6 @@ private bool TryFindItem(TKey key, out TValue? value)
621621
[MethodImpl(MethodImplOptions.AggressiveInlining)]
622622
private bool ContainsKeyArray(TKey key) => TryFindItem(key, out _);
623623

624-
625624
/// <inheritdoc />
626625
public struct Enumerator : IEnumerator<KeyValuePair<TKey, TValue>>
627626
{

0 commit comments

Comments
 (0)