Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore Jakarta Suspended and Context, Support BeanParam #1942

Merged
merged 2 commits into from
Feb 17, 2023
Merged

Ignore Jakarta Suspended and Context, Support BeanParam #1942

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cf83a6a
Ignore Jakarta Suspended and Context, Support BeanParam
JKomoroski Feb 17, 2023
0b007be
Merge branch 'master' into issue/1940/jakarta_support_jaxrs_2
velo Feb 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions jakarta/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,5 @@ Links the value of the corresponding parameter to a query parameter. When invok
Links the value of the corresponding parameter to a header.
#### `@FormParam`
Links the value of the corresponding parameter to a key passed to `Encoder.Text<Map<String, Object>>.encode()`.
#### `@BeanParm`
Aggregates the above supported parameter annotations under a single value object.
127 changes: 96 additions & 31 deletions jakarta/src/main/java/feign/jaxrs/JakartaContract.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,12 @@
import feign.MethodMetadata;
import feign.Request;
import java.lang.annotation.Annotation;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.util.Collections;
import jakarta.ws.rs.*;
import jakarta.ws.rs.container.Suspended;
import jakarta.ws.rs.core.Context;

public class JakartaContract extends DeclarativeContract {

Expand Down Expand Up @@ -93,6 +96,12 @@ public JakartaContract() {
super.registerMethodAnnotation(Consumes.class, this::handleConsumesAnnotation);
super.registerMethodAnnotation(Produces.class, this::handleProducesAnnotation);

// parameter with unsupported jax-rs annotations should not be passed as body params.
// this will prevent interfaces from becoming unusable entirely due to single (unsupported)
// endpoints.
// https://github.com/OpenFeign/feign/issues/669
super.registerParameterAnnotation(Suspended.class, (ann, data, i) -> data.ignoreParamater(i));
super.registerParameterAnnotation(Context.class, (ann, data, i) -> data.ignoreParamater(i));
// trying to minimize the diff
registerParamAnnotations();
}
Expand All @@ -113,37 +122,93 @@ private void handleConsumesAnnotation(Consumes consumes, MethodMetadata data) {
}

protected void registerParamAnnotations() {
{
registerParameterAnnotation(PathParam.class, (param, data, paramIndex) -> {
final String name = param.value();
checkState(emptyToNull(name) != null, "PathParam.value() was empty on parameter %s",
paramIndex);
nameParam(data, name, paramIndex);
});
registerParameterAnnotation(QueryParam.class, (param, data, paramIndex) -> {
final String name = param.value();
checkState(emptyToNull(name) != null, "QueryParam.value() was empty on parameter %s",
paramIndex);
final String query = addTemplatedParam(name);
data.template().query(name, query);
nameParam(data, name, paramIndex);
});
registerParameterAnnotation(HeaderParam.class, (param, data, paramIndex) -> {
final String name = param.value();
checkState(emptyToNull(name) != null, "HeaderParam.value() was empty on parameter %s",
paramIndex);
final String header = addTemplatedParam(name);
data.template().header(name, header);
nameParam(data, name, paramIndex);
});
registerParameterAnnotation(FormParam.class, (param, data, paramIndex) -> {
final String name = param.value();
checkState(emptyToNull(name) != null, "FormParam.value() was empty on parameter %s",
paramIndex);
data.formParams().add(name);
nameParam(data, name, paramIndex);
});
}

registerParameterAnnotation(PathParam.class, (param, data, paramIndex) -> {
final String name = param.value();
checkState(emptyToNull(name) != null, "PathParam.value() was empty on parameter %s",
paramIndex);
nameParam(data, name, paramIndex);
});
registerParameterAnnotation(QueryParam.class, (param, data, paramIndex) -> {
final String name = param.value();
checkState(emptyToNull(name) != null, "QueryParam.value() was empty on parameter %s",
paramIndex);
final String query = addTemplatedParam(name);
data.template().query(name, query);
nameParam(data, name, paramIndex);
});
registerParameterAnnotation(HeaderParam.class, (param, data, paramIndex) -> {
final String name = param.value();
checkState(emptyToNull(name) != null, "HeaderParam.value() was empty on parameter %s",
paramIndex);
final String header = addTemplatedParam(name);
data.template().header(name, header);
nameParam(data, name, paramIndex);
});
registerParameterAnnotation(FormParam.class, (param, data, paramIndex) -> {
final String name = param.value();
checkState(emptyToNull(name) != null, "FormParam.value() was empty on parameter %s",
paramIndex);
data.formParams().add(name);
nameParam(data, name, paramIndex);
});

// Reflect over the Bean Param looking for supported parameter annotations
registerParameterAnnotation(BeanParam.class, (param, data, paramIndex) -> {
final Field[] aggregatedParams = data.method()
.getParameters()[paramIndex]
.getType()
.getDeclaredFields();

for (Field aggregatedParam : aggregatedParams) {

if (aggregatedParam.isAnnotationPresent(PathParam.class)) {
final String name = aggregatedParam.getAnnotation(PathParam.class).value();
checkState(
emptyToNull(name) != null,
"BeanParam parameter %s contains PathParam with empty .value() on field %s",
paramIndex,
aggregatedParam.getName());
nameParam(data, name, paramIndex);
}

if (aggregatedParam.isAnnotationPresent(QueryParam.class)) {
final String name = aggregatedParam.getAnnotation(QueryParam.class).value();
checkState(
emptyToNull(name) != null,
"BeanParam parameter %s contains QueryParam with empty .value() on field %s",
paramIndex,
aggregatedParam.getName());
final String query = addTemplatedParam(name);
data.template().query(name, query);
nameParam(data, name, paramIndex);
}

if (aggregatedParam.isAnnotationPresent(HeaderParam.class)) {
final String name = aggregatedParam.getAnnotation(HeaderParam.class).value();
checkState(
emptyToNull(name) != null,
"BeanParam parameter %s contains HeaderParam with empty .value() on field %s",
paramIndex,
aggregatedParam.getName());
final String header = addTemplatedParam(name);
data.template().header(name, header);
nameParam(data, name, paramIndex);
}

if (aggregatedParam.isAnnotationPresent(FormParam.class)) {
final String name = aggregatedParam.getAnnotation(FormParam.class).value();
checkState(
emptyToNull(name) != null,
"BeanParam parameter %s contains FormParam with empty .value() on field %s",
paramIndex,
aggregatedParam.getName());
data.formParams().add(name);
nameParam(data, name, paramIndex);
}
}
});

}

// Not using override as the super-type's method is deprecated and will be removed.
Expand Down
73 changes: 61 additions & 12 deletions jakarta/src/test/java/feign/jaxrs/JakartaContractTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,32 +15,81 @@

import feign.MethodMetadata;
import feign.Response;
import jakarta.ws.rs.Consumes;
import jakarta.ws.rs.DELETE;
import jakarta.ws.rs.FormParam;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.HeaderParam;
import jakarta.ws.rs.HttpMethod;
import jakarta.ws.rs.POST;
import jakarta.ws.rs.PUT;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.PathParam;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.QueryParam;
import feign.jaxrs.JakartaContractTest.JakartaInternals.BeanParamInput;
import jakarta.ws.rs.*;
import jakarta.ws.rs.container.AsyncResponse;
import jakarta.ws.rs.container.Suspended;
import jakarta.ws.rs.core.Context;
import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.UriInfo;
import org.junit.Test;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.net.URI;
import java.util.List;
import static feign.assertj.FeignAssertions.assertThat;
import static java.util.Arrays.asList;
import static org.assertj.core.data.MapEntry.entry;

/**
* Tests interfaces defined per {@link JakartaContract} are interpreted into expected
* {@link feign .RequestTemplate template} instances.
*/
public class JakartaContractTest extends JAXRSContractTestSupport<JakartaContract> {

@Test
public void injectJaxrsInternals() throws Exception {
final MethodMetadata methodMetadata =
parseAndValidateMetadata(JakartaInternals.class, "inject", AsyncResponse.class,
UriInfo.class);
assertThat(methodMetadata.template())
.noRequestBody();
}

@Test
public void injectBeanParam() throws Exception {
final MethodMetadata methodMetadata =
parseAndValidateMetadata(JakartaInternals.class, "beanParameters", BeanParamInput.class);
assertThat(methodMetadata.template())
.noRequestBody();

assertThat(methodMetadata.template())
.hasHeaders(entry("X-Custom-Header", asList("{X-Custom-Header}")));
assertThat(methodMetadata.template())
.hasQueries(entry("query", asList("{query}")));
assertThat(methodMetadata.formParams())
.isNotEmpty()
.containsExactly("form");

}

public interface JakartaInternals {
@GET
@Path("/")
void inject(@Suspended AsyncResponse ar, @Context UriInfo info);

@Path("/{path}")
@POST
void beanParameters(@BeanParam BeanParamInput beanParam);

public class BeanParamInput {

@PathParam("path")
String path;

@QueryParam("query")
String query;

@FormParam("form")
String form;

@HeaderParam("X-Custom-Header")
String header;
}
}

interface Methods {

@POST
Expand Down