Fixes dispatch for toString, equals, hashCode in hystrix

Adrian Cole · Adrian Cole · commit f774a42623c4 · 2016-04-22T22:21:08.000+08:00
This copies missing logic from `FeignInvocationHandler` to
`HystrixInvocationHandler`, preventing NPEs when calling methods defined
on `java.lang.Object`.
diff --git a/hystrix/src/main/java/feign/hystrix/HystrixInvocationHandler.java b/hystrix/src/main/java/feign/hystrix/HystrixInvocationHandler.java
@@ -22,7 +22,8 @@
 import java.lang.reflect.InvocationHandler;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
-import java.util.HashMap;
+import java.lang.reflect.Proxy;
+import java.util.LinkedHashMap;
 import java.util.Map;
 
 import feign.InvocationHandlerFactory;
@@ -56,7 +57,7 @@ final class HystrixInvocationHandler implements InvocationHandler {
    * @return cached methods map for fallback invoking
    */
   private Map<Method, Method> toFallbackMethod(Map<Method, MethodHandler> dispatch) {
-    Map<Method, Method> result = new HashMap<Method, Method>();
+    Map<Method, Method> result = new LinkedHashMap<Method, Method>();
     for (Method method : dispatch.keySet()) {
       method.setAccessible(true);
       result.put(method, method);
@@ -67,6 +68,22 @@ private Map<Method, Method> toFallbackMethod(Map<Method, MethodHandler> dispatch
   @Override
   public Object invoke(final Object proxy, final Method method, final Object[] args)
       throws Throwable {
+    // early exit if the invoked method is from java.lang.Object
+    // code is the same as ReflectiveFeign.FeignInvocationHandler
+    if ("equals".equals(method.getName())) {
+      try {
+        Object otherHandler =
+            args.length > 0 && args[0] != null ? Proxy.getInvocationHandler(args[0]) : null;
+        return equals(otherHandler);
+      } catch (IllegalArgumentException e) {
+        return false;
+      }
+    } else if ("hashCode".equals(method.getName())) {
+      return hashCode();
+    } else if ("toString".equals(method.getName())) {
+      return toString();
+    }
+
     String groupKey = this.target.name();
     String commandKey = method.getName();
     HystrixCommand.Setter setter = HystrixCommand.Setter
@@ -137,6 +154,25 @@ private boolean isReturnsSingle(Method method) {
     return Single.class.isAssignableFrom(method.getReturnType());
   }
 
+  @Override
+  public boolean equals(Object obj) {
+    if (obj instanceof HystrixInvocationHandler) {
+      HystrixInvocationHandler other = (HystrixInvocationHandler) obj;
+      return target.equals(other.target);
+    }
+    return false;
+  }
+
+  @Override
+  public int hashCode() {
+    return target.hashCode();
+  }
+
+  @Override
+  public String toString() {
+    return target.toString();
+  }
+
   static final class Factory implements InvocationHandlerFactory {
 
     @Override
diff --git a/hystrix/src/test/java/feign/hystrix/HystrixBuilderTest.java b/hystrix/src/test/java/feign/hystrix/HystrixBuilderTest.java
@@ -20,6 +20,8 @@
 import feign.Headers;
 import feign.Param;
 import feign.RequestLine;
+import feign.Target;
+import feign.Target.HardCodedTarget;
 import feign.gson.GsonDecoder;
 import rx.Observable;
 import rx.Single;
@@ -426,13 +428,58 @@ public void plainListFallback() {
     assertThat(list).isNotNull().containsExactly("fallback");
   }
 
+  @Test
+  public void equalsHashCodeAndToStringWork() {
+    Target<TestInterface> t1 =
+        new HardCodedTarget<TestInterface>(TestInterface.class, "http://localhost:8080");
+    Target<TestInterface> t2 =
+        new HardCodedTarget<TestInterface>(TestInterface.class, "http://localhost:8888");
+    Target<OtherTestInterface> t3 =
+        new HardCodedTarget<OtherTestInterface>(OtherTestInterface.class, "http://localhost:8080");
+    TestInterface i1 = HystrixFeign.builder().target(t1);
+    TestInterface i2 = HystrixFeign.builder().target(t1);
+    TestInterface i3 = HystrixFeign.builder().target(t2);
+    OtherTestInterface i4 = HystrixFeign.builder().target(t3);
+
+    assertThat(i1)
+        .isEqualTo(i2)
+        .isNotEqualTo(i3)
+        .isNotEqualTo(i4);
+
+    assertThat(i1.hashCode())
+        .isEqualTo(i2.hashCode())
+        .isNotEqualTo(i3.hashCode())
+        .isNotEqualTo(i4.hashCode());
+
+    assertThat(i1.toString())
+        .isEqualTo(i2.toString())
+        .isNotEqualTo(i3.toString())
+        .isNotEqualTo(i4.toString());
+
+    assertThat(t1)
+        .isNotEqualTo(i1);
+
+    assertThat(t1.hashCode())
+        .isEqualTo(i1.hashCode());
+
+    assertThat(t1.toString())
+        .isEqualTo(i1.toString());
+  }
+
   private TestInterface target() {
     return HystrixFeign.builder()
         .decoder(new GsonDecoder())
         .target(TestInterface.class, "http://localhost:" + server.getPort(),
             new FallbackTestInterface());
   }
 
+  interface OtherTestInterface {
+
+    @RequestLine("GET /")
+    @Headers("Accept: application/json")
+    HystrixCommand<List<String>> listCommand();
+  }
+
   interface TestInterface {
 
     @RequestLine("GET /")
