From ca43a64c44c4c94fa967ce0fa1ead2d464b4ee77 Mon Sep 17 00:00:00 2001 From: Michael Cramer Date: Mon, 19 Jul 2021 02:39:20 +0200 Subject: [PATCH] adding support for meta-annotations (#1458) when trying to find the ErrorHandling annotation on method or class level also inspect annotations already present to see if they are annotated Co-authored-by: Marvin Froeder --- annotation-error-decoder/README.md | 112 +++++++++++++- .../feign/error/AnnotationErrorDecoder.java | 25 +++- ...ErrorDecoderAnnotationInheritanceTest.java | 105 +++++++++++++ ...erInheritanceClassLevelAnnotationTest.java | 119 +++++++++++++++ ...rInheritanceMethodLevelAnnotationTest.java | 141 ++++++++++++++++++ 5 files changed, 496 insertions(+), 6 deletions(-) create mode 100644 annotation-error-decoder/src/test/java/feign/error/AnnotationErrorDecoderAnnotationInheritanceTest.java create mode 100644 annotation-error-decoder/src/test/java/feign/error/AnnotationErrorDecoderInheritanceClassLevelAnnotationTest.java create mode 100644 annotation-error-decoder/src/test/java/feign/error/AnnotationErrorDecoderInheritanceMethodLevelAnnotationTest.java diff --git a/annotation-error-decoder/README.md b/annotation-error-decoder/README.md index 71deaf3d..f99d5db7 100644 --- a/annotation-error-decoder/README.md +++ b/annotation-error-decoder/README.md @@ -15,7 +15,7 @@ GitHub github = Feign.builder() ``` ## Leveraging the annotations and priority order -For annotation decoding to work, the class must be annotated with `@ErrorHandling` tags. +For annotation decoding to work, the class must be annotated with `@ErrorHandling` tags or meta-annotations. The tags are valid in both the class level as well as method level. They will be treated from 'most specific' to 'least specific' in the following order: * A code specific exception defined on the method @@ -248,4 +248,112 @@ interface GitHub3 extends FeignClientBase { @RequestLine("GET /repos/{owner}/{repo}/contributors") List contributors(@Param("owner") String owner, @Param("repo") String repo); } -``` \ No newline at end of file +``` + +## Meta-annotations +When you want to share the same configuration of one `@ErrorHandling` annotation the `@ErrorHandling` annotation +can be moved to a meta-annotation. Then later on this meta-annotation can be used on a method or at class level to +reduce the amount duplicated code. A meta-annotation is a special annotation that contains the `@ErrorHandling` +annotation and possibly other annotations, e.g. Spring-Rest annotations. + +There are some limitations and rules to keep in mind when using meta-annotation: +- inheritance for meta-annotations when using interface inheritance is supported and is following the same rules as for + interface inheritance (see above) + - `@ErrorHandling` has **precedence** over any meta-annotation when placed together on a class or method + - a meta-annotation on a child interface (method or class) has **precedence** over the error handling defined in the + parent interface +- having a meta-annotation on a meta-annotation is not supported, only the annotations on a type are checked for a + `@ErrorHandling` +- when multiple meta-annotations with an `@ErrorHandling` annotation are present on a class or method the first one + which is returned by java API is used to figure out the error handling, the others are not considered, so it is + advisable to have only one meta-annotation on each method or class as the order is not guaranteed. +- **no merging** of configurations is supported, e.g. multiple meta-annotations on the same type, meta-annotation with + `@ErrorHandling` on the same type + +Example: + +Let's assume multiple methods need to handle the response-code `404` in the same way but differently what is +specified in the `@ErrorHandling` annotation on the class-level. In that case, to avoid also duplicate annotation definitions +on the affected methods a meta-annotation can reduce the amount of code to be written to handle this `404` differently. + +In the following code the status-code `404` is handled on a class level which throws an `UnknownItemException` for all +methods inside this interface. For the methods `contributors` and `languages` a different exceptions needs to be thrown, +in this case it is a `NoDataFoundException`. The `teams`method will still use the exception defined by the class-level +error handling annotation. To simplify the code a meta-annotation can be created and be used in the interface to keep +the interface small and readable. + +```java +@ErrorHandling( + codeSpecific = { + @ErrorCodes(codes = {404}, generate = NoDataFoundException.class), + }, + defaultException = GithubRemoteException.class) +@Retention(RetentionPolicy.RUNTIME) +@interface NoDataErrorHandling { +} +``` + +Having this meta-annotation in place it can be used to transform the interface into a much smaller one, keeping the same +behavior. +- `contributers` will throw a `NoDataFoundException` for status code `404` as defined on method level and a + `GithubRemoteException` for all other status codes +- `languages` will throw a `NoDataFoundException` for status code `404` as defined on method level and a + `GithubRemoteException` for all other status codes +- `teams` will throw a `UnknownItemException` for status code `404` as defined on class level and a + `ClassLevelDefaultException` for all other status codes + +Before: +```java +@ErrorHandling(codeSpecific = + { + @ErrorCodes( codes = {404}, generate = UnknownItemException.class) + }, + defaultException = ClassLevelDefaultException.class +) +interface GitHub { + @ErrorHandling(codeSpecific = + { + @ErrorCodes( codes = {404}, generate = NoDataFoundException.class) + }, + defaultException = GithubRemoteException.class + ) + @RequestLine("GET /repos/{owner}/{repo}/contributors") + List contributors(@Param("owner") String owner, @Param("repo") String repo); + + @ErrorHandling(codeSpecific = + { + @ErrorCodes( codes = {404}, generate = NoDataFoundException.class) + }, + defaultException = GithubRemoteException.class + ) + @RequestLine("GET /repos/{owner}/{repo}/languages") + Map languages(@Param("owner") String owner, @Param("repo") String repo); + + @ErrorHandling + @RequestLine("GET /repos/{owner}/{repo}/team") + List languages(@Param("owner") String owner, @Param("repo") String repo); +} +``` + +After: +```java +@ErrorHandling(codeSpecific = + { + @ErrorCodes( codes = {404}, generate = UnknownItemException.class) + }, + defaultException = ClassLevelDefaultException.class +) +interface GitHub { + @NoDataErrorHandling + @RequestLine("GET /repos/{owner}/{repo}/contributors") + List contributors(@Param("owner") String owner, @Param("repo") String repo); + + @NoDataErrorHandling + @RequestLine("GET /repos/{owner}/{repo}/languages") + Map languages(@Param("owner") String owner, @Param("repo") String repo); + + @ErrorHandling + @RequestLine("GET /repos/{owner}/{repo}/team") + List languages(@Param("owner") String owner, @Param("repo") String repo); +} +``` diff --git a/annotation-error-decoder/src/main/java/feign/error/AnnotationErrorDecoder.java b/annotation-error-decoder/src/main/java/feign/error/AnnotationErrorDecoder.java index a8bede6f..14b703f2 100644 --- a/annotation-error-decoder/src/main/java/feign/error/AnnotationErrorDecoder.java +++ b/annotation-error-decoder/src/main/java/feign/error/AnnotationErrorDecoder.java @@ -16,6 +16,8 @@ package feign.error; import feign.Response; import feign.codec.Decoder; import feign.codec.ErrorDecoder; +import java.lang.annotation.Annotation; +import java.lang.reflect.AnnotatedElement; import java.lang.reflect.Method; import java.util.HashMap; import java.util.Map; @@ -93,9 +95,10 @@ public class AnnotationErrorDecoder implements ErrorDecoder { Map methodErrorHandlerMap = new HashMap(); for (Method method : apiType.getMethods()) { - if (method.isAnnotationPresent(ErrorHandling.class)) { + ErrorHandling methodLevelAnnotation = getErrorHandlingAnnotation(method); + if (methodLevelAnnotation != null) { ErrorHandlingDefinition methodErrorHandling = - readAnnotation(method.getAnnotation(ErrorHandling.class), responseBodyDecoder); + readAnnotation(methodLevelAnnotation, responseBodyDecoder); ExceptionGenerator methodDefault = methodErrorHandling.defaultThrow; if (methodDefault.getExceptionType().equals(ErrorHandling.NO_DEFAULT.class)) { methodDefault = classLevelDefault; @@ -113,8 +116,9 @@ public class AnnotationErrorDecoder implements ErrorDecoder { } Optional readErrorHandlingIncludingInherited(Class apiType) { - if (apiType.isAnnotationPresent(ErrorHandling.class)) { - return Optional.of(apiType.getAnnotation(ErrorHandling.class)); + ErrorHandling apiTypeAnnotation = getErrorHandlingAnnotation(apiType); + if (apiTypeAnnotation != null) { + return Optional.of(apiTypeAnnotation); } for (Class parentInterface : apiType.getInterfaces()) { Optional errorHandling = @@ -130,6 +134,19 @@ public class AnnotationErrorDecoder implements ErrorDecoder { return Optional.empty(); } + private static ErrorHandling getErrorHandlingAnnotation(AnnotatedElement element) { + ErrorHandling annotation = element.getAnnotation(ErrorHandling.class); + if (annotation == null) { + for (Annotation metaAnnotation : element.getAnnotations()) { + annotation = metaAnnotation.annotationType().getAnnotation(ErrorHandling.class); + if (annotation != null) { + break; + } + } + } + return annotation; + } + static ErrorHandlingDefinition readAnnotation(ErrorHandling errorHandling, Decoder responseBodyDecoder) { ExceptionGenerator defaultException = new ExceptionGenerator.Builder() diff --git a/annotation-error-decoder/src/test/java/feign/error/AnnotationErrorDecoderAnnotationInheritanceTest.java b/annotation-error-decoder/src/test/java/feign/error/AnnotationErrorDecoderAnnotationInheritanceTest.java new file mode 100644 index 00000000..796f08f9 --- /dev/null +++ b/annotation-error-decoder/src/test/java/feign/error/AnnotationErrorDecoderAnnotationInheritanceTest.java @@ -0,0 +1,105 @@ +/** + * Copyright 2012-2021 The Feign Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package feign.error; + +import static org.assertj.core.api.Assertions.assertThat; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.util.Arrays; + +@RunWith(Parameterized.class) +public class AnnotationErrorDecoderAnnotationInheritanceTest extends + AbstractAnnotationErrorDecoderTest { + @Override + public Class interfaceAtTest() { + return TestClientInterfaceWithWithMetaAnnotation.class; + } + + @Parameters( + name = "{0}: When error code ({1}) on method ({2}) should return exception type ({3})") + public static Iterable data() { + return Arrays.asList(new Object[][] { + {"Test Code Specific At Method", 402, "method1Test", MethodLevelDefaultException.class}, + {"Test Code Specific At Method", 403, "method1Test", MethodLevelNotFoundException.class}, + {"Test Code Specific At Method", 404, "method1Test", MethodLevelNotFoundException.class}, + {"Test Code Specific At Method", 402, "method2Test", ClassLevelDefaultException.class}, + {"Test Code Specific At Method", 403, "method2Test", MethodLevelNotFoundException.class}, + {"Test Code Specific At Method", 404, "method2Test", ClassLevelNotFoundException.class}, + }); + } + + @Parameter // first data value (0) is default + public String testType; + + @Parameter(1) + public int errorCode; + + @Parameter(2) + public String method; + + @Parameter(3) + public Class expectedExceptionClass; + + @Test + public void test() throws Exception { + AnnotationErrorDecoder decoder = + AnnotationErrorDecoder.builderFor(TestClientInterfaceWithWithMetaAnnotation.class).build(); + + assertThat(decoder.decode(feignConfigKey(method), testResponse(errorCode)).getClass()) + .isEqualTo(expectedExceptionClass); + } + + @ClassError + interface TestClientInterfaceWithWithMetaAnnotation { + @MethodError + void method1Test(); + + @ErrorHandling( + codeSpecific = {@ErrorCodes(codes = {403}, generate = MethodLevelNotFoundException.class)}) + void method2Test(); + } + + @ErrorHandling( + codeSpecific = {@ErrorCodes(codes = {404}, generate = ClassLevelNotFoundException.class),}, + defaultException = ClassLevelDefaultException.class) + @Retention(RetentionPolicy.RUNTIME) + @interface ClassError { + } + + @ErrorHandling( + codeSpecific = { + @ErrorCodes(codes = {404, 403}, generate = MethodLevelNotFoundException.class),}, + defaultException = MethodLevelDefaultException.class) + @Retention(RetentionPolicy.RUNTIME) + @interface MethodError { + } + + static class ClassLevelDefaultException extends Exception { + public ClassLevelDefaultException() {} + } + static class ClassLevelNotFoundException extends Exception { + public ClassLevelNotFoundException() {} + } + static class MethodLevelDefaultException extends Exception { + public MethodLevelDefaultException() {} + } + static class MethodLevelNotFoundException extends Exception { + public MethodLevelNotFoundException() {} + } +} diff --git a/annotation-error-decoder/src/test/java/feign/error/AnnotationErrorDecoderInheritanceClassLevelAnnotationTest.java b/annotation-error-decoder/src/test/java/feign/error/AnnotationErrorDecoderInheritanceClassLevelAnnotationTest.java new file mode 100644 index 00000000..47d37746 --- /dev/null +++ b/annotation-error-decoder/src/test/java/feign/error/AnnotationErrorDecoderInheritanceClassLevelAnnotationTest.java @@ -0,0 +1,119 @@ +/** + * Copyright 2012-2021 The Feign Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package feign.error; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.util.Arrays; +import static org.assertj.core.api.Assertions.assertThat; + +@RunWith(Parameterized.class) +public class AnnotationErrorDecoderInheritanceClassLevelAnnotationTest extends + AbstractAnnotationErrorDecoderTest { + @Override + public Class interfaceAtTest() { + return SecondLevelInterface.class; + } + + @Parameters( + name = "{0}: When error code ({1}) on method ({2}) should return exception type ({3})") + public static Iterable data() { + return Arrays.asList(new Object[][] { + {"Test Code Specific At Method", 403, "topLevelMethod", + SecondLevelClassDefaultException.class}, + {"Test Code Specific At Method", 403, "secondLevelMethod", + SecondLevelMethodDefaultException.class}, + {"Test Code Specific At Method", 404, "topLevelMethod", + SecondLevelClassAnnotationException.class}, + {"Test Code Specific At Method", 404, "secondLevelMethod", + SecondLevelMethodErrorHandlingException.class}, + }); + } + + @Parameter // first data value (0) is default + public String testType; + + @Parameter(1) + public int errorCode; + + @Parameter(2) + public String method; + + @Parameter(3) + public Class expectedExceptionClass; + + @Test + public void test() throws Exception { + AnnotationErrorDecoder decoder = + AnnotationErrorDecoder.builderFor(SecondLevelInterface.class).build(); + + assertThat(decoder.decode(feignConfigKey(method), testResponse(errorCode)).getClass()) + .isEqualTo(expectedExceptionClass); + } + + @TopLevelClassError + interface TopLevelInterface { + @ErrorHandling + void topLevelMethod(); + } + + @SecondLevelClassError + interface SecondLevelInterface extends TopLevelInterface { + @ErrorHandling( + codeSpecific = { + @ErrorCodes(codes = {404}, generate = SecondLevelMethodErrorHandlingException.class)}, + defaultException = SecondLevelMethodDefaultException.class) + void secondLevelMethod(); + } + + @ErrorHandling( + codeSpecific = { + @ErrorCodes(codes = {403, 404}, generate = TopLevelClassAnnotationException.class),}, + defaultException = TopLevelClassDefaultException.class) + @Retention(RetentionPolicy.RUNTIME) + @interface TopLevelClassError { + } + + @ErrorHandling( + codeSpecific = { + @ErrorCodes(codes = {404}, generate = SecondLevelClassAnnotationException.class),}, + defaultException = SecondLevelClassDefaultException.class) + @Retention(RetentionPolicy.RUNTIME) + @interface SecondLevelClassError { + } + + static class TopLevelClassDefaultException extends Exception { + public TopLevelClassDefaultException() {} + } + static class TopLevelClassAnnotationException extends Exception { + public TopLevelClassAnnotationException() {} + } + static class SecondLevelClassDefaultException extends Exception { + public SecondLevelClassDefaultException() {} + } + static class SecondLevelMethodDefaultException extends Exception { + public SecondLevelMethodDefaultException() {} + } + static class SecondLevelClassAnnotationException extends Exception { + public SecondLevelClassAnnotationException() {} + } + static class SecondLevelMethodErrorHandlingException extends Exception { + public SecondLevelMethodErrorHandlingException() {} + } +} diff --git a/annotation-error-decoder/src/test/java/feign/error/AnnotationErrorDecoderInheritanceMethodLevelAnnotationTest.java b/annotation-error-decoder/src/test/java/feign/error/AnnotationErrorDecoderInheritanceMethodLevelAnnotationTest.java new file mode 100644 index 00000000..5a4fbd82 --- /dev/null +++ b/annotation-error-decoder/src/test/java/feign/error/AnnotationErrorDecoderInheritanceMethodLevelAnnotationTest.java @@ -0,0 +1,141 @@ +/** + * Copyright 2012-2021 The Feign Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package feign.error; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.util.Arrays; +import static org.assertj.core.api.Assertions.assertThat; + +@RunWith(Parameterized.class) +public class AnnotationErrorDecoderInheritanceMethodLevelAnnotationTest extends + AbstractAnnotationErrorDecoderTest { + @Override + public Class interfaceAtTest() { + return SecondLevelInterface.class; + } + + @Parameters( + name = "{0}: When error code ({1}) on method ({2}) should return exception type ({3})") + public static Iterable data() { + return Arrays.asList(new Object[][] { + {"Test Code Specific At Method", 403, "topLevelMethod1", + MethodTopLevelDefaultException.class}, + {"Test Code Specific At Method", 404, "topLevelMethod1", + MethodTopLevelAnnotationException.class}, + {"Test Code Specific At Method", 403, "topLevelMethod2", + MethodSecondLevelDefaultException.class}, + {"Test Code Specific At Method", 404, "topLevelMethod2", + MethodSecondLevelAnnotationException.class}, + {"Test Code Specific At Method", 403, "topLevelMethod3", + MethodSecondLevelDefaultException.class}, + {"Test Code Specific At Method", 404, "topLevelMethod3", + MethodSecondLevelErrorHandlingException.class}, + {"Test Code Specific At Method", 403, "topLevelMethod4", + MethodSecondLevelDefaultException.class}, + {"Test Code Specific At Method", 404, "topLevelMethod4", + MethodSecondLevelAnnotationException.class}, + }); + } + + @Parameter // first data value (0) is default + public String testType; + + @Parameter(1) + public int errorCode; + + @Parameter(2) + public String method; + + @Parameter(3) + public Class expectedExceptionClass; + + @Test + public void test() throws Exception { + AnnotationErrorDecoder decoder = + AnnotationErrorDecoder.builderFor(SecondLevelInterface.class).build(); + + assertThat(decoder.decode(feignConfigKey(method), testResponse(errorCode)).getClass()) + .isEqualTo(expectedExceptionClass); + } + + interface TopLevelInterface { + @TopLevelMethodErrorHandling + void topLevelMethod1(); + + @TopLevelMethodErrorHandling + void topLevelMethod2(); + + @TopLevelMethodErrorHandling + void topLevelMethod3(); + + @ErrorHandling(codeSpecific = @ErrorCodes(codes = {404}, + generate = TopLevelMethodErrorHandlingException.class)) + void topLevelMethod4(); + } + + interface SecondLevelInterface extends TopLevelInterface { + @SecondLevelMethodErrorHandling + void topLevelMethod2(); + + @ErrorHandling( + codeSpecific = { + @ErrorCodes(codes = {404}, generate = MethodSecondLevelErrorHandlingException.class)}, + defaultException = MethodSecondLevelDefaultException.class) + void topLevelMethod3(); + + @SecondLevelMethodErrorHandling + void topLevelMethod4(); + } + + @ErrorHandling( + codeSpecific = { + @ErrorCodes(codes = {404}, generate = MethodTopLevelAnnotationException.class),}, + defaultException = MethodTopLevelDefaultException.class) + @Retention(RetentionPolicy.RUNTIME) + @interface TopLevelMethodErrorHandling { + } + + @ErrorHandling( + codeSpecific = { + @ErrorCodes(codes = {404}, generate = MethodSecondLevelAnnotationException.class),}, + defaultException = MethodSecondLevelDefaultException.class) + @Retention(RetentionPolicy.RUNTIME) + @interface SecondLevelMethodErrorHandling { + } + + static class MethodTopLevelDefaultException extends Exception { + public MethodTopLevelDefaultException() {} + } + static class TopLevelMethodErrorHandlingException extends Exception { + public TopLevelMethodErrorHandlingException() {} + } + static class MethodTopLevelAnnotationException extends Exception { + public MethodTopLevelAnnotationException() {} + } + static class MethodSecondLevelDefaultException extends Exception { + public MethodSecondLevelDefaultException() {} + } + static class MethodSecondLevelErrorHandlingException extends Exception { + public MethodSecondLevelErrorHandlingException() {} + } + static class MethodSecondLevelAnnotationException extends Exception { + public MethodSecondLevelAnnotationException() {} + } +}