Skip to content

Commit

Permalink
[Fix #3772] Duplicate function error (#3774)
Browse files Browse the repository at this point in the history
  • Loading branch information
fjtirado authored Nov 13, 2024
1 parent 5876428 commit 34a4323
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -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());
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@
"name": "logInfo",
"type": "custom",
"operation": "sysout:INFO"
},
{
"name": "pushData",
"type": "custom",
"operation": "script:python:print('javierito')"
}
],
"errors":[],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 34a4323

Please sign in to comment.