Refactor reflective feign (#1821)

* Extract ParseHandlersByName.createMethodHandler

Extract the function so that the ParseHandlersByName.apply function is easy to understand.

* Refactor ReflectiveFeign.newInstance

Extract the function so that the ReflectiveFeign.newInstance function is easy to understand.

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

core/src/main/java/feign/ReflectiveFeign.java

@@ -55,28 +55,18 @@ public <T> T newInstance(Target<T> target) {
   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>();
-
-    for (Method method : target.type().getMethods()) {
-      if (method.getDeclaringClass() == Object.class) {
-        continue;
-      } else if (Util.isDefault(method)) {
-        DefaultMethodHandler handler = new DefaultMethodHandler(method);
-        defaultMethodHandlers.add(handler);
-        methodToHandler.put(method, handler);
-      } else {
-        methodToHandler.put(method, nameToHandler.get(Feign.configKey(target.type(), method)));
-      }
-    }
+    Map<Method, MethodHandler> methodToHandler =
+        targetToHandlersByName.apply(target, requestContext);
     InvocationHandler handler = factory.create(target, methodToHandler);
     T proxy = (T) Proxy.newProxyInstance(target.type().getClassLoader(),
         new Class<?>[] {target.type()}, handler);
 
-    for (DefaultMethodHandler defaultMethodHandler : defaultMethodHandlers) {
-      defaultMethodHandler.bindTo(proxy);
+    for (MethodHandler methodHandler : methodToHandler.values()) {
+      if (methodHandler instanceof DefaultMethodHandler) {
+        ((DefaultMethodHandler) methodHandler).bindTo(proxy);
+      }
     }
+
     return proxy;
   }
 
@@ -156,31 +146,53 @@ static final class ParseHandlersByName<C> {
       this.decoder = checkNotNull(decoder, "decoder");
     }
 
-    public Map<String, MethodHandler> apply(Target target, C requestContext) {
-      List<MethodMetadata> metadata = contract.parseAndValidateMetadata(target.type());
-      Map<String, MethodHandler> result = new LinkedHashMap<String, MethodHandler>();
-      for (MethodMetadata md : metadata) {
-        BuildTemplateByResolvingArgs buildTemplate;
-        if (!md.formParams().isEmpty() && md.template().bodyTemplate() == null) {
-          buildTemplate =
-              new BuildFormEncodedTemplateFromArgs(md, encoder, queryMapEncoder, target);
-        } else if (md.bodyIndex() != null || md.alwaysEncodeBody()) {
-          buildTemplate = new BuildEncodedTemplateFromArgs(md, encoder, queryMapEncoder, target);
-        } else {
-          buildTemplate = new BuildTemplateByResolvingArgs(md, queryMapEncoder, target);
+    public Map<Method, MethodHandler> apply(Target target, C requestContext) {
+      final Map<Method, MethodHandler> result = new LinkedHashMap<>();
+
+      final List<MethodMetadata> metadataList = contract.parseAndValidateMetadata(target.type());
+      for (MethodMetadata md : metadataList) {
+        final Method method = md.method();
+        if (method.getDeclaringClass() == Object.class) {
+          continue;
         }
-        if (md.isIgnored()) {
-          result.put(md.configKey(), args -> {
-            throw new IllegalStateException(md.configKey() + " is not a method handled by feign");
-          });
-        } else {
-          result.put(md.configKey(),
-              factory.create(target, md, buildTemplate, options, decoder, errorDecoder,
-                  requestContext));
+
+        final MethodHandler handler = createMethodHandler(target, md, requestContext);
+        result.put(method, handler);
+      }
+
+      for (Method method : target.type().getMethods()) {
+        if (Util.isDefault(method)) {
+          final MethodHandler handler = new DefaultMethodHandler(method);
+          result.put(method, handler);
         }
       }
+
       return result;
     }
+
+    private MethodHandler createMethodHandler(final Target<?> target,
+                                              final MethodMetadata md,
+                                              final C requestContext) {
+      if (md.isIgnored()) {
+        return args -> {
+          throw new IllegalStateException(md.configKey() + " is not a method handled by feign");
+        };
+      }
+
+      BuildTemplateByResolvingArgs buildTemplate = getBuildTemplate(target, md);
+      return factory.create(
+          target, md, buildTemplate, options, decoder, errorDecoder, requestContext);
+    }
+
+    private BuildTemplateByResolvingArgs getBuildTemplate(Target target, MethodMetadata md) {
+      if (!md.formParams().isEmpty() && md.template().bodyTemplate() == null) {
+        return new BuildFormEncodedTemplateFromArgs(md, encoder, queryMapEncoder, target);
+      } else if (md.bodyIndex() != null || md.alwaysEncodeBody()) {
+        return new BuildEncodedTemplateFromArgs(md, encoder, queryMapEncoder, target);
+      } else {
+        return new BuildTemplateByResolvingArgs(md, queryMapEncoder, target);
+      }
+    }
   }
 
   private static class BuildTemplateByResolvingArgs implements RequestTemplate.Factory {