Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The builder clones itself before enrichment #2117

Merged
Prev Previous commit
Next Next commit
Alternative internal builder process
  • Loading branch information
velo committed Aug 21, 2023

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
commit 87ed0aff683e0343557dd1ac45f20fd26ab35b9a
37 changes: 17 additions & 20 deletions core/src/main/java/feign/AsyncFeign.java
Original file line number Diff line number Diff line change
@@ -65,7 +65,7 @@ private static class LazyInitializedExecutorService {
});
}

public static class AsyncBuilder<C> extends BaseBuilder<AsyncBuilder<C>> {
public static class AsyncBuilder<C> extends BaseBuilder<AsyncBuilder<C>, AsyncFeign<C>> {

private AsyncContextSupplier<C> defaultContextSupplier = () -> null;
private AsyncClient<C> client = new AsyncClient.Default<>(
@@ -190,33 +190,30 @@ public AsyncBuilder<C> invocationHandlerFactory(InvocationHandlerFactory invocat
return super.invocationHandlerFactory(invocationHandlerFactory);
}

public AsyncFeign<C> build() {
AsyncBuilder<C> enrichedBuilder = super.enrich();

@Override
public AsyncFeign<C> internalBuild() {
AsyncResponseHandler responseHandler =
(AsyncResponseHandler) Capability.enrich(
new AsyncResponseHandler(
enrichedBuilder.logLevel,
enrichedBuilder.logger,
enrichedBuilder.decoder,
enrichedBuilder.errorDecoder,
enrichedBuilder.dismiss404,
enrichedBuilder.closeAfterDecode, enrichedBuilder.responseInterceptor),
logLevel,
logger,
decoder,
errorDecoder,
dismiss404,
closeAfterDecode, decodeVoid, responseInterceptor),
AsyncResponseHandler.class,
enrichedBuilder.capabilities);
capabilities);

final MethodHandler.Factory<C> methodHandlerFactory =
new AsynchronousMethodHandler.Factory<>(
enrichedBuilder.client, enrichedBuilder.retryer, enrichedBuilder.requestInterceptors,
responseHandler, enrichedBuilder.logger, enrichedBuilder.logLevel,
enrichedBuilder.propagationPolicy, enrichedBuilder.methodInfoResolver,
new RequestTemplateFactoryResolver(enrichedBuilder.encoder,
enrichedBuilder.queryMapEncoder),
enrichedBuilder.options, enrichedBuilder.decoder, enrichedBuilder.errorDecoder);
client, retryer, requestInterceptors,
responseHandler, logger, logLevel,
propagationPolicy, methodInfoResolver,
new RequestTemplateFactoryResolver(encoder, queryMapEncoder),
options, decoder, errorDecoder);
final ReflectiveFeign<C> feign =
new ReflectiveFeign<>(enrichedBuilder.contract, methodHandlerFactory,
enrichedBuilder.invocationHandlerFactory,
enrichedBuilder.defaultContextSupplier);
new ReflectiveFeign<>(contract, methodHandlerFactory, invocationHandlerFactory,
defaultContextSupplier);
return new AsyncFeign<>(feign);
}
}
8 changes: 6 additions & 2 deletions core/src/main/java/feign/BaseBuilder.java
Original file line number Diff line number Diff line change
@@ -29,7 +29,7 @@
import java.util.Objects;
import java.util.stream.Collectors;

