Skip to content

Commit

Permalink
[GH-1464] Add appendHeader that supports Literals (#1781)
Browse files Browse the repository at this point in the history
This change adds a new `appendHeader` internal method to `RequestTemplate`
allowing for already resolved headers to be added to the resolved `RequestTemplate`
preventing duplicate expression processing by using another new method
`HeaderTemplate.literal` and `HeaderTemplate.appendLiteral` respectively.

I chose this route as it isolates the change to be applied only after the
original `HeaderTemplate` has been resolved.  While it does expose new
public `HeaderTemplate` APIs, I feel that is an OK trade off, allowing
a new escape-hatch for situations where URI template processing is not
acceptable for Header values.
  • Loading branch information
kdavisk6 committed Oct 6, 2022
1 parent 9a60ea6 commit 92b2f51
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 11 deletions.
30 changes: 26 additions & 4 deletions core/src/main/java/feign/RequestTemplate.java
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,8 @@ public RequestTemplate resolve(Map<String, ?> variables) {
String header = headerTemplate.expand(variables);
if (!header.isEmpty()) {
/* append the header as a new literal as the value has already been expanded. */
resolved.header(headerTemplate.getName(), header);
resolved.appendHeader(
headerTemplate.getName(), Collections.singletonList(header), true);
}
}
}
Expand Down Expand Up @@ -723,13 +724,26 @@ public RequestTemplate removeHeader(String name) {
}

/**
* Create a Header Template.
* Append the Header. Will create a new Header if it doesn't already exist. Treats all values as
* potentially expressions.
*
* @param name of the header
* @param values for the header, may be expressions.
* @return a RequestTemplate for chaining.
*/
private RequestTemplate appendHeader(String name, Iterable<String> values) {
return this.appendHeader(name, values, false);
}

/**
* Append the Header. Will create a new Header if it doesn't already exist.
*
* @param name of the header
* @param values for the header, may be expressions.
* @param literal indicator, to treat the values as literals and not expressions
* @return a RequestTemplate for chaining.
*/
private RequestTemplate appendHeader(String name, Iterable<String> values, boolean literal) {
if (!values.iterator().hasNext()) {
/* empty value, clear the existing values */
this.headers.remove(name);
Expand All @@ -745,9 +759,17 @@ private RequestTemplate appendHeader(String name, Iterable<String> values) {
}
this.headers.compute(name, (headerName, headerTemplate) -> {
if (headerTemplate == null) {
return HeaderTemplate.create(headerName, values);
if (literal) {
return HeaderTemplate.literal(headerName, values);
} else {
return HeaderTemplate.create(headerName, values);
}
} else {
return HeaderTemplate.append(headerTemplate, values);
if (literal) {
return HeaderTemplate.appendLiteral(headerTemplate, values);
} else {
return HeaderTemplate.append(headerTemplate, values);
}
}
});
return this;
Expand Down
65 changes: 58 additions & 7 deletions core/src/main/java/feign/template/HeaderTemplate.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,18 @@ public static HeaderTemplate create(String name, Iterable<String> values) {
return new HeaderTemplate(name, values, Util.UTF_8);
}

public static HeaderTemplate literal(String name, Iterable<String> values) {
if (name == null || name.isEmpty()) {
throw new IllegalArgumentException("name is required.");
}

if (values == null) {
throw new IllegalArgumentException("values are required");
}

return new HeaderTemplate(name, values, Util.UTF_8, true);
}

/**
* Append values to a Header Template.
*
Expand All @@ -63,6 +75,22 @@ public static HeaderTemplate append(HeaderTemplate headerTemplate, Iterable<Stri
return create(headerTemplate.getName(), headerValues);
}

/**
* Append values to a Header Template, as literals
*
* @param headerTemplate to append to.
* @param values to append.
* @return a new Header Template with the values added.
*/
public static HeaderTemplate appendLiteral(HeaderTemplate headerTemplate,
Iterable<String> values) {
LinkedHashSet<String> headerValues = new LinkedHashSet<>(headerTemplate.getValues());
headerValues.addAll(StreamSupport.stream(values.spliterator(), false)
.filter(Util::isNotBlank)
.collect(Collectors.toCollection(LinkedHashSet::new)));
return literal(headerTemplate.getName(), headerValues);
}

/**
* Create a new Header Template.
*
Expand All @@ -71,6 +99,18 @@ public static HeaderTemplate append(HeaderTemplate headerTemplate, Iterable<Stri
* @param charset to use when encoding the values.
*/
private HeaderTemplate(String name, Iterable<String> values, Charset charset) {
this(name, values, charset, false);
}

/**
* Create a new Header Template.
*
* @param name of the header
* @param values of the header
* @param charset for the header
* @param literal indicator. Will treat all values as literals instead of possible expressions.
*/
private HeaderTemplate(String name, Iterable<String> values, Charset charset, boolean literal) {
this.name = name;

for (String value : values) {
Expand All @@ -79,14 +119,25 @@ private HeaderTemplate(String name, Iterable<String> values, Charset charset) {
continue;
}

this.values.add(
new Template(
value,
ExpansionOptions.REQUIRED,
EncodingOptions.NOT_REQUIRED,
false,
charset));
if (literal) {
this.values.add(
new Template(
ExpansionOptions.ALLOW_UNRESOLVED,
EncodingOptions.NOT_REQUIRED,
false,
charset,
Collections.singletonList(Literal.create(value))));
} else {
this.values.add(
new Template(
value,
ExpansionOptions.REQUIRED,
EncodingOptions.NOT_REQUIRED,
false,
charset));
}
}

}

public Collection<String> getValues() {
Expand Down
20 changes: 20 additions & 0 deletions core/src/test/java/feign/FeignTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import okhttp3.mockwebserver.MockWebServer;
import okhttp3.mockwebserver.SocketPolicy;
import okio.Buffer;
import org.assertj.core.data.MapEntry;
import org.assertj.core.util.Maps;
import org.junit.Rule;
import org.junit.Test;
Expand Down Expand Up @@ -986,6 +987,21 @@ public void matrixParametersAlsoSupportMaps() throws Exception {

}

@Test
public void supportComplexHeaders() throws Exception {
TestInterface api = new TestInterfaceBuilder()
.target("http://localhost:" + server.getPort());

server.enqueue(new MockResponse());
/* demonstrate that a complex header, like a JSON document, is supported */
String complex = "{ \"object\": \"value\", \"second\": \"string\" }";

api.supportComplexHttpHeaders(complex);
assertThat(server.takeRequest())
.hasHeaders(MapEntry.entry("custom", Collections.singletonList(complex)))
.hasPath("/settings");
}

interface TestInterface {

@RequestLine("POST /")
Expand Down Expand Up @@ -1077,6 +1093,10 @@ void queryMapWithQueryParams(@Param("name") String name,
@RequestLine("GET /settings{;props}")
void matrixParametersWithMap(@Param("props") Map<String, Object> owners);

@RequestLine("GET /settings")
@Headers("Custom: {complex}")
void supportComplexHttpHeaders(@Param("complex") String complex);

class DateToMillis implements Param.Expander {

@Override
Expand Down

0 comments on commit 92b2f51

Please sign in to comment.