From 0fdbda658456ac9647c0ef716d998cd1bc35874a Mon Sep 17 00:00:00 2001 From: Paul Nepywoda Date: Thu, 10 Sep 2015 15:44:04 -0700 Subject: [PATCH 1/3] send empty response body for POST when no body parameter present see http://johnfeng.github.io/blog/2015/06/30/okhttp-updates-post-wouldnt-be-allowed-to-have-null-body/ --- core/src/main/java/feign/ReflectiveFeign.java | 16 ++++++++++++---- .../test/java/feign/okhttp/OkHttpClientTest.java | 14 ++++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/feign/ReflectiveFeign.java b/core/src/main/java/feign/ReflectiveFeign.java index 97cb735cf8..249708745c 100644 --- a/core/src/main/java/feign/ReflectiveFeign.java +++ b/core/src/main/java/feign/ReflectiveFeign.java @@ -18,6 +18,7 @@ import java.lang.reflect.InvocationHandler; import java.lang.reflect.Method; import java.lang.reflect.Proxy; +import java.lang.reflect.Type; import java.util.Collection; import java.util.LinkedHashMap; import java.util.List; @@ -143,7 +144,7 @@ public Map apply(Target key) { } else if (md.bodyIndex() != null) { buildTemplate = new BuildEncodedTemplateFromArgs(md, encoder); } else { - buildTemplate = new BuildTemplateByResolvingArgs(md); + buildTemplate = new BuildTemplateByResolvingArgs(md, encoder); } result.put(md.configKey(), factory.create(key, md, buildTemplate, options, decoder, errorDecoder)); @@ -156,9 +157,11 @@ private static class BuildTemplateByResolvingArgs implements RequestTemplate.Fac protected final MethodMetadata metadata; private final Map indexToExpander = new LinkedHashMap(); + private final Encoder encoder; - private BuildTemplateByResolvingArgs(MethodMetadata metadata) { + private BuildTemplateByResolvingArgs(MethodMetadata metadata, Encoder encoder) { this.metadata = metadata; + this.encoder = encoder; if (metadata.indexToExpanderClass().isEmpty()) { return; } @@ -201,6 +204,11 @@ public RequestTemplate create(Object[] argv) { protected RequestTemplate resolve(Object[] argv, RequestTemplate mutable, Map variables) { + if ("POST".equals(metadata.template().method()) && argv == null) { + // write an empty string for the 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/ + encoder.encode("", String.class, mutable); + } return mutable.resolve(variables); } } @@ -210,7 +218,7 @@ private static class BuildFormEncodedTemplateFromArgs extends BuildTemplateByRes private final Encoder encoder; private BuildFormEncodedTemplateFromArgs(MethodMetadata metadata, Encoder encoder) { - super(metadata); + super(metadata, encoder); this.encoder = encoder; } @@ -239,7 +247,7 @@ private static class BuildEncodedTemplateFromArgs extends BuildTemplateByResolvi private final Encoder encoder; private BuildEncodedTemplateFromArgs(MethodMetadata metadata, Encoder encoder) { - super(metadata); + super(metadata, encoder); this.encoder = encoder; } diff --git a/okhttp/src/test/java/feign/okhttp/OkHttpClientTest.java b/okhttp/src/test/java/feign/okhttp/OkHttpClientTest.java index bf43d088b6..0bbd2f165d 100644 --- a/okhttp/src/test/java/feign/okhttp/OkHttpClientTest.java +++ b/okhttp/src/test/java/feign/okhttp/OkHttpClientTest.java @@ -134,6 +134,17 @@ protected void log(String configKey, String format, Object... args) { api.post("foo"); } + @Test + public void noResponseBody() { + server.enqueue(new MockResponse()); + + TestInterface api = Feign.builder() + .client(new OkHttpClient()) + .target(TestInterface.class, "http://localhost:" + server.getPort()); + + api.noPostBody(); + } + interface TestInterface { @RequestLine("POST /?foo=bar&foo=baz&qux=") @@ -147,5 +158,8 @@ interface TestInterface { @RequestLine("PATCH /") @Headers("Accept: text/plain") String patch(String body); + + @RequestLine("POST") + String noPostBody(); } } From 777b06a43a2fd3210c62c9722f4f843f569cc7b8 Mon Sep 17 00:00:00 2001 From: Paul Nepywoda Date: Fri, 11 Sep 2015 12:21:52 -0700 Subject: [PATCH 2/3] only write empty body for okhttp --- core/src/main/java/feign/ReflectiveFeign.java | 5 ----- .../feign/httpclient/ApacheHttpClientTest.java | 15 +++++++++++++++ .../src/main/java/feign/okhttp/OkHttpClient.java | 10 +++++++++- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/feign/ReflectiveFeign.java b/core/src/main/java/feign/ReflectiveFeign.java index 249708745c..8ffafc67d3 100644 --- a/core/src/main/java/feign/ReflectiveFeign.java +++ b/core/src/main/java/feign/ReflectiveFeign.java @@ -204,11 +204,6 @@ public RequestTemplate create(Object[] argv) { protected RequestTemplate resolve(Object[] argv, RequestTemplate mutable, Map variables) { - if ("POST".equals(metadata.template().method()) && argv == null) { - // write an empty string for the 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/ - encoder.encode("", String.class, mutable); - } return mutable.resolve(variables); } } diff --git a/httpclient/src/test/java/feign/httpclient/ApacheHttpClientTest.java b/httpclient/src/test/java/feign/httpclient/ApacheHttpClientTest.java index c91ce3cc52..d9040003d6 100644 --- a/httpclient/src/test/java/feign/httpclient/ApacheHttpClientTest.java +++ b/httpclient/src/test/java/feign/httpclient/ApacheHttpClientTest.java @@ -23,6 +23,7 @@ import feign.Logger; import feign.RequestLine; import feign.Response; + import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -132,6 +133,17 @@ protected void log(String configKey, String format, Object... args) { api.post("foo"); } + @Test + public void noResponseBody() { + server.enqueue(new MockResponse()); + + TestInterface api = Feign.builder() + .client(new ApacheHttpClient()) + .target(TestInterface.class, "http://localhost:" + server.getPort()); + + api.noPostBody(); + } + interface TestInterface { @RequestLine("POST /?foo=bar&foo=baz&qux=") @@ -145,5 +157,8 @@ interface TestInterface { @RequestLine("PATCH /") @Headers("Accept: text/plain") String patch(); + + @RequestLine("POST") + String noPostBody(); } } diff --git a/okhttp/src/main/java/feign/okhttp/OkHttpClient.java b/okhttp/src/main/java/feign/okhttp/OkHttpClient.java index da3da5f738..e0ce87e843 100644 --- a/okhttp/src/main/java/feign/okhttp/OkHttpClient.java +++ b/okhttp/src/main/java/feign/okhttp/OkHttpClient.java @@ -25,6 +25,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.Reader; +import java.util.Arrays; import java.util.Collection; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -77,7 +78,14 @@ static Request toOkHttpRequest(feign.Request input) { requestBuilder.addHeader("Accept", "*/*"); } - RequestBody body = input.body() != null ? RequestBody.create(mediaType, input.body()) : null; + byte[] inputBody = input.body(); + if ("POST".equals(input.method()) && 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[] {}; + } + + RequestBody body = inputBody != null ? RequestBody.create(mediaType, inputBody) : null; requestBuilder.method(input.method(), body); return requestBuilder.build(); } From 5491f815072b6b5d078e5aae80e7688d149a391a Mon Sep 17 00:00:00 2001 From: Paul Nepywoda Date: Mon, 14 Sep 2015 01:53:48 -0700 Subject: [PATCH 3/3] more tests and cleanup for okhttp empty body fix --- CHANGELOG.md | 3 +++ core/src/main/java/feign/ReflectiveFeign.java | 11 ++++------- .../test/java/feign/client/DefaultClientTest.java | 13 +++++++++++++ okhttp/src/main/java/feign/okhttp/OkHttpClient.java | 2 +- 4 files changed, 21 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d09b251e0a..4c2629bbb4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +### Version 8.10 +* OkHttpClient sends an empty POST body to conform with okhttp 2.4+ + ### Version 8.9 * Skips error handling when return type is `Response` diff --git a/core/src/main/java/feign/ReflectiveFeign.java b/core/src/main/java/feign/ReflectiveFeign.java index 8ffafc67d3..97cb735cf8 100644 --- a/core/src/main/java/feign/ReflectiveFeign.java +++ b/core/src/main/java/feign/ReflectiveFeign.java @@ -18,7 +18,6 @@ import java.lang.reflect.InvocationHandler; import java.lang.reflect.Method; import java.lang.reflect.Proxy; -import java.lang.reflect.Type; import java.util.Collection; import java.util.LinkedHashMap; import java.util.List; @@ -144,7 +143,7 @@ public Map apply(Target key) { } else if (md.bodyIndex() != null) { buildTemplate = new BuildEncodedTemplateFromArgs(md, encoder); } else { - buildTemplate = new BuildTemplateByResolvingArgs(md, encoder); + buildTemplate = new BuildTemplateByResolvingArgs(md); } result.put(md.configKey(), factory.create(key, md, buildTemplate, options, decoder, errorDecoder)); @@ -157,11 +156,9 @@ private static class BuildTemplateByResolvingArgs implements RequestTemplate.Fac protected final MethodMetadata metadata; private final Map indexToExpander = new LinkedHashMap(); - private final Encoder encoder; - private BuildTemplateByResolvingArgs(MethodMetadata metadata, Encoder encoder) { + private BuildTemplateByResolvingArgs(MethodMetadata metadata) { this.metadata = metadata; - this.encoder = encoder; if (metadata.indexToExpanderClass().isEmpty()) { return; } @@ -213,7 +210,7 @@ private static class BuildFormEncodedTemplateFromArgs extends BuildTemplateByRes private final Encoder encoder; private BuildFormEncodedTemplateFromArgs(MethodMetadata metadata, Encoder encoder) { - super(metadata, encoder); + super(metadata); this.encoder = encoder; } @@ -242,7 +239,7 @@ private static class BuildEncodedTemplateFromArgs extends BuildTemplateByResolvi private final Encoder encoder; private BuildEncodedTemplateFromArgs(MethodMetadata metadata, Encoder encoder) { - super(metadata, encoder); + super(metadata); this.encoder = encoder; } diff --git a/core/src/test/java/feign/client/DefaultClientTest.java b/core/src/test/java/feign/client/DefaultClientTest.java index f0d2246369..c59a9afbc6 100644 --- a/core/src/test/java/feign/client/DefaultClientTest.java +++ b/core/src/test/java/feign/client/DefaultClientTest.java @@ -183,6 +183,16 @@ protected void log(String configKey, String format, Object... args) { api.post("foo"); } + @Test + public void noResponseBody() { + server.enqueue(new MockResponse()); + + TestInterface api = Feign.builder() + .target(TestInterface.class, "http://localhost:" + server.getPort()); + + api.noPostBody(); + } + interface TestInterface { @RequestLine("POST /?foo=bar&foo=baz&qux=") @@ -196,5 +206,8 @@ interface TestInterface { @RequestLine("PATCH /") @Headers("Accept: text/plain") String patch(); + + @RequestLine("POST") + String noPostBody(); } } diff --git a/okhttp/src/main/java/feign/okhttp/OkHttpClient.java b/okhttp/src/main/java/feign/okhttp/OkHttpClient.java index e0ce87e843..2e6abc9aec 100644 --- a/okhttp/src/main/java/feign/okhttp/OkHttpClient.java +++ b/okhttp/src/main/java/feign/okhttp/OkHttpClient.java @@ -82,7 +82,7 @@ static Request toOkHttpRequest(feign.Request input) { if ("POST".equals(input.method()) && 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[] {}; + inputBody = new byte[0]; } RequestBody body = inputBody != null ? RequestBody.create(mediaType, inputBody) : null;