Repository: Azure/azure-functions-java-worker
Pull request #711 (2023-04-06 18:44:00)

Fix durable return type void issue
<!-- Please provide all the information below.  -->

### Issue describing the changes in this PR

resolves https://github.com/microsoft/durabletask-java/issues/126

### Pull request checklist

* [X] My changes **do not** require documentation changes
  * [ ] Otherwise: Documentation issue linked to PR
* [X] My changes **should not** be added to the release notes for the next release
  * [ ] Otherwise: I've added my notes to `release_notes.md`
* [X] My changes **do not** need to be backported to a previous version
  * [ ] Otherwise: Backport tracked by issue/PR #issue_or_pr
* [X] I have added all required tests (Unit tests, E2E tests)

<!-- Optional: delete if not applicable  -->
### Additional information

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


Patch:
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
index 5048420..e4b605b 100644
--- 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 @@ public class ParameterResolver {
                 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 @@ public class ParameterResolver {
     }
 
     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;


Test patch:
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
index fd4c26c..1084d0d 100644
--- 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 org.mockito.junit.jupiter.MockitoExtension;
 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
new file mode 100644
index 0000000..385edad
--- /dev/null
+++ 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() {}
+}