Skip to content

Commit

Permalink
Allows @param value in default contract to be taken from param names. (
Browse files Browse the repository at this point in the history
…#1309)

JEP 118 introduced javac option `-parameters` that adds to bytecode
method parameter names, so if code is compiled with that option
and @param value is empty we can get that template parameter name
from method parameter name.

Fixes #1297.
  • Loading branch information
krzyk committed Dec 21, 2020
1 parent b439145 commit 17885da
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 3 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ should work. Feign's default contract defines the following annotations:
| Annotation | Interface Target | Usage |
|----------------|------------------|-------|
| `@RequestLine` | Method | Defines the `HttpMethod` and `UriTemplate` for request. `Expressions`, values wrapped in curly-braces `{expression}` are resolved using their corresponding `@Param` annotated parameters. |
| `@Param` | Parameter | Defines a template variable, whose value will be used to resolve the corresponding template `Expression`, by name. |
| `@Param` | Parameter | Defines a template variable, whose value will be used to resolve the corresponding template `Expression`, by name provided as annotation value. If value is missing it will try to get the name from bytecode method parameter name (if the code was compiled with `-parameters` flag). |
| `@Headers` | Method, Type | Defines a `HeaderTemplate`; a variation on a `UriTemplate`. that uses `@Param` annotated values to resolve the corresponding `Expressions`. When used on a `Type`, the template will be applied to every request. When used on a `Method`, the template will apply only to the annotated method. |
| `@QueryMap` | Parameter | Defines a `Map` of name-value pairs, or POJO, to expand into a query string. |
| `@HeaderMap` | Parameter | Defines a `Map` of name-value pairs, to expand into `Http Headers` |
Expand Down
9 changes: 8 additions & 1 deletion core/src/main/java/feign/Contract.java
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,14 @@ public Default() {
data.template().headers(toMap(headersOnMethod));
});
super.registerParameterAnnotation(Param.class, (paramAnnotation, data, paramIndex) -> {
final String name = paramAnnotation.value();
final String annotationName = paramAnnotation.value();
final Parameter parameter = data.method().getParameters()[paramIndex];
final String name;
if (emptyToNull(annotationName) == null && parameter.isNamePresent()) {
name = parameter.getName();
} else {
name = annotationName;
}
checkState(emptyToNull(name) != null, "Param annotation was empty on param %s.",
paramIndex);
nameParam(data, name, paramIndex);
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/feign/Param.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
/**
* The name of the template parameter.
*/
String value();
String value() default "";

/**
* How to expand the value of this parameter, if {@link ToStringExpander} isn't adequate.
Expand Down
21 changes: 21 additions & 0 deletions core/src/test/java/feign/DefaultContractTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,21 @@ public void pathAndQueryParams() throws Exception {
entry(2, asList("type")));
}

@Test
public void autoDiscoverParamNames() throws Exception {
final MethodMetadata md = parseAndValidateMetadata(AutoDiscoverParamNames.class,
"recordsByNameAndType", int.class, String.class,
String.class);

assertThat(md.template())
.hasQueries(entry("name", asList("{name}")), entry("type", asList("{type}")));

assertThat(md.indexToName()).containsExactly(
entry(0, asList("domainId")),
entry(1, asList("name")),
entry(2, asList("type")));
}

@Test
public void bodyWithTemplate() throws Exception {
final MethodMetadata md = parseAndValidateMetadata(FormParams.class,
Expand Down Expand Up @@ -517,6 +532,12 @@ Response recordsByNameAndType(@Param("domainId") int id,
@Param("type") String typeFilter);
}

interface AutoDiscoverParamNames {

@RequestLine("GET /domains/{domainId}/records?name={name}&type={type}")
Response recordsByNameAndType(@Param int domainId, @Param String name, @Param() String type);
}

interface FormParams {

@RequestLine("POST /")
Expand Down
15 changes: 15 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,21 @@
<target>${main.java.version}</target>
</configuration>
</execution>
<execution>
<id>default-test-compile</id>
<phase>test-compile</phase>
<goals>
<goal>testCompile</goal>
</goals>
<configuration>
<fork>true</fork>
<compilerArgs>
<arg>-parameters</arg>
</compilerArgs>
<source>${main.java.version}</source>
<target>${main.java.version}</target>
</configuration>
</execution>
</executions>
</plugin>

Expand Down

0 comments on commit 17885da

Please sign in to comment.