Skip to content

Add FeignClientBuilder.Builder#customize(FeignBuilderCustomizer) (#499) #515

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 13, 2021
Merged

Add FeignClientBuilder.Builder#customize(FeignBuilderCustomizer) (#499) #515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions ...-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package org.springframework.cloud.openfeign;

import feign.Feign;
import feign.hystrix.FallbackFactory;

import org.springframework.context.ApplicationContext;
Expand All @@ -28,6 +29,7 @@
*
* @author Sven Döring
* @author Matt King
* @author Sam Kruglov
*/
public class FeignClientBuilder {

Expand Down Expand Up @@ -69,6 +71,19 @@ public Builder<T> url(final String url) {
return this;
}

/**
* Applies a {@link FeignBuilderCustomizer} to the underlying {@link Feign.Builder}.
* May be called multiple times.
*
* @param customizer applied in the same order as supplied here
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a description line on top of the javadoc and the @return tag.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is self-explanatory but sure

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant sth more like the {@link Builder} with the customizer added rather than just this.

* after applying customizers found in the context.
* @return the {@link Builder} with the customizer added
*/
public Builder<T> customize(final FeignBuilderCustomizer customizer) {
this.feignClientFactoryBean.addCustomizer(customizer);
return this;
}

public Builder<T> contextId(final String contextId) {
this.feignClientFactoryBean.setContextId(contextId);
return this;
Expand Down
9 changes: 9 additions & 0 deletions ...nfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientFactoryBean.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.springframework.cloud.openfeign;

import java.util.ArrayList;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -62,6 +63,7 @@
* @author Olga Maciaszek-Sharma
* @author Ilia Ilinykh
* @author Marcin Grzejszczak
* @author Sam Kruglov
*/
public class FeignClientFactoryBean implements FactoryBean<Object>, InitializingBean,
ApplicationContextAware, BeanFactoryAware {
Expand Down Expand Up @@ -99,6 +101,8 @@ public class FeignClientFactoryBean implements FactoryBean<Object>, Initializing

private boolean followRedirects = new Request.Options().isFollowRedirects();

private List<FeignBuilderCustomizer> additionalCustomizers = new ArrayList<>();

@Override
public void afterPropertiesSet() {
Assert.hasText(contextId, "Context id must be set");
Expand Down Expand Up @@ -134,6 +138,7 @@ private void applyBuildCustomizers(FeignContext context, Feign.Builder builder)
.forEach(feignBuilderCustomizer -> feignBuilderCustomizer
.customize(builder));
}
additionalCustomizers.forEach(customizer -> customizer.customize(builder));
}

protected void configureFeign(FeignContext context, Feign.Builder builder) {
Expand Down Expand Up @@ -483,6 +488,10 @@ public void setInheritParentContext(boolean inheritParentContext) {
this.inheritParentContext = inheritParentContext;
}

public void addCustomizer(FeignBuilderCustomizer customizer) {
additionalCustomizers.add(customizer);
}

public ApplicationContext getApplicationContext() {
return applicationContext;
}
Expand Down
19 changes: 19 additions & 0 deletions ...n-core/src/test/java/org/springframework/cloud/openfeign/FeignBuilderCustomizerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

/**
* @author Matt King
* @author Sam Kruglov
*/
public class FeignBuilderCustomizerTests {

Expand Down Expand Up @@ -78,6 +79,24 @@ public void testBuildCustomizerOrdered() {
context.close();
}

@Test
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copied from above, diff 83, 84, 88, 90

public void testBuildCustomizerOrderedWithAdditional() {
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);

Feign.Builder builder = clientFactoryBean.feign(feignContext);
assertFeignBuilderField(builder, "logLevel", Logger.Level.BASIC);
assertFeignBuilderField(builder, "decode404", true);
assertFeignBuilderField(builder, "closeAfterDecode", false);

context.close();
}

private static FeignClientFactoryBean defaultFeignClientFactoryBean() {
FeignClientFactoryBean feignClientFactoryBean = new FeignClientFactoryBean();
feignClientFactoryBean.setContextId("test");
Expand Down
25 changes: 17 additions & 8 deletions ...feign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientBuilderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.Collections;
import java.util.List;

import feign.Feign;
import feign.hystrix.FallbackFactory;
import org.hamcrest.Matchers;
import org.junit.Before;
Expand All @@ -40,6 +41,7 @@

/**
* @author Sven Döring
* @author Sam Kruglov
*/
public class FeignClientBuilderTests {

Expand All @@ -56,20 +58,24 @@ private static Object getDefaultValueFromFeignClientAnnotation(
return method.getDefaultValue();
}

private static void assertFactoryBeanField(final FeignClientBuilder.Builder builder,
final String fieldName, final Object expectedValue) {
final Field factoryBeanField = ReflectionUtils
.findField(FeignClientBuilder.Builder.class, "feignClientFactoryBean");
private static void assertFactoryBeanField(final FeignClientBuilder.Builder builder, final String fieldName,
final Object expectedValue) {
final Object value = getFactoryBeanField(builder, fieldName);
assertThat(value).as("Expected value for the field '" + fieldName + "':").isEqualTo(expectedValue);
}

@SuppressWarnings("unchecked")
private static <T> T getFactoryBeanField(final FeignClientBuilder.Builder builder, final String fieldName) {
final Field factoryBeanField = ReflectionUtils.findField(FeignClientBuilder.Builder.class,
"feignClientFactoryBean");
ReflectionUtils.makeAccessible(factoryBeanField);
final FeignClientFactoryBean factoryBean = (FeignClientFactoryBean) ReflectionUtils
.getField(factoryBeanField, builder);

final Field field = ReflectionUtils.findField(FeignClientFactoryBean.class,
fieldName);
ReflectionUtils.makeAccessible(field);
final Object value = ReflectionUtils.getField(field, factoryBean);
assertThat(value).as("Expected value for the field '" + fieldName + "':")
.isEqualTo(expectedValue);
return (T) ReflectionUtils.getField(field, factoryBean);
}

@Before
Expand Down Expand Up @@ -131,7 +137,8 @@ public void forType_allFieldsSetOnBuilder() {
.forType(TestFeignClient.class, "TestClient").inheritParentContext(false)
.fallback(TestFeignClientFallback.class)
.fallbackFactory(TestFeignClientFallbackFactory.class).decode404(true)
.url("Url/").path("/Path").contextId("TestContext");
.url("Url/").path("/Path").contextId("TestContext")
.customize(Feign.Builder::doNotCloseAfterDecode);;

// then:
assertFactoryBeanField(builder, "applicationContext", this.applicationContext);
Expand All @@ -147,6 +154,8 @@ public void forType_allFieldsSetOnBuilder() {
assertFactoryBeanField(builder, "fallback", TestFeignClientFallback.class);
assertFactoryBeanField(builder, "fallbackFactory",
TestFeignClientFallbackFactory.class);
List<FeignBuilderCustomizer> additionalCustomizers = getFactoryBeanField(builder, "additionalCustomizers");
assertThat(additionalCustomizers).hasSize(1);
}

@Test
Expand Down