From c281800ccd4254780507e5988ae67b6b23a09df7 Mon Sep 17 00:00:00 2001 From: Michael Cramer Date: Tue, 24 Mar 2020 17:12:29 +0100 Subject: [PATCH] add support for an FeignErrorDecoderFactory in 2.2.x (#310) * add support for an FeignErrorDecoderFactory this factory will be used if no ErrorDecoder was found. This factory allows to create a ErrorDecoder based on the type. This fixes #308 * add assertions to make it more readable * add test for checking to not overwrite an existing ErrorDecoder when provided in the configuration * add test for checking to not overwrite an existing ErrorDecoder when provided in the configuration * add test for checking to not overwrite an existing ErrorDecoder when provided in the configuration --- .../openfeign/FeignClientFactoryBean.java | 8 + .../openfeign/FeignErrorDecoderFactory.java | 37 ++++ .../FeignClientErrorDecoderTests.java | 170 ++++++++++++++++++ .../FeignErrorDecoderFactoryTests.java | 125 +++++++++++++ 4 files changed, 340 insertions(+) create mode 100644 spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignErrorDecoderFactory.java create mode 100644 spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientErrorDecoderTests.java create mode 100644 spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignErrorDecoderFactoryTests.java diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientFactoryBean.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientFactoryBean.java index 7b4791a9..bb9c4551 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientFactoryBean.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientFactoryBean.java @@ -141,6 +141,14 @@ class FeignClientFactoryBean if (errorDecoder != null) { builder.errorDecoder(errorDecoder); } + else { + FeignErrorDecoderFactory errorDecoderFactory = getOptional(context, + FeignErrorDecoderFactory.class); + if (errorDecoderFactory != null) { + ErrorDecoder factoryErrorDecoder = errorDecoderFactory.create(this.type); + builder.errorDecoder(factoryErrorDecoder); + } + } Request.Options options = getOptional(context, Request.Options.class); if (options != null) { builder.options(options); diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignErrorDecoderFactory.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignErrorDecoderFactory.java new file mode 100644 index 00000000..c11d6d39 --- /dev/null +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignErrorDecoderFactory.java @@ -0,0 +1,37 @@ +/* + * Copyright 2020-2020 the original author or 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 + * + * https://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 org.springframework.cloud.openfeign; + +import feign.codec.ErrorDecoder; + +/** + * Allows an application to use a custom Feign {@link feign.codec.ErrorDecoder}. + * + * @author Michael Cramer + */ +public interface FeignErrorDecoderFactory { + + /** + * Factory method to provide a {@link feign.codec.ErrorDecoder} for a given + * {@link Class}. + * @param type the {@link Class} for which a {@link feign.codec.ErrorDecoder} instance + * is to be created + * @return a {@link feign.codec.ErrorDecoder} instance + */ + ErrorDecoder create(Class type); + +} diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientErrorDecoderTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientErrorDecoderTests.java new file mode 100644 index 00000000..d1cc5bc2 --- /dev/null +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientErrorDecoderTests.java @@ -0,0 +1,170 @@ +/* + * Copyright 2020-2020 the original author or 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 + * + * https://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 org.springframework.cloud.openfeign; + +import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.Map; + +import feign.Contract; +import feign.InvocationHandlerFactory; +import feign.RequestLine; +import feign.Response; +import feign.codec.ErrorDecoder; +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.cloud.openfeign.support.SpringMvcContract; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.test.util.ReflectionTestUtils; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestMethod; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Michael Cramer + */ +@RunWith(SpringJUnit4ClassRunner.class) +@SpringBootTest(classes = FeignClientErrorDecoderTests.TestConfiguration.class) +@DirtiesContext +public class FeignClientErrorDecoderTests { + + @Autowired + private FeignContext context; + + @Autowired + private FooClient foo; + + @Autowired + private BarClient bar; + + @Test + public void clientsAvailable() { + assertThat(this.foo).isNotNull(); + assertThat(this.bar).isNotNull(); + } + + @Test + public void errorDecoderInConfiguration() { + assertThat(this.context.getInstance("foo", ErrorDecoder.class)) + .isInstanceOf(ErrorDecoder.Default.class); + assertThat(this.context.getInstance("bar", ErrorDecoder.class)).isNull(); + } + + @Test + public void useConfiguredErrorDecoderWhenAlsoErrorDecoderFactoryIsAvailable() { + Object errorDecoder = getErrorDecoderFromClient(this.foo); + assertThat(errorDecoder).isInstanceOf(ErrorDecoder.Default.class); + } + + @Test + public void useErrorDecoderFromErrorDecoderFactory() { + Object errorDecoder = getErrorDecoderFromClient(this.bar); + assertThat(errorDecoder).isInstanceOf(ErrorDecoderImpl.class); + } + + @SuppressWarnings({ "unchecked", "ConstantConditions" }) + private Object getErrorDecoderFromClient(final Object client) { + Object invocationHandler = ReflectionTestUtils.getField(client, "h"); + Map dispatch = (Map) ReflectionTestUtils + .getField(invocationHandler, "dispatch"); + Method key = new ArrayList<>(dispatch.keySet()).get(0); + return ReflectionTestUtils.getField(dispatch.get(key), "errorDecoder"); + } + + @Configuration(proxyBeanMethods = false) + @EnableFeignClients(clients = { FooClient.class, BarClient.class }) + @EnableAutoConfiguration + protected static class TestConfiguration { + + } + + @FeignClient(name = "foo", url = "http://foo", configuration = FooConfiguration.class) + interface FooClient { + + @RequestLine("GET /") + String get(); + + } + + public static class FooConfiguration { + + @Bean + Contract feignContract() { + return new Contract.Default(); + } + + @Bean + FeignErrorDecoderFactory errorDecoderFactory() { + return new FeignErrorDecoderFactoryImpl(); + } + + @Bean + ErrorDecoder feignErrorDecoder() { + return new ErrorDecoder.Default(); + } + + } + + @FeignClient(name = "bar", url = "http://bar", configuration = BarConfiguration.class) + interface BarClient { + + @RequestMapping(value = "/", method = RequestMethod.GET) + String get(); + + } + + public static class BarConfiguration { + + @Bean + Contract feignContract() { + return new SpringMvcContract(); + } + + @Bean + FeignErrorDecoderFactory errorDecoderFactory() { + return new FeignErrorDecoderFactoryImpl(); + } + + } + + public static class FeignErrorDecoderFactoryImpl implements FeignErrorDecoderFactory { + + @Override + public ErrorDecoder create(final Class type) { + return new ErrorDecoderImpl(); + } + + } + + public static class ErrorDecoderImpl implements ErrorDecoder { + + @Override + public Exception decode(final String methodKey, final Response response) { + return null; + } + + } + +} diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignErrorDecoderFactoryTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignErrorDecoderFactoryTests.java new file mode 100644 index 00000000..2c677e4a --- /dev/null +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignErrorDecoderFactoryTests.java @@ -0,0 +1,125 @@ +/* + * Copyright 2020-2020 the original author or 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 + * + * https://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 org.springframework.cloud.openfeign; + +import feign.Response; +import feign.codec.ErrorDecoder; +import org.junit.Test; + +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Michael Cramer + */ +public class FeignErrorDecoderFactoryTests { + + @Test + public void testNoDefaultFactory() { + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext( + SampleConfiguration1.class); + String[] beanNamesForType = context + .getBeanNamesForType(FeignErrorDecoderFactory.class); + assertThat(beanNamesForType).isEmpty(); + context.close(); + } + + @Test + public void testCustomErrorDecoderFactory() { + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext( + SampleConfiguration2.class); + FeignErrorDecoderFactory errorDecoderFactory = context + .getBean(FeignErrorDecoderFactory.class); + assertThat(errorDecoderFactory).isNotNull(); + ErrorDecoder errorDecoder = errorDecoderFactory.create(Object.class); + assertThat(errorDecoder).isNotNull(); + assertThat(errorDecoder instanceof ErrorDecoderImpl).isTrue(); + context.close(); + } + + @Test + public void testCustomErrorDecoderFactoryNotOverwritingErrorDecoder() { + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext( + SampleConfiguration3.class); + FeignErrorDecoderFactory errorDecoderFactory = context + .getBean(FeignErrorDecoderFactory.class); + assertThat(errorDecoderFactory).isNotNull(); + ErrorDecoder errorDecoderFromFactory = errorDecoderFactory.create(Object.class); + assertThat(errorDecoderFromFactory).isNotNull(); + assertThat(errorDecoderFromFactory instanceof ErrorDecoderImpl).isTrue(); + ErrorDecoder errorDecoder = context.getBean(ErrorDecoder.class); + assertThat(errorDecoder).isNotNull(); + assertThat(errorDecoder instanceof ErrorDecoder.Default).isTrue(); + context.close(); + } + + @Configuration + @Import(FeignClientsConfiguration.class) + protected static class SampleConfiguration1 { + + } + + @Configuration + @Import(FeignClientsConfiguration.class) + protected static class SampleConfiguration2 { + + @Bean + public FeignErrorDecoderFactory errorDecoderFactory() { + return new ErrorDecoderFactoryImpl(); + } + + } + + @Configuration + @Import(FeignClientsConfiguration.class) + protected static class SampleConfiguration3 { + + @Bean + public FeignErrorDecoderFactory errorDecoderFactory() { + return new ErrorDecoderFactoryImpl(); + } + + @Bean + public ErrorDecoder errorDecoder() { + return new ErrorDecoder.Default(); + } + + } + + static class ErrorDecoderFactoryImpl implements FeignErrorDecoderFactory { + + @Override + public ErrorDecoder create(final Class type) { + return new ErrorDecoderImpl(); + } + + } + + static class ErrorDecoderImpl implements ErrorDecoder { + + @Override + public Exception decode(final String methodKey, final Response response) { + return null; + } + + } + +}