From 989fbd7c1f4029c6ad5900f0c3e591323cc11ffc Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Sat, 29 Sep 2018 15:05:08 -0400 Subject: [PATCH] fix #797 fix #798: JAXRSContract sets a single Content-Type value (#799) Closes #797 Closes #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 --- core/src/main/java/feign/RequestTemplate.java | 5 +++++ .../test/java/feign/assertj/RequestTemplateAssert.java | 2 +- jaxrs/README.md | 2 +- jaxrs/src/main/java/feign/jaxrs/JAXRSContract.java | 9 ++++++--- jaxrs/src/test/java/feign/jaxrs/JAXRSContractTest.java | 8 ++++---- jaxrs2/README.md | 2 +- 6 files changed, 18 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/feign/RequestTemplate.java b/core/src/main/java/feign/RequestTemplate.java index 87745b05..ddbf90c8 100644 --- a/core/src/main/java/feign/RequestTemplate.java +++ b/core/src/main/java/feign/RequestTemplate.java @@ -663,6 +663,11 @@ public final class RequestTemplate implements Serializable { * @return a RequestTemplate for chaining. */ private RequestTemplate appendHeader(String name, Iterable 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 index 599a42a3..6a36ed27 100644 --- a/core/src/test/java/feign/assertj/RequestTemplateAssert.java +++ b/core/src/test/java/feign/assertj/RequestTemplateAssert.java @@ -85,7 +85,7 @@ public final class RequestTemplateAssert 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 index 7a16ce7c..966ed66c 100644 --- 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 index 78d611f7..5a1f954e 100644 --- a/jaxrs/src/main/java/feign/jaxrs/JAXRSContract.java +++ b/jaxrs/src/main/java/feign/jaxrs/JAXRSContract.java @@ -20,7 +20,10 @@ import javax.ws.rs.*; 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,7 +110,7 @@ public class JAXRSContract extends Contract.BaseContract { 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); } @@ -115,8 +118,8 @@ public class JAXRSContract extends Contract.BaseContract { 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 index 5fea5cfa..95b8ed1d 100644 --- a/jaxrs/src/test/java/feign/jaxrs/JAXRSContractTest.java +++ b/jaxrs/src/test/java/feign/jaxrs/JAXRSContractTest.java @@ -120,7 +120,7 @@ public class JAXRSContractTest { 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 class JAXRSContractTest { 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 class JAXRSContractTest { /* 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 class JAXRSContractTest { 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 index faecd81a..cbdd4be7 100644 --- 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