Refactored Header and Query parameter JAXRS Contract Parsing (#896)

As part of 10.x, the `headers()` and `queries()` collections on the `RequestTemplate` were made read only. The `JAXRSContract` was still attempting to manipulate those directly. THis was missed due to a use case not accounted for in the tests. I've added the appropriate use case and corrected the usage of the template.
OpenFeign · Feb 12, 2019 · f2e1226 · f2e1226
1 parent c248bba
commit f2e1226
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 10 deletions.
diff --git a/jaxrs/src/main/java/feign/jaxrs/JAXRSContract.java b/jaxrs/src/main/java/feign/jaxrs/JAXRSContract.java
@@ -22,8 +22,6 @@
 import feign.Request;
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Method;
-import java.util.ArrayList;
-import java.util.Collection;
 import java.util.Collections;
 import javax.ws.rs.*;
 
@@ -155,15 +153,15 @@ protected boolean processAnnotationsOnParameter(
         String name = QueryParam.class.cast(parameterAnnotation).value();
         checkState(
             emptyToNull(name) != null, "QueryParam.value() was empty on parameter %s", paramIndex);
-        Collection<String> query = addTemplatedParam(data.template().queries().get(name), name);
+        String query = addTemplatedParam(name);
         data.template().query(name, query);
         nameParam(data, name, paramIndex);
         isHttpParam = true;
       } else if (annotationType == HeaderParam.class) {
         String name = HeaderParam.class.cast(parameterAnnotation).value();
         checkState(
             emptyToNull(name) != null, "HeaderParam.value() was empty on parameter %s", paramIndex);
-        Collection<String> header = addTemplatedParam(data.template().headers().get(name), name);
+        String header = addTemplatedParam(name);
         data.template().header(name, header);
         nameParam(data, name, paramIndex);
         isHttpParam = true;
@@ -180,11 +178,7 @@ protected boolean processAnnotationsOnParameter(
   }
 
   // Not using override as the super-type's method is deprecated and will be removed.
-  protected Collection<String> addTemplatedParam(Collection<String> possiblyNull, String name) {
-    if (possiblyNull == null) {
-      possiblyNull = new ArrayList<String>();
-    }
-    possiblyNull.add(String.format("{%s}", name));
-    return possiblyNull;
+  private String addTemplatedParam(String name) {
+    return String.format("{%s}", name);
   }
 }
diff --git a/jaxrs/src/test/java/feign/jaxrs/JAXRSContractTest.java b/jaxrs/src/test/java/feign/jaxrs/JAXRSContractTest.java
@@ -25,6 +25,7 @@
 import java.lang.annotation.Target;
 import java.net.URI;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import javax.ws.rs.Consumes;
@@ -396,6 +397,22 @@ public void methodPathWithoutLeadingSlashParsesCorrectly() throws Exception {
         .hasUrl("/base/specific");
   }
 
+  @Test
+  public void producesWithHeaderParamContainAllHeaders() throws Exception {
+    assertThat(
+            parseAndValidateMetadata(
+                    MixedAnnotations.class,
+                    "getWithHeaders",
+                    String.class,
+                    String.class,
+                    String.class)
+                .template())
+        .hasHeaders(entry("Accept", Arrays.asList("{Accept}", "application/json")))
+        .hasQueries(
+            entry("multiple", Arrays.asList("stuff", "{multiple}")),
+            entry("another", Collections.singletonList("{another}")));
+  }
+
   interface Methods {
 
     @POST
@@ -637,4 +654,15 @@ private MethodMetadata parseAndValidateMetadata(
     return contract.parseAndValidateMetadata(
         targetType, targetType.getMethod(method, parameterTypes));
   }
+
+  interface MixedAnnotations {
+
+    @GET
+    @Path("/api/stuff?multiple=stuff")
+    @Produces("application/json")
+    Response getWithHeaders(
+        @HeaderParam("Accept") String accept,
+        @QueryParam("multiple") String multiple,
+        @QueryParam("another") String another);
+  }
 }