[Fix apache#3772] Duplicate function error (apache#3774)

kiegroup · Nov 13, 2024 · b1b9e2c · b1b9e2c
1 parent 1dbb058
commit b1b9e2c
Show file tree

Hide file tree

Showing 7 changed files with 56 additions and 4 deletions.
diff --git a/...java/org/kie/kogito/serverless/workflow/parser/handlers/validation/WorkflowValidator.java b/...java/org/kie/kogito/serverless/workflow/parser/handlers/validation/WorkflowValidator.java
@@ -18,11 +18,14 @@
  */
 package org.kie.kogito.serverless.workflow.parser.handlers.validation;
 
+import java.util.Map;
+
 import org.kie.kogito.serverless.workflow.parser.ParserContext;
 
 import io.serverlessworkflow.api.Workflow;
 
 import static org.kie.kogito.internal.utils.ConversionUtils.isEmpty;
+import static org.kie.kogito.serverless.workflow.utils.ServerlessWorkflowUtils.findDuplicates;
 
 public class WorkflowValidator {
 
@@ -33,5 +36,13 @@ public static void validateStart(Workflow workflow, ParserContext context) {
         if (workflow.getStart() == null || isEmpty(workflow.getStart().getStateName())) {
             context.addValidationError("Workflow does not define a starting state");
         }
+        if (workflow.getFunctions() != null) {
+            Map<String, Integer> functionDuplicates = findDuplicates(workflow.getFunctions().getFunctionDefs(), f -> f.getName());
+            if (!functionDuplicates.isEmpty()) {
+                StringBuilder sb = new StringBuilder("There are duplicated function definitions: ");
+                functionDuplicates.forEach((k, v) -> sb.append(String.format("\nFunction %s appears %d times", k, v)));
+                context.addValidationError(sb.toString());
+            }
+        }
     }
 }
diff --git a/...ilder/src/main/java/org/kie/kogito/serverless/workflow/utils/ServerlessWorkflowUtils.java b/...ilder/src/main/java/org/kie/kogito/serverless/workflow/utils/ServerlessWorkflowUtils.java
@@ -27,8 +27,13 @@
 import java.nio.file.Path;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
+import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.function.Supplier;
 
@@ -251,4 +256,19 @@ private static ModelMetaData getModelMetadata(WorkflowProcess process, Class<?>
         return new ModelMetaData(process.getId(), modelClass.getPackage().getName(), modelClass.getSimpleName(), KogitoWorkflowProcess.PUBLIC_VISIBILITY,
                 VariableDeclarations.of(Collections.emptyMap()), false);
     }
+
+    public static <T, V> Map<V, Integer> findDuplicates(List<T> items, Function<T, V> converter) {
+        if (items == null) {
+            return Map.of();
+        }
+        Map<V, Integer> duplicates = new LinkedHashMap<>();
+        Set<V> helper = new HashSet<>();
+        items.forEach(item -> {
+            V toAdd = converter.apply(item);
+            if (!helper.add(toAdd)) {
+                duplicates.compute(toAdd, (k, v) -> v == null ? 2 : ++v);
+            }
+        });
+        return duplicates;
+    }
 }
diff --git a/...low-builder/src/test/java/org/kie/kogito/serverless/workflow/utils/WorkflowUtilsTest.java b/...low-builder/src/test/java/org/kie/kogito/serverless/workflow/utils/WorkflowUtilsTest.java
@@ -19,7 +19,9 @@
 package org.kie.kogito.serverless.workflow.utils;
 
 import java.io.File;
+import java.util.Arrays;
 import java.util.Collections;
+import java.util.Map;
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -51,4 +53,9 @@ public void testResolveFunctionMetadata() {
         assertThat(resolveFunctionMetadata(function, "testprop1", context)).isNotNull().isEqualTo("customtestprop1val");
         assertThat(resolveFunctionMetadata(function, "testprop2", context)).isNotNull().isEqualTo("testprop2val");
     }
