FIX: interceptors disapearing on BaseBuilder#clone

Author: hosuaby

core/src/main/java/feign/BaseBuilder.java

@@ -32,9 +32,6 @@
 import java.util.stream.Collectors;
 
 public abstract class BaseBuilder<B extends BaseBuilder<B, T>, T> implements Cloneable {
-
-  private final B thisB;
-
   protected final List<RequestInterceptor> requestInterceptors = new ArrayList<>();
   protected final List<ResponseInterceptor> responseInterceptors = new ArrayList<>();
   protected Logger.Level logLevel = Logger.Level.NONE;
@@ -56,37 +53,42 @@ public abstract class BaseBuilder<B extends BaseBuilder<B, T>, T> implements Clo
 
   public BaseBuilder() {
     super();
-    thisB = (B) this;
   }
 
+  @SuppressWarnings("unchecked")
   public B logLevel(Logger.Level logLevel) {
     this.logLevel = logLevel;
-    return thisB;
+    return (B) this;
   }
 
+  @SuppressWarnings("unchecked")
   public B contract(Contract contract) {
     this.contract = contract;
-    return thisB;
+    return (B) this;
   }
 
+  @SuppressWarnings("unchecked")
   public B retryer(Retryer retryer) {
     this.retryer = retryer;
-    return thisB;
+    return (B) this;
   }
 
+  @SuppressWarnings("unchecked")
   public B logger(Logger logger) {
     this.logger = logger;
-    return thisB;
+    return (B) this;
   }
 
+  @SuppressWarnings("unchecked")
   public B encoder(Encoder encoder) {
     this.encoder = encoder;
-    return thisB;
+    return (B) this;
   }
 
+  @SuppressWarnings("unchecked")
   public B decoder(Decoder decoder) {
     this.decoder = decoder;
-    return thisB;
+    return (B) this;
   }
 
   /**
@@ -99,25 +101,29 @@ public B decoder(Decoder decoder) {
    *
    * @since 9.6
    */
+  @SuppressWarnings("unchecked")
   public B doNotCloseAfterDecode() {
     this.closeAfterDecode = false;
-    return thisB;
+    return (B) this;
   }
 
+  @SuppressWarnings("unchecked")
   public B decodeVoid() {
     this.decodeVoid = true;
-    return thisB;
+    return (B) this;
   }
 
+  @SuppressWarnings("unchecked")
   public B queryMapEncoder(QueryMapEncoder queryMapEncoder) {
     this.queryMapEncoder = queryMapEncoder;
-    return thisB;
+    return (B) this;
   }
 
   /** Allows to map the response before passing it to the decoder. */
+  @SuppressWarnings("unchecked")
   public B mapAndDecode(ResponseMapper mapper, Decoder decoder) {
     this.decoder = new ResponseMappingDecoder(mapper, decoder);
-    return thisB;
+    return (B) this;
   }
 
   /**
@@ -135,9 +141,10 @@ public B mapAndDecode(ResponseMapper mapper, Decoder decoder) {
    *
    * @since 11.9
    */
+  @SuppressWarnings("unchecked")
   public B dismiss404() {
     this.dismiss404 = true;
-    return thisB;
+    return (B) this;
   }
 
   /**
@@ -157,81 +164,91 @@ public B dismiss404() {
    * @deprecated use {@link #dismiss404()} instead.
    */
   @Deprecated
+  @SuppressWarnings("unchecked")
   public B decode404() {
     this.dismiss404 = true;
-    return thisB;
+    return (B) this;
   }
 
+  @SuppressWarnings("unchecked")
   public B errorDecoder(ErrorDecoder errorDecoder) {
     this.errorDecoder = errorDecoder;
-    return thisB;
+    return (B) this;
   }
 
+  @SuppressWarnings("unchecked")
   public B options(Options options) {
     this.options = options;
-    return thisB;
+    return (B) this;
   }
 
   /** Adds a single request interceptor to the builder. */
+  @SuppressWarnings("unchecked")
   public B requestInterceptor(RequestInterceptor requestInterceptor) {
     this.requestInterceptors.add(requestInterceptor);
-    return thisB;
+    return (B) this;
   }
 
   /**
    * Sets the full set of request interceptors for the builder, overwriting any previous
    * interceptors.
    */
+  @SuppressWarnings("unchecked")
   public B requestInterceptors(Iterable<RequestInterceptor> requestInterceptors) {
     this.requestInterceptors.clear();
     for (RequestInterceptor requestInterceptor : requestInterceptors) {
       this.requestInterceptors.add(requestInterceptor);
     }
-    return thisB;
+    return (B) this;
   }
 
   /**
    * Sets the full set of request interceptors for the builder, overwriting any previous
    * interceptors.
    */
+  @SuppressWarnings("unchecked")
   public B responseInterceptors(Iterable<ResponseInterceptor> responseInterceptors) {
     this.responseInterceptors.clear();
     for (ResponseInterceptor responseInterceptor : responseInterceptors) {
       this.responseInterceptors.add(responseInterceptor);
     }
-    return thisB;
+    return (B) this;
   }
 
   /** Adds a single response interceptor to the builder. */
+  @SuppressWarnings("unchecked")
   public B responseInterceptor(ResponseInterceptor responseInterceptor) {
     this.responseInterceptors.add(responseInterceptor);
-    return thisB;
+    return (B) this;
   }
 
   /** Allows you to override how reflective dispatch works inside of Feign. */
+  @SuppressWarnings("unchecked")
   public B invocationHandlerFactory(InvocationHandlerFactory invocationHandlerFactory) {
     this.invocationHandlerFactory = invocationHandlerFactory;
-    return thisB;
+    return (B) this;
   }
 
+  @SuppressWarnings("unchecked")
   public B exceptionPropagationPolicy(ExceptionPropagationPolicy propagationPolicy) {
     this.propagationPolicy = propagationPolicy;
-    return thisB;
+    return (B) this;
   }
 
+  @SuppressWarnings("unchecked")
   public B addCapability(Capability capability) {
     this.capabilities.add(capability);
-    return thisB;
+    return (B) this;
   }
 
   @SuppressWarnings("unchecked")
   B enrich() {
     if (capabilities.isEmpty()) {
-      return thisB;
+      return (B) this;
     }
 
     try {
-      B clone = (B) thisB.clone();
+      B clone = (B) this.clone();
 
       getFieldsToEnrich()
           .forEach(
@@ -274,8 +291,6 @@ List<Field> 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"))
         // skip primitive types
         .filter(field -> !field.getType().isPrimitive())
         // skip enumerations

core/src/test/java/feign/BaseBuilderTest.java

@@ -67,4 +67,27 @@ void checkEnrichTouchesAllBuilderFields()
             .responseInterceptor((ic, c) -> c.next(ic)),
         12);
   }
+
+  @Test
+  void checkCloneDontLooseInterceptors() {
+    Feign.Builder originalBuilder = Feign.builder().addCapability(new Capability() {});
+
+    // There is no any interceptors
+    assertThat(originalBuilder.requestInterceptors).isNotNull().isEmpty();
+
+    Feign.Builder enrichedBuilder = originalBuilder.enrich();
+    Feign.Builder enrichedBuilderWithInterceptor = enrichedBuilder.requestInterceptor((req) -> {});
+
+    // Original builder should have no interceptor
+    assertThat(originalBuilder.requestInterceptors).isNotNull().isEmpty();
+
+    // enrichedBuilder should have an interceptor
+    assertThat(enrichedBuilder.requestInterceptors).isNotNull().isNotEmpty().hasSize(1);
+
+    // enrichedBuilderWithInterceptor should have an interceptor
+    assertThat(enrichedBuilderWithInterceptor.requestInterceptors)
+        .isNotNull()
+        .isNotEmpty()
+        .hasSize(1);
+  }
 }

oauth2/src/test/resources/test.env

@@ -0,0 +1,14 @@
+AWS_DOMAIN=feign
+AWS_REGION=us-west-2
+AWS_CLIENT_ID=25fhico3bdqmssemu6ur5op98m
+AWS_CLIENT_SECRET=jpc3av0it1j32us3ijcf066u3cqg9euoo0pc920b6jqg9i0h1ki
+AWS_TENANT=us-west-2_WbMEZgSZt
+AWS_ISSUER_URI=https://cognito-idp.us-west-2.amazonaws.com/us-west-2_WbMEZgSZt
+AWS_JWK_SET_URI=https://cognito-idp.us-west-2.amazonaws.com/us-west-2_WbMEZgSZt/.well-known/jwks.json
+
+AUTH0_CLIENT_ID=oleVJYeNgas7mJrRzmg5LQNR4aTpxDln
+AUTH0_CLIENT_SECRET=bZxSqpuH4m41PYjD5STHsTzOSgAMRWS5alc6JPz3zZLBFxggwgvgfc3qkdFzjUEN
+AUTH0_TENANT=hosuaby
+AUTH0_AUDIENCE=https://hosuaby.auth0.com/api/v2/
+AUTH0_ISSUER_URI=https://hosuaby.auth0.com/
+AUTH0_JWK_SET_URI=https://hosuaby.auth0.com/.well-known/jwks.json