Skip to content

Commit

Permalink
Merge commit '2c00066d4a7a1f1882708166f8b2cbaabe721efa' into feature/…
Browse files Browse the repository at this point in the history
…decoder-interceptor-to-response-interceptor

# Conflicts:
#	core/src/main/java/feign/InvocationContext.java
#	core/src/main/java/feign/ResponseHandler.java
#	core/src/main/java/feign/ResponseInterceptor.java
  • Loading branch information
iain-henderson committed Aug 29, 2023
2 parents bdb890b + 2c00066 commit ec675b9
Show file tree
Hide file tree
Showing 51 changed files with 164 additions and 83 deletions.
2 changes: 1 addition & 1 deletion annotation-error-decoder/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
<parent>
<groupId>io.github.openfeign</groupId>
<artifactId>parent</artifactId>
<version>12.5-SNAPSHOT</version>
<version>13.0-SNAPSHOT</version>
</parent>

<artifactId>feign-annotation-error-decoder</artifactId>
Expand Down
2 changes: 1 addition & 1 deletion apt-test-generator/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
<parent>
<groupId>io.github.openfeign</groupId>
<artifactId>parent</artifactId>
<version>12.5-SNAPSHOT</version>
<version>13.0-SNAPSHOT</version>
</parent>

<groupId>io.github.openfeign.experimental</groupId>
Expand Down
2 changes: 1 addition & 1 deletion benchmark/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
<parent>
<groupId>io.github.openfeign</groupId>
<artifactId>parent</artifactId>
<version>12.5-SNAPSHOT</version>
<version>13.0-SNAPSHOT</version>
</parent>

<artifactId>feign-benchmark</artifactId>
Expand Down
10 changes: 9 additions & 1 deletion core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
<parent>
<groupId>io.github.openfeign</groupId>
<artifactId>parent</artifactId>
<version>12.5-SNAPSHOT</version>
<version>13.0-SNAPSHOT</version>
</parent>

<artifactId>feign-core</artifactId>
Expand Down Expand Up @@ -109,6 +109,14 @@
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<source>11</source>
<target>11</target>
</configuration>
</plugin>
</plugins>
</build>

Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/feign/AsyncFeign.java
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ public AsyncFeign<C> internalBuild() {
decoder,
errorDecoder,
dismiss404,
closeAfterDecode, decodeVoid, responseInterceptor),
closeAfterDecode, decodeVoid, responseInterceptorChain()),
AsyncResponseHandler.class,
capabilities);

