diff --git a/CHANGES.md b/CHANGES.md index cccc683f..5e5c24ca 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,6 @@ +### Version 4.2/3.3 +* Document and enforce JAX-RS annotation processing from server POV + ### Version 4.1/3.2 * update to dagger 1.1 * Add wikipedia search example diff --git a/feign-jaxrs/README.md b/feign-jaxrs/README.md new file mode 100644 index 00000000..5f53d92f --- /dev/null +++ b/feign-jaxrs/README.md @@ -0,0 +1,37 @@ +# Feign JAXRS +This module overrides annotation processing to instead use standard ones supplied by the JAX-RS specification. This is currently targeted at the 1.1 spec. + +## Limitations +While it may appear possible to reuse the same interface across client and server, bear in mind that JAX-RS resource + annotations were not designed to be processed by clients. Moreover, JAX-RS 2.0 has a different package hierarchy for +client invocation. Finally, JAX-RS is a large spec and attempts to implement it completely would be a project larger +than feign itself. In other words, this implementation is *best efforts* and concedes far from 100% compatibility with +server interface behavior. + +## Currently Supported Annotation Processing +Feign only supports processing java interfaces (not abstract or concrete classes). + +ISE is raised when any annotation's value is empty or null. Ex. `Path("")` raises an ISE. + +Here are a list of behaviors currently supported. +### Type Annotations +#### `@Path` +Appends the value to `Target.url()`. Can have tokens corresponding to `@PathParam` annotations. +### Method Annotations +#### `@HttpMethod` meta-annotation (present on `@GET`, `@POST`, etc.) +Sets the request method. +#### `@Path` +Appends the value to `Target.url()`. Can have tokens corresponding to `@PathParam` annotations. +#### `@Produces` +Adds the first value as the `Accept` header. +#### `@Consumes` +Adds the first value as the `Content-Type` header. +### Parameter Annotations +#### `@PathParam` +Links the value of the corresponding parameter to a template variable declared in the path. +#### `@QueryParam` +Links the value of the corresponding parameter to a query parameter. +#### `@HeaderParam` +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()`. diff --git a/feign-jaxrs/src/main/java/feign/jaxrs/JAXRSModule.java b/feign-jaxrs/src/main/java/feign/jaxrs/JAXRSModule.java index db5d7883..dc84c8e2 100644 --- a/feign-jaxrs/src/main/java/feign/jaxrs/JAXRSModule.java +++ b/feign-jaxrs/src/main/java/feign/jaxrs/JAXRSModule.java @@ -33,7 +33,12 @@ import java.lang.reflect.Method; import java.util.Collection; import static feign.Util.checkState; +import static feign.Util.emptyToNull; +/** + * Please refer to the + * Feign JAX-RS README. + */ @dagger.Module(library = true, overrides = true) public final class JAXRSModule { static final String ACCEPT = "Accept"; @@ -50,7 +55,9 @@ public final class JAXRSModule { MethodMetadata md = super.parseAndValidatateMetadata(method); Path path = method.getDeclaringClass().getAnnotation(Path.class); if (path != null) { - md.template().insert(0, path.value()); + String pathValue = emptyToNull(path.value()); + checkState(pathValue != null, "Path.value() was empty on type %s", method.getDeclaringClass().getName()); + md.template().insert(0, pathValue); } return md; } @@ -64,19 +71,20 @@ public final class JAXRSModule { "Method %s contains multiple HTTP methods. Found: %s and %s", method.getName(), data.template() .method(), http.value()); data.template().method(http.value()); - } else if (annotationType == Body.class) { - String body = Body.class.cast(methodAnnotation).value(); - if (body.indexOf('{') == -1) { - data.template().body(body); - } else { - data.template().bodyTemplate(body); - } } else if (annotationType == Path.class) { + String pathValue = emptyToNull(Path.class.cast(methodAnnotation).value()); + checkState(pathValue != null, "Path.value() was empty on method %s", method.getName()); data.template().append(Path.class.cast(methodAnnotation).value()); } else if (annotationType == Produces.class) { - data.template().header(CONTENT_TYPE, join(',', ((Produces) methodAnnotation).value())); + String[] serverProduces = ((Produces) methodAnnotation).value(); + String clientAccepts = serverProduces.length == 0 ? null: emptyToNull(serverProduces[0]); + checkState(clientAccepts != null, "Produces.value() was empty on method %s", method.getName()); + data.template().header(ACCEPT, clientAccepts); } else if (annotationType == Consumes.class) { - data.template().header(ACCEPT, join(',', ((Consumes) methodAnnotation).value())); + String[] serverConsumes = ((Consumes) methodAnnotation).value(); + String clientProduces = serverConsumes.length == 0 ? null: emptyToNull(serverConsumes[0]); + checkState(clientProduces != null, "Consumes.value() was empty on method %s", method.getName()); + data.template().header(CONTENT_TYPE, clientProduces); } } @@ -87,22 +95,26 @@ public final class JAXRSModule { Class annotationType = parameterAnnotation.annotationType(); if (annotationType == PathParam.class) { String name = PathParam.class.cast(parameterAnnotation).value(); + checkState(emptyToNull(name) != null, "PathParam.value() was empty on parameter %s", paramIndex); nameParam(data, name, paramIndex); isHttpParam = true; } else if (annotationType == QueryParam.class) { String name = QueryParam.class.cast(parameterAnnotation).value(); + checkState(emptyToNull(name) != null, "QueryParam.value() was empty on parameter %s", paramIndex); Collection query = addTemplatedParam(data.template().queries().get(name), 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 header = addTemplatedParam(data.template().headers().get(name), name); data.template().header(name, header); nameParam(data, name, paramIndex); isHttpParam = true; } else if (annotationType == FormParam.class) { String name = FormParam.class.cast(parameterAnnotation).value(); + checkState(emptyToNull(name) != null, "FormParam.value() was empty on parameter %s", paramIndex); data.formParams().add(name); nameParam(data, name, paramIndex); isHttpParam = true; @@ -111,17 +123,4 @@ public final class JAXRSModule { return isHttpParam; } } - - private static String join(char separator, String... parts) { - if (parts == null || parts.length == 0) - return ""; - StringBuilder to = new StringBuilder(); - for (int i = 0; i < parts.length; i++) { - to.append(parts[i]); - if (i + 1 < parts.length) { - to.append(separator); - } - } - return to.toString(); - } } diff --git a/feign-jaxrs/src/test/java/feign/jaxrs/JAXRSContractTest.java b/feign-jaxrs/src/test/java/feign/jaxrs/JAXRSContractTest.java index cad033a8..1669e369 100644 --- a/feign-jaxrs/src/test/java/feign/jaxrs/JAXRSContractTest.java +++ b/feign-jaxrs/src/test/java/feign/jaxrs/JAXRSContractTest.java @@ -18,13 +18,13 @@ package feign.jaxrs; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.gson.reflect.TypeToken; -import feign.Body; import feign.MethodMetadata; import feign.Observable; import feign.Observer; import feign.Response; import org.testng.annotations.Test; +import javax.ws.rs.Consumes; import javax.ws.rs.DELETE; import javax.ws.rs.FormParam; import javax.ws.rs.GET; @@ -44,14 +44,15 @@ import java.lang.reflect.Type; import java.net.URI; import java.util.List; +import static feign.jaxrs.JAXRSModule.ACCEPT; import static feign.jaxrs.JAXRSModule.CONTENT_TYPE; import static javax.ws.rs.HttpMethod.DELETE; import static javax.ws.rs.HttpMethod.GET; import static javax.ws.rs.HttpMethod.POST; import static javax.ws.rs.HttpMethod.PUT; +import static javax.ws.rs.core.MediaType.APPLICATION_JSON; import static javax.ws.rs.core.MediaType.APPLICATION_XML; import static org.testng.Assert.assertEquals; -import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertNull; import static org.testng.Assert.assertTrue; @@ -154,21 +155,48 @@ public class JAXRSContractTest { } } - interface BodyWithoutParameters { - @POST @Produces(APPLICATION_XML) @Body("") Response post(); + interface ProducesAndConsumes { + @GET @Produces(APPLICATION_XML) Response produces(); + + @GET @Produces({}) Response producesNada(); + + @GET @Produces({""}) Response producesEmpty(); + + @POST @Consumes(APPLICATION_JSON) Response consumes(); + + @POST @Consumes({}) Response consumesNada(); + + @POST @Consumes({""}) Response consumesEmpty(); + } + + @Test public void producesAddsAcceptHeader() throws Exception { + MethodMetadata md = contract.parseAndValidatateMetadata(ProducesAndConsumes.class.getDeclaredMethod("produces")); + assertEquals(md.template().headers().get(ACCEPT), ImmutableSet.of(APPLICATION_XML)); + } + + @Test(expectedExceptions = IllegalStateException.class, expectedExceptionsMessageRegExp = "Produces.value\\(\\) was empty on method producesNada") + public void producesNada() throws Exception { + contract.parseAndValidatateMetadata(ProducesAndConsumes.class.getDeclaredMethod("producesNada")); } - @Test public void bodyWithoutParameters() throws Exception { - MethodMetadata md = contract.parseAndValidatateMetadata(BodyWithoutParameters.class.getDeclaredMethod("post")); - assertEquals(md.template().body(), ""); - assertFalse(md.template().bodyTemplate() != null); - assertTrue(md.formParams().isEmpty()); - assertTrue(md.indexToName().isEmpty()); + @Test(expectedExceptions = IllegalStateException.class, expectedExceptionsMessageRegExp = "Produces.value\\(\\) was empty on method producesEmpty") + public void producesEmpty() throws Exception { + contract.parseAndValidatateMetadata(ProducesAndConsumes.class.getDeclaredMethod("producesEmpty")); } - @Test public void producesAddsContentTypeHeader() throws Exception { - MethodMetadata md = contract.parseAndValidatateMetadata(BodyWithoutParameters.class.getDeclaredMethod("post")); - assertEquals(md.template().headers().get(CONTENT_TYPE), ImmutableSet.of(APPLICATION_XML)); + @Test public void consumesAddsContentTypeHeader() throws Exception { + MethodMetadata md = contract.parseAndValidatateMetadata(ProducesAndConsumes.class.getDeclaredMethod("consumes")); + assertEquals(md.template().headers().get(CONTENT_TYPE), ImmutableSet.of(APPLICATION_JSON)); + } + + @Test(expectedExceptions = IllegalStateException.class, expectedExceptionsMessageRegExp = "Consumes.value\\(\\) was empty on method consumesNada") + public void consumesNada() throws Exception { + contract.parseAndValidatateMetadata(ProducesAndConsumes.class.getDeclaredMethod("consumesNada")); + } + + @Test(expectedExceptions = IllegalStateException.class, expectedExceptionsMessageRegExp = "Consumes.value\\(\\) was empty on method consumesEmpty") + public void consumesEmpty() throws Exception { + contract.parseAndValidatateMetadata(ProducesAndConsumes.class.getDeclaredMethod("consumesEmpty")); } interface BodyParams { @@ -193,11 +221,23 @@ public class JAXRSContractTest { contract.parseAndValidatateMetadata(BodyParams.class.getDeclaredMethod("tooMany", List.class, List.class)); } - @Path("/base") - interface PathOnType { + @Path("") interface EmptyPathOnType { + @GET Response base(); + } + + @Test(expectedExceptions = IllegalStateException.class, expectedExceptionsMessageRegExp = "Path.value\\(\\) was empty on type .*") + public void emptyPathOnType() throws Exception { + contract.parseAndValidatateMetadata(EmptyPathOnType.class.getDeclaredMethod("base")); + } + + @Path("/base") interface PathOnType { @GET Response base(); @GET @Path("/specific") Response get(); + + @GET @Path("") Response emptyPath(); + + @GET @Path("/{param}") Response emptyPathParam(@PathParam("") String empty); } @Test public void pathOnType() throws Exception { @@ -207,6 +247,16 @@ public class JAXRSContractTest { assertEquals(md.template().url(), "/base/specific"); } + @Test(expectedExceptions = IllegalStateException.class, expectedExceptionsMessageRegExp = "Path.value\\(\\) was empty on method emptyPath") + public void emptyPathOnMethod() throws Exception { + contract.parseAndValidatateMetadata(PathOnType.class.getDeclaredMethod("emptyPath")); + } + + @Test(expectedExceptions = IllegalStateException.class, expectedExceptionsMessageRegExp = "PathParam.value\\(\\) was empty on parameter 0") + public void emptyPathParam() throws Exception { + contract.parseAndValidatateMetadata(PathOnType.class.getDeclaredMethod("emptyPathParam", String.class)); + } + interface WithURIParam { @GET @Path("/{1}/{2}") Response uriParam(@PathParam("1") String one, URI endpoint, @PathParam("2") String two); } @@ -229,6 +279,8 @@ public class JAXRSContractTest { @GET @Path("/domains/{domainId}/records") Response recordsByNameAndType(@PathParam("domainId") int id, @QueryParam("name") String nameFilter, @QueryParam("type") String typeFilter); + + @GET Response emptyQueryParam(@QueryParam("") String empty); } @Test public void mixedRequestLineParams() throws Exception { @@ -246,29 +298,40 @@ public class JAXRSContractTest { assertEquals(md.template().toString(), "GET /domains/{domainId}/records?name={name}&type={type} HTTP/1.1\n"); } + @Test(expectedExceptions = IllegalStateException.class, expectedExceptionsMessageRegExp = "QueryParam.value\\(\\) was empty on parameter 0") + public void emptyQueryParam() throws Exception { + contract.parseAndValidatateMetadata(WithPathAndQueryParams.class.getDeclaredMethod("emptyQueryParam", String.class)); + } + interface FormParams { - @POST - @Body("%7B\"customer_name\": \"{customer_name}\", \"user_name\": \"{user_name}\", \"password\": \"{password}\"%7D") - void login( + @POST void login( @FormParam("customer_name") String customer, @FormParam("user_name") String user, @FormParam("password") String password); + + @GET Response emptyFormParam(@FormParam("") String empty); } @Test public void formParamsParseIntoIndexToName() throws Exception { MethodMetadata md = contract.parseAndValidatateMetadata(FormParams.class.getDeclaredMethod("login", String.class, String.class, String.class)); - assertFalse(md.template().body() != null); - assertEquals(md.template().bodyTemplate(), - "%7B\"customer_name\": \"{customer_name}\", \"user_name\": \"{user_name}\", \"password\": \"{password}\"%7D"); + assertNull(md.template().body()); + assertNull(md.template().bodyTemplate()); assertEquals(md.formParams(), ImmutableList.of("customer_name", "user_name", "password")); assertEquals(md.indexToName().get(0), ImmutableSet.of("customer_name")); assertEquals(md.indexToName().get(1), ImmutableSet.of("user_name")); assertEquals(md.indexToName().get(2), ImmutableSet.of("password")); } + @Test(expectedExceptions = IllegalStateException.class, expectedExceptionsMessageRegExp = "FormParam.value\\(\\) was empty on parameter 0") + public void emptyFormParam() throws Exception { + contract.parseAndValidatateMetadata(FormParams.class.getDeclaredMethod("emptyFormParam", String.class)); + } + interface HeaderParams { @POST void logout(@HeaderParam("Auth-Token") String token); + + @GET Response emptyHeaderParam(@HeaderParam("") String empty); } @Test public void headerParamsParseIntoIndexToName() throws Exception { @@ -278,6 +341,11 @@ public class JAXRSContractTest { assertEquals(md.indexToName().get(0), ImmutableSet.of("Auth-Token")); } + @Test(expectedExceptions = IllegalStateException.class, expectedExceptionsMessageRegExp = "HeaderParam.value\\(\\) was empty on parameter 0") + public void emptyHeaderParam() throws Exception { + contract.parseAndValidatateMetadata(HeaderParams.class.getDeclaredMethod("emptyHeaderParam", String.class)); + } + interface WithObservable { @GET @Path("/") Observable> valid();