From 0cb3a0aa5a46b092e975164155b950db3f07e02c Mon Sep 17 00:00:00 2001 From: Jared Komoroski Date: Fri, 17 Feb 2023 16:09:15 -0700 Subject: [PATCH] Ignore Jakarta Suspended and Context, Support BeanParam (#1942) Co-authored-by: Marvin Froeder --- jakarta/README.md | 2 + .../java/feign/jaxrs/JakartaContract.java | 127 +++++++++++++----- .../java/feign/jaxrs/JakartaContractTest.java | 73 ++++++++-- 3 files changed, 159 insertions(+), 43 deletions(-) diff --git a/jakarta/README.md b/jakarta/README.md index da4cae8e..ea18412d 100644 --- a/jakarta/README.md +++ b/jakarta/README.md @@ -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>.encode()`. +#### `@BeanParm` +Aggregates the above supported parameter annotations under a single value object. diff --git a/jakarta/src/main/java/feign/jaxrs/JakartaContract.java b/jakarta/src/main/java/feign/jaxrs/JakartaContract.java index 8e81f6dd..c89ac398 100644 --- a/jakarta/src/main/java/feign/jaxrs/JakartaContract.java +++ b/jakarta/src/main/java/feign/jaxrs/JakartaContract.java @@ -20,9 +20,12 @@ import feign.DeclarativeContract; 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 { @@ -93,6 +96,12 @@ public class JakartaContract extends DeclarativeContract { 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(); } @@ -113,37 +122,93 @@ public class JakartaContract extends DeclarativeContract { } 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. diff --git a/jakarta/src/test/java/feign/jaxrs/JakartaContractTest.java b/jakarta/src/test/java/feign/jaxrs/JakartaContractTest.java index 98ec2c0d..23f6c656 100644 --- a/jakarta/src/test/java/feign/jaxrs/JakartaContractTest.java +++ b/jakarta/src/test/java/feign/jaxrs/JakartaContractTest.java @@ -15,25 +15,23 @@ package feign.jaxrs; 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 @@ -41,6 +39,57 @@ import java.util.List; */ public class JakartaContractTest extends JAXRSContractTestSupport { + @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