diff --git a/README.md b/README.md index 14045f0296..74c1fb2807 100644 --- a/README.md +++ b/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. diff --git a/core/src/main/java/feign/InvocationContext.java b/core/src/main/java/feign/InvocationContext.java new file mode 100755 index 0000000000..b6ec9de496 --- /dev/null +++ b/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"); + } +} diff --git a/core/src/main/java/feign/RedirectionInterceptor.java b/core/src/main/java/feign/RedirectionInterceptor.java new file mode 100755 index 0000000000..2451abe1df --- /dev/null +++ b/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 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; + } +} diff --git a/core/src/main/java/feign/Request.java b/core/src/main/java/feign/Request.java index d8d0478405..b236caa134 100644 --- a/core/src/main/java/feign/Request.java +++ b/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; } + } /** diff --git a/core/src/main/java/feign/ResponseHandler.java b/core/src/main/java/feign/ResponseHandler.java old mode 100644 new mode 100755 index a9e0c0903b..b9e1d5876d --- a/core/src/main/java/feign/ResponseHandler.java +++ b/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()); - } - } } diff --git a/core/src/main/java/feign/ResponseInterceptor.java b/core/src/main/java/feign/ResponseInterceptor.java old mode 100644 new mode 100755 index dc9496421a..cc9377d751 --- a/core/src/main/java/feign/ResponseInterceptor.java +++ b/core/src/main/java/feign/ResponseInterceptor.java @@ -13,28 +13,24 @@ */ package feign; -import static feign.FeignException.errorReading; -import feign.codec.DecodeException; -import feign.codec.Decoder; -import java.io.IOException; -import java.lang.reflect.Type; -import java.util.function.Function; - /** - * Interceptor for purposes such as verify or modify headers of response, verify the business status - * of decoded object. Once interceptors are applied, - * {@link ResponseInterceptor#intercept(Response, Function)} is called around decode method called + * {@code ResponseInterceptor}s may be configured for purposes such as verifying or modifying + * headers of response, verifying the business status of decoded object, or processing responses to + * unsuccessful requests. Once the interceptors are applied, + * {@link ResponseInterceptor#intercept(InvocationContext, Chain)} is called, then the response is + * decoded. */ public interface ResponseInterceptor { /** - * Called for response around decode, must either manually invoke {@link Chain#next(Context)} or - * manually create a new response object + * Called by {@link ResponseHandler} after refreshing the response and wrapped around the whole + * decode process, must either manually invoke {@link Chain#next(InvocationContext)} or manually + * create a new response object * - * @param invocationContext information surrounding the response been decoded + * @param invocationContext information surrounding the response being decoded * @return decoded response */ - Object intercept(Context context, Chain chain) throws IOException; + Object intercept(InvocationContext invocationContext, Chain chain) throws Exception; /** * Return a new {@link ResponseInterceptor} that invokes the current interceptor first and then @@ -51,19 +47,8 @@ default ResponseInterceptor andThen(ResponseInterceptor nextInterceptor) { /** * Contract for delegation to the rest of the chain. */ - public interface Chain { - Chain DEFAULT = ic -> { - try { - return ic.decoder().decode(ic.response(), ic.returnType()); - } catch (final FeignException e) { - throw e; - } catch (final RuntimeException e) { - throw new DecodeException(ic.response().status(), e.getMessage(), - ic.response().request(), e); - } catch (IOException e) { - throw errorReading(ic.response().request(), ic.response(), e); - } - }; + interface Chain { + Chain DEFAULT = InvocationContext::proceed; /** * Delegate to the rest of the chain to execute the request. @@ -71,7 +56,7 @@ public interface Chain { * @param context the request to execute the {@link Chain} . * @return the response */ - Object next(Context context) throws IOException; + Object next(InvocationContext context) throws Exception; } /** @@ -83,31 +68,4 @@ public interface Chain { default Chain apply(Chain chain) { return request -> intercept(request, chain); } - - public class Context { - - private final Decoder decoder; - private final Type returnType; - private final Response response; - - Context(Decoder decoder, Type returnType, Response response) { - this.decoder = decoder; - this.returnType = returnType; - this.response = response; - } - - public Decoder decoder() { - return decoder; - } - - public Type returnType() { - return returnType; - } - - public Response response() { - return response; - } - - } - } diff --git a/core/src/main/java/feign/ResponseMapper.java b/core/src/main/java/feign/ResponseMapper.java index 994847b72f..f9ecfc5228 100644 --- a/core/src/main/java/feign/ResponseMapper.java +++ b/core/src/main/java/feign/ResponseMapper.java @@ -21,18 +21,18 @@ *
  * {@code
  * new ResponseMapper() {
- *      @Override
- *      public Response map(Response response, Type type) {
- *          try {
- *            return response
- *              .toBuilder()
- *              .body(Util.toString(response.body().asReader()).toUpperCase().getBytes())
- *              .build();
- *          } catch (IOException e) {
- *              throw new RuntimeException(e);
- *          }
- *      }
- *  };
+ *   @Override
+ *   public Response map(Response response, Type type) {
+ *     try {
+ *       return response
+ *           .toBuilder()
+ *           .body(Util.toString(response.body().asReader()).toUpperCase().getBytes())
+ *           .build();
+ *     } catch (IOException e) {
+ *       throw new RuntimeException(e);
+ *     }
+ *   }
+ * };
  * }
  * 
