Eliminates unnecessary overhead

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

@@ -13,12 +13,9 @@
  */
 package feign;
 
-import static feign.FeignException.errorReading;
-import static feign.Util.ensureClosed;
 import feign.Logger.Level;
 import feign.codec.Decoder;
 import feign.codec.ErrorDecoder;
-import java.io.IOException;
 import java.lang.reflect.Type;
 import java.util.concurrent.CompletableFuture;
 
@@ -28,93 +25,40 @@
  */
 @Experimental
 class AsyncResponseHandler {
-
-  private static final long MAX_RESPONSE_BUFFER_SIZE = 8192L;
-
-  private final Level logLevel;
-  private final Logger logger;
-
-  private final Decoder decoder;
-  private final ErrorDecoder errorDecoder;
-  private final boolean dismiss404;
-  private final boolean closeAfterDecode;
-
-  private final ResponseInterceptor responseInterceptor;
+  private final ResponseHandler responseHandler;
 
   AsyncResponseHandler(Level logLevel, Logger logger, Decoder decoder,
       ErrorDecoder errorDecoder, boolean dismiss404, boolean closeAfterDecode,
       ResponseInterceptor responseInterceptor) {
-    super();
-    this.logLevel = logLevel;
-    this.logger = logger;
-    this.decoder = decoder;
-    this.errorDecoder = errorDecoder;
-    this.dismiss404 = dismiss404;
-    this.closeAfterDecode = closeAfterDecode;
-    this.responseInterceptor = responseInterceptor;
+    this.responseHandler = new ResponseHandler(
+        logLevel, logger, decoder,
+        errorDecoder, dismiss404, closeAfterDecode,
+        responseInterceptor);
   }
 
-  boolean isVoidType(Type returnType) {
-    return Void.class == returnType || void.class == returnType
-        || returnType.getTypeName().equals("kotlin.Unit");
+  public CompletableFuture<Object> handleResponse(String configKey,
+                                                  Response response,
+                                                  Type returnType,
+                                                  long elapsedTime) {
+    CompletableFuture<Object> resultFuture = new CompletableFuture<>();
+    handleResponse(resultFuture, configKey, response, returnType, elapsedTime);
+    return resultFuture;
   }
 
+  /**
+   * @deprecated use {@link #handleResponse(String, Response, Type, long)} instead.
+   */
+  @Deprecated()
   void handleResponse(CompletableFuture<Object> resultFuture,
                       String configKey,
                       Response response,
                       Type returnType,
                       long elapsedTime) {
-    // copied fairly liberally from SynchronousMethodHandler
-    boolean shouldClose = true;
-
     try {
-      if (logLevel != Level.NONE) {
-        response = logger.logAndRebufferResponse(configKey, logLevel, response,
-            elapsedTime);
-      }
-      if (Response.class == returnType) {
-        if (response.body() == null) {
-          resultFuture.complete(response);
-        } else if (response.body().length() == null
-            || response.body().length() > MAX_RESPONSE_BUFFER_SIZE) {
-          shouldClose = false;
-          resultFuture.complete(response);
-        } else {
-          // Ensure the response body is disconnected
-          final byte[] bodyData = Util.toByteArray(response.body().asInputStream());
-          resultFuture.complete(response.toBuilder().body(bodyData).build());
-        }
-      } else if (response.status() >= 200 && response.status() < 300) {
-        if (isVoidType(returnType)) {
-          resultFuture.complete(null);
-        } else {
-          final Object result = decode(response, returnType);
-          shouldClose = closeAfterDecode;
-          resultFuture.complete(result);
-        }
-      } else if (dismiss404 && response.status() == 404 && !isVoidType(returnType)) {
-        final Object result = decode(response, returnType);
-        shouldClose = closeAfterDecode;
-        resultFuture.complete(result);
-      } else {
-        resultFuture.completeExceptionally(errorDecoder.decode(configKey, response));
-      }
-    } catch (final IOException e) {
-      if (logLevel != Level.NONE) {
-        logger.logIOException(configKey, logLevel, e, elapsedTime);
-      }
-      resultFuture.completeExceptionally(errorReading(response.request(), response, e));
-    } catch (final Exception e) {
+      resultFuture.complete(
+          this.responseHandler.handleResponse(configKey, response, returnType, elapsedTime));
+    } catch (Exception e) {
       resultFuture.completeExceptionally(e);
-    } finally {
-      if (shouldClose) {
-        ensureClosed(response.body());
-      }
     }
-
-  }
-
-  Object decode(Response response, Type type) throws IOException {
-    return responseInterceptor.aroundDecode(new InvocationContext(decoder, type, response));
   }
 }

core/src/main/java/feign/AsynchronousMethodHandler.java

@@ -188,16 +188,8 @@ private static Response ensureRequestIsSet(Response response,
   }
 
   private CompletableFuture<Object> handleResponse(Response response, long elapsedTime) {
-    CompletableFuture<Object> resultFuture = new CompletableFuture<>();
-
-    asyncResponseHandler.handleResponse(resultFuture, metadata.configKey(), response,
-        methodInfo.underlyingReturnType(), elapsedTime);
-
-    if (!resultFuture.isDone()) {
-      resultFuture.completeExceptionally(new IllegalStateException("Response handling not done"));
-    }
-
-    return resultFuture;
+    return asyncResponseHandler.handleResponse(
+        metadata.configKey(), response, methodInfo.underlyingReturnType(), elapsedTime);
   }
 
   private long elapsedTime(long start) {

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

@@ -0,0 +1,141 @@
+/*
+ * Copyright 2012-2022 The Feign Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
+ * in compliance with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package feign;
+
+import feign.Logger.Level;
+import feign.codec.Decoder;
+import feign.codec.ErrorDecoder;
+import java.io.IOException;
+import java.lang.reflect.Type;
+import static feign.FeignException.errorReading;
+import static feign.Util.ensureClosed;
+
+/**
+ * The response handler that is used to provide synchronous support on top of standard response
+ * handling
+ */
+public class ResponseHandler {
+
+  private static final long MAX_RESPONSE_BUFFER_SIZE = 8192L;
+
+  private final Level logLevel;
+  private final Logger logger;
+
+  private final Decoder decoder;
+  private final ErrorDecoder errorDecoder;
+  private final boolean dismiss404;
+  private final boolean closeAfterDecode;
+
+  private final ResponseInterceptor responseInterceptor;
+
+  public ResponseHandler(Level logLevel, Logger logger, Decoder decoder,
+      ErrorDecoder errorDecoder, boolean dismiss404, boolean closeAfterDecode,
+      ResponseInterceptor responseInterceptor) {
+    super();
+    this.logLevel = logLevel;
+    this.logger = logger;
+    this.decoder = decoder;
+    this.errorDecoder = errorDecoder;
+    this.dismiss404 = dismiss404;
+    this.closeAfterDecode = closeAfterDecode;
+    this.responseInterceptor = responseInterceptor;
+  }
+
+  public Object handleResponse(String configKey,
+                               Response response,
+                               Type returnType,
+                               long elapsedTime)
+      throws Exception {
+    try {
+      response = logAndRebufferResponseIfNeeded(configKey, response, elapsedTime);
+      if (returnType == Response.class) {
+        return disconnectResponseBodyIfNeeded(response);
+      }
+
+      final boolean shouldDecodeResponseBody = (response.status() >= 200 && response.status() < 300)
+          || (response.status() == 404 && dismiss404 && !isVoidType(returnType));
+
+      if (!shouldDecodeResponseBody) {
+        throw decodeError(configKey, response);
+      }
+
+      return decode(response, returnType);
+    } catch (final IOException e) {
+      if (logLevel != Level.NONE) {
+        logger.logIOException(configKey, logLevel, e, elapsedTime);
+      }
+      throw errorReading(response.request(), response, e);
+    }
+  }
+
+  private boolean isVoidType(Type returnType) {
+    return returnType == Void.class
+        || returnType == void.class
+        || returnType.getTypeName().equals("kotlin.Unit");
+  }
+
+  private Response logAndRebufferResponseIfNeeded(String configKey,
+                                                  Response response,
+                                                  long elapsedTime)
+      throws IOException {
+    if (logLevel == Level.NONE) {
+      return response;
+    }
+
+    return logger.logAndRebufferResponse(configKey, logLevel, response, elapsedTime);
+  }
+
+  private static Response disconnectResponseBodyIfNeeded(Response response) throws IOException {
+    final boolean shouldDisconnectResponseBody = response.body() != null
+        && response.body().length() != null
+        && response.body().length() <= MAX_RESPONSE_BUFFER_SIZE;
+    if (!shouldDisconnectResponseBody) {
+      return response;
+    }
+
+    try {
+      final byte[] bodyData = Util.toByteArray(response.body().asInputStream());
+      return response.toBuilder().body(bodyData).build();
+    } finally {
+      ensureClosed(response.body());
+    }
+  }
+
+  private Object decode(Response response, Type type) throws IOException {
+    if (isVoidType(type)) {
+      ensureClosed(response.body());
+      return null;
+    }
+
+    try {
+      final Object result = responseInterceptor.aroundDecode(
+          new InvocationContext(decoder, type, response));
+      if (closeAfterDecode) {
+        ensureClosed(response.body());
+      }
+      return result;
+    } catch (Exception e) {
+      ensureClosed(response.body());
+      throw e;
+    }
+  }
+
+  private Exception decodeError(String methodKey, Response response) {
+    try {
+      return errorDecoder.decode(methodKey, response);
+    } finally {
+      ensureClosed(response.body());
+    }
+  }
+}

core/src/main/java/feign/SynchronousMethodHandler.java

@@ -22,27 +22,22 @@
 import feign.codec.ErrorDecoder;
 import java.io.IOException;
 import java.util.List;
-import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.CompletionException;
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Stream;
 
 final class SynchronousMethodHandler implements MethodHandler {
 
-  private static final long MAX_RESPONSE_BUFFER_SIZE = 8192L;
-
   private final MethodMetadata metadata;
   private final Target<?> target;
   private final Client client;
   private final Retryer retryer;
   private final List<RequestInterceptor> requestInterceptors;
-  private final ResponseInterceptor responseInterceptor;
   private final Logger logger;
   private final Logger.Level logLevel;
   private final RequestTemplate.Factory buildTemplateFromArgs;
   private final Options options;
   private final ExceptionPropagationPolicy propagationPolicy;
-  private final AsyncResponseHandler asyncResponseHandler;
+  private final ResponseHandler responseHandler;
 
 
   private SynchronousMethodHandler(Target<?> target, Client client, Retryer retryer,
@@ -63,8 +58,7 @@ private SynchronousMethodHandler(Target<?> target, Client client, Retryer retrye
     this.buildTemplateFromArgs = checkNotNull(buildTemplateFromArgs, "metadata for %s", target);
     this.options = checkNotNull(options, "options for %s", target);
     this.propagationPolicy = propagationPolicy;
-    this.responseInterceptor = responseInterceptor;
-    this.asyncResponseHandler = new AsyncResponseHandler(logLevel, logger, decoder, errorDecoder,
+    this.responseHandler = new ResponseHandler(logLevel, logger, decoder, errorDecoder,
         dismiss404, closeAfterDecode, responseInterceptor);
   }
 
@@ -117,22 +111,10 @@ Object executeAndDecode(RequestTemplate template, Options options) throws Throwa
       }
       throw errorExecuting(request, e);
     }
-    long elapsedTime = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);
-
-    CompletableFuture<Object> resultFuture = new CompletableFuture<>();
-    asyncResponseHandler.handleResponse(resultFuture, metadata.configKey(), response,
-        metadata.returnType(), elapsedTime);
 
-    try {
-      if (!resultFuture.isDone())
-        throw new IllegalStateException("Response handling not done");
-      return resultFuture.join();
-    } catch (CompletionException e) {
-      Throwable cause = e.getCause();
-      if (cause != null)
-        throw cause;
-      throw e;
-    }
+    long elapsedTime = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);
+    return responseHandler.handleResponse(
+        metadata.configKey(), response, metadata.returnType(), elapsedTime);
   }
 
   long elapsedTime(long start) {