Remove some code duplication in System.Net.Http.Headers (#48297)

* Remove some code duplication in System.Net.Http.Headers

* Update src/libraries/System.Net.Http/tests/UnitTests/Headers/NameValueHeaderValueTest.cs

Co-authored-by: Karel Zikmund <karelz@microsoft.com>
* Apply suggestions from code review

Co-authored-by: Karel Zikmund <karelz@microsoft.com>
Co-authored-by: Stephen Toub <stoub@microsoft.com>

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/CacheControlHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/CacheControlHeaderValue.cs
index 55c966d..b38c339 100644 (file)
--- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/CacheControlHeaderValue.cs
+++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/CacheControlHeaderValue.cs
@@ -167,13 +167,7 @@ namespace System.Net.Http.Headers
                 }
             }
 
-            if (source._extensions != null)
-            {
-                foreach (var extension in source._extensions)
-                {
-                    Extensions.Add((NameValueHeaderValue)((ICloneable)extension).Clone());
-                }
-            }
+            _extensions = source._extensions.Clone();
         }
 
         public override string ToString()

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/ContentDispositionHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/ContentDispositionHeaderValue.cs
index 079b8a5..08f295b 100644 (file)
--- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/ContentDispositionHeaderValue.cs
+++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/ContentDispositionHeaderValue.cs
@@ -134,14 +134,7 @@ namespace System.Net.Http.Headers
             Debug.Assert(source != null);
 
             _dispositionType = source._dispositionType;
-
-            if (source._parameters != null)
-            {
-                foreach (var parameter in source._parameters)
-                {
-                    this.Parameters.Add((NameValueHeaderValue)((ICloneable)parameter).Clone());
-                }
-            }
+            _parameters = source._parameters.Clone();
         }
 
         public ContentDispositionHeaderValue(string dispositionType)

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HeaderUtilities.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HeaderUtilities.cs
index f8bf7ec..91967be 100644 (file)
--- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HeaderUtilities.cs
+++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HeaderUtilities.cs
@@ -390,5 +390,19 @@ namespace System.Net.Http.Headers
         {
             CheckValidToken(value, "item");
         }
+
+        internal static ObjectCollection<NameValueHeaderValue>? Clone(this ObjectCollection<NameValueHeaderValue>? source)
+        {
+            if (source == null)
+                return null;
+
+            var copy = new ObjectCollection<NameValueHeaderValue>();
+            foreach (NameValueHeaderValue item in source)
+            {
+                copy.Add(new NameValueHeaderValue(item));
+            }
+
+            return copy;
+        }
     }
 }

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/MediaTypeHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/MediaTypeHeaderValue.cs
index 4a486c5..f505db8 100644 (file)
--- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/MediaTypeHeaderValue.cs
+++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/MediaTypeHeaderValue.cs
@@ -76,14 +76,7 @@ namespace System.Net.Http.Headers
             Debug.Assert(source != null);
 
             _mediaType = source._mediaType;
-
-            if (source._parameters != null)
-            {
-                foreach (var parameter in source._parameters)
-                {
-                    this.Parameters.Add((NameValueHeaderValue)((ICloneable)parameter).Clone());
-                }
-            }
+            _parameters = source._parameters.Clone();
         }
 
         public MediaTypeHeaderValue(string mediaType)

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/NameValueHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/NameValueHeaderValue.cs
index 09970b4..61caf76 100644 (file)
--- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/NameValueHeaderValue.cs
+++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/NameValueHeaderValue.cs
@@ -51,7 +51,7 @@ namespace System.Net.Http.Headers
             _value = value;
         }
 
-        protected NameValueHeaderValue(NameValueHeaderValue source)
+        protected internal NameValueHeaderValue(NameValueHeaderValue source)
         {
             Debug.Assert(source != null);
 

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/NameValueWithParametersHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/NameValueWithParametersHeaderValue.cs
index 7ab592c..38ab866 100644 (file)
--- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/NameValueWithParametersHeaderValue.cs
+++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/NameValueWithParametersHeaderValue.cs
@@ -36,13 +36,7 @@ namespace System.Net.Http.Headers
         protected NameValueWithParametersHeaderValue(NameValueWithParametersHeaderValue source)
             : base(source)
         {
-            if (source._parameters != null)
-            {
-                foreach (var parameter in source._parameters)
-                {
-                    this.Parameters.Add((NameValueHeaderValue)((ICloneable)parameter).Clone());
-                }
-            }
+            _parameters = source._parameters.Clone();
         }
 
         public override bool Equals([NotNullWhen(true)] object? obj)

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/RangeHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/RangeHeaderValue.cs
index 503f54d..9462bd1 100644 (file)
--- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/RangeHeaderValue.cs
+++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/RangeHeaderValue.cs
@@ -47,7 +47,7 @@ namespace System.Net.Http.Headers
             {
                 foreach (RangeItemHeaderValue range in source._ranges)
                 {
-                    this.Ranges.Add((RangeItemHeaderValue)((ICloneable)range).Clone());
+                    this.Ranges.Add(new RangeItemHeaderValue(range));
                 }
             }
         }

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/RangeItemHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/RangeItemHeaderValue.cs
index 68197f0..cba711f 100644 (file)
--- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/RangeItemHeaderValue.cs
+++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/RangeItemHeaderValue.cs
@@ -46,7 +46,7 @@ namespace System.Net.Http.Headers
             _to = to;
         }
 
-        private RangeItemHeaderValue(RangeItemHeaderValue source)
+        internal RangeItemHeaderValue(RangeItemHeaderValue source)
         {
             Debug.Assert(source != null);
 

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/TransferCodingHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/TransferCodingHeaderValue.cs
index cbcc18c..4953fdb 100644 (file)
--- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/TransferCodingHeaderValue.cs
+++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/TransferCodingHeaderValue.cs
@@ -31,14 +31,7 @@ namespace System.Net.Http.Headers
             Debug.Assert(source != null);
 
             _value = source._value;
-
-            if (source._parameters != null)
-            {
-                foreach (var parameter in source._parameters)
-                {
-                    this.Parameters.Add((NameValueHeaderValue)((ICloneable)parameter).Clone());
-                }
-            }
+            _parameters = source._parameters.Clone();
         }
 
         public TransferCodingHeaderValue(string value)

diff --git a/src/libraries/System.Net.Http/tests/UnitTests/Headers/NameValueHeaderValueTest.cs b/src/libraries/System.Net.Http/tests/UnitTests/Headers/NameValueHeaderValueTest.cs
index e7ba87b..00ca0d3 100644 (file)
--- a/src/libraries/System.Net.Http/tests/UnitTests/Headers/NameValueHeaderValueTest.cs
+++ b/src/libraries/System.Net.Http/tests/UnitTests/Headers/NameValueHeaderValueTest.cs
@@ -16,7 +16,7 @@ namespace System.Net.Http.Tests
         [Fact]
         public void Ctor_NameNull_Throw()
         {
-            AssertExtensions.Throws<ArgumentException>("name", () => { NameValueHeaderValue nameValue = new NameValueHeaderValue(null); });
+            AssertExtensions.Throws<ArgumentException>("name", () => { NameValueHeaderValue nameValue = new NameValueHeaderValue((string)null); });
         }
 
         [Fact]