fix OpenFeign#797 fix OpenFeign#798: JAXRSContract sets a single Content-Type value (OpenFeign#799)

Carter Kozak · kdavisk6 · commit 989fbd7c1f40 · 2018-09-29T15:05:08.000-04:00
Closes OpenFeign#797 Closes OpenFeign#798 JAXRSContract sets a single Content-Type value This change allows headers to be cleared by passing a null value for backwards compatibility. Multiple Content-Type values are not valid because the body that we send with any given request will only have a single type. Updated header entry assertion to be agnostic to header name order. * RequestTemplate.headers clear behavior matches that of query params
diff --git a/core/src/main/java/feign/RequestTemplate.java b/core/src/main/java/feign/RequestTemplate.java
@@ -663,6 +663,11 @@ public RequestTemplate header(String name, Iterable<String> values) {
    * @return a RequestTemplate for chaining.
    */
   private RequestTemplate appendHeader(String name, Iterable<String> values) {
+    if (!values.iterator().hasNext()) {
+      /* empty value, clear the existing values */
+      this.headers.remove(name);
+      return this;
+    }
     this.headers.compute(name, (headerName, headerTemplate) -> {
       if (headerTemplate == null) {
         return HeaderTemplate.create(headerName, values);
diff --git a/core/src/test/java/feign/assertj/RequestTemplateAssert.java b/core/src/test/java/feign/assertj/RequestTemplateAssert.java
@@ -85,7 +85,7 @@ public RequestTemplateAssert hasQueries(MapEntry... entries) {
 
   public RequestTemplateAssert hasHeaders(MapEntry... entries) {
     isNotNull();
-    maps.assertContainsExactly(info, actual.headers(), entries);
+    maps.assertContainsOnly(info, actual.headers(), entries);
     return this;
   }
 
diff --git a/jaxrs/README.md b/jaxrs/README.md
@@ -23,7 +23,7 @@ Appends the value to `Target.url()`.  Can have tokens corresponding to `@PathPar
 #### `@Produces`
 Adds all values into the `Accept` header.
 #### `@Consumes`
-Adds all values into the `Content-Type` header.
+Adds the first value as the `Content-Type` header.
 ### Parameter Annotations
 #### `@PathParam`
 Links the value of the corresponding parameter to a template variable declared in the path.
diff --git a/jaxrs/src/main/java/feign/jaxrs/JAXRSContract.java b/jaxrs/src/main/java/feign/jaxrs/JAXRSContract.java
@@ -20,7 +20,10 @@
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Method;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
+
 import static feign.Util.checkState;
 import static feign.Util.emptyToNull;
 import static feign.Util.removeValues;
@@ -107,16 +110,16 @@ private void handleProducesAnnotation(MethodMetadata data, Produces produces, St
     String[] serverProduces =
         removeValues(produces.value(), (mediaType) -> emptyToNull(mediaType) == null, String.class);
     checkState(serverProduces.length > 0, "Produces.value() was empty on %s", name);
-    data.template().header(ACCEPT, (String) null); // remove any previous produces
+    data.template().header(ACCEPT, Collections.emptyList()); // remove any previous produces
     data.template().header(ACCEPT, serverProduces);
   }
 
   private void handleConsumesAnnotation(MethodMetadata data, Consumes consumes, String name) {
     String[] serverConsumes =
         removeValues(consumes.value(), (mediaType) -> emptyToNull(mediaType) == null, String.class);
     checkState(serverConsumes.length > 0, "Consumes.value() was empty on %s", name);
-    data.template().header(CONTENT_TYPE, (String) null); // remove any previous consumes
-    data.template().header(CONTENT_TYPE, serverConsumes);
+    data.template().header(CONTENT_TYPE, Collections.emptyList()); // remove any previous consumes
+    data.template().header(CONTENT_TYPE, serverConsumes[0]);
   }
 
   /**
diff --git a/jaxrs/src/test/java/feign/jaxrs/JAXRSContractTest.java b/jaxrs/src/test/java/feign/jaxrs/JAXRSContractTest.java
@@ -120,7 +120,7 @@ public void producesAddsAcceptHeader() throws Exception {
     assertThat(md.template())
         .hasHeaders(
             entry("Content-Type", asList("application/json")),
-            entry("Accept", asList("application/xml", "text/html")));
+            entry("Accept", asList("application/xml")));
   }
 
   @Test
@@ -130,7 +130,7 @@ public void producesMultipleAddsAcceptHeader() throws Exception {
     assertThat(md.template())
         .hasHeaders(
             entry("Content-Type", Collections.singletonList("application/json")),
-            entry("Accept", asList("application/xml", "text/html", "text/plain")));
+            entry("Accept", asList("application/xml", "text/plain")));
   }
 
   @Test
@@ -156,7 +156,7 @@ public void consumesAddsContentTypeHeader() throws Exception {
     /* multiple @Consumes annotations are additive */
     assertThat(md.template())
         .hasHeaders(
-            entry("Content-Type", asList("application/xml", "application/json")),
+            entry("Content-Type", asList("application/xml")),
             entry("Accept", asList("text/html")));
   }
 
@@ -165,7 +165,7 @@ public void consumesMultipleAddsContentTypeHeader() throws Exception {
     MethodMetadata md = parseAndValidateMetadata(ProducesAndConsumes.class, "consumesMultiple");
 
     assertThat(md.template())
-        .hasHeaders(entry("Content-Type", asList("application/xml", "application/json")),
+        .hasHeaders(entry("Content-Type", asList("application/xml")),
             entry("Accept", Collections.singletonList("text/html")));
   }
 
diff --git a/jaxrs2/README.md b/jaxrs2/README.md
@@ -23,7 +23,7 @@ Sets the request method.
 #### `@Path`
 Appends the value to `Target.url()`.  Can have tokens corresponding to `@PathParam` annotations.
 #### `@Produces`
-Adds the first value as the `Accept` header.
+Adds all values into the `Accept` header.
 #### `@Consumes`
 Adds the first value as the `Content-Type` header.
 ### Parameter Annotations

Original file line number	Diff line number	Diff line change
`@@ -85,7 +85,7 @@ public RequestTemplateAssert hasQueries(MapEntry... entries) {`
`85`	`85`
`86`	`86`	`public RequestTemplateAssert hasHeaders(MapEntry... entries) {`
`87`	`87`	`isNotNull();`
`88`		`- maps.assertContainsExactly(info, actual.headers(), entries);`
	`88`	`+ maps.assertContainsOnly(info, actual.headers(), entries);`
`89`	`89`	`return this;`
`90`	`90`	`}`
`91`	`91`