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 · f821a93 · f821a93
1 parent 4a5d1c2
commit f821a93
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 8 deletions.
diff --git a/jaxrs/src/main/java/feign/jaxrs/JAXRSContract.java b/jaxrs/src/main/java/feign/jaxrs/JAXRSContract.java
@@ -154,15 +154,15 @@ protected boolean processAnnotationsOnParameter(MethodMetadata data,
         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;
@@ -179,11 +179,7 @@ protected boolean processAnnotationsOnParameter(MethodMetadata data,
   }
 
   // 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
@@ -14,6 +14,7 @@
 package feign.jaxrs;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import org.junit.Rule;
 import org.junit.Test;
@@ -394,6 +395,18 @@ 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
@@ -638,4 +651,14 @@ private MethodMetadata parseAndValidateMetadata(Class<?> targetType,
     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);
+  }
 }