From 79b1fdff352e724b7a516df49e7cbd1440844c44 Mon Sep 17 00:00:00 2001 From: Francisco Javier Tirado Sarti <65240126+fjtirado@users.noreply.github.com> Date: Wed, 13 Nov 2024 16:54:44 +0100 Subject: [PATCH] [Fix #3772] Duplicate function error (#3774) --- .../validation/WorkflowValidator.java | 11 ++++++++++ .../utils/ServerlessWorkflowUtils.java | 20 +++++++++++++++++++ .../workflow/utils/WorkflowUtilsTest.java | 7 +++++++ .../StaticWorkflowApplicationTest.java | 5 +++-- .../src/test/resources/wrong.sw.json | 5 +++++ .../workflow/fluent/StateBuilder.java | 6 ++++++ .../workflow/fluent/TransitionBuilder.java | 6 ++++-- 7 files changed, 56 insertions(+), 4 deletions(-) diff --git a/kogito-serverless-workflow/kogito-serverless-workflow-builder/src/main/java/org/kie/kogito/serverless/workflow/parser/handlers/validation/WorkflowValidator.java b/kogito-serverless-workflow/kogito-serverless-workflow-builder/src/main/java/org/kie/kogito/serverless/workflow/parser/handlers/validation/WorkflowValidator.java index 82de55e0af7..21cd5af355c 100644 --- a/kogito-serverless-workflow/kogito-serverless-workflow-builder/src/main/java/org/kie/kogito/serverless/workflow/parser/handlers/validation/WorkflowValidator.java +++ b/kogito-serverless-workflow/kogito-serverless-workflow-builder/src/main/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 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/kogito-serverless-workflow/kogito-serverless-workflow-builder/src/main/java/org/kie/kogito/serverless/workflow/utils/ServerlessWorkflowUtils.java b/kogito-serverless-workflow/kogito-serverless-workflow-builder/src/main/java/org/kie/kogito/serverless/workflow/utils/ServerlessWorkflowUtils.java index e21330ef2e1..13a04e970b2 100644 --- a/kogito-serverless-workflow/kogito-serverless-workflow-builder/src/main/java/org/kie/kogito/serverless/workflow/utils/ServerlessWorkflowUtils.java +++ b/kogito-serverless-workflow/kogito-serverless-workflow-builder/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 Map findDuplicates(List items, Function converter) { + if (items == null) { + return Map.of(); + } + Map duplicates = new LinkedHashMap<>(); + Set 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/kogito-serverless-workflow/kogito-serverless-workflow-builder/src/test/java/org/kie/kogito/serverless/workflow/utils/WorkflowUtilsTest.java b/kogito-serverless-workflow/kogito-serverless-workflow-builder/src/test/java/org/kie/kogito/serverless/workflow/utils/WorkflowUtilsTest.java index 68749171037..a2c8b9dcb43 100644 --- a/kogito-serverless-workflow/kogito-serverless-workflow-builder/src/test/java/org/kie/kogito/serverless/workflow/utils/WorkflowUtilsTest.java +++ b/kogito-serverless-workflow/kogito-serverless-workflow-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/kogito-serverless-workflow/kogito-serverless-workflow-executor-core/src/test/java/org/kie/kogito/serverless/workflow/executor/StaticWorkflowApplicationTest.java b/kogito-serverless-workflow/kogito-serverless-workflow-executor-core/src/test/java/org/kie/kogito/serverless/workflow/executor/StaticWorkflowApplicationTest.java index bed9c81d532..1c5612782e6 100644 --- a/kogito-serverless-workflow/kogito-serverless-workflow-executor-core/src/test/java/org/kie/kogito/serverless/workflow/executor/StaticWorkflowApplicationTest.java +++ b/kogito-serverless-workflow/kogito-serverless-workflow-executor-core/src/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/kogito-serverless-workflow/kogito-serverless-workflow-executor-core/src/test/resources/wrong.sw.json b/kogito-serverless-workflow/kogito-serverless-workflow-executor-core/src/test/resources/wrong.sw.json index 41c48f309a9..848da0b48e3 100644 --- a/kogito-serverless-workflow/kogito-serverless-workflow-executor-core/src/test/resources/wrong.sw.json +++ b/kogito-serverless-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/kogito-serverless-workflow/kogito-serverless-workflow-fluent/src/main/java/org/kie/kogito/serverless/workflow/fluent/StateBuilder.java b/kogito-serverless-workflow/kogito-serverless-workflow-fluent/src/main/java/org/kie/kogito/serverless/workflow/fluent/StateBuilder.java index ca01f32307a..21460baf12b 100644 --- a/kogito-serverless-workflow/kogito-serverless-workflow-fluent/src/main/java/org/kie/kogito/serverless/workflow/fluent/StateBuilder.java +++ b/kogito-serverless-workflow/kogito-serverless-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 functionDefinitions = new HashSet<>(); protected final Collection eventDefinitions = new HashSet<>(); + private short buildCount; Collection 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 ensureName(T state) { diff --git a/kogito-serverless-workflow/kogito-serverless-workflow-fluent/src/main/java/org/kie/kogito/serverless/workflow/fluent/TransitionBuilder.java b/kogito-serverless-workflow/kogito-serverless-workflow-fluent/src/main/java/org/kie/kogito/serverless/workflow/fluent/TransitionBuilder.java index d6736245a19..10ad1020f2f 100644 --- a/kogito-serverless-workflow/kogito-serverless-workflow-fluent/src/main/java/org/kie/kogito/serverless/workflow/fluent/TransitionBuilder.java +++ b/kogito-serverless-workflow/kogito-serverless-workflow-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 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;