Skip to content

Commit 94ce071

Browse files
masc3dvelo
masc3d
authored and
velo
committed
FIXED unsupported jaxrs-2.1 annotations should not break entire interface (OpenFeign#672)
* FIXED unsupported jaxrs-2.1 annotations should not break entire interface, resolving OpenFeign#669 * UPDATED jaxrs: more defensive jaxrs2 support * ADDED jsr311-api dependency to httpclient (as jsr311 is `provided` in feign-jaxrs now) * UPDATED httpclient `jsr311-api` scope to test UPDATED jaxrs readme
1 parent 7ce0727 commit 94ce071

7 files changed

Lines changed: 180 additions & 23 deletions

File tree

‎httpclient/pom.xml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,13 @@
5555
<scope>test</scope>
5656
</dependency>
5757

58+
<dependency>
59+
<artifactId>jsr311-api</artifactId>
60+
<version>1.1.1</version>
61+
<groupId>javax.ws.rs</groupId>
62+
<scope>test</scope>
63+
</dependency>
64+
5865
<dependency>
5966
<groupId>com.squareup.okhttp3</groupId>
6067
<artifactId>mockwebserver</artifactId>

‎jaxrs/pom.xml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,10 @@
3737
</dependency>
3838

3939
<dependency>
40-
<groupId>javax.ws.rs</groupId>
4140
<artifactId>jsr311-api</artifactId>
4241
<version>1.1.1</version>
42+
<groupId>javax.ws.rs</groupId>
43+
<scope>provided</scope>
4344
</dependency>
4445

4546
<!-- for example -->

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

Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -13,31 +13,23 @@
1313
*/
1414
package feign.jaxrs;
1515

16+
import feign.Contract;
17+
import feign.MethodMetadata;
18+
19+
import javax.ws.rs.*;
1620
import java.lang.annotation.Annotation;
1721
import java.lang.reflect.Method;
1822
import java.util.ArrayList;
1923
import java.util.Collection;
2024

21-
import javax.ws.rs.Consumes;
22-
import javax.ws.rs.FormParam;
23-
import javax.ws.rs.HeaderParam;
24-
import javax.ws.rs.HttpMethod;
25-
import javax.ws.rs.Path;
26-
import javax.ws.rs.PathParam;
27-
import javax.ws.rs.Produces;
28-
import javax.ws.rs.QueryParam;
29-
30-
import feign.Contract;
31-
import feign.MethodMetadata;
32-
3325
import static feign.Util.checkState;
3426
import static feign.Util.emptyToNull;
3527

3628
/**
3729
* Please refer to the <a href="https://github.com/Netflix/feign/tree/master/feign-jaxrs">Feign
3830
* JAX-RS README</a>.
3931
*/
40-
public final class JAXRSContract extends Contract.BaseContract {
32+
public class JAXRSContract extends Contract.BaseContract {
4133

4234
static final String ACCEPT = "Accept";
4335
static final String CONTENT_TYPE = "Content-Type";
@@ -58,8 +50,8 @@ protected void processAnnotationOnClass(MethodMetadata data, Class<?> clz) {
5850
pathValue = "/" + pathValue;
5951
}
6052
if (pathValue.endsWith("/")) {
61-
// Strip off any trailing slashes, since the template has already had slashes appropriately added
62-
pathValue = pathValue.substring(0, pathValue.length() - 1);
53+
// Strip off any trailing slashes, since the template has already had slashes appropriately added
54+
pathValue = pathValue.substring(0, pathValue.length() - 1);
6355
}
6456
data.template().insert(0, pathValue);
6557
}
@@ -80,8 +72,8 @@ protected void processAnnotationOnMethod(MethodMetadata data, Annotation methodA
8072
HttpMethod http = annotationType.getAnnotation(HttpMethod.class);
8173
if (http != null) {
8274
checkState(data.template().method() == null,
83-
"Method %s contains multiple HTTP methods. Found: %s and %s", method.getName(),
84-
data.template().method(), http.value());
75+
"Method %s contains multiple HTTP methods. Found: %s and %s", method.getName(),
76+
data.template().method(), http.value());
8577
data.template().method(http.value());
8678
} else if (annotationType == Path.class) {
8779
String pathValue = emptyToNull(Path.class.cast(methodAnnotation).value());
@@ -119,38 +111,51 @@ private void handleConsumesAnnotation(MethodMetadata data, Consumes consumes, St
119111
data.template().header(CONTENT_TYPE, clientProduces);
120112
}
121113

114+
/**
115+
* Allows derived contracts to specify unsupported jax-rs parameter annotations which should be ignored.
116+
* Required for JAX-RS 2 compatibility.
117+
*/
118+
protected boolean isUnsupportedHttpParameterAnnotation(Annotation parameterAnnotation) {
119+
return false;
120+
}
121+
122122
@Override
123123
protected boolean processAnnotationsOnParameter(MethodMetadata data, Annotation[] annotations,
124124
int paramIndex) {
125125
boolean isHttpParam = false;
126126
for (Annotation parameterAnnotation : annotations) {
127127
Class<? extends Annotation> annotationType = parameterAnnotation.annotationType();
128-
if (annotationType == PathParam.class) {
128+
// masc20180327. parameter with unsupported jax-rs annotations should not be passed as body params.
129+
// this will prevent interfaces from becoming unusable entirely due to single (unsupported) endpoints.
130+
// https://github.com/OpenFeign/feign/issues/669
131+
if (this.isUnsupportedHttpParameterAnnotation(parameterAnnotation)) {
132+
isHttpParam = true;
133+
} else if (annotationType == PathParam.class) {
129134
String name = PathParam.class.cast(parameterAnnotation).value();
130135
checkState(emptyToNull(name) != null, "PathParam.value() was empty on parameter %s",
131-
paramIndex);
136+
paramIndex);
132137
nameParam(data, name, paramIndex);
133138
isHttpParam = true;
134139
} else if (annotationType == QueryParam.class) {
135140
String name = QueryParam.class.cast(parameterAnnotation).value();
136141
checkState(emptyToNull(name) != null, "QueryParam.value() was empty on parameter %s",
137-
paramIndex);
142+
paramIndex);
138143
Collection<String> query = addTemplatedParam(data.template().queries().get(name), name);
139144
data.template().query(name, query);
140145
nameParam(data, name, paramIndex);
141146
isHttpParam = true;
142147
} else if (annotationType == HeaderParam.class) {
143148
String name = HeaderParam.class.cast(parameterAnnotation).value();
144149
checkState(emptyToNull(name) != null, "HeaderParam.value() was empty on parameter %s",
145-
paramIndex);
150+
paramIndex);
146151
Collection<String> header = addTemplatedParam(data.template().headers().get(name), name);
147152
data.template().header(name, header);
148153
nameParam(data, name, paramIndex);
149154
isHttpParam = true;
150155
} else if (annotationType == FormParam.class) {
151156
String name = FormParam.class.cast(parameterAnnotation).value();
152157
checkState(emptyToNull(name) != null, "FormParam.value() was empty on parameter %s",
153-
paramIndex);
158+
paramIndex);
154159
data.formParams().add(name);
155160
nameParam(data, name, paramIndex);
156161
isHttpParam = true;

‎jaxrs2/README.md‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
# Feign JAXRS 2
2+
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.
3+
4+
## Limitations
5+
While it may appear possible to reuse the same interface across client and server, bear in mind that JAX-RS resource
6+
annotations were not designed to be processed by clients. Moreover, JAX-RS 2.0 has a different package hierarchy for
7+
client invocation. Finally, JAX-RS is a large spec and attempts to implement it completely would be a project larger
8+
than feign itself. In other words, this implementation is *best efforts* and concedes far from 100% compatibility with
9+
server interface behavior.
10+
11+
## Currently Supported Annotation Processing
12+
Feign only supports processing java interfaces (not abstract or concrete classes).
13+
14+
ISE is raised when any annotation's value is empty or null. Ex. `Path("")` raises an ISE.
15+
16+
Here are a list of behaviors currently supported.
17+
### Type Annotations
18+
#### `@Path`
19+
Appends the value to `Target.url()`. Can have tokens corresponding to `@PathParam` annotations.
20+
### Method Annotations
21+
#### `@HttpMethod` meta-annotation (present on `@GET`, `@POST`, etc.)
22+
Sets the request method.
23+
#### `@Path`
24+
Appends the value to `Target.url()`. Can have tokens corresponding to `@PathParam` annotations.
25+
#### `@Produces`
26+
Adds the first value as the `Accept` header.
27+
#### `@Consumes`
28+
Adds the first value as the `Content-Type` header.
29+
### Parameter Annotations
30+
#### `@PathParam`
31+
Links the value of the corresponding parameter to a template variable declared in the path.
32+
#### `@QueryParam`
33+
Links the value of the corresponding parameter to a query parameter. When invoked, null will skip the query param.
34+
#### `@HeaderParam`
35+
Links the value of the corresponding parameter to a header.
36+
#### `@FormParam`
37+
Links the value of the corresponding parameter to a key passed to `Encoder.Text<Map<String, Object>>.encode()`.

‎jaxrs2/pom.xml‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
<!--
2+
3+
Copyright 2012-2018 The Feign Authors
4+
5+
Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
6+
in compliance with the License. You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software distributed under the License
11+
is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
12+
or implied. See the License for the specific language governing permissions and limitations under
13+
the License.
14+
15+
-->
16+
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
17+
<modelVersion>4.0.0</modelVersion>
18+
19+
<parent>
20+
<groupId>io.github.openfeign</groupId>
21+
<artifactId>parent</artifactId>
22+
<version>9.7.0-SNAPSHOT</version>
23+
</parent>
24+
25+
<artifactId>feign-jaxrs2</artifactId>
26+
<name>Feign JAX-RS 2</name>
27+
<description>Feign JAX-RS 2</description>
28+
29+
<properties>
30+
<main.basedir>${project.basedir}/..</main.basedir>
31+
</properties>
32+
33+
<dependencies>
34+
<dependency>
35+
<groupId>${project.groupId}</groupId>
36+
<artifactId>feign-core</artifactId>
37+
</dependency>
38+
39+
<dependency>
40+
<groupId>${project.groupId}</groupId>
41+
<artifactId>feign-jaxrs</artifactId>
42+
</dependency>
43+
44+
<dependency>
45+
<groupId>javax.ws.rs</groupId>
46+
<artifactId>javax.ws.rs-api</artifactId>
47+
<version>2.1</version>
48+
<scope>provided</scope>
49+
</dependency>
50+
51+
<!-- for example -->
52+
<dependency>
53+
<groupId>${project.groupId}</groupId>
54+
<artifactId>feign-gson</artifactId>
55+
<scope>test</scope>
56+
</dependency>
57+
58+
<dependency>
59+
<groupId>${project.groupId}</groupId>
60+
<artifactId>feign-core</artifactId>
61+
<type>test-jar</type>
62+
<scope>test</scope>
63+
</dependency>
64+
</dependencies>
65+
</project>

‎jaxrs2/src/main/java/feign/jaxrs/JAXRS2Contract.java‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/**
2+
* Copyright 2012-2018 The Feign Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
5+
* in compliance with the License. You may obtain a copy of the License at
6+
*
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
*
9+
* Unless required by applicable law or agreed to in writing, software distributed under the License
10+
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
11+
* or implied. See the License for the specific language governing permissions and limitations under
12+
* the License.
13+
*/
14+
package feign.jaxrs;
15+
16+
import javax.ws.rs.container.Suspended;
17+
import javax.ws.rs.core.Context;
18+
import java.lang.annotation.Annotation;
19+
20+
/**
21+
* Please refer to the <a href="https://github.com/Netflix/feign/tree/master/feign-jaxrs2">Feign
22+
* JAX-RS 2 README</a>.
23+
*/
24+
public final class JAXRS2Contract extends JAXRSContract {
25+
@Override
26+
protected boolean isUnsupportedHttpParameterAnnotation(Annotation parameterAnnotation) {
27+
Class<? extends Annotation> annotationType = parameterAnnotation.annotationType();
28+
29+
// masc20180327. parameter with unsupported jax-rs annotations should not be passed as body params.
30+
// this will prevent interfaces from becoming unusable entirely due to single (unsupported) endpoints.
31+
// https://github.com/OpenFeign/feign/issues/669
32+
return (annotationType == Suspended.class ||
33+
annotationType == Context.class);
34+
}
35+
}

‎pom.xml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
<module>jackson</module>
3232
<module>jaxb</module>
3333
<module>jaxrs</module>
34+
<module>jaxrs2</module>
3435
<module>okhttp</module>
3536
<module>ribbon</module>
3637
<module>sax</module>
@@ -184,6 +185,12 @@
184185
<version>${project.version}</version>
185186
</dependency>
186187

188+
<dependency>
189+
<groupId>${project.groupId}</groupId>
190+
<artifactId>feign-jaxrs2</artifactId>
191+
<version>${project.version}</version>
192+
</dependency>
193+
187194
<dependency>
188195
<groupId>${project.groupId}</groupId>
189196
<artifactId>feign-okhttp</artifactId>

0 commit comments

Comments
 (0)