*/ diff --git a/core/src/test/java/feign/FeignTest.java b/core/src/test/java/feign/FeignTest.java old mode 100644 new mode 100755 index a2ed6b0c9a..b6bea3c026 --- a/core/src/test/java/feign/FeignTest.java +++ b/core/src/test/java/feign/FeignTest.java @@ -18,6 +18,12 @@ import feign.Feign.ResponseMappingDecoder; import feign.Request.HttpMethod; import feign.Target.HardCodedTarget; +import feign.codec.DecodeException; +import feign.codec.Decoder; +import feign.codec.EncodeException; +import feign.codec.Encoder; +import feign.codec.ErrorDecoder; +import feign.codec.StringDecoder; import feign.querymap.BeanQueryMapEncoder; import feign.querymap.FieldQueryMapEncoder; import okhttp3.mockwebserver.MockResponse; @@ -29,23 +35,21 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import org.mockito.ArgumentMatchers; import java.io.IOException; import java.lang.reflect.Type; import java.net.URI; import java.util.*; import java.util.concurrent.atomic.AtomicReference; -import feign.codec.DecodeException; -import feign.codec.Decoder; -import feign.codec.EncodeException; -import feign.codec.Encoder; -import feign.codec.ErrorDecoder; -import feign.codec.StringDecoder; import static feign.ExceptionPropagationPolicy.UNWRAP; import static feign.Util.UTF_8; import static feign.assertj.MockWebServerAssertions.assertThat; +import static java.util.Collections.emptyList; import static org.assertj.core.data.MapEntry.entry; -import static org.hamcrest.CoreMatchers.*; +import static org.hamcrest.CoreMatchers.isA; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.*; @SuppressWarnings("deprecation") public class FeignTest { @@ -893,7 +897,7 @@ public void mapAndDecodeExecutesMapFunction() throws Exception { @Test public void beanQueryMapEncoderWithPrivateGetterIgnored() throws Exception { - TestInterface api = new TestInterfaceBuilder().queryMapEndcoder(new BeanQueryMapEncoder()) + TestInterface api = new TestInterfaceBuilder().queryMapEncoder(new BeanQueryMapEncoder()) .target("http://localhost:" + server.getPort()); PropertyPojo.ChildPojoClass propertyPojo = new PropertyPojo.ChildPojoClass(); @@ -909,7 +913,7 @@ public void beanQueryMapEncoderWithPrivateGetterIgnored() throws Exception { @Test public void queryMap_with_child_pojo() throws Exception { - TestInterface api = new TestInterfaceBuilder().queryMapEndcoder(new FieldQueryMapEncoder()) + TestInterface api = new TestInterfaceBuilder().queryMapEncoder(new FieldQueryMapEncoder()) .target("http://localhost:" + server.getPort()); ChildPojo childPojo = new ChildPojo(); @@ -928,7 +932,7 @@ public void queryMap_with_child_pojo() throws Exception { @Test public void beanQueryMapEncoderWithNullValueIgnored() throws Exception { - TestInterface api = new TestInterfaceBuilder().queryMapEndcoder(new BeanQueryMapEncoder()) + TestInterface api = new TestInterfaceBuilder().queryMapEncoder(new BeanQueryMapEncoder()) .target("http://localhost:" + server.getPort()); PropertyPojo.ChildPojoClass propertyPojo = new PropertyPojo.ChildPojoClass(); @@ -943,7 +947,7 @@ public void beanQueryMapEncoderWithNullValueIgnored() throws Exception { @Test public void beanQueryMapEncoderWithEmptyParams() throws Exception { - TestInterface api = new TestInterfaceBuilder().queryMapEndcoder(new BeanQueryMapEncoder()) + TestInterface api = new TestInterfaceBuilder().queryMapEncoder(new BeanQueryMapEncoder()) .target("http://localhost:" + server.getPort()); PropertyPojo.ChildPojoClass propertyPojo = new PropertyPojo.ChildPojoClass(); @@ -1002,8 +1006,126 @@ public void supportComplexHeaders() throws Exception { .hasPath("/settings"); } + @Test + public void decodeVoid() throws Exception { + Decoder mockDecoder = mock(Decoder.class); + server.enqueue(new MockResponse().setResponseCode(200).setBody("OK")); + + TestInterface api = new TestInterfaceBuilder().decodeVoid().decoder(mockDecoder) + .target("http://localhost:" + server.getPort()); + + api.body(emptyList()); + verify(mockDecoder, times(1)).decode(ArgumentMatchers.any(), eq(void.class)); + } + + @Test + public void redirectionInterceptorString() throws Exception { + String location = "https://redirect.example.com"; + server.enqueue(new MockResponse().setResponseCode(302).setHeader("Location", location)); + + TestInterface api = new TestInterfaceBuilder().responseInterceptor(new RedirectionInterceptor()) + .target("http://localhost:" + server.getPort()); + + assertEquals("RedirectionInterceptor did not extract the location header", location, + api.post()); + } + + @Test + public void redirectionInterceptorCollection() throws Exception { + String location = "https://redirect.example.com"; + Collection locations = Collections.singleton("https://redirect.example.com"); + + server.enqueue(new MockResponse().setResponseCode(302).setHeader("Location", location)); + + TestInterface api = new TestInterfaceBuilder().responseInterceptor(new RedirectionInterceptor()) + .target("http://localhost:" + server.getPort()); + + Collection response = api.collection(); + assertEquals("RedirectionInterceptor did not extract the location header", locations.size(), + response.size()); + assertTrue("RedirectionInterceptor did not extract the location header", + response.contains(location)); + } + + @Test + public void responseInterceptor400Error() throws Exception { + String body = "BACK OFF!!"; + server.enqueue(new MockResponse().setResponseCode(429).setBody(body)); + + TestInterface api = new TestInterfaceBuilder().responseInterceptor(new ErrorInterceptor()) + .target("http://localhost:" + server.getPort()); + assertEquals("ResponseInterceptor did not extract the response body", body, api.post()); + } + + @Test + public void responseInterceptor500Error() throws Exception { + String body = "One moment, please."; + server.enqueue(new MockResponse().setResponseCode(503).setBody(body)); + + TestInterface api = new TestInterfaceBuilder().responseInterceptor(new ErrorInterceptor()) + .target("http://localhost:" + server.getPort()); + assertEquals("ResponseInterceptor did not extract the response body", body, api.post()); + } + + @Test + public void responseInterceptorChain() throws Exception { + String location = "https://redirect.example.com"; + server.enqueue(new MockResponse().setResponseCode(302).setHeader("Location", location)); + + String body = "One moment, please."; + server.enqueue(new MockResponse().setResponseCode(503).setBody(body)); + + TestInterface api = new TestInterfaceBuilder().responseInterceptor(new RedirectionInterceptor()) + .responseInterceptor(new ErrorInterceptor()).target("http://localhost:" + server.getPort()); + + assertEquals("RedirectionInterceptor did not extract the location header", location, + api.post()); + assertEquals("ResponseInterceptor did not extract the response body", body, api.post()); + } + + @Test + public void responseInterceptorChainList() throws Exception { + String location = "https://redirect.example.com"; + server.enqueue(new MockResponse().setResponseCode(302).setHeader("Location", location)); + + String body = "One moment, please."; + server.enqueue(new MockResponse().setResponseCode(503).setBody(body)); + + TestInterface api = new TestInterfaceBuilder() + .responseInterceptors(List.of(new RedirectionInterceptor(), new ErrorInterceptor())) + .target("http://localhost:" + server.getPort()); + + assertEquals("RedirectionInterceptor did not extract the location header", location, + api.post()); + assertEquals("ResponseInterceptor did not extract the response body", body, api.post()); + } + + @Test + public void responseInterceptorChainOrder() throws Exception { + String location = "https://redirect.example.com"; + String redirectBody = "Not the location"; + server.enqueue(new MockResponse().setResponseCode(302).setHeader("Location", location) + .setBody(redirectBody)); + + String body = "One moment, please."; + server.enqueue(new MockResponse().setResponseCode(503).setBody(body)); + + // ErrorInterceptor WILL extract the body of redirects, so we re-order our interceptors to + // verify that chain ordering is maintained + TestInterface api = new TestInterfaceBuilder() + .responseInterceptors(List.of(new ErrorInterceptor(), new RedirectionInterceptor())) + .target("http://localhost:" + server.getPort()); + + assertEquals("RedirectionInterceptor did not extract the redirect response body", redirectBody, + api.post()); + assertEquals("ResponseInterceptor did not extract the response body", body, api.post()); + } + interface TestInterface { + @RequestLine("POST /") + Collection collection(); + @RequestLine("POST /") Response response(); @@ -1211,6 +1333,16 @@ TestInterfaceBuilder requestInterceptor(RequestInterceptor requestInterceptor) { return this; } + TestInterfaceBuilder responseInterceptor(ResponseInterceptor responseInterceptor) { + delegate.responseInterceptor(responseInterceptor); + return this; + } + + TestInterfaceBuilder responseInterceptors(Iterable responseInterceptors) { + delegate.responseInterceptors(responseInterceptors); + return this; + } + TestInterfaceBuilder encoder(Encoder encoder) { delegate.encoder(encoder); return this; @@ -1231,7 +1363,12 @@ TestInterfaceBuilder dismiss404() { return this; } - TestInterfaceBuilder queryMapEndcoder(QueryMapEncoder queryMapEncoder) { + TestInterfaceBuilder decodeVoid() { + delegate.decodeVoid(); + return this; + } + + TestInterfaceBuilder queryMapEncoder(QueryMapEncoder queryMapEncoder) { delegate.queryMapEncoder(queryMapEncoder); return this; } @@ -1240,4 +1377,19 @@ TestInterface target(String url) { return delegate.target(TestInterface.class, url); } } + + class ErrorInterceptor implements ResponseInterceptor { + @Override + public Object intercept(InvocationContext invocationContext, Chain chain) throws Exception { + Response response = invocationContext.response(); + if (300 <= response.status()) { + if (String.class.equals(invocationContext.returnType())) { + String body = Util.toString(response.body().asReader(Util.UTF_8)); + response.close(); + return body; + } + } + return chain.next(invocationContext); + } + } }