From 83b6f043ddeef8ae2f68e95fefc04e285fbb7877 Mon Sep 17 00:00:00 2001 From: Felix Dittrich <31076102+f11h@users.noreply.github.com> Date: Fri, 28 Jan 2022 10:42:00 +0100 Subject: [PATCH] Move Call of applyBuildCustomizers at end of getTarget Method (#674) --- .../openfeign/FeignClientFactoryBean.java | 5 +- .../FeignBuilderCustomizerTests.java | 71 +++++++++++++++++-- 2 files changed, 69 insertions(+), 7 deletions(-) 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 38552632..dca79f85 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 @@ -69,6 +69,7 @@ import org.springframework.util.StringUtils; * @author Sam Kruglov * @author Jasbir Singh * @author Hyeonmin Park + * @author Felix Dittrich */ public class FeignClientFactoryBean implements FactoryBean, InitializingBean, ApplicationContextAware, BeanFactoryAware { @@ -132,7 +133,6 @@ public class FeignClientFactoryBean // @formatter:on configureFeign(context, builder); - applyBuildCustomizers(context, builder); return builder; } @@ -443,6 +443,9 @@ public class FeignClientFactoryBean } builder.client(client); } + + applyBuildCustomizers(context, builder); + Targeter targeter = get(context, Targeter.class); return (T) targeter.target(this, builder, context, new HardCodedTarget<>(type, name, url)); } diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignBuilderCustomizerTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignBuilderCustomizerTests.java index ebef9c14..6c33843e 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignBuilderCustomizerTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignBuilderCustomizerTests.java @@ -18,9 +18,12 @@ package org.springframework.cloud.openfeign; import java.lang.reflect.Field; +import feign.Client; import feign.Feign; import feign.Logger; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; @@ -30,22 +33,35 @@ import org.springframework.core.annotation.Order; import org.springframework.util.ReflectionUtils; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doCallRealMethod; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; /** * @author Matt King * @author Sam Kruglov + * @author Felix Dittrich */ class FeignBuilderCustomizerTests { + private static final Targeter targeterSpy = spy(DefaultTargeter.class); + + private static final Client defaultClient = mock(Client.class); + @Test void testBuilderCustomizer() { + ArgumentCaptor feignBuilderCaptor = ArgumentCaptor.forClass(Feign.Builder.class); + doCallRealMethod().when(targeterSpy).target(any(), feignBuilderCaptor.capture(), any(), any()); + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext( FeignBuilderCustomizerTests.SampleConfiguration2.class); FeignClientFactoryBean clientFactoryBean = context.getBean(FeignClientFactoryBean.class); - FeignContext feignContext = context.getBean(FeignContext.class); + clientFactoryBean.getTarget(); - Feign.Builder builder = clientFactoryBean.feign(feignContext); + Assertions.assertNotNull(feignBuilderCaptor.getValue()); + Feign.Builder builder = feignBuilderCaptor.getValue(); assertFeignBuilderField(builder, "logLevel", Logger.Level.HEADERS); assertFeignBuilderField(builder, "decode404", true); @@ -62,13 +78,17 @@ class FeignBuilderCustomizerTests { @Test void testBuildCustomizerOrdered() { + ArgumentCaptor feignBuilderCaptor = ArgumentCaptor.forClass(Feign.Builder.class); + doCallRealMethod().when(targeterSpy).target(any(), feignBuilderCaptor.capture(), any(), any()); + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext( FeignBuilderCustomizerTests.SampleConfiguration3.class); FeignClientFactoryBean clientFactoryBean = context.getBean(FeignClientFactoryBean.class); - FeignContext feignContext = context.getBean(FeignContext.class); + clientFactoryBean.getTarget(); - Feign.Builder builder = clientFactoryBean.feign(feignContext); + Assertions.assertNotNull(feignBuilderCaptor.getValue()); + Feign.Builder builder = feignBuilderCaptor.getValue(); assertFeignBuilderField(builder, "logLevel", Logger.Level.FULL); assertFeignBuilderField(builder, "decode404", true); @@ -77,15 +97,19 @@ class FeignBuilderCustomizerTests { @Test void testBuildCustomizerOrderedWithAdditional() { + ArgumentCaptor feignBuilderCaptor = ArgumentCaptor.forClass(Feign.Builder.class); + doCallRealMethod().when(targeterSpy).target(any(), feignBuilderCaptor.capture(), any(), any()); + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext( FeignBuilderCustomizerTests.SampleConfiguration3.class); FeignClientFactoryBean clientFactoryBean = context.getBean(FeignClientFactoryBean.class); clientFactoryBean.addCustomizer(builder -> builder.logLevel(Logger.Level.BASIC)); clientFactoryBean.addCustomizer(Feign.Builder::doNotCloseAfterDecode); - FeignContext feignContext = context.getBean(FeignContext.class); + clientFactoryBean.getTarget(); - Feign.Builder builder = clientFactoryBean.feign(feignContext); + Assertions.assertNotNull(feignBuilderCaptor.getValue()); + Feign.Builder builder = feignBuilderCaptor.getValue(); assertFeignBuilderField(builder, "logLevel", Logger.Level.BASIC); assertFeignBuilderField(builder, "decode404", true); assertFeignBuilderField(builder, "closeAfterDecode", false); @@ -93,6 +117,26 @@ class FeignBuilderCustomizerTests { context.close(); } + @Test + void testBuildCustomizerWithCustomHttpClient() { + ArgumentCaptor feignBuilderCaptor = ArgumentCaptor.forClass(Feign.Builder.class); + doCallRealMethod().when(targeterSpy).target(any(), feignBuilderCaptor.capture(), any(), any()); + + Client customClientMock = mock(Client.class); + + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext( + FeignBuilderCustomizerTests.SampleConfiguration3.class); + FeignClientFactoryBean clientFactoryBean = context.getBean(FeignClientFactoryBean.class); + clientFactoryBean.addCustomizer(builder -> builder.client(customClientMock)); + clientFactoryBean.getTarget(); + + Assertions.assertNotNull(feignBuilderCaptor.getValue()); + Feign.Builder builder = feignBuilderCaptor.getValue(); + assertFeignBuilderField(builder, "client", customClientMock); + + context.close(); + } + private static FeignClientFactoryBean defaultFeignClientFactoryBean() { FeignClientFactoryBean feignClientFactoryBean = new FeignClientFactoryBean(); feignClientFactoryBean.setContextId("test"); @@ -132,6 +176,11 @@ class FeignBuilderCustomizerTests { return defaultFeignClientFactoryBean(); } + @Bean + Targeter targeter() { + return targeterSpy; + } + } @Configuration(proxyBeanMethods = false) @@ -170,6 +219,16 @@ class FeignBuilderCustomizerTests { return defaultFeignClientFactoryBean(); } + @Bean + Targeter targeter() { + return targeterSpy; + } + + @Bean + Client client() { + return defaultClient; + } + } }