From bbfa85eb9fbcdab4b2950dc7261d05d06a08f281 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Tue, 16 Aug 2016 10:39:03 +0200 Subject: [PATCH] Ensured reusability of Feign components Tests have been refactored to ensure that the custom components registered as beans are working properly. fixes #374 --- .../web/client/feign/SleuthFeignBuilder.java | 13 ++++- .../feign/SleuthHystrixFeignBuilder.java | 13 ++++- .../cloud/sleuth/AdhocTestSuite.java | 2 +- .../WebClientDiscoveryExceptionTests.java | 2 +- .../WebClientExceptionTests.java | 2 +- .../issues/{ => issue307}/Issue307Tests.java | 2 +- .../feign/issues/issue362/Issue362Tests.java | 49 ++++++++++++++++--- .../FeignClientServerErrorTests.java | 2 +- .../{ => integration}/WebClientTests.java | 2 +- 9 files changed, 73 insertions(+), 14 deletions(-) rename spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/{ => discoveryexception}/WebClientDiscoveryExceptionTests.java (98%) rename spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/{ => exception}/WebClientExceptionTests.java (98%) rename spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/{ => issue307}/Issue307Tests.java (99%) rename spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/{ => servererrors}/FeignClientServerErrorTests.java (99%) rename spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/{ => integration}/WebClientTests.java (99%) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/SleuthFeignBuilder.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/SleuthFeignBuilder.java index b8486118e..6553a6cc1 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/SleuthFeignBuilder.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/SleuthFeignBuilder.java @@ -16,8 +16,10 @@ package org.springframework.cloud.sleuth.instrument.web.client.feign; +import org.springframework.beans.BeansException; import org.springframework.beans.factory.BeanFactory; +import feign.Client; import feign.Feign; /** @@ -34,6 +36,15 @@ final class SleuthFeignBuilder { static Feign.Builder builder(BeanFactory beanFactory) { return Feign.builder() - .client(new TraceFeignClient(beanFactory)); + .client(client(beanFactory)); + } + + private static Client client(BeanFactory beanFactory) { + try { + Client client = beanFactory.getBean(Client.class); + return (Client) new TraceFeignObjectWrapper(beanFactory).wrap(client); + } catch (BeansException e) { + return new TraceFeignClient(beanFactory); + } } } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/SleuthHystrixFeignBuilder.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/SleuthHystrixFeignBuilder.java index 8b69adae8..1efe8cadf 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/SleuthHystrixFeignBuilder.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/SleuthHystrixFeignBuilder.java @@ -16,8 +16,10 @@ package org.springframework.cloud.sleuth.instrument.web.client.feign; +import org.springframework.beans.BeansException; import org.springframework.beans.factory.BeanFactory; +import feign.Client; import feign.Feign; import feign.hystrix.HystrixFeign; @@ -36,6 +38,15 @@ final class SleuthHystrixFeignBuilder { static Feign.Builder builder(BeanFactory beanFactory) { return HystrixFeign.builder() - .client(new TraceFeignClient(beanFactory)); + .client(client(beanFactory)); + } + + private static Client client(BeanFactory beanFactory) { + try { + Client client = beanFactory.getBean(Client.class); + return (Client) new TraceFeignObjectWrapper(beanFactory).wrap(client); + } catch (BeansException e) { + return new TraceFeignClient(beanFactory); + } } } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/AdhocTestSuite.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/AdhocTestSuite.java index 7ffdbed5b..b2bbc570c 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/AdhocTestSuite.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/AdhocTestSuite.java @@ -21,7 +21,7 @@ import org.junit.runner.RunWith; import org.junit.runners.Suite; import org.junit.runners.Suite.SuiteClasses; import org.springframework.cloud.sleuth.instrument.web.RestTemplateTraceAspectIntegrationTests; -import org.springframework.cloud.sleuth.instrument.web.client.WebClientDiscoveryExceptionTests; +import org.springframework.cloud.sleuth.instrument.web.client.discoveryexception.WebClientDiscoveryExceptionTests; /** * A test suite for probing weird ordering problems in the tests. diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/WebClientDiscoveryExceptionTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/discoveryexception/WebClientDiscoveryExceptionTests.java similarity index 98% rename from spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/WebClientDiscoveryExceptionTests.java rename to spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/discoveryexception/WebClientDiscoveryExceptionTests.java index 4f215f627..fde2db7ed 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/WebClientDiscoveryExceptionTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/discoveryexception/WebClientDiscoveryExceptionTests.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package org.springframework.cloud.sleuth.instrument.web.client; +package org.springframework.cloud.sleuth.instrument.web.client.discoveryexception; import java.io.IOException; import java.util.Map; diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/WebClientExceptionTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/exception/WebClientExceptionTests.java similarity index 98% rename from spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/WebClientExceptionTests.java rename to spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/exception/WebClientExceptionTests.java index 5d19dec21..207495380 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/WebClientExceptionTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/exception/WebClientExceptionTests.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package org.springframework.cloud.sleuth.instrument.web.client; +package org.springframework.cloud.sleuth.instrument.web.client.exception; import java.io.IOException; import java.util.Collections; diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/Issue307Tests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/issue307/Issue307Tests.java similarity index 99% rename from spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/Issue307Tests.java rename to spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/issue307/Issue307Tests.java index c0d14ae8c..bd3eb5856 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/Issue307Tests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/issue307/Issue307Tests.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package org.springframework.cloud.sleuth.instrument.web.client.feign.issues; +package org.springframework.cloud.sleuth.instrument.web.client.feign.issues.issue307; import java.util.ArrayList; import java.util.List; diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/issue362/Issue362Tests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/issue362/Issue362Tests.java index c6474b3e5..a997375e7 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/issue362/Issue362Tests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/issue362/Issue362Tests.java @@ -16,7 +16,10 @@ package org.springframework.cloud.sleuth.instrument.web.client.feign.issues.issue362; +import java.io.IOException; import java.util.Date; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutionException; import org.junit.Before; @@ -43,6 +46,7 @@ import org.springframework.web.client.RestTemplate; import feign.Client; import feign.Logger; +import feign.Request; import feign.Response; import feign.RetryableException; import feign.Retryer; @@ -61,9 +65,11 @@ import static org.assertj.core.api.BDDAssertions.then; public class Issue362Tests { RestTemplate template = new RestTemplate(); + @Autowired FeignComponentAsserter feignComponentAsserter; @Before public void setup() { + this.feignComponentAsserter.executedComponents.clear(); ExceptionUtils.setFail(true); } @@ -75,6 +81,7 @@ public class Issue362Tests { SleuthAssertions.then(response.getBody()).isEqualTo("I'm OK"); then(ExceptionUtils.getLastException()).isNull(); + then(this.feignComponentAsserter.executedComponents).containsEntry(Client.class, true); } @Test @@ -87,6 +94,9 @@ public class Issue362Tests { } catch (Exception e) { } then(ExceptionUtils.getLastException()).isNull(); + then(this.feignComponentAsserter.executedComponents) + .containsEntry(ErrorDecoder.class, true) + .containsEntry(Client.class, true); } } @@ -117,17 +127,20 @@ class Application { } @Bean - public Client client() { - return new Client.Default(null, null); - } + public FeignComponentAsserter testHolder() { return new FeignComponentAsserter(); } + +} + +class FeignComponentAsserter { + Map executedComponents = new ConcurrentHashMap<>(); } @Configuration class CustomConfig { @Bean - public ErrorDecoder errorDecoder() { - return new CustomErrorDecoder(); + public ErrorDecoder errorDecoder(FeignComponentAsserter feignComponentAsserter) { + return new CustomErrorDecoder(feignComponentAsserter); } @Bean @@ -137,18 +150,42 @@ class CustomConfig { public static class CustomErrorDecoder extends ErrorDecoder.Default { - public CustomErrorDecoder() { + private final FeignComponentAsserter feignComponentAsserter; + + public CustomErrorDecoder(FeignComponentAsserter feignComponentAsserter) { + this.feignComponentAsserter = feignComponentAsserter; } @Override public Exception decode(String methodKey, Response response) { + this.feignComponentAsserter.executedComponents.put(ErrorDecoder.class, true); if (response.status() == 409) { return new RetryableException("Article not Ready", new Date()); } else { return super.decode(methodKey, response); } } + } + @Bean + public Client client(FeignComponentAsserter feignComponentAsserter) { + return new CustomClient(feignComponentAsserter); + } + + public static class CustomClient extends Client.Default { + + private final FeignComponentAsserter feignComponentAsserter; + + public CustomClient(FeignComponentAsserter feignComponentAsserter) { + super(null, null); + this.feignComponentAsserter = feignComponentAsserter; + } + + @Override public Response execute(Request request, Request.Options options) + throws IOException { + this.feignComponentAsserter.executedComponents.put(Client.class, true); + return super.execute(request, options); + } } } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignClientServerErrorTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/servererrors/FeignClientServerErrorTests.java similarity index 99% rename from spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignClientServerErrorTests.java rename to spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/servererrors/FeignClientServerErrorTests.java index 3420000aa..fd58109b8 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignClientServerErrorTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/servererrors/FeignClientServerErrorTests.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package org.springframework.cloud.sleuth.instrument.web.client.feign; +package org.springframework.cloud.sleuth.instrument.web.client.feign.servererrors; import java.util.ArrayList; import java.util.Collections; diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/WebClientTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/integration/WebClientTests.java similarity index 99% rename from spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/WebClientTests.java rename to spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/integration/WebClientTests.java index 43bfe2de4..bb7c7adb5 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/WebClientTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/integration/WebClientTests.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package org.springframework.cloud.sleuth.instrument.web.client; +package org.springframework.cloud.sleuth.instrument.web.client.integration; import java.lang.invoke.MethodHandles; import java.util.ArrayList;