Skip to content

Commit 88a253d

Browse files
author
Fernando Saint-Jean
committed
Merge remote-tracking branch 'origin/master'
# Conflicts: # .travis.yml
2 parents 2ce6881 + e63a2af commit 88a253d

File tree

7 files changed

+388
-16
lines changed

7 files changed

+388
-16
lines changed

.travis.yml

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,20 @@ cache:
2424
directories:
2525
- $HOME/.m2
2626

27-
#Remove these for now as they cause issues at release.
28-
#matrix:
29-
# include:
30-
# - os: linux
31-
# jdk: oraclejdk8
32-
# addons:
33-
# apt:
34-
# packages:
35-
# - oracle-java8-installer
27+
28+
matrix:
29+
include:
30+
- os: linux
31+
jdk: oraclejdk8
32+
addons:
33+
apt:
34+
packages:
35+
- oracle-java8-installer
36+
#removing openjdk8 from matrix for now as it causes issues on release
3637
# - os: linux
3738
# jdk: openjdk8
38-
# - os: linux
39-
# jdk: openjdk11
39+
- os: linux
40+
jdk: openjdk11
4041

4142
# Don't build release tags. This avoids publish conflicts because the version commit exists both on master and the release tag.
4243
# See https://github.com/travis-ci/travis-ci/issues/1532

README.md

Lines changed: 84 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ GitHub github = Feign.builder()
7777

7878
## Complex Exceptions
7979

80-
Finally, Any exception can be used if they have a default constructor:
80+
Any exception can be used if they have a default constructor:
8181

8282
```java
8383
class DefaultConstructorException extends Exception {}
@@ -147,4 +147,86 @@ class GithubExceptionResponse {
147147
```
148148

