[GH-1464] Add appendHeader that supports Literals (#1781)

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.

Author: kdavisk6

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

@@ -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);
         }
       }
     }
@@ -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);
@@ -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;

core/src/main/java/feign/template/HeaderTemplate.java

@@ -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.
    *
@@ -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.
    *
@@ -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) {
@@ -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() {

core/src/test/java/feign/FeignTest.java

@@ -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;
@@ -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 /")
@@ -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