Make headers.set(self) a no-op instead of throwing. Makes it consistent with setAll

Louis Ryan · nmittler · commit 3eb65797ed11 · 2015-11-06T07:00:54.000-08:00
Motivation:

Makes the API contract of headers more consistent and simpler.

Modifications:

If self is passed to set then simply return

Result:

set and setAll will be consistent
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/HttpHeadersTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/HttpHeadersTest.java
@@ -79,9 +79,11 @@ public void testAddSelf() {
         headers.add(headers);
     }
 
-    @Test(expected = IllegalArgumentException.class)
-    public void testSetSelf() {
+    @Test
+    public void testSetSelfIsNoOp() {
         HttpHeaders headers = new DefaultHttpHeaders(false);
+        headers.add("name", "value");
         headers.set(headers);
+        assertEquals(1, headers.size());
     }
 }
diff --git a/codec/src/main/java/io/netty/handler/codec/DefaultHeaders.java b/codec/src/main/java/io/netty/handler/codec/DefaultHeaders.java
@@ -543,7 +543,7 @@ public T setShort(K name, short value) {
     public T set(Headers<? extends K, ? extends V, ?> headers) {
         checkNotNull(headers, "headers");
         if (headers == this) {
-            throw new IllegalArgumentException("can't add to itself.");
+            return thisT();
         }
         clear();
         if (headers instanceof DefaultHeaders) {
diff --git a/codec/src/test/java/io/netty/handler/codec/DefaultHeadersTest.java b/codec/src/test/java/io/netty/handler/codec/DefaultHeadersTest.java
@@ -390,9 +390,11 @@ public void testAddSelf() {
         headers.add(headers);
     }
 
-    @Test(expected = IllegalArgumentException.class)
-    public void testSetSelf() {
+    @Test
+    public void testSetSelfIsNoOp() {
         TestDefaultHeaders headers = newInstance();
+        headers.add("name", "value");
         headers.set(headers);
+        assertEquals(1, headers.size());
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -79,9 +79,11 @@ public void testAddSelf() {`
`79`	`79`	`headers.add(headers);`
`80`	`80`	`}`
`81`	`81`
`82`		`- @Test(expected = IllegalArgumentException.class)`
`83`		`- public void testSetSelf() {`
	`82`	`+ @Test`
	`83`	`+ public void testSetSelfIsNoOp() {`
`84`	`84`	`HttpHeaders headers = new DefaultHttpHeaders(false);`
	`85`	`+ headers.add("name", "value");`
`85`	`86`	`headers.set(headers);`
	`87`	`+ assertEquals(1, headers.size());`
`86`	`88`	`}`
`87`	`89`	`}`
Original file line number	Diff line number	Diff line change
`@@ -543,7 +543,7 @@ public T setShort(K name, short value) {`
`543`	`543`	`public T set(Headers<? extends K, ? extends V, ?> headers) {`
`544`	`544`	`checkNotNull(headers, "headers");`
`545`	`545`	`if (headers == this) {`
`546`		`- throw new IllegalArgumentException("can't add to itself.");`
	`546`	`+ return thisT();`
`547`	`547`	`}`
`548`	`548`	`clear();`
`549`	`549`	`if (headers instanceof DefaultHeaders) {`
Original file line number	Diff line number	Diff line change
`@@ -390,9 +390,11 @@ public void testAddSelf() {`
`390`	`390`	`headers.add(headers);`
`391`	`391`	`}`
`392`	`392`
`393`		`- @Test(expected = IllegalArgumentException.class)`
`394`		`- public void testSetSelf() {`
	`393`	`+ @Test`
	`394`	`+ public void testSetSelfIsNoOp() {`
`395`	`395`	`TestDefaultHeaders headers = newInstance();`
	`396`	`+ headers.add("name", "value");`
`396`	`397`	`headers.set(headers);`
	`398`	`+ assertEquals(1, headers.size());`
`397`	`399`	`}`
`398`	`400`	`}`