Expand Down
12 changes: 5 additions & 7 deletions core/src/main/java/feign/AsyncResponseHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,11 @@
class AsyncResponseHandler {
private final ResponseHandler responseHandler;

AsyncResponseHandler(Level logLevel, Logger logger, Decoder decoder,
ErrorDecoder errorDecoder, boolean dismiss404, boolean closeAfterDecode, boolean decodeVoid,
ResponseInterceptor responseInterceptor) {
this.responseHandler = new ResponseHandler(
logLevel, logger, decoder,
errorDecoder, dismiss404, closeAfterDecode, decodeVoid,
responseInterceptor);
AsyncResponseHandler(Level logLevel, Logger logger, Decoder decoder, ErrorDecoder errorDecoder,
boolean dismiss404, boolean closeAfterDecode, boolean decodeVoid,
ResponseInterceptor.Chain executionChain) {
this.responseHandler = new ResponseHandler(logLevel, logger, decoder, errorDecoder, dismiss404,
closeAfterDecode, decodeVoid, executionChain);
}

public CompletableFuture<Object> handleResponse(String configKey,
Expand Down
30 changes: 28 additions & 2 deletions core/src/main/java/feign/BaseBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public abstract class BaseBuilder<B extends BaseBuilder<B, T>, T> implements Clo

protected final List<RequestInterceptor> requestInterceptors =
new ArrayList<>();
protected ResponseInterceptor responseInterceptor = ResponseInterceptor.DEFAULT;
protected final List<ResponseInterceptor> responseInterceptors = new ArrayList<>();
protected Logger.Level logLevel = Logger.Level.NONE;
protected Contract contract = new Contract.Default();
protected Retryer retryer = new Retryer.Default();
Expand Down Expand Up @@ -203,11 +203,23 @@ public B requestInterceptors(Iterable<RequestInterceptor> requestInterceptors) {
return thisB;
}

/**
* Sets the full set of request interceptors for the builder, overwriting any previous
* interceptors.
*/
public B responseInterceptors(Iterable<ResponseInterceptor> responseInterceptors) {
this.responseInterceptors.clear();
for (ResponseInterceptor responseInterceptor : responseInterceptors) {
this.responseInterceptors.add(responseInterceptor);
}
return thisB;
}

/**
* Adds a single response interceptor to the builder.
*/
public B responseInterceptor(ResponseInterceptor responseInterceptor) {
this.responseInterceptor = responseInterceptor;
this.responseInterceptors.add(responseInterceptor);
return thisB;
}

Expand Down Expand Up @@ -288,4 +300,18 @@ public final T build() {
}

protected abstract T internalBuild();

protected ResponseInterceptor.Chain responseInterceptorChain() {
ResponseInterceptor.Chain endOfChain =
ResponseInterceptor.Chain.DEFAULT;
ResponseInterceptor.Chain executionChain = this.responseInterceptors.stream()
.reduce(ResponseInterceptor::andThen)
.map(interceptor -> interceptor.apply(endOfChain))
.orElse(endOfChain);

return (ResponseInterceptor.Chain) Capability.enrich(executionChain,
ResponseInterceptor.Chain.class, capabilities);
}


}
4 changes: 4 additions & 0 deletions core/src/main/java/feign/Capability.java
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@ default ResponseInterceptor enrich(ResponseInterceptor responseInterceptor) {
return responseInterceptor;
}

default ResponseInterceptor.Chain enrich(ResponseInterceptor.Chain chain) {
return chain;
}

default Logger enrich(Logger logger) {
return logger;
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/feign/Feign.java
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ public <T> T target(Target<T> target) {
public Feign internalBuild() {
final ResponseHandler responseHandler =
new ResponseHandler(logLevel, logger, decoder, errorDecoder,
dismiss404, closeAfterDecode, decodeVoid, responseInterceptor);
dismiss404, closeAfterDecode, decodeVoid, responseInterceptorChain());
MethodHandler.Factory<Object> methodHandlerFactory =
new SynchronousMethodHandler.Factory(client, retryer, requestInterceptors,
responseHandler, logger, logLevel, propagationPolicy,
Expand Down
5 changes: 3 additions & 2 deletions core/src/main/java/feign/RedirectionInterceptor.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
*/
package feign;

import java.io.IOException;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
import java.util.Collection;
Expand All @@ -26,7 +27,7 @@
*/
public class RedirectionInterceptor implements ResponseInterceptor {
@Override
public Object aroundDecode(InvocationContext invocationContext) throws Exception {
public Object intercept(InvocationContext invocationContext, Chain chain) throws Exception {
Response response = invocationContext.response();
int status = response.status();
Object returnValue = null;
Expand All @@ -44,7 +45,7 @@ public Object aroundDecode(InvocationContext invocationContext) throws Exception
}
}
if (returnValue == null) {
return invocationContext.proceed();
return chain.next(invocationContext);
} else {
response.close();
return returnValue;
Expand Down
2 changes: 2 additions & 0 deletions core/src/main/java/feign/Request.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -66,6 +67,7 @@ public enum ProtocolVersion {
public String toString() {
return protocolVersion;
}

}

/**
Expand Down
16 changes: 8 additions & 8 deletions core/src/main/java/feign/ResponseHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@
*/
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 static feign.FeignException.errorReading;
import static feign.Util.ensureClosed;

/**
* The response handler that is used to provide synchronous support on top of standard response
Expand All @@ -37,11 +37,11 @@ public class ResponseHandler {

private final boolean decodeVoid;

private final ResponseInterceptor responseInterceptor;
private final ResponseInterceptor.Chain executionChain;

public ResponseHandler(Level logLevel, Logger logger, Decoder decoder,
ErrorDecoder errorDecoder, boolean dismiss404, boolean closeAfterDecode, boolean decodeVoid,
ResponseInterceptor responseInterceptor) {
public ResponseHandler(Level logLevel, Logger logger, Decoder decoder, ErrorDecoder errorDecoder,
boolean dismiss404, boolean closeAfterDecode, boolean decodeVoid,
ResponseInterceptor.Chain executionChain) {
super();
this.logLevel = logLevel;
this.logger = logger;
Expand All @@ -50,7 +50,7 @@ public ResponseHandler(Level logLevel, Logger logger, Decoder decoder,
this.dismiss404 = dismiss404;
this.closeAfterDecode = closeAfterDecode;
this.decodeVoid = decodeVoid;
this.responseInterceptor = responseInterceptor;
this.executionChain = executionChain;
}

public Object handleResponse(String configKey,
Expand All @@ -60,7 +60,7 @@ public Object handleResponse(String configKey,
throws Exception {
try {
response = logAndRebufferResponseIfNeeded(configKey, response, elapsedTime);
return responseInterceptor.aroundDecode(
return executionChain.next(
new InvocationContext(configKey, decoder, errorDecoder, dismiss404, closeAfterDecode,
decodeVoid, response, returnType));
} catch (final IOException e) {
Expand Down
52 changes: 46 additions & 6 deletions core/src/main/java/feign/ResponseInterceptor.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,63 @@
*/
package feign;

import java.io.IOException;

/**
* Zero or One {@code ResponseInterceptor} may be configured for purposes such as verify or modify
* headers of response, verify the business status of decoded object. Once the interceptor is
* applied, {@link ResponseInterceptor#aroundDecode(InvocationContext)} is called around decode
* method called
* applied, {@link ResponseInterceptor#intercept(InvocationContext, Chain)} is called around decode
* method called 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;
*
*/
public interface ResponseInterceptor {

ResponseInterceptor DEFAULT = InvocationContext::proceed;

/**
* Called by {@link ResponseHandler} after refreshing the response and wrapped around the whole
* decode process, must either manually invoke {@link InvocationContext#proceed} or manually
* decode process, must either manually invoke {@link Chain#next(InvocationContext)} or manually
* create a new response object
*
* @param invocationContext information surrounding the response being decoded
* @return decoded response
*/
Object aroundDecode(InvocationContext invocationContext) throws Exception;
Object intercept(InvocationContext invocationContext, Chain chain) throws Exception;

/**
* Return a new {@link ResponseInterceptor} that invokes the current interceptor first and then
* the one that is passed in.
*
* @param nextInterceptor the interceptor to delegate to after the current
* @return a new interceptor that chains the two
*/
default ResponseInterceptor andThen(ResponseInterceptor nextInterceptor) {
return (ic, chain) -> intercept(ic,
nextContext -> nextInterceptor.intercept(nextContext, chain));
}

/**
* Contract for delegation to the rest of the chain.
*/
public interface Chain {
Chain DEFAULT = InvocationContext::proceed;

/**
* Delegate to the rest of the chain to execute the request.
*
* @param context the request to execute the {@link Chain} .
* @return the response
*/
Object next(InvocationContext context) throws Exception;
}

/**
* Apply this interceptor to the given {@code Chain} resulting in an intercepted chain.
*
* @param chain the chain to add interception around
* @return a new chain instance
*/
default Chain apply(Chain chain) {
return request -> intercept(request, chain);
}
}
24 changes: 12 additions & 12 deletions core/src/main/java/feign/ResponseMapper.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,18 @@
* <pre>
* {@code
* new ResponseMapper() {
* &#64;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);
* }
* }
* };
* &#64;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);
* }
* }
* };
* }
* </pre>
*/
Expand Down
6 changes: 4 additions & 2 deletions core/src/test/java/feign/BaseBuilderTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public class BaseBuilderTest {
public void checkEnrichTouchesAllAsyncBuilderFields()
throws IllegalArgumentException, IllegalAccessException {
test(AsyncFeign.builder().requestInterceptor(template -> {
}), 14);
}).responseInterceptor((ic, c) -> c.next(ic)), 14);
}

