Add the Content-Type if specified, unless we are setting the body (OpenFeign#569)

DavidTanner · adriancole · commit e70bb4fb7a6a · 2017-07-18T16:20:43.000+08:00
* Add the Content-Type if specified, unless we are setting the body

* Update changelog
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,6 @@
+### Version 9.5.1
+* Update Okhttp client so that if specified, the content-type is included even without a body.
+
 ### Version 9.5
 * Introduces `feign-java8` with support for `java.util.Optional`
 * Adds `Feign.Builder.mapAndDecode()` to allow response preprocessing before decoding it.
diff --git a/okhttp/src/main/java/feign/okhttp/OkHttpClient.java b/okhttp/src/main/java/feign/okhttp/OkHttpClient.java
@@ -62,13 +62,12 @@ static Request toOkHttpRequest(feign.Request input) {
       }
 
       for (String value : input.headers().get(field)) {
+        requestBuilder.addHeader(field, value);
         if (field.equalsIgnoreCase("Content-Type")) {
           mediaType = MediaType.parse(value);
           if (input.charset() != null) {
             mediaType.charset(input.charset());
           }
-        } else {
-          requestBuilder.addHeader(field, value);
         }
       }
     }
@@ -79,10 +78,13 @@ static Request toOkHttpRequest(feign.Request input) {
 
     byte[] inputBody = input.body();
     boolean isMethodWithBody = "POST".equals(input.method()) || "PUT".equals(input.method());
-    if (isMethodWithBody && inputBody == null) {
-      // write an empty BODY to conform with okhttp 2.4.0+
-      // http://johnfeng.github.io/blog/2015/06/30/okhttp-updates-post-wouldnt-be-allowed-to-have-null-body/
-      inputBody = new byte[0];
+    if (isMethodWithBody) {
+      requestBuilder.removeHeader("Content-Type");
+      if (inputBody == null) {
+        // write an empty BODY to conform with okhttp 2.4.0+
+        // http://johnfeng.github.io/blog/2015/06/30/okhttp-updates-post-wouldnt-be-allowed-to-have-null-body/
+        inputBody = new byte[0];
+      }
     }
 
     RequestBody body = inputBody != null ? RequestBody.create(mediaType, inputBody) : null;
diff --git a/okhttp/src/test/java/feign/okhttp/OkHttpClientTest.java b/okhttp/src/test/java/feign/okhttp/OkHttpClientTest.java
@@ -16,9 +16,22 @@
 package feign.okhttp;
 
 import feign.Feign.Builder;
+import feign.Headers;
+import feign.Param;
+import feign.RequestLine;
+import feign.Response;
+import feign.Util;
+import feign.assertj.MockWebServerAssertions;
 import feign.client.AbstractClientTest;
 
 import feign.Feign;
+import okhttp3.mockwebserver.MockResponse;
+import org.junit.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
 
 /** Tests client-specific behavior, such as ensuring Content-Length is sent when specified. */
 public class OkHttpClientTest extends AbstractClientTest {
@@ -27,4 +40,29 @@ public class OkHttpClientTest extends AbstractClientTest {
   public Builder newBuilder() {
     return Feign.builder().client(new OkHttpClient());
   }
+
+
+  @Test
+  public void testContentTypeWithoutCharset() throws Exception {
+    server.enqueue(new MockResponse()
+            .setBody("AAAAAAAA"));
+    OkHttpClientTestInterface api = newBuilder()
+            .target(OkHttpClientTestInterface.class, "http://localhost:" + server.getPort());
+
+    Response response = api.getWithContentType();
+    // Response length should not be null
+    assertEquals("AAAAAAAA", Util.toString(response.body().asReader()));
+
+    MockWebServerAssertions.assertThat(server.takeRequest())
+            .hasHeaders("Accept: text/plain", "Content-Type: text/plain") // Note: OkHttp adds content length.
+            .hasMethod("GET");
+  }
+
+
+  public interface OkHttpClientTestInterface {
+
+    @RequestLine("GET /")
+    @Headers({"Accept: text/plain", "Content-Type: text/plain"})
+    Response getWithContentType();
+  }
 }

Original file line number	Diff line number	Diff line change
`@@ -62,13 +62,12 @@ static Request toOkHttpRequest(feign.Request input) {`
`62`	`62`	`}`
`63`	`63`
`64`	`64`	`for (String value : input.headers().get(field)) {`
	`65`	`+ requestBuilder.addHeader(field, value);`
`65`	`66`	`if (field.equalsIgnoreCase("Content-Type")) {`
`66`	`67`	`mediaType = MediaType.parse(value);`
`67`	`68`	`if (input.charset() != null) {`
`68`	`69`	`mediaType.charset(input.charset());`
`69`	`70`	`}`
`70`		`- } else {`
`71`		`- requestBuilder.addHeader(field, value);`
`72`	`71`	`}`
`73`	`72`	`}`
`74`	`73`	`}`
`@@ -79,10 +78,13 @@ static Request toOkHttpRequest(feign.Request input) {`
`79`	`78`
`80`	`79`	`byte[] inputBody = input.body();`
`81`	`80`	`boolean isMethodWithBody = "POST".equals(input.method()) \|\| "PUT".equals(input.method());`
`82`		`- if (isMethodWithBody && inputBody == null) {`
`83`		`- // write an empty BODY to conform with okhttp 2.4.0+`
`84`		`- // http://johnfeng.github.io/blog/2015/06/30/okhttp-updates-post-wouldnt-be-allowed-to-have-null-body/`
`85`		`- inputBody = new byte[0];`
	`81`	`+ if (isMethodWithBody) {`
	`82`	`+ requestBuilder.removeHeader("Content-Type");`
	`83`	`+ if (inputBody == null) {`
	`84`	`+ // write an empty BODY to conform with okhttp 2.4.0+`
	`85`	`+ // http://johnfeng.github.io/blog/2015/06/30/okhttp-updates-post-wouldnt-be-allowed-to-have-null-body/`
	`86`	`+ inputBody = new byte[0];`
	`87`	`+ }`
`86`	`88`	`}`
`87`	`89`
`88`	`90`	`RequestBody body = inputBody != null ? RequestBody.create(mediaType, inputBody) : null;`