From 39afd1f46f8f03582b0414591ff7a1e2f1809979 Mon Sep 17 00:00:00 2001 From: Marek Safar Date: Tue, 16 Feb 2021 03:48:40 +0100 Subject: [PATCH] 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 * Apply suggestions from code review Co-authored-by: Karel Zikmund Co-authored-by: Stephen Toub --- .../src/System/Net/Http/Headers/CacheControlHeaderValue.cs | 8 +------- .../Net/Http/Headers/ContentDispositionHeaderValue.cs | 9 +-------- .../src/System/Net/Http/Headers/HeaderUtilities.cs | 14 ++++++++++++++ .../src/System/Net/Http/Headers/MediaTypeHeaderValue.cs | 9 +-------- .../src/System/Net/Http/Headers/NameValueHeaderValue.cs | 2 +- .../Net/Http/Headers/NameValueWithParametersHeaderValue.cs | 8 +------- .../src/System/Net/Http/Headers/RangeHeaderValue.cs | 2 +- .../src/System/Net/Http/Headers/RangeItemHeaderValue.cs | 2 +- .../System/Net/Http/Headers/TransferCodingHeaderValue.cs | 9 +-------- .../tests/UnitTests/Headers/NameValueHeaderValueTest.cs | 2 +- 10 files changed, 23 insertions(+), 42 deletions(-) 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 --- 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 --- 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 --- 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? Clone(this ObjectCollection? source) + { + if (source == null) + return null; + + var copy = new ObjectCollection(); + 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 4a486c5b..f505db8 100644 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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("name", () => { NameValueHeaderValue nameValue = new NameValueHeaderValue(null); }); + AssertExtensions.Throws("name", () => { NameValueHeaderValue nameValue = new NameValueHeaderValue((string)null); }); } [Fact] -- 2.7.4