private void test(BaseBuilder<?, ?> builder, int expectedFieldsCount)
Expand All @@ -43,6 +43,8 @@ private void test(BaseBuilder<?, ?> builder, int expectedFieldsCount)
field.setAccessible(true);
Object mockedValue = field.get(enriched);
if (mockedValue instanceof List) {
assertThat((List) mockedValue).withFailMessage("Enriched list missing contents %s", field)
.isNotEmpty();
mockedValue = ((List<Object>) mockedValue).get(0);
}
assertTrue("Field was not enriched " + field, Mockito.mockingDetails(mockedValue)
Expand All @@ -56,7 +58,7 @@ private void test(BaseBuilder<?, ?> builder, int expectedFieldsCount)
public void checkEnrichTouchesAllBuilderFields()
throws IllegalArgumentException, IllegalAccessException {
test(Feign.builder().requestInterceptor(template -> {
}), 12);
}).responseInterceptor((ic, c) -> c.next(ic)), 12);
}

}
4 changes: 2 additions & 2 deletions core/src/test/java/feign/DefaultContractTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@
import static org.assertj.core.data.MapEntry.entry;

/**
* Tests interfaces defined per {@link Contract.Default} are interpreted into expected
* {@link feign .RequestTemplate template} instances.
* Tests interfaces defined per {@link Contract.Default} are interpreted into expected {@link feign
* .RequestTemplate template} instances.
*/
public class DefaultContractTest {

Expand Down
Loading

0 comments on commit ec675b9

Please sign in to comment.