Fix durable return type void issue (#711)

* fix durable return type void issue add unit tests * fix stupid test name
Azure · Apr 14, 2023 · 2bc845a · 2bc845a
1 parent 9881b3a
commit 2bc845a
Show file tree

Hide file tree

Showing 3 changed files with 84 additions and 3 deletions.
diff --git a/src/main/java/com/microsoft/azure/functions/worker/broker/ParameterResolver.java b/src/main/java/com/microsoft/azure/functions/worker/broker/ParameterResolver.java
@@ -52,7 +52,11 @@ else if (paramName == null && !paramBindingNameAnnotation.isEmpty()) {
                 BindingData actualArg = argument.orElseThrow(WrongMethodTypeException::new);
                 invokeInfo.appendArgument(actualArg.getValue());
             }
-            if (!method.getMethod().getReturnType().equals(void.class) && !method.getMethod().getReturnType().equals(Void.class)) {
+            // For function annotated with @HasImplicitOutput, we should allow it to send back data even function's return type is void
+            // Reference to https://github.com/microsoft/durabletask-java/issues/126
+            if (!method.getMethod().getReturnType().equals(void.class)
+                    && !method.getMethod().getReturnType().equals(Void.class)
+                    || method.hasImplicitOutput()) {
                 dataStore.getOrAddDataTarget(invokeInfo.getOutputsId(), BindingDataStore.RETURN_NAME, method.getMethod().getReturnType(), method.hasImplicitOutput());
             }
             return invokeInfo;
@@ -63,8 +67,8 @@ else if (paramName == null && !paramBindingNameAnnotation.isEmpty()) {
     }
 
     public static final class InvokeInfoBuilder extends JavaMethodInvokeInfo.Builder {
-        public InvokeInfoBuilder(MethodBindInfo method) { super.setMethod(method.getMethod()); }
         private final UUID outputsId = UUID.randomUUID();
+        public InvokeInfoBuilder(MethodBindInfo method) { super.setMethod(method.getMethod()); }
 
         public UUID getOutputsId() {
             return outputsId;

diff --git a/src/test/java/com/microsoft/azure/functions/worker/broker/JavaFunctionBrokerTest.java b/src/test/java/com/microsoft/azure/functions/worker/broker/JavaFunctionBrokerTest.java
@@ -12,7 +12,6 @@
 import java.util.*;
 
 import static org.junit.jupiter.api.Assertions.*;
-import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.when;
 
 @ExtendWith(MockitoExtension.class)

diff --git a/src/test/java/com/microsoft/azure/functions/worker/broker/ParameterResolverTest.java b/src/test/java/com/microsoft/azure/functions/worker/broker/ParameterResolverTest.java
@@ -0,0 +1,78 @@
+package com.microsoft.azure.functions.worker.broker;
+
+import com.microsoft.azure.functions.rpc.messages.RpcException;
+import com.microsoft.azure.functions.spi.inject.FunctionInstanceInjector;
+import com.microsoft.azure.functions.worker.WorkerLogManager;
+import com.microsoft.azure.functions.worker.binding.BindingDataStore;
+import com.microsoft.azure.functions.worker.binding.ExecutionContextDataSource;
+import com.microsoft.azure.functions.worker.binding.ExecutionRetryContext;
+import com.microsoft.azure.functions.worker.binding.ExecutionTraceContext;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.logging.Logger;
+
+import static org.junit.jupiter.api.Assertions.*;
+import static org.mockito.Mockito.when;
+
+
+@ExtendWith(MockitoExtension.class)
+public class ParameterResolverTest {
+
+    private ExecutionContextDataSource executionContextDataSource;
+    @Mock
+    private MethodBindInfo methodBindInfo;
+
+    @BeforeEach
+    public void setup() {
+        String invocationId = "testInvocationId";
+        ExecutionTraceContext traceContext = new ExecutionTraceContext("traceParent", "traceState", new HashMap<>());
+        ExecutionRetryContext retryContext = new ExecutionRetryContext(1, 2, RpcException.newBuilder().build());
+        String functionName = "ParameterResolverTest";
+        BindingDataStore dataStore = new BindingDataStore();
+        dataStore.setBindingDefinitions(new HashMap<>());
+        try (MockedStatic<WorkerLogManager> workerLogManagerMockedStatic = Mockito.mockStatic(WorkerLogManager.class)) {
+            workerLogManagerMockedStatic.when(() -> WorkerLogManager.getInvocationLogger(invocationId))
+                    .thenReturn(Logger.getAnonymousLogger());
+            executionContextDataSource = new ExecutionContextDataSource(invocationId,
+                    traceContext, retryContext, functionName, dataStore, methodBindInfo,
+                    this.getClass(), new ArrayList<>(), new FunctionInstanceInjector() {
+                @Override
+                public <T> T getInstance(Class<T> functionClass) throws Exception {
+                    return null;
+                }
+            });
+        }
+
+    }
+
+    @Test
+    public void testResolveArgumentsHasImplicitOutputTrue() throws Exception {
+        Method testMethod = this.getClass().getDeclaredMethod("testMethod");
+        when(methodBindInfo.hasImplicitOutput()).thenReturn(true);
+        when(methodBindInfo.getMethod()).thenReturn(testMethod);
+        when(methodBindInfo.getParams()).thenReturn(new ArrayList<>());
+        ParameterResolver.resolveArguments(executionContextDataSource);
+        assertTrue(executionContextDataSource.getDataStore().getDataTargetTypedValue(BindingDataStore.RETURN_NAME).isPresent());
+    }
+
+    @Test
+    public void testResolveArgumentsHasImplicitOutputFalse() throws Exception {
+        Method testMethod = this.getClass().getDeclaredMethod("testMethod");
+        when(methodBindInfo.hasImplicitOutput()).thenReturn(false);
+        when(methodBindInfo.getMethod()).thenReturn(testMethod);
+        when(methodBindInfo.getParams()).thenReturn(new ArrayList<>());
+        ParameterResolver.resolveArguments(executionContextDataSource);
+        assertFalse(executionContextDataSource.getDataStore().getDataTargetTypedValue(BindingDataStore.RETURN_NAME).isPresent());
+    }
+
+    public void testMethod() {}
+}