Merge branch 'master' into fix-http2-response-timeout

Author: goutamadwant

CHANGELOG.md

@@ -1,3 +1,9 @@
+### Version 13.14
+
+* Add support for the HTTP QUERY method (RFC 10008) — safe, idempotent, and cacheable with a
+  request body. `HttpCacheInterceptor` includes QUERY in its default cacheable set and
+  incorporates a body hash into the cache key to reduce cross-body collisions.
+
 ### Version 13.12
 
 * `UrlencodedFormContentProcessor` now honors `CollectionFormat` from `@RequestLine`/`RequestTemplate` for array and

benchmark/pom.xml

@@ -32,7 +32,7 @@
     <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.2.15.Final</netty.version>
+    <netty.version>4.2.16.Final</netty.version>
     <moditect.skip>true</moditect.skip>
   </properties>
 

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

@@ -40,8 +40,6 @@
 
 public abstract class BaseBuilder<B extends BaseBuilder<B, T>, T> implements Cloneable {
 
-  private final B thisB;
-
   protected List<RequestInterceptor> requestInterceptors = new ArrayList<>();
   protected List<ResponseInterceptor> responseInterceptors = new ArrayList<>();
   protected List<MethodInterceptor> methodInterceptors = new ArrayList<>();
@@ -64,43 +62,47 @@ public abstract class BaseBuilder<B extends BaseBuilder<B, T>, 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<RequestInterceptor> requestInterceptors) {
     for (RequestInterceptor requestInterceptor : requestInterceptors) {
       this.requestInterceptors.add(requestInterceptor);
     }
-    return thisB;
+    return thisB();
   }
 
   /**
@@ -213,13 +215,13 @@ public B responseInterceptors(Iterable<ResponseInterceptor> 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<MethodInterceptor> 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(
@@ -316,10 +318,21 @@ B enrich() {
       // enrich each response interceptor, then enrich the list as a whole
       ResponseInterceptor[] responseArray =
           clone.responseInterceptors.toArray(new ResponseInterceptor[0]);
-      for (int i = 0; i < responseArray.length; i++) {
-        responseArray[i] =
+      if (responseArray.length == 0) {
+        ResponseInterceptor defaultResponseInterceptor = (context, chain) -> chain.next(context);
+        ResponseInterceptor enrichedResponseInterceptor =
             (ResponseInterceptor)
-                Capability.enrich(responseArray[i], ResponseInterceptor.class, capabilities);
+                Capability.enrich(
+                    defaultResponseInterceptor, ResponseInterceptor.class, capabilities);
+        if (enrichedResponseInterceptor != defaultResponseInterceptor) {
+          responseArray = new ResponseInterceptor[] {enrichedResponseInterceptor};
+        }
+      } else {
+        for (int i = 0; i < responseArray.length; i++) {
+          responseArray[i] =
+              (ResponseInterceptor)
+                  Capability.enrich(responseArray[i], ResponseInterceptor.class, capabilities);
+        }
       }
       ResponseInterceptors responseInterceptors =
           (ResponseInterceptors)
@@ -356,8 +369,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"))
         // 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"))

core/src/main/java/feign/DefaultClient.java

@@ -111,7 +111,9 @@ Response convertResponse(HttpURLConnection connection, Request request) throws I
     }
 
     Integer length = connection.getContentLength();
-    if (length == -1) {
+    if (length < 0) {
+      // -1 signals unknown or above Integer.MAX_VALUE; any other negative value is a malformed
+      // header that HttpURLConnection surfaces verbatim
       length = null;
     }
     InputStream stream;
@@ -122,8 +124,11 @@ Response convertResponse(HttpURLConnection connection, Request request) throws I
     }
     if (stream != null && this.isGzip(headers.get(CONTENT_ENCODING))) {
       stream = new GZIPInputStream(stream);
+      // the body is now decompressed, the Content-Length described the compressed bytes
+      length = null;
     } else if (stream != null && this.isDeflate(headers.get(CONTENT_ENCODING))) {
       stream = new InflaterInputStream(stream);
+      length = null;
     }
     return Response.builder()
         .status(status)
@@ -170,7 +175,6 @@ HttpURLConnection convertAndSend(Request request, Options options) throws IOExce
         if (field.equals(CONTENT_LENGTH)) {
           if (!gzipEncodedRequest && !deflateEncodedRequest) {
             contentLength = Integer.valueOf(value);
-            connection.addRequestProperty(field, value);
           }
         }
         // Avoid add "Accept-encoding" twice or more when "compression" option is enabled
@@ -189,7 +193,7 @@ else if (field.equals(ACCEPT_ENCODING)) {
 
     byte[] body = request.body();
 
-    if (body != null) {
+    if (body != null && (body.length > 0 || request.httpMethod() != Request.HttpMethod.GET)) {
       /*
        * Ignore disableRequestBuffering flag if the empty body was set, to ensure that internal
        * retry logic applies to such requests.

core/src/main/java/feign/Logger.java

@@ -21,10 +21,14 @@
 import static feign.Util.valuesOrEmpty;
 import static java.util.Objects.nonNull;
 
+import java.io.Closeable;
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.io.StringWriter;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.logging.FileHandler;
+import java.util.logging.Handler;
 import java.util.logging.LogRecord;
 import java.util.logging.SimpleFormatter;
 
@@ -182,9 +186,10 @@ protected void log(String configKey, String format, Object... args) {
   }
 
   /** Logs to the category {@link Logger} at {@link java.util.logging.Level#FINE}, if loggable. */
