Browse Source

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
pull/784/merge
Carter Kozak 6 years ago committed by Kevin Davis
parent
commit
989fbd7c1f
  1. 5
      core/src/main/java/feign/RequestTemplate.java
  2. 2
      core/src/test/java/feign/assertj/RequestTemplateAssert.java
  3. 2
      jaxrs/README.md
  4. 9
      jaxrs/src/main/java/feign/jaxrs/JAXRSContract.java
  5. 8
      jaxrs/src/test/java/feign/jaxrs/JAXRSContractTest.java
  6. 2
      jaxrs2/README.md

5
core/src/main/java/feign/RequestTemplate.java

@ -663,6 +663,11 @@ public final class RequestTemplate implements Serializable { @@ -663,6 +663,11 @@ public final class RequestTemplate implements Serializable {
* @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);

2
core/src/test/java/feign/assertj/RequestTemplateAssert.java

@ -85,7 +85,7 @@ public final class RequestTemplateAssert @@ -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;
}

2
jaxrs/README.md

@ -23,7 +23,7 @@ Appends the value to `Target.url()`. Can have tokens corresponding to `@PathPar @@ -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.

9
jaxrs/src/main/java/feign/jaxrs/JAXRSContract.java

@ -20,7 +20,10 @@ import javax.ws.rs.*; @@ -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 { @@ -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 { @@ -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]);
}
/**

8
jaxrs/src/test/java/feign/jaxrs/JAXRSContractTest.java

@ -120,7 +120,7 @@ public class JAXRSContractTest { @@ -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 { @@ -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 { @@ -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 { @@ -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")));
}

2
jaxrs2/README.md

@ -23,7 +23,7 @@ Sets the request method. @@ -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

Loading…
Cancel
Save