Skip to content

Commit f2e1226

Browse files
authored
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.
1 parent c248bba commit f2e1226

File tree

2 files changed

+32
-10
lines changed

2 files changed

+32
-10
lines changed

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

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@
2222
import feign.Request;
2323
import java.lang.annotation.Annotation;
2424
import java.lang.reflect.Method;
25-
import java.util.ArrayList;
26-
import java.util.Collection;
2725
import java.util.Collections;
2826
import javax.ws.rs.*;
2927

@@ -155,15 +153,15 @@ protected boolean processAnnotationsOnParameter(
155153
String name = QueryParam.class.cast(parameterAnnotation).value();
156154
checkState(
157155
emptyToNull(name) != null, "QueryParam.value() was empty on parameter %s", paramIndex);
158-
Collection<String> query = addTemplatedParam(data.template().queries().get(name), name);
156+
String query = addTemplatedParam(name);
159157
data.template().query(name, query);
160158
nameParam(data, name, paramIndex);
161159
isHttpParam = true;
162160
} else if (annotationType == HeaderParam.class) {
163161
String name = HeaderParam.class.cast(parameterAnnotation).value();
164162
checkState(
165163
emptyToNull(name) != null, "HeaderParam.value() was empty on parameter %s", paramIndex);
166-
Collection<String> header = addTemplatedParam(data.template().headers().get(name), name);
164+
String header = addTemplatedParam(name);
167165
data.template().header(name, header);
168166
nameParam(data, name, paramIndex);
169167
isHttpParam = true;
@@ -180,11 +178,7 @@ protected boolean processAnnotationsOnParameter(
180178
}
181179

182180
// Not using override as the super-type's method is deprecated and will be removed.
183-
protected Collection<String> addTemplatedParam(Collection<String> possiblyNull, String name) {
184-
if (possiblyNull == null) {
185-
possiblyNull = new ArrayList<String>();
186-
}
187-
possiblyNull.add(String.format("{%s}", name));
188-
return possiblyNull;
181+
private String addTemplatedParam(String name) {
182+
return String.format("{%s}", name);
189183
}
190184
}

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import java.lang.annotation.Target;
2626
import java.net.URI;
2727
import java.util.ArrayList;
28+
import java.util.Arrays;
2829
import java.util.Collections;
2930
import java.util.List;
3031
import javax.ws.rs.Consumes;
@@ -396,6 +397,22 @@ public void methodPathWithoutLeadingSlashParsesCorrectly() throws Exception {
396397
.hasUrl("/base/specific");
397398
}
398399

400+
@Test
401+
public void producesWithHeaderParamContainAllHeaders() throws Exception {
402+
assertThat(
403+
parseAndValidateMetadata(
404+
MixedAnnotations.class,
405+
"getWithHeaders",
406+
String.class,
407+
String.class,
408+
String.class)
409+
.template())
410+
.hasHeaders(entry("Accept", Arrays.asList("{Accept}", "application/json")))
411+
.hasQueries(
412+
entry("multiple", Arrays.asList("stuff", "{multiple}")),
413+
entry("another", Collections.singletonList("{another}")));
414+
}
415+
399416
interface Methods {
400417

401418
@POST
@@ -637,4 +654,15 @@ private MethodMetadata parseAndValidateMetadata(
637654
return contract.parseAndValidateMetadata(
638655
targetType, targetType.getMethod(method, parameterTypes));
639656
}
657+
658+
interface MixedAnnotations {
659+
660+
@GET
661+
@Path("/api/stuff?multiple=stuff")
662+
@Produces("application/json")
663+
Response getWithHeaders(
664+
@HeaderParam("Accept") String accept,
665+
@QueryParam("multiple") String multiple,
666+
@QueryParam("another") String another);
667+
}
640668
}

0 commit comments

Comments
 (0)