Refactor AsyncFeign (#1789)

* Refactor to supply defaultContext on ReflectiveFeign

* Move target specification verification logic

* Refactor TargetSpecificationVerifier

Co-authored-by: Marvin Froeder <velo@users.noreply.github.com>

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<C> build() {
               decoder, queryMapEncoder,
               errorDecoder, methodHandlerFactory);
       final ReflectiveFeign<C> feign =
-          new ReflectiveFeign<>(handlersByName, invocationHandlerFactory, queryMapEncoder);
-      return new AsyncFeign<>(feign, defaultContextSupplier);
+          new ReflectiveFeign<>(handlersByName, invocationHandlerFactory, defaultContextSupplier);
+      return new AsyncFeign<>(feign);
     }
   }
 
   private final ReflectiveFeign<C> feign;
-  private final AsyncContextSupplier<C> defaultContextSupplier;
 
-  private AsyncFeign(ReflectiveFeign<C> feign, AsyncContextSupplier<C> defaultContextSupplier) {
+  private AsyncFeign(ReflectiveFeign<C> feign) {
     this.feign = feign;
-    this.defaultContextSupplier = defaultContextSupplier;
   }
 
   public <T> T newInstance(Target<T> target) {
-    return newInstance(target, defaultContextSupplier.newContext());
+    return feign.newInstance(target);
   }
 
   public <T> T newInstance(Target<T> target, C context) {
-    verifyTargetSpecfication(target);
     return feign.newInstance(target, context);
   }
-
-  private <T> void verifyTargetSpecfication(Target<T> target) {
-    Class<T> 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();
-  }
 }

core/src/main/java/feign/Feign.java

@@ -207,7 +207,7 @@ public Feign build() {
       ParseHandlersByName<Object> handlersByName =
           new ParseHandlersByName<>(contract, options, encoder, decoder, queryMapEncoder,
               errorDecoder, synchronousMethodHandlerFactory);
-      return new ReflectiveFeign<>(handlersByName, invocationHandlerFactory, queryMapEncoder);
+      return new ReflectiveFeign<>(handlersByName, invocationHandlerFactory, () -> null);
     }
   }
 

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,25 +34,27 @@ public class ReflectiveFeign<C> extends Feign {
 
   private final ParseHandlersByName<C> targetToHandlersByName;
   private final InvocationHandlerFactory factory;
-  private final QueryMapEncoder queryMapEncoder;
+  private final AsyncContextSupplier<C> defaultContextSupplier;
 
   ReflectiveFeign(ParseHandlersByName<C> targetToHandlersByName, InvocationHandlerFactory factory,
-      QueryMapEncoder queryMapEncoder) {
+      AsyncContextSupplier<C> defaultContextSupplier) {
     this.targetToHandlersByName = targetToHandlersByName;
     this.factory = factory;
-    this.queryMapEncoder = queryMapEncoder;
+    this.defaultContextSupplier = defaultContextSupplier;
   }
 
   /**
    * creates an api binding to the {@code target}. As this invokes reflection, care should be taken
    * to cache the result.
    */
   public <T> T newInstance(Target<T> target) {
-    return newInstance(target, null);
+    return newInstance(target, defaultContextSupplier.newContext());
   }
 
   @SuppressWarnings("unchecked")
   public <T> T newInstance(Target<T> target, C requestContext) {
+    TargetSpecificationVerifier.verify(target);
+
     Map<String, MethodHandler> nameToHandler = targetToHandlersByName.apply(target, requestContext);
     Map<Method, MethodHandler> methodToHandler = new LinkedHashMap<Method, MethodHandler>();
     List<DefaultMethodHandler> defaultMethodHandlers = new LinkedList<DefaultMethodHandler>();
@@ -408,4 +414,43 @@ protected RequestTemplate resolve(Object[] argv,
       return super.resolve(argv, mutable, variables);
     }
   }
+
+  private static class TargetSpecificationVerifier {
+    public static <T> void verify(Target<T> target) {
+      Class<T> 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();
+    }
+  }
 }