Headers overwrite (OpenFeign#1219)

velo · Pierre de Soyres · web-flow · commit 4ee8cd8f1ec6 · 2020-05-03T08:42:17.000+12:00
* Add unit test illustrating OpenFeign#1217 * Make sure content-type is limited to a single value Co-authored-by: Pierre de Soyres <pierre.de-soyres@eptica.com>
diff --git a/core/src/main/java/feign/RequestTemplate.java b/core/src/main/java/feign/RequestTemplate.java
@@ -696,7 +696,7 @@ public RequestTemplate header(String name, String... values) {
 
   /**
    * Add a header using the supplied Chunks.
-   * 
+   *
    * @param name of the header.
    * @param chunks to add.
    * @return a RequestTemplate for chaining.
@@ -753,6 +753,14 @@ private RequestTemplate appendHeader(String name, Iterable<String> values) {
       this.headers.remove(name);
       return this;
     }
+    if (name.equals("Content-Type")) {
+      // a client can only produce content of one single type, so always override Content-Type and
+      // only add a single type
+      this.headers.remove(name);
+      this.headers.put(name,
+          HeaderTemplate.create(name, Collections.singletonList(values.iterator().next())));
+      return this;
+    }
     this.headers.compute(name, (headerName, headerTemplate) -> {
       if (headerTemplate == null) {
         return HeaderTemplate.create(headerName, values);
diff --git a/jaxrs/src/main/java/feign/jaxrs/JAXRSContract.java b/jaxrs/src/main/java/feign/jaxrs/JAXRSContract.java
@@ -66,12 +66,12 @@ public JAXRSContract() {
     super.registerClassAnnotation(Produces.class, this::handleProducesAnnotation);
 
     registerMethodAnnotation(methodAnnotation -> {
-      Class<? extends Annotation> annotationType = methodAnnotation.annotationType();
-      HttpMethod http = annotationType.getAnnotation(HttpMethod.class);
+      final Class<? extends Annotation> annotationType = methodAnnotation.annotationType();
+      final HttpMethod http = annotationType.getAnnotation(HttpMethod.class);
       return http != null;
     }, (methodAnnotation, data) -> {
-      Class<? extends Annotation> annotationType = methodAnnotation.annotationType();
-      HttpMethod http = annotationType.getAnnotation(HttpMethod.class);
+      final Class<? extends Annotation> annotationType = methodAnnotation.annotationType();
+      final HttpMethod http = annotationType.getAnnotation(HttpMethod.class);
       checkState(data.template().method() == null,
           "Method %s contains multiple HTTP methods. Found: %s and %s", data.configKey(),
           data.template().method(), http.value());
@@ -103,18 +103,17 @@ public JAXRSContract() {
 
   private void handleProducesAnnotation(Produces produces, MethodMetadata data) {
     final String[] serverProduces =
-        removeValues(produces.value(), (mediaType) -> emptyToNull(mediaType) == null, String.class);
+        removeValues(produces.value(), mediaType -> emptyToNull(mediaType) == null, String.class);
     checkState(serverProduces.length > 0, "Produces.value() was empty on %s", data.configKey());
     data.template().header(ACCEPT, Collections.emptyList()); // remove any previous produces
     data.template().header(ACCEPT, serverProduces);
   }
 
   private void handleConsumesAnnotation(Consumes consumes, MethodMetadata data) {
     final String[] serverConsumes =
-        removeValues(consumes.value(), (mediaType) -> emptyToNull(mediaType) == null, String.class);
+        removeValues(consumes.value(), mediaType -> emptyToNull(mediaType) == null, String.class);
     checkState(serverConsumes.length > 0, "Consumes.value() was empty on %s", data.configKey());
-    data.template().header(CONTENT_TYPE, Collections.emptyList()); // remove any previous consumes
-    data.template().header(CONTENT_TYPE, serverConsumes[0]);
+    data.template().header(CONTENT_TYPE, serverConsumes);
   }
 
   protected void registerParamAnnotations() {
diff --git a/jaxrs2/src/test/java/feign/jaxrs2/JAXRSClientTest.java b/jaxrs2/src/test/java/feign/jaxrs2/JAXRSClientTest.java
@@ -28,7 +28,12 @@
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.util.Collections;
+import javax.ws.rs.Consumes;
+import javax.ws.rs.HeaderParam;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
 import javax.ws.rs.ProcessingException;
+import feign.jaxrs.JAXRSContract;
 import okhttp3.mockwebserver.MockResponse;
 import org.assertj.core.data.MapEntry;
 import org.junit.Assume;
@@ -126,11 +131,36 @@ public void testContentTypeWithoutCharset2() throws Exception {
         .hasMethod("GET");
   }
 
+  @Test
+  public void testConsumesMultipleWithContentTypeHeaderAndBody() throws Exception {
+    server.enqueue(new MockResponse().setBody("AAAAAAAA"));
+    final JaxRSClientTestInterfaceWithJaxRsContract api = newBuilder()
+        .contract(new JAXRSContract()) // use JAXRSContract
+        .target(JaxRSClientTestInterfaceWithJaxRsContract.class,
+            "http://localhost:" + server.getPort());
+
+    final Response response =
+        api.consumesMultipleWithContentTypeHeaderAndBody("application/json;charset=utf-8", "body");
+    assertEquals("AAAAAAAA", Util.toString(response.body().asReader(UTF_8)));
+
+    MockWebServerAssertions.assertThat(server.takeRequest())
+        .hasHeaders(MapEntry.entry("Content-Type",
+            Collections.singletonList("application/json;charset=utf-8")))
+        .hasMethod("POST");
+  }
 
   public interface JaxRSClientTestInterface {
 
     @RequestLine("GET /")
     @Headers({"Accept: text/plain", "Content-Type: text/plain"})
     Response getWithContentType();
   }
+
+  public interface JaxRSClientTestInterfaceWithJaxRsContract {
+    @Path("/")
+    @POST
+    @Consumes({"application/xml", "application/json"})
+    Response consumesMultipleWithContentTypeHeaderAndBody(@HeaderParam("Content-Type") String contentType,
+                                                          String body);
+  }
 }
