TryAddWithoutValidation for multiple values could be more efficient (#…

…102845)
dotnet · Jun 10, 2024 · 7ffb9a4 · 7ffb9a4
1 parent a848547
commit 7ffb9a4
Show file tree

Hide file tree

Showing 2 changed files with 104 additions and 13 deletions.
diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs
@@ -169,27 +169,58 @@ internal bool TryAddWithoutValidation(HeaderDescriptor descriptor, IEnumerable<s
         {
             ArgumentNullException.ThrowIfNull(values);
 
-            using IEnumerator<string?> enumerator = values.GetEnumerator();
-            if (enumerator.MoveNext())
+            if (values is IList<string?> valuesList)
             {
-                TryAddWithoutValidation(descriptor, enumerator.Current);
-                if (enumerator.MoveNext())
+                int count = valuesList.Count;
+
+                if (count > 0)
                 {
+                    // The store value is either a string (a single unparsed value) or a HeaderStoreItemInfo.
+                    // The RawValue on HeaderStoreItemInfo can likewise be either a single string or a List<string>.
+
                     ref object? storeValueRef = ref GetValueRefOrAddDefault(descriptor);
-                    Debug.Assert(storeValueRef is not null);
+                    object? storeValue = storeValueRef;
 
-                    object value = storeValueRef;
-                    if (value is not HeaderStoreItemInfo info)
+                    // If the storeValue was already set or we're adding more than 1 value,
+                    // we'll have to store the values in a List<string> on HeaderStoreItemInfo.
+                    if (storeValue is not null || count > 1)
                     {
-                        Debug.Assert(value is string);
-                        storeValueRef = info = new HeaderStoreItemInfo { RawValue = value };
-                    }
+                        if (storeValue is not HeaderStoreItemInfo info)
+                        {
+                            storeValueRef = info = new HeaderStoreItemInfo { RawValue = storeValue };
+                        }
+
+                        object? rawValue = info.RawValue;
+                        if (rawValue is not List<string> rawValues)
+                        {
+                            info.RawValue = rawValues = new List<string>();
+
+                            if (rawValue != null)
+                            {
+                                rawValues.EnsureCapacity(count + 1);
+                                rawValues.Add((string)rawValue);
+                            }
+                        }
 
-                    do
+                        rawValues.EnsureCapacity(rawValues.Count + count);
+
+                        for (int i = 0; i < count; i++)
+                        {
+                            rawValues.Add(valuesList[i] ?? string.Empty);
+                        }
+                    }
+                    else
                     {
-                        AddRawValue(info, enumerator.Current ?? string.Empty);
+                        // We're adding a single value to a new header entry. We can store the unparsed value as-is.
+                        storeValueRef = valuesList[0] ?? string.Empty;
                     }
-                    while (enumerator.MoveNext());
+                }
+            }
+            else
+            {
+                foreach (string? value in values)
+                {
+                    TryAddWithoutValidation(descriptor, value ?? string.Empty);
                 }
             }
 

diff --git a/src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpHeadersTest.cs b/src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpHeadersTest.cs
@@ -2532,6 +2532,66 @@ public void TryGetValues_InvalidValuesContainingNewLines_ShouldNotRemoveInvalidV
             Assert.Equal(value, values.Single());
         }
 
+        [Fact]
+        public void TryAddWithoutValidation_OneValidValueHeader_UseSpecialListImplementation()
+        {
+            const string Name = "customHeader1";
+            const string Value = "Value1";
+
+            var response = new HttpResponseMessage();
+            Assert.True(response.Headers.TryAddWithoutValidation(Name, new List<string> { Value }));
+
+            Assert.True(response.Headers.Contains(Name));
+
+            Assert.True(response.Headers.TryGetValues(Name, out IEnumerable<string> values));
+            Assert.Equal(Value, values.Single());
+        }
+
+        [Fact]
+        public void TryAddWithoutValidation_ThreeValidValueHeader_UseSpecialListImplementation()
+        {
+            const string Name = "customHeader1";
+            List<string> expectedValues = [ "Value1", "Value2", "Value3" ];
+
+            var response = new HttpResponseMessage();
+            Assert.True(response.Headers.TryAddWithoutValidation(Name, expectedValues));
+
+            Assert.True(response.Headers.Contains(Name));
+
+            Assert.True(response.Headers.TryGetValues(Name, out IEnumerable<string> values));
+            Assert.True(expectedValues.SequenceEqual(values));
+        }
+
+        [Fact]
+        public void TryAddWithoutValidation_OneValidValueHeader_UseGenericImplementation()
+        {
+            const string Name = "customHeader1";
+            const string Value = "Value1";
+
+            var response = new HttpResponseMessage();
+            Assert.True(response.Headers.TryAddWithoutValidation(Name, new HashSet<string> { Value }));
+
+            Assert.True(response.Headers.Contains(Name));
+
+            Assert.True(response.Headers.TryGetValues(Name, out IEnumerable<string> values));
+            Assert.Equal(Value, values.Single());
+        }
+
+        [Fact]
+        public void TryAddWithoutValidation_ThreeValidValueHeader_UseGenericImplementation()
+        {
+            const string Name = "customHeader1";
+            List<string> expectedValues = ["Value1", "Value2", "Value3"];
+
+            var response = new HttpResponseMessage();
+            Assert.True(response.Headers.TryAddWithoutValidation(Name, new HashSet<string>(expectedValues)));
+
+            Assert.True(response.Headers.Contains(Name));
+
+            Assert.True(response.Headers.TryGetValues(Name, out IEnumerable<string> values));
+            Assert.True(expectedValues.SequenceEqual(values));
+        }
+
         public static IEnumerable<object[]> NumberOfHeadersUpToArrayThreshold_AddNonValidated_EnumerateNonValidated()
         {
             for (int i = 0; i <= HttpHeaders.ArrayThreshold; i++)