public abstract class BaseBuilder<B extends BaseBuilder<B>> implements Cloneable {
public abstract class BaseBuilder<B extends BaseBuilder<B, T>, T> implements Cloneable {

private final B thisB;

@@ -231,7 +231,7 @@ public B addCapability(Capability capability) {
}

@SuppressWarnings("unchecked")
protected B enrich() {
B enrich() {
if (capabilities.isEmpty()) {
return thisB;
}
@@ -283,5 +283,9 @@ List<Field> getFieldsToEnrich() {
.collect(Collectors.toList());
}

public final T build() {
return enrich().internalBuild();
}

protected abstract T internalBuild();
}
29 changes: 11 additions & 18 deletions core/src/main/java/feign/Feign.java
Original file line number Diff line number Diff line change
@@ -13,12 +13,12 @@
*/
package feign;

import feign.InvocationHandlerFactory.MethodHandler;
import feign.Request.Options;
import feign.Target.HardCodedTarget;
import feign.codec.Decoder;
import feign.codec.Encoder;
import feign.codec.ErrorDecoder;
import feign.InvocationHandlerFactory.MethodHandler;
import java.io.IOException;
import java.lang.reflect.Method;
import java.lang.reflect.Type;
@@ -92,7 +92,7 @@ public static String configKey(Method method) {
*/
public abstract <T> T newInstance(Target<T> target);

public static class Builder extends BaseBuilder<Builder> {
public static class Builder extends BaseBuilder<Builder, Feign> {

private Client client = new Client.Default(null, null);

@@ -201,24 +201,17 @@ public <T> T target(Target<T> target) {
return build().newInstance(target);
}

public Feign build() {
Builder enrichedBuilder = super.enrich();

@Override
public Feign internalBuild() {
final ResponseHandler responseHandler =
new ResponseHandler(enrichedBuilder.logLevel, enrichedBuilder.logger,
enrichedBuilder.decoder, enrichedBuilder.errorDecoder,
enrichedBuilder.dismiss404, enrichedBuilder.closeAfterDecode,
enrichedBuilder.responseInterceptor);
new ResponseHandler(logLevel, logger, decoder, errorDecoder,
dismiss404, closeAfterDecode, decodeVoid, responseInterceptor);
MethodHandler.Factory<Object> methodHandlerFactory =
new SynchronousMethodHandler.Factory(enrichedBuilder.client, enrichedBuilder.retryer,
enrichedBuilder.requestInterceptors,
responseHandler, enrichedBuilder.logger, enrichedBuilder.logLevel,
enrichedBuilder.propagationPolicy,
new RequestTemplateFactoryResolver(enrichedBuilder.encoder,
enrichedBuilder.queryMapEncoder),
enrichedBuilder.options);
return new ReflectiveFeign<>(enrichedBuilder.contract, methodHandlerFactory,
enrichedBuilder.invocationHandlerFactory,
new SynchronousMethodHandler.Factory(client, retryer, requestInterceptors,
responseHandler, logger, logLevel, propagationPolicy,
new RequestTemplateFactoryResolver(encoder, queryMapEncoder),
options);
return new ReflectiveFeign<>(contract, methodHandlerFactory, invocationHandlerFactory,
() -> null);
}
}
4 changes: 2 additions & 2 deletions core/src/test/java/feign/BaseBuilderTest.java
Original file line number Diff line number Diff line change
@@ -31,10 +31,10 @@ public void checkEnrichTouchesAllAsyncBuilderFields()
}), 14);
}

private void test(BaseBuilder<?> builder, int expectedFieldsCount)
private void test(BaseBuilder<?, ?> builder, int expectedFieldsCount)
throws IllegalArgumentException, IllegalAccessException {
Capability mockingCapability = Mockito.mock(Capability.class, RETURNS_MOCKS);
BaseBuilder<?> enriched = builder.addCapability(mockingCapability).enrich();
BaseBuilder<?, ?> enriched = builder.addCapability(mockingCapability).enrich();

List<Field> fields = enriched.getFieldsToEnrich();
assertThat(fields).hasSize(expectedFieldsCount);
8 changes: 4 additions & 4 deletions hystrix/src/main/java/feign/hystrix/HystrixFeign.java
Original file line number Diff line number Diff line change
@@ -14,9 +14,6 @@
package feign.hystrix;

import com.netflix.hystrix.HystrixCommand;
import java.lang.reflect.InvocationHandler;
import java.lang.reflect.Method;
import java.util.Map;
import feign.Client;
import feign.Contract;
import feign.Feign;
@@ -30,6 +27,9 @@
import feign.codec.Decoder;
import feign.codec.Encoder;
import feign.codec.ErrorDecoder;
import java.lang.reflect.InvocationHandler;
import java.lang.reflect.Method;
import java.util.Map;

/**
* Allows Feign interfaces to return HystrixCommand or rx.Observable or rx.Single objects. Also
@@ -133,7 +133,7 @@ public Builder contract(Contract contract) {
}

@Override
public Feign build() {
public Feign internalBuild() {
return build(null);
}

39 changes: 19 additions & 20 deletions kotlin/src/main/java/feign/kotlin/CoroutineFeign.java
Original file line number Diff line number Diff line change
@@ -112,7 +112,8 @@ public String toString() {
}
}

public static class CoroutineBuilder<C> extends BaseBuilder<CoroutineBuilder<C>> {
public static class CoroutineBuilder<C>
extends BaseBuilder<CoroutineBuilder<C>, CoroutineFeign<C>> {

private AsyncContextSupplier<C> defaultContextSupplier = () -> null;
private AsyncClient<C> client = new AsyncClient.Default<>(
@@ -156,27 +157,25 @@ public <T> T target(Target<T> target, C context) {
return build().newInstance(target, context);
}

@Override
@SuppressWarnings("unchecked")
public CoroutineFeign<C> build() {
CoroutineBuilder<C> enrichedBuilder = super.enrich();

public CoroutineFeign<C> internalBuild() {
AsyncFeign<C> asyncFeign = (AsyncFeign<C>) AsyncFeign.builder()
.logLevel(enrichedBuilder.logLevel)
.client((AsyncClient<Object>) enrichedBuilder.client)
.decoder(enrichedBuilder.decoder)
.errorDecoder(enrichedBuilder.errorDecoder)
.contract(enrichedBuilder.contract)
.retryer(enrichedBuilder.retryer)
.logger(enrichedBuilder.logger)
.encoder(enrichedBuilder.encoder)
.queryMapEncoder(enrichedBuilder.queryMapEncoder)
.options(enrichedBuilder.options)
.requestInterceptors(enrichedBuilder.requestInterceptors)
.responseInterceptor(enrichedBuilder.responseInterceptor)
.invocationHandlerFactory(enrichedBuilder.invocationHandlerFactory)
.defaultContextSupplier(
(AsyncContextSupplier<Object>) enrichedBuilder.defaultContextSupplier)
.methodInfoResolver(enrichedBuilder.methodInfoResolver)
.logLevel(logLevel)
.client((AsyncClient<Object>) client)
.decoder(decoder)
.errorDecoder(errorDecoder)
.contract(contract)
.retryer(retryer)
.logger(logger)
.encoder(encoder)
.queryMapEncoder(queryMapEncoder)
.options(options)
.requestInterceptors(requestInterceptors)
.responseInterceptor(responseInterceptor)
.invocationHandlerFactory(invocationHandlerFactory)
.defaultContextSupplier((AsyncContextSupplier<Object>) defaultContextSupplier)
.methodInfoResolver(methodInfoResolver)
.build();
return new CoroutineFeign<>(asyncFeign);
}
You are viewing a condensed version of this merge commit. You can view the full changes here.