Feature/decoder interceptor to response interceptor (#2116)

* Refactor so that ResponseInterceptor intercepts the response (in the same manner that RequestInterceptor does) rather than intercepting the decoding process.

Signed-off-by: Iain Henderson <Iain.henderson@mac.com>

* Add a default RedirectionInterceptor as an implementation of ResponseInterceptor and include unit tests for redirection interception, error interception, and void decoding in FeignTest.

* Update README to include ResponseInterceptor

* Add copyright notice to RedirectionInterceptor

* Correct formatting using maven

* Updates in response to CodeRabbit

* more CodeRabbitAI suggestions

* Add unit tests for chained ResponseInterceptor instances

* fixing formatting

* formatting and responding to CodeRabbitAI comment

* Reverting Feign-core pom

* Cleanup Javadocs in ResponseInterceptor and RedirectionInterceptor

---------

Signed-off-by: Iain Henderson <Iain.henderson@mac.com>
Co-authored-by: Marvin Froeder <velo@users.noreply.github.com>

README.md

@@ -1066,6 +1066,28 @@ created for each `Client` execution, allowing you to maintain state bewteen each
 If the retry is determined to be unsuccessful, the last `RetryException` will be thrown.  To throw the original
 cause that led to the unsuccessful retry, build your Feign client with the `exceptionPropagationPolicy()` option.
 
+#### Response Interceptor
+If you need to treat what would otherwise be an error as a success and return a result rather than throw an exception then you may use a `ResponseInterceptor`.
+
+As an example Feign includes a simple `RedirectionInterceptor` that can be used to extract the location header from redirection responses.
+```java
+public interface Api {
+  // returns a 302 response
+  @RequestLine("GET /location")
+  String location();
+}
+
+public class MyApp {
+  public static void main(String[] args) {
+    // Configure the HTTP client to ignore redirection
+    Api api = Feign.builder()
+                   .options(new Options(10, TimeUnit.SECONDS, 60, TimeUnit.SECONDS, false))
+                   .responseInterceptor(new RedirectionInterceptor())
+                   .target(Api.class, "https://redirect.example.com");
+  }
+}
+```
+
 ### Metrics
 By default, feign won't collect any metrics.
 

core/src/main/java/feign/InvocationContext.java

@@ -0,0 +1,125 @@
+/*
+ * Copyright 2012-2023 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 static feign.FeignException.errorReading;
+import static feign.Util.ensureClosed;
+import feign.codec.DecodeException;
+import feign.codec.Decoder;
+import feign.codec.ErrorDecoder;
+import java.io.IOException;
+import java.lang.reflect.Type;
+
+public class InvocationContext {
+  private static final long MAX_RESPONSE_BUFFER_SIZE = 8192L;
+  private final String configKey;
+  private final Decoder decoder;
+  private final ErrorDecoder errorDecoder;
+  private final boolean dismiss404;
+  private final boolean closeAfterDecode;
+  private final boolean decodeVoid;
+  private final Response response;
+  private final Type returnType;
+
+  InvocationContext(String configKey, Decoder decoder, ErrorDecoder errorDecoder,
+      boolean dismiss404, boolean closeAfterDecode, boolean decodeVoid, Response response,
+      Type returnType) {
+    this.configKey = configKey;
+    this.decoder = decoder;
+    this.errorDecoder = errorDecoder;
+    this.dismiss404 = dismiss404;
+    this.closeAfterDecode = closeAfterDecode;
+    this.decodeVoid = decodeVoid;
+    this.response = response;
+    this.returnType = returnType;
+  }
+
+  public Decoder decoder() {
+    return decoder;
+  }
+
+  public Type returnType() {
+    return returnType;
+  }
+
+  public Response response() {
+    return response;
+  }
+
+  public Object proceed() throws Exception {
+    if (returnType == Response.class) {
+      return disconnectResponseBodyIfNeeded(response);
+    }
+
+    try {
+      final boolean shouldDecodeResponseBody =
+          (response.status() >= 200 && response.status() < 300)
+              || (response.status() == 404 && dismiss404
+                  && !isVoidType(returnType));
+
+      if (!shouldDecodeResponseBody) {
+        throw decodeError(configKey, response);
+      }
+
+      if (isVoidType(returnType) && !decodeVoid) {
+        ensureClosed(response.body());
+        return null;
+      }
+
+      try {
+        return decoder.decode(response, returnType);
+      } catch (final FeignException e) {
+        throw e;
+      } catch (final RuntimeException e) {
+        throw new DecodeException(response.status(), e.getMessage(), response.request(), e);
+      } catch (IOException e) {
+        throw errorReading(response.request(), response, e);
+      }
+    } finally {
+      if (closeAfterDecode) {
+        ensureClosed(response.body());
+      }
+    }
+  }
+
+  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 Exception decodeError(String methodKey, Response response) {
+    try {
+      return errorDecoder.decode(methodKey, response);
+    } finally {
+      ensureClosed(response.body());
+    }
+  }
+
+  private boolean isVoidType(Type returnType) {
+    return returnType == Void.class
+        || returnType == void.class
+        || returnType.getTypeName().equals("kotlin.Unit");
+  }
+}

core/src/main/java/feign/RedirectionInterceptor.java

@@ -0,0 +1,54 @@
+/*
+ * Copyright 2012-2023 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 java.lang.reflect.ParameterizedType;
+import java.lang.reflect.Type;
+import java.util.Collection;
+
+/**
+ * An implementation of {@link ResponseInterceptor} the returns the value of the location header
+ * when appropriate.
+ */
+public class RedirectionInterceptor implements ResponseInterceptor {
+  @Override
+  public Object intercept(InvocationContext invocationContext, Chain chain) throws Exception {
+    Response response = invocationContext.response();
+    int status = response.status();
+    Object returnValue = null;
+    if (300 <= status && status < 400 && response.headers().containsKey("Location")) {
+      Type returnType = rawType(invocationContext.returnType());
+      Collection<String> locations = response.headers().get("Location");
+      if (Collection.class.equals(returnType)) {
+        returnValue = locations;
+      } else if (String.class.equals(returnType)) {
+        if (locations.isEmpty()) {
+          returnValue = "";
+        } else {
+          returnValue = locations.stream().findFirst().orElse("");
+        }
+      }
+    }
+    if (returnValue == null) {
+      return chain.next(invocationContext);
+    } else {
+      response.close();
+      return returnValue;
+    }
+  }
+
+  private Type rawType(Type type) {
+    return type instanceof ParameterizedType ? ((ParameterizedType) type).getRawType() : type;
+  }
+}

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

@@ -50,6 +50,7 @@ public boolean isWithBody() {
   }
 
   public enum ProtocolVersion {
+
     HTTP_1_0("HTTP/1.0"), HTTP_1_1("HTTP/1.1"), HTTP_2("HTTP/2.0"), MOCK;
 
     final String protocolVersion;
@@ -66,6 +67,7 @@ public enum ProtocolVersion {
     public String toString() {
       return protocolVersion;
     }
+
   }
 
   /**

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

@@ -27,8 +27,6 @@
  */
 public class ResponseHandler {
 
-  private static final long MAX_RESPONSE_BUFFER_SIZE = 8192L;
-
   private final Level logLevel;
   private final Logger logger;
 
@@ -62,32 +60,20 @@ public Object handleResponse(String configKey,
       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);
+      return executionChain.next(
+          new InvocationContext(configKey, decoder, errorDecoder, dismiss404, closeAfterDecode,
+              decodeVoid, response, returnType));
     } catch (final IOException e) {
       if (logLevel != Level.NONE) {
         logger.logIOException(configKey, logLevel, e, elapsedTime);
       }
       throw errorReading(response.request(), response, e);
+    } catch (Exception e) {
+      ensureClosed(response.body());
+      throw 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)
@@ -98,47 +84,4 @@ private Response logAndRebufferResponseIfNeeded(String configKey,
 
     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) && !decodeVoid) {
-      ensureClosed(response.body());
-      return null;
-    }
-
-    try {
-      final Object result = executionChain.next(
-          new ResponseInterceptor.Context(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());
-    }
-  }
 }