From 542312e6163327bee0eee481094e9f932fd67140 Mon Sep 17 00:00:00 2001 From: Donghyeon Kim Date: Wed, 12 Oct 2022 03:03:29 +0900 Subject: [PATCH] Refactor AsyncFeign (#1789) * Refactor to supply defaultContext on ReflectiveFeign * Move target specification verification logic * Refactor TargetSpecificationVerifier Co-authored-by: Marvin Froeder --- core/src/main/java/feign/AsyncFeign.java | 53 ++----------------- core/src/main/java/feign/Feign.java | 2 +- core/src/main/java/feign/ReflectiveFeign.java | 53 +++++++++++++++++-- 3 files changed, 54 insertions(+), 54 deletions(-) diff --git a/core/src/main/java/feign/AsyncFeign.java b/core/src/main/java/feign/AsyncFeign.java index 564a50ebd..0b0d5b1bf 100644 --- a/core/src/main/java/feign/AsyncFeign.java +++ b/core/src/main/java/feign/AsyncFeign.java @@ -21,10 +21,6 @@ import feign.codec.Decoder; import feign.codec.Encoder; import feign.codec.ErrorDecoder; -import java.lang.reflect.Method; -import java.lang.reflect.ParameterizedType; -import java.lang.reflect.Type; -import java.lang.reflect.WildcardType; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -215,63 +211,22 @@ public AsyncFeign build() { decoder, queryMapEncoder, errorDecoder, methodHandlerFactory); final ReflectiveFeign feign = - new ReflectiveFeign<>(handlersByName, invocationHandlerFactory, queryMapEncoder); - return new AsyncFeign<>(feign, defaultContextSupplier); + new ReflectiveFeign<>(handlersByName, invocationHandlerFactory, defaultContextSupplier); + return new AsyncFeign<>(feign); } } private final ReflectiveFeign feign; - private final AsyncContextSupplier defaultContextSupplier; - private AsyncFeign(ReflectiveFeign feign, AsyncContextSupplier defaultContextSupplier) { + private AsyncFeign(ReflectiveFeign feign) { this.feign = feign; - this.defaultContextSupplier = defaultContextSupplier; } public T newInstance(Target target) { - return newInstance(target, defaultContextSupplier.newContext()); + return feign.newInstance(target); } public T newInstance(Target target, C context) { - verifyTargetSpecfication(target); return feign.newInstance(target, context); } - - private void verifyTargetSpecfication(Target target) { - Class type = target.type(); - if (!type.isInterface()) { - throw new IllegalArgumentException("Type must be an interface: " + type); - } - - for (final Method m : type.getMethods()) { - final Class retType = m.getReturnType(); - - if (!CompletableFuture.class.isAssignableFrom(retType)) { - continue; // synchronous case - } - - if (retType != CompletableFuture.class) { - throw new IllegalArgumentException("Method return type is not CompleteableFuture: " - + getFullMethodName(type, retType, m)); - } - - final Type genRetType = m.getGenericReturnType(); - - if (!ParameterizedType.class.isInstance(genRetType)) { - throw new IllegalArgumentException("Method return type is not parameterized: " - + getFullMethodName(type, genRetType, m)); - } - - if (WildcardType.class - .isInstance(ParameterizedType.class.cast(genRetType).getActualTypeArguments()[0])) { - throw new IllegalArgumentException( - "Wildcards are not supported for return-type parameters: " - + getFullMethodName(type, genRetType, m)); - } - } - } - - private String getFullMethodName(Class type, Type retType, Method m) { - return retType.getTypeName() + " " + type.toGenericString() + "." + m.getName(); - } } diff --git a/core/src/main/java/feign/Feign.java b/core/src/main/java/feign/Feign.java index 9e3cbf1c0..bb28fef8a 100644 --- a/core/src/main/java/feign/Feign.java +++ b/core/src/main/java/feign/Feign.java @@ -207,7 +207,7 @@ public Feign build() { ParseHandlersByName handlersByName = new ParseHandlersByName<>(contract, options, encoder, decoder, queryMapEncoder, errorDecoder, synchronousMethodHandlerFactory); - return new ReflectiveFeign<>(handlersByName, invocationHandlerFactory, queryMapEncoder); + return new ReflectiveFeign<>(handlersByName, invocationHandlerFactory, () -> null); } } diff --git a/core/src/main/java/feign/ReflectiveFeign.java b/core/src/main/java/feign/ReflectiveFeign.java index 71eb65a7a..df81b1254 100644 --- a/core/src/main/java/feign/ReflectiveFeign.java +++ b/core/src/main/java/feign/ReflectiveFeign.java @@ -17,9 +17,13 @@ import static feign.Util.checkNotNull; import java.lang.reflect.InvocationHandler; import java.lang.reflect.Method; +import java.lang.reflect.ParameterizedType; import java.lang.reflect.Proxy; +import java.lang.reflect.Type; +import java.lang.reflect.WildcardType; import java.util.*; import java.util.Map.Entry; +import java.util.concurrent.CompletableFuture; import feign.InvocationHandlerFactory.MethodHandler; import feign.Param.Expander; import feign.Request.Options; @@ -30,13 +34,13 @@ public class ReflectiveFeign extends Feign { private final ParseHandlersByName targetToHandlersByName; private final InvocationHandlerFactory factory; - private final QueryMapEncoder queryMapEncoder; + private final AsyncContextSupplier defaultContextSupplier; ReflectiveFeign(ParseHandlersByName targetToHandlersByName, InvocationHandlerFactory factory, - QueryMapEncoder queryMapEncoder) { + AsyncContextSupplier defaultContextSupplier) { this.targetToHandlersByName = targetToHandlersByName; this.factory = factory; - this.queryMapEncoder = queryMapEncoder; + this.defaultContextSupplier = defaultContextSupplier; } /** @@ -44,11 +48,13 @@ public class ReflectiveFeign extends Feign { * to cache the result. */ public T newInstance(Target target) { - return newInstance(target, null); + return newInstance(target, defaultContextSupplier.newContext()); } @SuppressWarnings("unchecked") public T newInstance(Target target, C requestContext) { + TargetSpecificationVerifier.verify(target); + Map nameToHandler = targetToHandlersByName.apply(target, requestContext); Map methodToHandler = new LinkedHashMap(); List defaultMethodHandlers = new LinkedList(); @@ -408,4 +414,43 @@ protected RequestTemplate resolve(Object[] argv, return super.resolve(argv, mutable, variables); } } + + private static class TargetSpecificationVerifier { + public static void verify(Target target) { + Class type = target.type(); + if (!type.isInterface()) { + throw new IllegalArgumentException("Type must be an interface: " + type); + } + + for (final Method m : type.getMethods()) { + final Class retType = m.getReturnType(); + + if (!CompletableFuture.class.isAssignableFrom(retType)) { + continue; // synchronous case + } + + if (retType != CompletableFuture.class) { + throw new IllegalArgumentException("Method return type is not CompleteableFuture: " + + getFullMethodName(type, retType, m)); + } + + final Type genRetType = m.getGenericReturnType(); + + if (!(genRetType instanceof ParameterizedType)) { + throw new IllegalArgumentException("Method return type is not parameterized: " + + getFullMethodName(type, genRetType, m)); + } + + if (((ParameterizedType) genRetType).getActualTypeArguments()[0] instanceof WildcardType) { + throw new IllegalArgumentException( + "Wildcards are not supported for return-type parameters: " + + getFullMethodName(type, genRetType, m)); + } + } + } + + private static String getFullMethodName(Class type, Type retType, Method m) { + return retType.getTypeName() + " " + type.toGenericString() + "." + m.getName(); + } + } }