+
+    @Test
+    void findDuplicates() {
+        assertThat(ServerlessWorkflowUtils.findDuplicates(Arrays.asList(1, 3, 3, 2, 2, 1, 8, 7, 3), v -> v)).isEqualTo(Map.of(3, 3, 2, 2, 1, 2));
+    }
 }
diff --git a/.../test/java/org/kie/kogito/serverless/workflow/executor/StaticWorkflowApplicationTest.java b/.../test/java/org/kie/kogito/serverless/workflow/executor/StaticWorkflowApplicationTest.java
@@ -115,8 +115,9 @@ void testValidationError() throws IOException {
                 StaticWorkflowApplication application = StaticWorkflowApplication.create()) {
             Workflow workflow = getWorkflow(reader, WorkflowFormat.JSON);
             ValidationException validationException = catchThrowableOfType(() -> application.process(workflow), ValidationException.class);
-            assertThat(validationException.getErrors()).hasSizeGreaterThanOrEqualTo(4);
-            assertThat(validationException).hasMessageContaining("error").hasMessageContaining("function").hasMessageContaining("connect").hasMessageContaining("transition");
+            assertThat(validationException.getErrors()).hasSizeGreaterThanOrEqualTo(5);
+            assertThat(validationException).hasMessageContaining("error").hasMessageContaining("function").hasMessageContaining("connect").hasMessageContaining("transition")
+                    .hasMessageContaining("duplicated");
         }
     }
 

diff --git a/...erless-workflow/kogito-serverless-workflow-executor-core/src/test/resources/wrong.sw.json b/...erless-workflow/kogito-serverless-workflow-executor-core/src/test/resources/wrong.sw.json
@@ -19,6 +19,11 @@
       "name": "logInfo",
       "type": "custom",
       "operation": "sysout:INFO"
+    },
+    {
+      "name": "pushData",
+      "type": "custom",
+      "operation": "script:python:print('javierito')"
     }
   ],
   "errors":[],

diff --git a/...workflow-fluent/src/main/java/org/kie/kogito/serverless/workflow/fluent/StateBuilder.java b/...workflow-fluent/src/main/java/org/kie/kogito/serverless/workflow/fluent/StateBuilder.java
@@ -58,6 +58,7 @@ public static ForEachStateBuilder forEach(String inputExpr) {
     protected final S state;
     protected final Collection<FunctionBuilder> functionDefinitions = new HashSet<>();
     protected final Collection<EventDefBuilder> eventDefinitions = new HashSet<>();
+    private short buildCount;
 
     Collection<FunctionBuilder> getFunctions() {
         return functionDefinitions;
@@ -115,9 +116,14 @@ public T outputFilter(String filter) {
     }
 
     public S build() {
+        buildCount++;
         return ensureName(state);
     }
 
+    short buildCount() {
+        return buildCount;
+    }
+
     private static int counter;
 
     protected static <T extends DefaultState> T ensureName(T state) {

diff --git a/...low-fluent/src/main/java/org/kie/kogito/serverless/workflow/fluent/TransitionBuilder.java b/...low-fluent/src/main/java/org/kie/kogito/serverless/workflow/fluent/TransitionBuilder.java
@@ -43,8 +43,10 @@ protected TransitionBuilder(T container, WorkflowBuilder workflow, DefaultState
 
     public TransitionBuilder<T> next(StateBuilder<?, ?> stateBuilder) {
         DefaultState state = stateBuilder.build();
-        workflow.addFunctions(stateBuilder.getFunctions());
-        workflow.addEvents(stateBuilder.getEvents());
+        if (stateBuilder.buildCount() == 1) {
+            workflow.addFunctions(stateBuilder.getFunctions());
+            workflow.addEvents(stateBuilder.getEvents());
+        }
         next(state);
         lastState = state;
         return this;