Issue/565/skilltrigger returnval (#797)

Introduced hasNonVoidReturnType and hasEffectiveReturnType methods to streamline return type checks and implicit output handling. Updated relevant logic in BindingDataStore, MethodBindInfo, and ParameterResolver to enhance clarity and maintainability. --------- Co-authored-by: Gavin Aguiar <80794152+gavin-aguiar@users.noreply.github.com>
Azure · Nov 26, 2024 · 70d2fa5 · 70d2fa5
1 parent 0204a6f
commit 70d2fa5
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 11 deletions.
diff --git a/src/main/java/com/microsoft/azure/functions/worker/binding/BindingDataStore.java b/src/main/java/com/microsoft/azure/functions/worker/binding/BindingDataStore.java
@@ -119,11 +119,11 @@ public Optional<TypedData> getDataTargetTypedValue(String name) throws Exception
 		});
     }
 
-    public Optional<BindingData> getOrAddDataTarget(UUID outputId, String name, Type target, boolean hasImplicitOutput) {
+    public Optional<BindingData> getOrAddDataTarget(UUID outputId, String name, Type target, boolean ignoreDefinition) {
         DataTarget output = null;
         if (this.isDataTargetValid(name, target)) {
             output = this.getTarget(outputId).get(name);
-            if (output == null && (this.isDefinitionOutput(name) || hasImplicitOutput)) {
+            if (output == null && (this.isDefinitionOutput(name) || ignoreDefinition)) {
                 this.getTarget(outputId).put(name, output = rpcDataTargetFromType(target));
             }
         }

diff --git a/src/main/java/com/microsoft/azure/functions/worker/broker/MethodBindInfo.java b/src/main/java/com/microsoft/azure/functions/worker/broker/MethodBindInfo.java
@@ -28,9 +28,22 @@ public List<ParamBindInfo> getParams() {
         return params;
     }
 
+    public boolean hasEffectiveReturnType(){
+        boolean nonVoidReturn = this.hasNonVoidReturnType();
+        // 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
+        boolean implicitOutput = this.hasImplicitOutput();
+        return nonVoidReturn || implicitOutput;
+    }
+
     public boolean hasImplicitOutput() {
         return hasImplicitOutput;
     }
+
+    public boolean hasNonVoidReturnType() {
+        Class<?> returnType = this.getMethod().getReturnType();
+        return !returnType.equals(void.class) && !returnType.equals(Void.class);
+    }
 }
 
 
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,13 +52,11 @@ else if (paramName == null && !paramBindingNameAnnotation.isEmpty()) {
                 BindingData actualArg = argument.orElseThrow(WrongMethodTypeException::new);
                 invokeInfo.appendArgument(actualArg.getValue());
             }
-            // 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());
+
+            if (method.hasEffectiveReturnType()){
+                dataStore.getOrAddDataTarget(invokeInfo.getOutputsId(), BindingDataStore.RETURN_NAME, method.getMethod().getReturnType(), true);
             }
+
             return invokeInfo;
         } catch (Exception ex) {
             ExceptionUtils.rethrow(ex);

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
@@ -56,7 +56,9 @@ public <T> T getInstance(Class<T> functionClass) throws Exception {
 
     @Test
     public void testResolveArgumentsHasImplicitOutputTrue() throws Exception {
-        Method testMethod = this.getClass().getDeclaredMethod("testMethod");
+        Method testMethod = this.getClass().getDeclaredMethod("testMethodVoid");
+        when(methodBindInfo.hasNonVoidReturnType()).thenCallRealMethod();
+        when(methodBindInfo.hasEffectiveReturnType()).thenCallRealMethod();
         when(methodBindInfo.hasImplicitOutput()).thenReturn(true);
         when(methodBindInfo.getMethod()).thenReturn(testMethod);
         when(methodBindInfo.getParams()).thenReturn(new ArrayList<>());
@@ -66,13 +68,40 @@ public void testResolveArgumentsHasImplicitOutputTrue() throws Exception {
 
     @Test
     public void testResolveArgumentsHasImplicitOutputFalse() throws Exception {
-        Method testMethod = this.getClass().getDeclaredMethod("testMethod");
+        Method testMethod = this.getClass().getDeclaredMethod("testMethodVoid");
+        when(methodBindInfo.hasNonVoidReturnType()).thenCallRealMethod();
+        when(methodBindInfo.hasEffectiveReturnType()).thenCallRealMethod();
         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() {}
+    @Test
+    public void testResolveArgumentsNonVoidReturnTypeTrue() throws Exception {
+        Method testMethod = this.getClass().getDeclaredMethod("testMethodNonVoid");
+        when(methodBindInfo.getMethod()).thenReturn(testMethod);
+        when(methodBindInfo.getParams()).thenReturn(new ArrayList<>());
+        when(methodBindInfo.hasNonVoidReturnType()).thenCallRealMethod(); // Ensures real logic is executed
+        when(methodBindInfo.hasImplicitOutput()).thenReturn(false);
+        when(methodBindInfo.hasEffectiveReturnType()).thenCallRealMethod();
+        ParameterResolver.resolveArguments(executionContextDataSource);
+        assertTrue(executionContextDataSource.getDataStore().getDataTargetTypedValue(BindingDataStore.RETURN_NAME).isPresent());
+    }
+
+    @Test
+    public void testResolveArgumentsNonVoidReturnTypeFalse() throws Exception {
+        Method testMethod = this.getClass().getDeclaredMethod("testMethodVoid");
+        when(methodBindInfo.getMethod()).thenReturn(testMethod);
+        when(methodBindInfo.getParams()).thenReturn(new ArrayList<>());
+        when(methodBindInfo.hasNonVoidReturnType()).thenCallRealMethod(); // Ensures real logic is executed
+        when(methodBindInfo.hasImplicitOutput()).thenReturn(false);
+        when(methodBindInfo.hasEffectiveReturnType()).thenCallRealMethod();
+        ParameterResolver.resolveArguments(executionContextDataSource);
+        assertFalse(executionContextDataSource.getDataStore().getDataTargetTypedValue(BindingDataStore.RETURN_NAME).isPresent());
+    }
+
+    public void testMethodVoid() {}
+    public String testMethodNonVoid() { return "test"; }
 }