Merge pull request #1340 from mattrjacobs/refactor-user-defined-fallback

Made the user-defined fallback calculation more general
Netflix · Aug 31, 2016 · 12ad3e0 · 12ad3e0
2 parents 5ca732a + ce1caab
commit 12ad3e0
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 25 deletions.
diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/AbstractCommand.java b/hystrix-core/src/main/java/com/netflix/hystrix/AbstractCommand.java
@@ -134,7 +134,7 @@ protected enum ThreadState {
     // on the repetitive string processing that will occur on the same classes over and over again
     private static ConcurrentHashMap<Class<?>, String> defaultNameCache = new ConcurrentHashMap<Class<?>, String>();
 
-    private static ConcurrentHashMap<HystrixCommandKey, Boolean> commandContainsFallback = new ConcurrentHashMap<HystrixCommandKey, Boolean>();
+    protected static ConcurrentHashMap<HystrixCommandKey, Boolean> commandContainsFallback = new ConcurrentHashMap<HystrixCommandKey, Boolean>();
 
     /* package */static String getDefaultNameFromClass(Class<?> cls) {
         String fromCache = defaultNameCache.get(cls);
@@ -833,7 +833,7 @@ public void call() {
                 // acquire a permit
                 if (fallbackSemaphore.tryAcquire()) {
                     try {
-                        if (isFallbackUserSupplied(this)) {
+                        if (isFallbackUserDefined()) {
                             executionHook.onFallbackStart(this);
                             fallbackExecutionChain = getFallbackObservable();
                         } else {
@@ -1254,32 +1254,13 @@ protected TryableSemaphore getExecutionSemaphore() {
     /**
      * Each concrete implementation of AbstractCommand should return the name of the fallback method as a String
      * This will be used to determine if the fallback "exists" for firing the onFallbackStart/onFallbackError hooks
+     * @deprecated This functionality is replaced by {@link #isFallbackUserDefined}, which is less implementation-aware
      * @return method name of fallback
      */
+    @Deprecated
     protected abstract String getFallbackMethodName();
 
-    /**
-     * For the given command instance, does it define an actual fallback method?
-     * @param cmd command instance
-     * @return true iff there is a user-supplied fallback method on the given command instance
-     */
-    /*package-private*/ static boolean isFallbackUserSupplied(final AbstractCommand<?> cmd) {
-        HystrixCommandKey commandKey = cmd.commandKey;
-        Boolean containsFromMap = commandContainsFallback.get(commandKey);
-        if (containsFromMap != null) {
-            return containsFromMap;
-        } else {
-            Boolean toInsertIntoMap;
-            try {
-                cmd.getClass().getDeclaredMethod(cmd.getFallbackMethodName());
-                toInsertIntoMap = true;
-            } catch (NoSuchMethodException nsme) {
-                toInsertIntoMap = false;
-            }
-            commandContainsFallback.put(commandKey, toInsertIntoMap);
-            return toInsertIntoMap;
-        }
-    }
+    protected abstract boolean isFallbackUserDefined();
 
     /**
      * @return {@link HystrixCommandGroupKey} used to group together multiple {@link AbstractCommand} objects.
@@ -1497,7 +1478,7 @@ private Exception wrapWithOnExecutionErrorHook(Throwable t) {
     private Exception wrapWithOnFallbackErrorHook(Throwable t) {
         Exception e = getExceptionFromThrowable(t);
         try {
-            if (isFallbackUserSupplied(this)) {
+            if (isFallbackUserDefined()) {
                 return executionHook.onFallbackError(this, e);
             } else {
                 return e;

diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommand.java b/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommand.java
@@ -15,6 +15,7 @@
  */
 package com.netflix.hystrix;
 
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
@@ -44,6 +45,7 @@
  */
 public abstract class HystrixCommand<R> extends AbstractCommand<R> implements HystrixExecutable<R>, HystrixInvokableInfo<R>, HystrixObservable<R> {
 
+
     /**
      * Construct a {@link HystrixCommand} with defined {@link HystrixCommandGroupKey}.
      * <p>
@@ -461,6 +463,24 @@ protected String getFallbackMethodName() {
         return "getFallback";
     }
 
+    @Override
+    protected boolean isFallbackUserDefined() {
+        Boolean containsFromMap = commandContainsFallback.get(commandKey);
+        if (containsFromMap != null) {
+            return containsFromMap;
+        } else {
+            Boolean toInsertIntoMap;
+            try {
+                getClass().getDeclaredMethod("getFallback");
+                toInsertIntoMap = true;
+            } catch (NoSuchMethodException nsme) {
+                toInsertIntoMap = false;
+            }
+            commandContainsFallback.put(commandKey, toInsertIntoMap);
+            return toInsertIntoMap;
+        }
+    }
+
     @Override
     protected boolean commandIsScalar() {
         return true;

diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/HystrixObservableCommand.java b/hystrix-core/src/main/java/com/netflix/hystrix/HystrixObservableCommand.java
@@ -67,6 +67,24 @@ protected String getFallbackMethodName() {
         return "resumeWithFallback";
     }
 
+    @Override
+    protected boolean isFallbackUserDefined() {
+        Boolean containsFromMap = commandContainsFallback.get(commandKey);
+        if (containsFromMap != null) {
+            return containsFromMap;
+        } else {
+            Boolean toInsertIntoMap;
+            try {
+                getClass().getDeclaredMethod("resumeWithFallback");
+                toInsertIntoMap = true;
+            } catch (NoSuchMethodException nsme) {
+                toInsertIntoMap = false;
+            }
+            commandContainsFallback.put(commandKey, toInsertIntoMap);
+            return toInsertIntoMap;
+        }
+    }
+
     @Override
     protected boolean commandIsScalar() {
         return false;