Skip to content

Commit 2768e45

Browse files
author
Fernando Saint-Jean
committed
makes annotation be 'inherited' as requested in #10
1 parent d046a08 commit 2768e45

File tree

4 files changed

+230
-4
lines changed

4 files changed

+230
-4
lines changed

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+
```

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

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.lang.reflect.Method;
2121
import java.util.HashMap;
2222
import java.util.Map;
23+
import java.util.Optional;
2324

2425
import static feign.Feign.configKey;
2526

@@ -80,8 +81,9 @@ Map<String, MethodErrorHandler> generateErrorHandlerMapFromApi(Class<?> apiType)
8081
.build();
8182
Map<Integer, ExceptionGenerator> classLevelStatusCodeDefinitions = new HashMap<Integer, ExceptionGenerator>();
8283

83-
if(apiType.isAnnotationPresent(ErrorHandling.class)) {
84-
ErrorHandlingDefinition classErrorHandlingDefinition = readAnnotation(apiType.getAnnotation(ErrorHandling.class), responseBodyDecoder);
84+
Optional<ErrorHandling> classLevelErrorHandling = readErrorHandlingIncludingInherited(apiType);
85+
if(classLevelErrorHandling.isPresent()) {
86+
ErrorHandlingDefinition classErrorHandlingDefinition = readAnnotation(classLevelErrorHandling.get(), responseBodyDecoder);
8587
classLevelDefault = classErrorHandlingDefinition.defaultThrow;
8688
classLevelStatusCodeDefinitions = classErrorHandlingDefinition.statusCodesMap;
8789
}
@@ -105,6 +107,19 @@ Map<String, MethodErrorHandler> generateErrorHandlerMapFromApi(Class<?> apiType)
105107
return methodErrorHandlerMap;
106108
}
107109

110+
Optional<ErrorHandling> readErrorHandlingIncludingInherited(Class<?> apiType) {
111+
if(apiType.isAnnotationPresent(ErrorHandling.class)) {
112+
return Optional.of(apiType.getAnnotation(ErrorHandling.class));
113+
}
114+
for(Class<?> parentInterface: apiType.getInterfaces()) {
115+
Optional<ErrorHandling> errorHandling = readErrorHandlingIncludingInherited(parentInterface);
116+
if(errorHandling.isPresent()) {
117+
return errorHandling;
118+
}
119+
}
120+
return Optional.empty();
121+
}
122+
108123
static ErrorHandlingDefinition readAnnotation(ErrorHandling errorHandling, Decoder responseBodyDecoder) {
109124
ExceptionGenerator defaultException = new ExceptionGenerator.Builder()
110125
.withResponseBodyDecoder(responseBodyDecoder)

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

0 commit comments

Comments
 (0)