Alternative internal builder process

benchmark/pom.xml

@@ -27,10 +27,10 @@
   <name>Feign Benchmark (JMH)</name>
 
   <properties>
-    <jmh.version>1.36</jmh.version>
+    <jmh.version>1.37</jmh.version>
     <rx.netty.version>0.5.3</rx.netty.version>
     <rx.java.version>1.3.8</rx.java.version>
-    <netty.version>4.1.94.Final</netty.version>
+    <netty.version>4.1.96.Final</netty.version>
     <main.basedir>${project.basedir}/..</main.basedir>
 
     <moditect.skip>true</moditect.skip>

core/pom.xml

@@ -47,7 +47,7 @@
     <dependency>
       <groupId>org.springframework</groupId>
       <artifactId>spring-context</artifactId>
-      <version>6.0.10</version>
+      <version>6.0.11</version>
       <scope>test</scope>
     </dependency>
 

core/src/main/java/feign/AsyncFeign.java

@@ -65,7 +65,7 @@ private static class LazyInitializedExecutorService {
             });
   }
 
-  public static class AsyncBuilder<C> extends BaseBuilder<AsyncBuilder<C>> {
+  public static class AsyncBuilder<C> extends BaseBuilder<AsyncBuilder<C>, AsyncFeign<C>> {
 
     private AsyncContextSupplier<C> defaultContextSupplier = () -> null;
     private AsyncClient<C> client = new AsyncClient.Default<>(
@@ -119,6 +119,11 @@ public AsyncBuilder<C> doNotCloseAfterDecode() {
       return super.doNotCloseAfterDecode();
     }
 
+    @Override
+    public AsyncBuilder<C> decodeVoid() {
+      return super.decodeVoid();
+    }
+
     public AsyncBuilder<C> defaultContextSupplier(AsyncContextSupplier<C> supplier) {
       this.defaultContextSupplier = supplier;
       return this;
@@ -185,33 +190,30 @@ public AsyncBuilder<C> invocationHandlerFactory(InvocationHandlerFactory invocat
       return super.invocationHandlerFactory(invocationHandlerFactory);
     }
 
-    public AsyncFeign<C> build() {
-      AsyncBuilder<C> enrichedBuilder = super.enrich();
-
+    @Override
+    public AsyncFeign<C> internalBuild() {
       AsyncResponseHandler responseHandler =
           (AsyncResponseHandler) Capability.enrich(
               new AsyncResponseHandler(
-                  enrichedBuilder.logLevel,
-                  enrichedBuilder.logger,
-                  enrichedBuilder.decoder,
-                  enrichedBuilder.errorDecoder,
-                  enrichedBuilder.dismiss404,
-                  enrichedBuilder.closeAfterDecode, enrichedBuilder.responseInterceptor),
+                  logLevel,
+                  logger,
+                  decoder,
+                  errorDecoder,
+                  dismiss404,
+                  closeAfterDecode, decodeVoid, responseInterceptor),
               AsyncResponseHandler.class,
-              enrichedBuilder.capabilities);
+              capabilities);
 
       final MethodHandler.Factory<C> methodHandlerFactory =
           new AsynchronousMethodHandler.Factory<>(
-              enrichedBuilder.client, enrichedBuilder.retryer, enrichedBuilder.requestInterceptors,
-              responseHandler, enrichedBuilder.logger, enrichedBuilder.logLevel,
-              enrichedBuilder.propagationPolicy, enrichedBuilder.methodInfoResolver,
-              new RequestTemplateFactoryResolver(enrichedBuilder.encoder,
-                  enrichedBuilder.queryMapEncoder),
-              enrichedBuilder.options, enrichedBuilder.decoder, enrichedBuilder.errorDecoder);
+              client, retryer, requestInterceptors,
+              responseHandler, logger, logLevel,
+              propagationPolicy, methodInfoResolver,
+              new RequestTemplateFactoryResolver(encoder, queryMapEncoder),
+              options, decoder, errorDecoder);
       final ReflectiveFeign<C> feign =
-          new ReflectiveFeign<>(enrichedBuilder.contract, methodHandlerFactory,
-              enrichedBuilder.invocationHandlerFactory,
-              enrichedBuilder.defaultContextSupplier);
+          new ReflectiveFeign<>(contract, methodHandlerFactory, invocationHandlerFactory,
+              defaultContextSupplier);
       return new AsyncFeign<>(feign);
     }
   }

core/src/main/java/feign/AsyncResponseHandler.java

@@ -28,11 +28,11 @@ class AsyncResponseHandler {
   private final ResponseHandler responseHandler;
 
   AsyncResponseHandler(Level logLevel, Logger logger, Decoder decoder,
-      ErrorDecoder errorDecoder, boolean dismiss404, boolean closeAfterDecode,
+      ErrorDecoder errorDecoder, boolean dismiss404, boolean closeAfterDecode, boolean decodeVoid,
       ResponseInterceptor responseInterceptor) {
     this.responseHandler = new ResponseHandler(
         logLevel, logger, decoder,
-        errorDecoder, dismiss404, closeAfterDecode,
+        errorDecoder, dismiss404, closeAfterDecode, decodeVoid,
         responseInterceptor);
   }
 

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

@@ -29,7 +29,7 @@
 import java.util.Objects;
 import java.util.stream.Collectors;
 
-public abstract class BaseBuilder<B extends BaseBuilder<B>> implements Cloneable {
+public abstract class BaseBuilder<B extends BaseBuilder<B, T>, T> implements Cloneable {
 
   private final B thisB;
 
@@ -43,6 +43,7 @@ public abstract class BaseBuilder<B extends BaseBuilder<B>> implements Cloneable
   protected Encoder encoder = new Encoder.Default();
   protected Decoder decoder = new Decoder.Default();
   protected boolean closeAfterDecode = true;
+  protected boolean decodeVoid = false;
   protected QueryMapEncoder queryMapEncoder = new FieldQueryMapEncoder();
   protected ErrorDecoder errorDecoder = new ErrorDecoder.Default();
   protected Options options = new Options();
@@ -106,6 +107,11 @@ public B doNotCloseAfterDecode() {
     return thisB;
   }
 
+  public B decodeVoid() {
+    this.decodeVoid = true;
+    return thisB;
+  }
+
   public B queryMapEncoder(QueryMapEncoder queryMapEncoder) {
     this.queryMapEncoder = queryMapEncoder;
     return thisB;
@@ -225,7 +231,7 @@ public B addCapability(Capability capability) {
   }
 
   @SuppressWarnings("unchecked")
-  protected B enrich() {
+  B enrich() {
     if (capabilities.isEmpty()) {
       return thisB;
     }
@@ -277,5 +283,9 @@ List<Field> getFieldsToEnrich() {
         .collect(Collectors.toList());
   }
 
+  public final T build() {
+    return enrich().internalBuild();
+  }
 
+  protected abstract T internalBuild();
 }

core/src/main/java/feign/Client.java

@@ -14,6 +14,7 @@
 package feign;
 
 import static feign.Util.CONTENT_ENCODING;
+import static feign.Util.ACCEPT_ENCODING;
 import static feign.Util.CONTENT_LENGTH;
 import static feign.Util.ENCODING_DEFLATE;
 import static feign.Util.ENCODING_GZIP;
@@ -186,6 +187,11 @@ HttpURLConnection convertAndSend(Request request, Options options) throws IOExce
               contentLength = Integer.valueOf(value);
               connection.addRequestProperty(field, value);
             }
+          }
+          // Avoid add "Accept-encoding" twice or more when "compression" option is enabled
+          if (field.equals(ACCEPT_ENCODING)) {
+            connection.addRequestProperty(field, String.join(", ", request.headers().get(field)));
+            break;
           } else {
             connection.addRequestProperty(field, value);
           }

core/src/main/java/feign/Feign.java

@@ -13,12 +13,12 @@
  */
 package feign;
 
+import feign.InvocationHandlerFactory.MethodHandler;
 import feign.Request.Options;
 import feign.Target.HardCodedTarget;
 import feign.codec.Decoder;
 import feign.codec.Encoder;
 import feign.codec.ErrorDecoder;
-import feign.InvocationHandlerFactory.MethodHandler;
 import java.io.IOException;
 import java.lang.reflect.Method;
 import java.lang.reflect.Type;
@@ -92,7 +92,7 @@ public static String configKey(Method method) {
    */
   public abstract <T> T newInstance(Target<T> target);
 
-  public static class Builder extends BaseBuilder<Builder> {
+  public static class Builder extends BaseBuilder<Builder, Feign> {
 
     private Client client = new Client.Default(null, null);
 
@@ -178,6 +178,11 @@ public Builder doNotCloseAfterDecode() {
       return super.doNotCloseAfterDecode();
     }
 
+    @Override
+    public Builder decodeVoid() {
+      return super.decodeVoid();
+    }
+
     @Override
     public Builder exceptionPropagationPolicy(ExceptionPropagationPolicy propagationPolicy) {
       return super.exceptionPropagationPolicy(propagationPolicy);
@@ -196,24 +201,17 @@ public <T> T target(Target<T> target) {
       return build().newInstance(target);
     }
 
-    public Feign build() {
-      Builder enrichedBuilder = super.enrich();
-
+    @Override
+    public Feign internalBuild() {
       final ResponseHandler responseHandler =
-          new ResponseHandler(enrichedBuilder.logLevel, enrichedBuilder.logger,
-              enrichedBuilder.decoder, enrichedBuilder.errorDecoder,
-              enrichedBuilder.dismiss404, enrichedBuilder.closeAfterDecode,
-              enrichedBuilder.responseInterceptor);
+          new ResponseHandler(logLevel, logger, decoder, errorDecoder,
+              dismiss404, closeAfterDecode, decodeVoid, responseInterceptor);
       MethodHandler.Factory<Object> methodHandlerFactory =
-          new SynchronousMethodHandler.Factory(enrichedBuilder.client, enrichedBuilder.retryer,
-              enrichedBuilder.requestInterceptors,
-              responseHandler, enrichedBuilder.logger, enrichedBuilder.logLevel,
-              enrichedBuilder.propagationPolicy,
-              new RequestTemplateFactoryResolver(enrichedBuilder.encoder,
-                  enrichedBuilder.queryMapEncoder),
-              enrichedBuilder.options);
-      return new ReflectiveFeign<>(enrichedBuilder.contract, methodHandlerFactory,
-          enrichedBuilder.invocationHandlerFactory,
+          new SynchronousMethodHandler.Factory(client, retryer, requestInterceptors,
+              responseHandler, logger, logLevel, propagationPolicy,
+              new RequestTemplateFactoryResolver(encoder, queryMapEncoder),
+              options);
+      return new ReflectiveFeign<>(contract, methodHandlerFactory, invocationHandlerFactory,
           () -> null);
     }
   }

core/src/main/java/feign/ResponseHandler.java

@@ -37,10 +37,12 @@ public class ResponseHandler {
   private final boolean dismiss404;
   private final boolean closeAfterDecode;
 
+  private final boolean decodeVoid;
+
   private final ResponseInterceptor responseInterceptor;
 
   public ResponseHandler(Level logLevel, Logger logger, Decoder decoder,
-      ErrorDecoder errorDecoder, boolean dismiss404, boolean closeAfterDecode,
+      ErrorDecoder errorDecoder, boolean dismiss404, boolean closeAfterDecode, boolean decodeVoid,
       ResponseInterceptor responseInterceptor) {
     super();
     this.logLevel = logLevel;
@@ -50,6 +52,7 @@ public ResponseHandler(Level logLevel, Logger logger, Decoder decoder,
     this.dismiss404 = dismiss404;
     this.closeAfterDecode = closeAfterDecode;
     this.responseInterceptor = responseInterceptor;
+    this.decodeVoid = decodeVoid;
   }
 
   public Object handleResponse(String configKey,
@@ -113,7 +116,7 @@ private static Response disconnectResponseBodyIfNeeded(Response response) throws
   }
 
   private Object decode(Response response, Type type) throws IOException {
-    if (isVoidType(type)) {
+    if (isVoidType(type) && !decodeVoid) {
       ensureClosed(response.body());
       return null;
     }

core/src/main/java/feign/RetryableException.java

@@ -14,7 +14,9 @@
 package feign;
 
 import feign.Request.HttpMethod;
+import java.util.Collection;
 import java.util.Date;
+import java.util.Map;
 
 /**
  * This exception is raised when the {@link Response} is deemed to be retryable, typically via an
@@ -47,6 +49,16 @@ public RetryableException(int status, String message, HttpMethod httpMethod, Dat
     this.retryAfter = retryAfter != null ? retryAfter.getTime() : null;
   }
 
+  /**
+   * @param retryAfter usually corresponds to the {@link feign.Util#RETRY_AFTER} header.
+   */
+  public RetryableException(int status, String message, HttpMethod httpMethod, Date retryAfter,
+      Request request, byte[] responseBody, Map<String, Collection<String>> responseHeaders) {
+    super(status, message, request, responseBody, responseHeaders);
+    this.httpMethod = httpMethod;
+    this.retryAfter = retryAfter != null ? retryAfter.getTime() : null;
+  }
+
   /**
    * Sometimes corresponds to the {@link feign.Util#RETRY_AFTER} header present in {@code 503}
    * status. Other times parsed from an application-specific response. Null if unknown.

core/src/main/java/feign/Util.java

@@ -64,6 +64,10 @@ public class Util {
    * The HTTP Content-Encoding header field name.
    */
   public static final String CONTENT_ENCODING = "Content-Encoding";
+  /**
+   * The HTTP Accept-Encoding header field name.
+   */
+  public static final String ACCEPT_ENCODING = "Accept-Encoding";
   /**
    * The HTTP Retry-After header field name.
    */

core/src/main/java/feign/template/Expressions.java

@@ -22,6 +22,8 @@
 
 public final class Expressions {
 
+  private static final int MAX_EXPRESSION_LENGTH = 10000;
+
   private static final String PATH_STYLE_OPERATOR = ";";
   /**
    * Literals may be present and preceded the expression.
@@ -38,10 +40,10 @@ public final class Expressions {
    *
    * This is not a complete implementation of the rfc
    *
-   * <a href="https://www.rfc-editor.org/rfc/rfc6570#section-2.2>RFC 6570 Expressions</a>
+   * <a href="https://www.rfc-editor.org/rfc/rfc6570#section-2.2">RFC 6570 Expressions</a>
    */
-  private static final Pattern EXPRESSION_PATTERN =
-      Pattern.compile("^(\\{([+#./;?&=,!@|]?)(.+)})$");
+  static final Pattern EXPRESSION_PATTERN =
+      Pattern.compile("^(\\{([+#./;?&=,!@|]?)(.+)\\})$");
 
   // Partially From:
   // https://stackoverflow.com/questions/29494608/regex-for-uri-templates-rfc-6570-wanted -- I
@@ -68,6 +70,12 @@ public static Expression create(final String value) {
       throw new IllegalArgumentException("an expression is required.");
     }
 
+    /* Check if the expression is too long */
+    if (expression.length() > MAX_EXPRESSION_LENGTH) {
+      throw new IllegalArgumentException(
+          "expression is too long. Max length: " + MAX_EXPRESSION_LENGTH);
+    }
+
     /* create a new regular expression matcher for the expression */
     String variableName = null;
     String variablePattern = null;
@@ -80,8 +88,8 @@ public static Expression create(final String value) {
       /* we have a valid variable expression, extract the name from the first group */
       variableName = matcher.group(3).trim();
       if (variableName.contains(":")) {
-        /* split on the colon */
-        String[] parts = variableName.split(":");
+        /* split on the colon and ensure the size of parts array must be 2 */
+        String[] parts = variableName.split(":", 2);
         variableName = parts[0];
         variablePattern = parts[1];
       }

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

@@ -31,10 +31,10 @@ public void checkEnrichTouchesAllAsyncBuilderFields()
     }), 14);
   }
 
-  private void test(BaseBuilder<?> builder, int expectedFieldsCount)
+  private void test(BaseBuilder<?, ?> builder, int expectedFieldsCount)
       throws IllegalArgumentException, IllegalAccessException {
     Capability mockingCapability = Mockito.mock(Capability.class, RETURNS_MOCKS);
-    BaseBuilder<?> enriched = builder.addCapability(mockingCapability).enrich();
+    BaseBuilder<?, ?> enriched = builder.addCapability(mockingCapability).enrich();
 
     List<Field> fields = enriched.getFieldsToEnrich();
     assertThat(fields).hasSize(expectedFieldsCount);