Execution type is now correctly determined from method return type

sawano · sawano · commit 2137cb15bf6a · 2014-11-27T20:32:45.000+01:00
diff --git a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/ExecutionType.java b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/ExecutionType.java
@@ -39,16 +39,15 @@ public enum ExecutionType {
      */
     OBSERVABLE;
 
-
     /**
      * Gets execution type for specified class type.
      * @param type the type
      * @return the execution type {@link ExecutionType}
      */
     public static ExecutionType getExecutionType(Class<?> type) {
-        if (type.isAssignableFrom(Future.class)) {
+        if (Future.class.isAssignableFrom(type)) {
             return ExecutionType.ASYNCHRONOUS;
-        } else if (type.isAssignableFrom(Observable.class)) {
+        } else if (Observable.class.isAssignableFrom(type)) {
             return ExecutionType.OBSERVABLE;
         } else {
             return ExecutionType.SYNCHRONOUS;
diff --git a/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/command/ExecutionTypeTest.java b/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/command/ExecutionTypeTest.java
@@ -0,0 +1,60 @@
+package com.netflix.hystrix.contrib.javanica.command;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import rx.Observable;
+import rx.internal.operators.OperatorMulticast;
+
+import java.util.List;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Future;
+import java.util.concurrent.RunnableFuture;
+
+import static com.netflix.hystrix.contrib.javanica.command.ExecutionType.ASYNCHRONOUS;
+import static com.netflix.hystrix.contrib.javanica.command.ExecutionType.OBSERVABLE;
+import static com.netflix.hystrix.contrib.javanica.command.ExecutionType.SYNCHRONOUS;
+import static java.util.Arrays.asList;
+import static org.junit.Assert.assertEquals;
+
+@RunWith(Parameterized.class)
+public class ExecutionTypeTest {
+
+    @Parameterized.Parameters
+    public static List<Object[]> data() {
+        return asList(new Object[][]{
+                {returnType(Integer.class), shouldHaveExecutionType(SYNCHRONOUS)},
+                {returnType(List.class), shouldHaveExecutionType(SYNCHRONOUS)},
+                {returnType(Object.class), shouldHaveExecutionType(SYNCHRONOUS)},
+                {returnType(Class.class), shouldHaveExecutionType(SYNCHRONOUS)},
+                {returnType(Future.class), shouldHaveExecutionType(ASYNCHRONOUS)},
+                {returnType(AsyncResult.class), shouldHaveExecutionType(ASYNCHRONOUS)},
+                {returnType(RunnableFuture.class), shouldHaveExecutionType(ASYNCHRONOUS)},
+                {returnType(CompletableFuture.class), shouldHaveExecutionType(ASYNCHRONOUS)},
+                {returnType(Observable.class), shouldHaveExecutionType(OBSERVABLE)},
+                {returnType(OperatorMulticast.class), shouldHaveExecutionType(OBSERVABLE)},
+        });
+    }
+
+    @Test
+    public void should_return_correct_execution_type() throws Exception {
+        assertEquals("Unexpected execution type for method return type: " + methodReturnType, expectedType, ExecutionType.getExecutionType(methodReturnType));
+
+    }
+
+    private static ExecutionType shouldHaveExecutionType(final ExecutionType type) {
+        return type;
+    }
+
+    private static Class<?> returnType(final Class<?> aClass) {
+        return aClass;
+    }
+
+    private final Class<?> methodReturnType;
+    private final ExecutionType expectedType;
+
+    public ExecutionTypeTest(final Class<?> methodReturnType, final ExecutionType expectedType) {
+        this.methodReturnType = methodReturnType;
+        this.expectedType = expectedType;
+    }
+}
diff --git a/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/spring/command/CommandTest.java b/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/spring/command/CommandTest.java
@@ -77,6 +77,14 @@ public void testGetUserSync() {
         assertTrue(command.getExecutionEvents().contains(HystrixEventType.SUCCESS));
     }
 
+    @Test
+    public void should_work_with_parameterized_method() throws Exception {
+        assertEquals(Integer.valueOf(1), userService.echo(1));
+
+        assertEquals(1, HystrixRequestLog.getCurrentRequest().getExecutedCommands().size());
+        assertTrue(getCommand().getExecutionEvents().contains(HystrixEventType.SUCCESS));
+    }
+
     private com.netflix.hystrix.HystrixCommand<?> getCommand() {
         return HystrixRequestLog.getCurrentRequest().getExecutedCommands().iterator().next();
     }
@@ -98,6 +106,11 @@ public User getUserSync(String id, String name) {
             return new User(id, name + id); // it should be network call
         }
 
+        @HystrixCommand
+        public <T> T echo(T value) {
+            return value;
+        }
+
     }
 
     @Configurable

Original file line number	Diff line number	Diff line change
`@@ -77,6 +77,14 @@ public void testGetUserSync() {`
`77`	`77`	`assertTrue(command.getExecutionEvents().contains(HystrixEventType.SUCCESS));`
`78`	`78`	`}`
`79`	`79`
	`80`	`+ @Test`
	`81`	`+ public void should_work_with_parameterized_method() throws Exception {`
	`82`	`+ assertEquals(Integer.valueOf(1), userService.echo(1));`
	`83`	`+`
	`84`	`+ assertEquals(1, HystrixRequestLog.getCurrentRequest().getExecutedCommands().size());`
	`85`	`+ assertTrue(getCommand().getExecutionEvents().contains(HystrixEventType.SUCCESS));`
	`86`	`+ }`
	`87`	`+`
`80`	`88`	`private com.netflix.hystrix.HystrixCommand<?> getCommand() {`
`81`	`89`	`return HystrixRequestLog.getCurrentRequest().getExecutedCommands().iterator().next();`
`82`	`90`	`}`
`@@ -98,6 +106,11 @@ public User getUserSync(String id, String name) {`
`98`	`106`	`return new User(id, name + id); // it should be network call`
`99`	`107`	`}`
`100`	`108`
	`109`	`+ @HystrixCommand`
	`110`	`+ public <T> T echo(T value) {`
	`111`	`+ return value;`
	`112`	`+ }`
	`113`	`+`
`101`	`114`	`}`
`102`	`115`
`103`	`116`	`@Configurable`