149149
It's worth noting that at setup/startup time, the generators are checked with a null value of the body.
150-
If you don't do the null-checker, you'll get an NPE and startup will fail.
150+
If you don't do the null-checker, you'll get an NPE and startup will fail.
151+
152+
153+
## Inheriting from other interface definitions
154+
You can create a client interface that inherits from a different one. However, there are some limitations that
155+
you should be aware of (for most cases, these shouldn't be an issue):
156+
* The inheritance is not natural java inheritance of annotations - as these don't work on interfaces
157+
* Instead, the error looks at the class and if it finds the `@ErrorHandling` annotation, it uses that one.
158+
* If not, it will look at *all* the interfaces the main interface `extends` - but it does so in the order the
159+
java API gives it - so order is not guaranteed.
160+
* If it finds the annotation in one of those parents, it uses that definition, without looking at any other
161+
* That means that if more than one interface was extended which contained the `@ErrorHandling` annotation, we can't
162+
really guarantee which one of the parents will be selected and you should really do handling at the child interface
163+
* so far, the java API seems to return in order of definition after the `extends`, but it's a really bad practice
164+
if you have to depend on that... so our suggestion: don't.
165+
166+
That means that as long as you only ever extend from a base interface (where you may decide that all 404's are "NotFoundException", for example)
167+
then you should be ok. But if you get complex in polymorphism, all bets are off - so don't go crazy!
168+
169+
Example:
170+
In the following code:
171+
* The base `FeignClientBase` interface defines a default set of exceptions at class level
172+
* the `GitHub1` and `GitHub2` interfaces will inherit the class-level error handling, which means that
173+
any 401/403/404 will be handled correctly (provided the method doesn't specify a more specific exception)
174+
* the `GitHub3` interface however, by defining its own error handling, will handle all 401's, but not the
175+
403/404's since there's no merging/etc (not really in the plan to implement either...)
176+
```java
177+
178+
@ErrorHandling(codeSpecific =
179+
{
180+
@ErrorCodes( codes = {401}, generate = UnAuthorizedException.class),
181+
@ErrorCodes( codes = {403}, generate = ForbiddenException.class),
182+
@ErrorCodes( codes = {404}, generate = UnknownItemException.class),
183+
},
184+
defaultException = ClassLevelDefaultException.class
185+
)
186+
interface FeignClientBase {}
187+
188+
interface GitHub1 extends FeignClientBase {
189+
190+
@ErrorHandling(codeSpecific =
191+
{
192+
@ErrorCodes( codes = {404}, generate = NonExistentRepoException.class),
193+
@ErrorCodes( codes = {502, 503, 504}, generate = RetryAfterCertainTimeException.class),
194+
},
195+
defaultException = FailedToGetContributorsException.class
196+
)
197+
@RequestLine("GET /repos/{owner}/{repo}/contributors")
198+
List<Contributor> contributors(@Param("owner") String owner, @Param("repo") String repo);
199+
}
200+
201+
interface GitHub2 extends FeignClientBase {
202+
203+
@ErrorHandling(codeSpecific =
204+
{
205+
@ErrorCodes( codes = {404}, generate = NonExistentRepoException.class),
206+
@ErrorCodes( codes = {502, 503, 504}, generate = RetryAfterCertainTimeException.class),
207+
},
208+
defaultException = FailedToGetContributorsException.class
209+
)
210+
@RequestLine("GET /repos/{owner}/{repo}/contributors")
211+
List<Contributor> contributors(@Param("owner") String owner, @Param("repo") String repo);
212+
}
213+
214+
@ErrorHandling(codeSpecific =
215+
{
216+
@ErrorCodes( codes = {401}, generate = UnAuthorizedException.class)
217+
},
218+
defaultException = ClassLevelDefaultException.class
219+
)
220+
interface GitHub3 extends FeignClientBase {
221+
222+
@ErrorHandling(codeSpecific =
223+
{
224+
@ErrorCodes( codes = {404}, generate = NonExistentRepoException.class),
225+
@ErrorCodes( codes = {502, 503, 504}, generate = RetryAfterCertainTimeException.class),
226+
},
227+
defaultException = FailedToGetContributorsException.class
228+
)
229+
@RequestLine("GET /repos/{owner}/{repo}/contributors")
230+
List<Contributor> contributors(@Param("owner") String owner, @Param("repo") String repo);
231+
}
232+
```

pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
</parent>
2525

2626
<artifactId>feign-annotation-error-decoder</artifactId>
27-
<version>1.1.1-SNAPSHOT</version>
27+
<version>1.1.2-SNAPSHOT</version>
2828
<name>Feign Annotation Error Decoder</name>
2929
<description>Feign Annotation Error Decoder</description>
3030
<url>https://github.com/OpenFeign/feign-annotation-error-decoder</url>

src/main/java/feign/error/AnnotationErrorDecoder.java

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.lang.reflect.Method;
2020
import java.util.HashMap;
2121
import java.util.Map;
22+
import java.util.Optional;
2223
import static feign.Feign.configKey;
2324

2425
public class AnnotationErrorDecoder implements ErrorDecoder {
@@ -80,9 +81,11 @@ Map<String, MethodErrorHandler> generateErrorHandlerMapFromApi(Class<?> apiType)
8081
Map<Integer, ExceptionGenerator> classLevelStatusCodeDefinitions =
8182
new HashMap<Integer, ExceptionGenerator>();
8283

83-
if (apiType.isAnnotationPresent(ErrorHandling.class)) {
84+
Optional<ErrorHandling> classLevelErrorHandling =
85+
readErrorHandlingIncludingInherited(apiType);
86+
if (classLevelErrorHandling.isPresent()) {
8487
ErrorHandlingDefinition classErrorHandlingDefinition =
85-
readAnnotation(apiType.getAnnotation(ErrorHandling.class), responseBodyDecoder);
88+
readAnnotation(classLevelErrorHandling.get(), responseBodyDecoder);
8689
classLevelDefault = classErrorHandlingDefinition.defaultThrow;
8790
classLevelStatusCodeDefinitions = classErrorHandlingDefinition.statusCodesMap;
8891
}
@@ -109,6 +112,24 @@ Map<String, MethodErrorHandler> generateErrorHandlerMapFromApi(Class<?> apiType)
109112
return methodErrorHandlerMap;
110113
}
111114

115+
Optional<ErrorHandling> readErrorHandlingIncludingInherited(Class<?> apiType) {
116+
if (apiType.isAnnotationPresent(ErrorHandling.class)) {
117+
return Optional.of(apiType.getAnnotation(ErrorHandling.class));
118+
}
119+
for (Class<?> parentInterface : apiType.getInterfaces()) {
120+
Optional<ErrorHandling> errorHandling =
121+
readErrorHandlingIncludingInherited(parentInterface);
122+
if (errorHandling.isPresent()) {
123+
return errorHandling;
124+
}
125+
}
126+
// Finally, if there's a superclass that isn't Object check if the superclass has anything
127+
if (!apiType.isInterface() && !apiType.getSuperclass().equals(Object.class)) {
128+
return readErrorHandlingIncludingInherited(apiType.getSuperclass());
129+
}
130+
return Optional.empty();
131+
}
132+
112133
static ErrorHandlingDefinition readAnnotation(ErrorHandling errorHandling,
113134
Decoder responseBodyDecoder) {
114135
ExceptionGenerator defaultException = new ExceptionGenerator.Builder()

src/main/java/feign/error/ErrorHandling.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,12 @@
1414
package feign.error;
1515

1616
import java.lang.annotation.ElementType;
17+
import java.lang.annotation.Inherited;
1718
import java.lang.annotation.Retention;
1819
import java.lang.annotation.RetentionPolicy;
1920
import java.lang.annotation.Target;
2021

22+
@Inherited
2123
@Retention(RetentionPolicy.RUNTIME)
2224
@Target({ElementType.TYPE, ElementType.METHOD})
2325
public @interface ErrorHandling {
Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
/**
2+
* Copyright 2017-2019 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.error;
15+
16+
import static org.assertj.core.api.Assertions.assertThat;
17+
import feign.error.AnnotationErrorDecoderClassInheritanceTest.ParentInterfaceWithErrorHandling.Method1NotFoundException;
18+
import feign.error.AnnotationErrorDecoderClassInheritanceTest.ParentInterfaceWithErrorHandling.UnauthenticatedOrUnauthorizedException;
19+
import feign.error.AnnotationErrorDecoderClassInheritanceTest.ParentInterfaceWithErrorHandling.Method2NotFoundException;
20+
import feign.error.AnnotationErrorDecoderClassInheritanceTest.ParentInterfaceWithErrorHandling.ServeErrorException;
21+
import feign.error.AnnotationErrorDecoderClassInheritanceTest.ParentInterfaceWithErrorHandling.ClassLevelNotFoundException;
22+
import feign.error.AnnotationErrorDecoderClassInheritanceTest.ParentInterfaceWithErrorHandling.Method1DefaultException;
23+
import feign.error.AnnotationErrorDecoderClassInheritanceTest.ParentInterfaceWithErrorHandling.Method3DefaultException;
24+
import feign.error.AnnotationErrorDecoderClassInheritanceTest.ParentInterfaceWithErrorHandling.ClassLevelDefaultException;
25+
import java.util.Arrays;
26+
import org.junit.Test;
27+
import org.junit.runner.RunWith;
28+
import org.junit.runners.Parameterized;
29+
import org.junit.runners.Parameterized.Parameter;
30+
import org.junit.runners.Parameterized.Parameters;
31+
32+
@RunWith(Parameterized.class)
33+
public class AnnotationErrorDecoderClassInheritanceTest extends
34+
AbstractAnnotationErrorDecoderTest<AnnotationErrorDecoderClassInheritanceTest.GrandChild> {
35+
36+
@Override
37+
public Class<GrandChild> interfaceAtTest() {
38+
return GrandChild.class;
39+
}
40+
41+
@Parameters(
42+
name = "{0}: When error code ({1}) on method ({2}) should return exception type ({3})")
43+
public static Iterable<Object[]> data() {
44+
return Arrays.asList(new Object[][] {
45+
{"Test Code Specific At Method", 404, "method1Test", Method1NotFoundException.class},
46+
{"Test Code Specific At Method", 401, "method1Test",
47+
UnauthenticatedOrUnauthorizedException.class},
48+
{"Test Code Specific At Method", 404, "method2Test", Method2NotFoundException.class},
49+
{"Test Code Specific At Method", 500, "method2Test", ServeErrorException.class},
50+
{"Test Code Specific At Method", 503, "method2Test", ServeErrorException.class},
51+
{"Test Code Specific At Class", 403, "method1Test",
52+
UnauthenticatedOrUnauthorizedException.class},
53+
{"Test Code Specific At Class", 403, "method2Test",
54+
UnauthenticatedOrUnauthorizedException.class},
55+
{"Test Code Specific At Class", 404, "method3Test", ClassLevelNotFoundException.class},
56+
{"Test Code Specific At Class", 403, "method3Test",
57+
UnauthenticatedOrUnauthorizedException.class},
58+
{"Test Default At Method", 504, "method1Test", Method1DefaultException.class},
59+
{"Test Default At Method", 504, "method3Test", Method3DefaultException.class},
60+
{"Test Default At Class", 504, "method2Test", ClassLevelDefaultException.class},
61+
});
62+
}
63+
64+
@Parameter // first data value (0) is default
65+
public String testType;
66+
67+
@Parameter(1)
68+
public int errorCode;
69+
70+
@Parameter(2)
71+
public String method;
72+
73+
@Parameter(3)
74+
public Class<? extends Exception> expectedExceptionClass;
75+
76+
@Test
77+
public void test() throws Exception {
78+
AnnotationErrorDecoder decoder =
79+
AnnotationErrorDecoder.builderFor(GrandChild.class).build();
80+
81+
assertThat(decoder.decode(feignConfigKey(method), testResponse(errorCode)).getClass())
82+
.isEqualTo(expectedExceptionClass);
83+
}
84+
85+
@ErrorHandling(codeSpecific = {
86+
@ErrorCodes(codes = {404}, generate = ClassLevelNotFoundException.class),
87+
@ErrorCodes(codes = {403}, generate = UnauthenticatedOrUnauthorizedException.class)
88+
},
89+
defaultException = ClassLevelDefaultException.class)
90+
interface ParentInterfaceWithErrorHandling {
91+
@ErrorHandling(codeSpecific = {
92+
@ErrorCodes(codes = {404}, generate = Method1NotFoundException.class),
93+
@ErrorCodes(codes = {401}, generate = UnauthenticatedOrUnauthorizedException.class)
94+
},
95+
defaultException = Method1DefaultException.class)
96+
void method1Test();
97+
98+
@ErrorHandling(codeSpecific = {
99+
@ErrorCodes(codes = {404}, generate = Method2NotFoundException.class),
100+
@ErrorCodes(codes = {500, 503}, generate = ServeErrorException.class)
101+
})
102+
void method2Test();
103+
104+
@ErrorHandling(
105+
defaultException = Method3DefaultException.class)
106+
void method3Test();
107+
108+
class ClassLevelDefaultException extends Exception {
109+
}
110+
class Method1DefaultException extends Exception {
111+
}
112+
class Method3DefaultException extends Exception {
113+
}
114+
class Method1NotFoundException extends Exception {
115+
}
116+
class Method2NotFoundException extends Exception {
117+
}
118+
class ClassLevelNotFoundException extends Exception {
119+
}
120+
class UnauthenticatedOrUnauthorizedException extends Exception {
121+
}
122+
class ServeErrorException extends Exception {
123+
}
124+
}
125+
126+
abstract class Child implements ParentInterfaceWithErrorHandling {
127+
}
128+
129+
abstract class GrandChild extends Child {
130+
}
131+
}

0 commit comments

Comments
 (0)