-  public static class JavaLogger extends Logger {
+  public static class JavaLogger extends Logger implements Closeable {
 
     final java.util.logging.Logger logger;
+    private final List<Handler> handlers = new ArrayList<>();
 
     /**
      * @deprecated Use {@link #JavaLogger(String)} or {@link #JavaLogger(Class)} instead.
@@ -256,11 +261,21 @@ public String format(LogRecord record) {
               }
             });
         logger.addHandler(handler);
+        handlers.add(handler);
       } catch (IOException e) {
         throw new IllegalStateException("Could not add file handler.", e);
       }
       return this;
     }
+
+    @Override
+    public void close() {
+      for (Handler handler : handlers) {
+        logger.removeHandler(handler);
+        handler.close();
+      }
+      handlers.clear();
+    }
   }
 
   public static class NoOpLogger extends Logger {

core/src/main/java/feign/Request.java

@@ -44,7 +44,8 @@ public enum HttpMethod {
     CONNECT,
     OPTIONS,
     TRACE,
-    PATCH(true);
+    PATCH(true),
+    QUERY(true);
 
     private final boolean withBody;
 
@@ -554,7 +555,7 @@ public boolean isBinary() {
     }
 
     public static Body create(String data) {
-      return new Body(data.getBytes());
+      return create(data, Util.UTF_8);
     }
 
     public static Body create(String data, Charset charset) {

core/src/main/java/feign/RequestLine.java

@@ -35,9 +35,10 @@
    *
    * <p>The string must begin with a valid {@linkplain feign.Request.HttpMethod HTTP method name}
    * (e.g. {@linkplain feign.Request.HttpMethod#GET GET}, {@linkplain feign.Request.HttpMethod#POST
-   * POST}, {@linkplain feign.Request.HttpMethod#PUT PUT}), followed by a space and a URI template.
-   * If only the HTTP method is specified (e.g. {@code "DELETE"}), the request will use the base URL
-   * defined for the client.
+   * POST}, {@linkplain feign.Request.HttpMethod#PUT PUT}, {@linkplain
+   * feign.Request.HttpMethod#QUERY QUERY}), followed by a space and a URI template. If only the
+   * HTTP method is specified (e.g. {@code "DELETE"}), the request will use the base URL defined for
+   * the client.
    *
    * <p>Example:
    *

core/src/main/java/feign/codec/StringDecoder.java

@@ -31,7 +31,7 @@ public Object decode(Response response, Type type) throws IOException {
       return null;
     }
     if (String.class.equals(type)) {
-      return Util.toString(body.asReader(Util.UTF_8));
+      return Util.toString(body.asReader(response.charset()));
     }
     throw new DecodeException(
         response.status(),