diff --git a/core/src/main/java/feign/BaseBuilder.java b/core/src/main/java/feign/BaseBuilder.java index d279ed764..43c903b8c 100644 --- a/core/src/main/java/feign/BaseBuilder.java +++ b/core/src/main/java/feign/BaseBuilder.java @@ -40,8 +40,6 @@ public abstract class BaseBuilder, T> implements Cloneable { - private final B thisB; - protected List requestInterceptors = new ArrayList<>(); protected List responseInterceptors = new ArrayList<>(); protected List methodInterceptors = new ArrayList<>(); @@ -64,43 +62,47 @@ public abstract class BaseBuilder, T> implements Clo public BaseBuilder() { super(); - thisB = (B) this; + } + + @SuppressWarnings("unchecked") + private B thisB() { + return (B) this; } public B logLevel(Logger.Level logLevel) { this.logLevel = logLevel; - return thisB; + return thisB(); } public B contract(Contract contract) { this.contract = contract; - return thisB; + return thisB(); } public B retryer(Retryer retryer) { this.retryer = retryer; - return thisB; + return thisB(); } public B logger(Logger logger) { this.logger = logger; - return thisB; + return thisB(); } public B encoder(Encoder encoder) { this.encoder = encoder; - return thisB; + return thisB(); } public B decoder(Decoder decoder) { this.decoder = decoder; - return thisB; + return thisB(); } public B codec(Codec codec) { this.encoder = codec.encoder(); this.decoder = codec.decoder(); - return thisB; + return thisB(); } /** @@ -115,23 +117,23 @@ public B codec(Codec codec) { */ public B doNotCloseAfterDecode() { this.closeAfterDecode = false; - return thisB; + return thisB(); } public B decodeVoid() { this.decodeVoid = true; - return thisB; + return thisB(); } public B queryMapEncoder(QueryMapEncoder queryMapEncoder) { this.queryMapEncoder = queryMapEncoder; - return thisB; + return thisB(); } /** Allows to map the response before passing it to the decoder. */ public B mapAndDecode(ResponseMapper mapper, Decoder decoder) { this.decoder = new ResponseMappingDecoder(mapper, decoder); - return thisB; + return thisB(); } /** @@ -151,7 +153,7 @@ public B mapAndDecode(ResponseMapper mapper, Decoder decoder) { */ public B dismiss404() { this.dismiss404 = true; - return thisB; + return thisB(); } /** @@ -173,23 +175,23 @@ public B dismiss404() { @Deprecated public B decode404() { this.dismiss404 = true; - return thisB; + return thisB(); } public B errorDecoder(ErrorDecoder errorDecoder) { this.errorDecoder = errorDecoder; - return thisB; + return thisB(); } public B options(Options options) { this.options = options; - return thisB; + return thisB(); } /** Adds a single request interceptor to the builder. */ public B requestInterceptor(RequestInterceptor requestInterceptor) { this.requestInterceptors.add(requestInterceptor); - return thisB; + return thisB(); } /** @@ -201,7 +203,7 @@ public B requestInterceptors(Iterable requestInterceptors) { for (RequestInterceptor requestInterceptor : requestInterceptors) { this.requestInterceptors.add(requestInterceptor); } - return thisB; + return thisB(); } /** @@ -213,13 +215,13 @@ public B responseInterceptors(Iterable responseInterceptors for (ResponseInterceptor responseInterceptor : responseInterceptors) { this.responseInterceptors.add(responseInterceptor); } - return thisB; + return thisB(); } /** Adds a single response interceptor to the builder. */ public B responseInterceptor(ResponseInterceptor responseInterceptor) { this.responseInterceptors.add(responseInterceptor); - return thisB; + return thisB(); } /** @@ -230,7 +232,7 @@ public B responseInterceptor(ResponseInterceptor responseInterceptor) { @Experimental public B methodInterceptor(MethodInterceptor methodInterceptor) { this.methodInterceptors.add(methodInterceptor); - return thisB; + return thisB(); } /** Sets the full set of method interceptors, overwriting any previously configured. */ @@ -240,33 +242,33 @@ public B methodInterceptors(Iterable methodInterceptors) { for (MethodInterceptor methodInterceptor : methodInterceptors) { this.methodInterceptors.add(methodInterceptor); } - return thisB; + return thisB(); } /** Allows you to override how reflective dispatch works inside of Feign. */ public B invocationHandlerFactory(InvocationHandlerFactory invocationHandlerFactory) { this.invocationHandlerFactory = invocationHandlerFactory; - return thisB; + return thisB(); } public B exceptionPropagationPolicy(ExceptionPropagationPolicy propagationPolicy) { this.propagationPolicy = propagationPolicy; - return thisB; + return thisB(); } public B addCapability(Capability capability) { this.capabilities.add(capability); - return thisB; + return thisB(); } @SuppressWarnings("unchecked") B enrich() { if (capabilities.isEmpty()) { - return thisB; + return thisB(); } try { - B clone = (B) thisB.clone(); + B clone = (B) thisB().clone(); getFieldsToEnrich() .forEach( @@ -356,8 +358,6 @@ List getFieldsToEnrich() { .filter(field -> !field.isSynthetic()) // and capabilities itself .filter(field -> !Objects.equals(field.getName(), "capabilities")) - // and thisB helper field - .filter(field -> !Objects.equals(field.getName(), "thisB")) // interceptor lists are enriched per-element then as a whole via custom types .filter(field -> !Objects.equals(field.getName(), "requestInterceptors")) .filter(field -> !Objects.equals(field.getName(), "responseInterceptors")) diff --git a/core/src/test/java/feign/BaseBuilderTest.java b/core/src/test/java/feign/BaseBuilderTest.java index 30abf2d48..a1874fb74 100644 --- a/core/src/test/java/feign/BaseBuilderTest.java +++ b/core/src/test/java/feign/BaseBuilderTest.java @@ -19,6 +19,7 @@ import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.mockito.Mockito.RETURNS_MOCKS; +import feign.codec.Decoder; import java.lang.reflect.Field; import java.util.List; import org.junit.jupiter.api.Test; @@ -62,4 +63,20 @@ void checkEnrichTouchesAllBuilderFields() test( Feign.builder().requestInterceptor(_ -> {}).responseInterceptor((ic, c) -> c.next(ic)), 10); } + + @Test + void clonedBuilderFluentMethodsReturnClone() throws CloneNotSupportedException { + CloneableFeignBuilder original = new CloneableFeignBuilder(); + Feign.Builder clone = original.copy(); + + assertThat(clone.requestInterceptor(_ -> {})).isSameAs(clone); + assertThat(clone.decoder(Mockito.mock(Decoder.class))).isSameAs(clone); + } + + private static final class CloneableFeignBuilder extends Feign.Builder { + + Feign.Builder copy() throws CloneNotSupportedException { + return (Feign.Builder) super.clone(); + } + } }