From 2b8d62a20b436b6cbaac152014ec6fc3e0cbb3d7 Mon Sep 17 00:00:00 2001 From: kaibocai <89094811+kaibocai@users.noreply.github.com> Date: Thu, 6 Apr 2023 12:31:21 -0500 Subject: [PATCH 1/2] fix durable return type void issue add unit tests --- .../worker/broker/ParameterResolver.java | 8 +- .../worker/broker/JavaFunctionBrokerTest.java | 1 - .../worker/broker/ParameterResolverTest.java | 78 +++++++++++++++++++ 3 files changed, 84 insertions(+), 3 deletions(-) create mode 100644 src/test/java/com/microsoft/azure/functions/worker/broker/ParameterResolverTest.java 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 50484209..e4b605b0 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 @@ 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 index fd4c26ca..1084d0d5 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 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 00000000..d873299c --- /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 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 getInstance(Class functionClass) throws Exception { + return null; + } + }); + } + + } + + @Test + public void testResolveArguments() 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 testResolveArguments1() 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() {} +} From d0b9fce376905869860fe2d983c7e83ad37d0f92 Mon Sep 17 00:00:00 2001 From: kaibocai <89094811+kaibocai@users.noreply.github.com> Date: Thu, 6 Apr 2023 14:20:30 -0500 Subject: [PATCH 2/2] fix stupid test name --- .../azure/functions/worker/broker/ParameterResolverTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 index d873299c..385edade 100644 --- a/src/test/java/com/microsoft/azure/functions/worker/broker/ParameterResolverTest.java +++ b/src/test/java/com/microsoft/azure/functions/worker/broker/ParameterResolverTest.java @@ -55,7 +55,7 @@ public T getInstance(Class functionClass) throws Exception { } @Test - public void testResolveArguments() throws Exception { + public void testResolveArgumentsHasImplicitOutputTrue() throws Exception { Method testMethod = this.getClass().getDeclaredMethod("testMethod"); when(methodBindInfo.hasImplicitOutput()).thenReturn(true); when(methodBindInfo.getMethod()).thenReturn(testMethod); @@ -65,7 +65,7 @@ public void testResolveArguments() throws Exception { } @Test - public void testResolveArguments1() throws Exception { + public void testResolveArgumentsHasImplicitOutputFalse() throws Exception { Method testMethod = this.getClass().getDeclaredMethod("testMethod"); when(methodBindInfo.hasImplicitOutput()).thenReturn(false); when(methodBindInfo.getMethod()).thenReturn(testMethod);