Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix #3733] Collecting more than one error message #3756

Merged
merged 3 commits into from
Oct 31, 2024
Merged

[Fix #3733] Collecting more than one error message #3756

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
50926f3
[Fix #3733] Collecting more than one error message
fjtirado Oct 29, 2024
44e8f2f
[Fix #3733] Including process validation check
fjtirado Oct 30, 2024
666b2b3
[Fix #3733] Adding transition check
fjtirado Oct 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions jbpm/jbpm-flow/src/main/java/org/jbpm/ruleflow/core/validation/RuleFlowProcessValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,7 @@ public static RuleFlowProcessValidator getInstance() {
return INSTANCE;
}

public ProcessValidationError[] validateProcess(final RuleFlowProcess process) {
final List<ProcessValidationError> errors = new ArrayList<>();

public List<ProcessValidationError> validateProcess(final RuleFlowProcess process, List<ProcessValidationError> errors) {
if (process.getName() == null) {
errors.add(new ProcessValidationErrorImpl(process,
"Process has no name."));
Expand Down Expand Up @@ -151,6 +149,12 @@ public ProcessValidationError[] validateProcess(final RuleFlowProcess process) {
errors,
process);

return errors;

}

public ProcessValidationError[] validateProcess(final RuleFlowProcess process) {
final List<ProcessValidationError> errors = validateProcess(process, new ArrayList<>());
return errors.toArray(new ProcessValidationError[errors.size()]);
}

Expand Down
21 changes: 14 additions & 7 deletions ...va/org/kie/kogito/serverless/workflow/operationid/AbstractWorkflowOperationIdFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public abstract class AbstractWorkflowOperationIdFactory implements WorkflowOper

@Override
public WorkflowOperationId from(Workflow workflow, FunctionDefinition function, Optional<ParserContext> context) {
ActionResource actionResource = ActionResourceFactory.getActionResource(function);
ActionResource actionResource = ActionResourceFactory.getActionResource(function, context);
Optional<String> convertedUri = convertURI(workflow, context, actionResource.getUri());
final String fileName;
final String uri;
Expand All @@ -56,14 +56,19 @@ public WorkflowOperationId from(Workflow workflow, FunctionDefinition function,
fileName = getFileName(workflow, function, context, uri, actionResource.getOperation(), actionResource.getService());
}
if (fileName == null || fileName.isBlank()) {
throw new IllegalArgumentException(
format("Empty file name for function '%s', please review uri '%s' or consider using a different strategy defined in the kogito.sw.operationIdStrategy property",
function.getName(), uri));
String msg = format("Empty file name for function '%s', please review uri '%s' or consider using a different strategy defined in the kogito.sw.operationIdStrategy property",
function.getName(), uri);
context.ifPresentOrElse(c -> c.addValidationError(msg), () -> {
throw new IllegalArgumentException(msg);
});
}
String packageName = onlyChars(removeExt(fileName.toLowerCase()));
if (packageName.isBlank()) {
throw new IllegalArgumentException(
format("Empty package for file '%s'. A file name should contain at least one letter which is not part of the extension", fileName));
String msg =
format("Empty package for file '%s'. A file name should contain at least one letter which is not part of the extension", fileName);
context.ifPresentOrElse(c -> c.addValidationError(msg), () -> {
throw new IllegalArgumentException(msg);
});
}
return new WorkflowOperationId(uri, actionResource.getOperation(), actionResource.getService(), fileName, packageName);
}
Expand All @@ -80,7 +85,9 @@ private JsonNode getUriDefinitions(Workflow workflow, Optional<ParserContext> co
try {
definitions = uri == null ? NullNode.instance : ObjectMapperFactory.get().readTree(readBytes(uri, workflow, context));
} catch (IOException e) {
throw new UncheckedIOException(e);
context.ifPresentOrElse(c -> c.addValidationError(e.getMessage()), () -> {
throw new UncheckedIOException(e);
});
}
uriDefinitions.setDefinitions(definitions);
}
Expand Down
12 changes: 10 additions & 2 deletions ...rkflow-builder/src/main/java/org/kie/kogito/serverless/workflow/parser/ParserContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,9 @@ public class ParserContext {
private final NodeIdGenerator idGenerator;
private final WorkflowOperationIdFactory operationIdFactory;
private final KogitoBuildContext context;
private final Collection<GeneratedFile> generatedFiles;
private final Collection<GeneratedFile> generatedFiles = new ArrayList<>();
private final AsyncInfoResolver asyncInfoResolver;
private final Collection<String> validationErrors = new ArrayList<>();

public static final String ASYNC_CONVERTER_KEY = "asyncInfoConverter";

Expand All @@ -60,7 +61,6 @@ public ParserContext(NodeIdGenerator idGenerator, RuleFlowProcessFactory factory
this.context = context;
this.operationIdFactory = operationIdFactory;
this.asyncInfoResolver = asyncInfoResolver;
this.generatedFiles = new ArrayList<>();
}

public void add(StateHandler<?> stateHandler) {
Expand Down Expand Up @@ -114,4 +114,12 @@ public void setCompensation() {
public KogitoBuildContext getContext() {
return context;
}

public void addValidationError(String message) {
validationErrors.add(message);
}

public Collection<String> validationErrors() {
return validationErrors;
}
}
22 changes: 17 additions & 5 deletions ...der/src/main/java/org/kie/kogito/serverless/workflow/parser/ServerlessWorkflowParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,24 +20,28 @@

import java.io.IOException;
import java.io.Reader;
import java.io.UncheckedIOException;
import java.net.URI;
import java.net.URL;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;

import org.jbpm.process.core.datatype.impl.type.ObjectDataType;
import org.jbpm.process.core.validation.ProcessValidationError;
import org.jbpm.process.core.validation.impl.ProcessValidationErrorImpl;
import org.jbpm.ruleflow.core.Metadata;
import org.jbpm.ruleflow.core.RuleFlowProcessFactory;
import org.jbpm.ruleflow.core.validation.RuleFlowProcessValidator;
import org.jbpm.workflow.core.WorkflowModelValidator;
import org.kie.kogito.codegen.api.GeneratedInfo;
import org.kie.kogito.codegen.api.context.KogitoBuildContext;
import org.kie.kogito.internal.process.runtime.KogitoWorkflowProcess;
import org.kie.kogito.internal.utils.ConversionUtils;
import org.kie.kogito.jackson.utils.ObjectMapperFactory;
import org.kie.kogito.process.validation.ValidationException;
import org.kie.kogito.serverless.workflow.SWFConstants;
import org.kie.kogito.serverless.workflow.extensions.OutputSchema;
import org.kie.kogito.serverless.workflow.operationid.WorkflowOperationIdFactoryProvider;
Expand Down Expand Up @@ -121,7 +125,7 @@ private ServerlessWorkflowParser(Workflow workflow, KogitoBuildContext context)
}

private GeneratedInfo<KogitoWorkflowProcess> parseProcess() {
WorkflowValidator.validateStart(workflow);

RuleFlowProcessFactory factory = RuleFlowProcessFactory.createProcess(workflow.getId(), !workflow.isKeepActive())
.name(workflow.getName() == null ? DEFAULT_NAME : workflow.getName())
.version(workflow.getVersion() == null ? DEFAULT_VERSION : workflow.getVersion())
Expand All @@ -134,6 +138,7 @@ private GeneratedInfo<KogitoWorkflowProcess> parseProcess() {
.type(KogitoWorkflowProcess.SW_TYPE);
ParserContext parserContext =
new ParserContext(idGenerator, factory, context, WorkflowOperationIdFactoryProvider.getFactory(context.getApplicationProperty(WorkflowOperationIdFactoryProvider.PROPERTY_NAME)));
WorkflowValidator.validateStart(workflow, parserContext);
modelValidator(parserContext, Optional.ofNullable(workflow.getDataInputSchema())).ifPresent(factory::inputValidator);
modelValidator(parserContext, ServerlessWorkflowUtils.getExtension(workflow, OutputSchema.class).map(OutputSchema::getOutputSchema)).ifPresent(factory::outputValidator);
loadConstants(factory, parserContext);
Expand All @@ -145,6 +150,7 @@ private GeneratedInfo<KogitoWorkflowProcess> parseProcess() {
handlers.forEach(StateHandler::handleState);
handlers.forEach(StateHandler::handleTransitions);
handlers.forEach(StateHandler::handleConnections);

if (parserContext.isCompensation()) {
factory.metaData(Metadata.COMPENSATION, true);
factory.metaData(Metadata.COMPENSATE_WHEN_ABORTED, true);
Expand All @@ -170,8 +176,13 @@ private GeneratedInfo<KogitoWorkflowProcess> parseProcess() {
if (!annotations.isEmpty()) {
factory.metaData(Metadata.ANNOTATIONS, annotations);
}

return new GeneratedInfo<>(factory.validate().getProcess(), parserContext.generatedFiles());
factory.link();
List<ProcessValidationError> errors = RuleFlowProcessValidator.getInstance().validateProcess(factory.getProcess(), new ArrayList<>());
parserContext.validationErrors().forEach(m -> errors.add(new ProcessValidationErrorImpl(factory.getProcess(), m)));
if (!errors.isEmpty()) {
throw new ValidationException(factory.getProcess().getId(), errors);
}
return new GeneratedInfo<>(factory.getProcess(), parserContext.generatedFiles());
}

private Optional<WorkflowModelValidator> modelValidator(ParserContext parserContext, Optional<DataInputSchema> schema) {
Expand All @@ -194,7 +205,8 @@ private void loadConstants(RuleFlowProcessFactory factory, ParserContext parserC
try {
constants.setConstantsDef(ObjectMapperFactory.get().readValue(readBytes(constants.getRefValue(), workflow, parserContext), JsonNode.class));
} catch (IOException e) {
throw new UncheckedIOException("Invalid file " + constants.getRefValue(), e);
parserContext.addValidationError("Invalid file " + constants.getRefValue() + e);
return;
}
}
factory.metaData(Metadata.CONSTANTS, constants.getConstantsDef());
Expand Down
17 changes: 11 additions & 6 deletions ...der/src/main/java/org/kie/kogito/serverless/workflow/parser/handlers/ActionNodeUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,21 @@ public class ActionNodeUtils {
return embeddedSubProcess.actionNode(context.newId()).name(functionDef.getName());
}

public static void checkArgs(FunctionRef functionRef, String... requiredArgs) {
public static boolean checkArgs(ParserContext context, FunctionRef functionRef, String... requiredArgs) {
JsonNode args = functionRef.getArguments();
boolean isOk = true;
gmunozfe marked this conversation as resolved.
Show resolved Hide resolved
if (args == null) {
throw new IllegalArgumentException("Arguments cannot be null for function " + functionRef.getRefName());
}
for (String arg : requiredArgs) {
if (!args.has(arg)) {
throw new IllegalArgumentException("Missing mandatory " + arg + " argument for function " + functionRef.getRefName());
context.addValidationError("Arguments cannot be null for function " + functionRef.getRefName());
isOk = false;
} else {
for (String arg : requiredArgs) {
if (!args.has(arg)) {
context.addValidationError("Missing mandatory " + arg + " argument for function " + functionRef.getRefName());
isOk = false;
}
}
}
return isOk;
gmunozfe marked this conversation as resolved.
Show resolved Hide resolved
}

private ActionNodeUtils() {
Expand Down
38 changes: 25 additions & 13 deletions ...c/main/java/org/kie/kogito/serverless/workflow/parser/handlers/ActionResourceFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@
package org.kie.kogito.serverless.workflow.parser.handlers;

import java.util.Map;
import java.util.function.Function;
import java.util.Optional;
import java.util.function.BiFunction;

import org.kie.kogito.serverless.workflow.parser.ParserContext;

import io.serverlessworkflow.api.functions.FunctionDefinition;
import io.serverlessworkflow.api.functions.FunctionDefinition.Type;
Expand All @@ -28,38 +31,47 @@

public class ActionResourceFactory {

private static final Map<Type, Function<String, ActionResource>> map =
private static final Map<Type, BiFunction<String, Optional<ParserContext>, ActionResource>> map =
Map.of(FunctionDefinition.Type.REST, ActionResourceFactory::justOperation, FunctionDefinition.Type.ASYNCAPI, ActionResourceFactory::justOperation, FunctionDefinition.Type.RPC,
ActionResourceFactory::withService);

private static ActionResource justOperation(String operationStr) {
String[] tokens = getTokens(operationStr, 2);
private static ActionResource justOperation(String operationStr, Optional<ParserContext> context) {
String[] tokens = getTokens(operationStr, 2, context);
return new ActionResource(tokens[0], tokens[1], null);
}

private static ActionResource withService(String operationStr) {
String[] tokens = getTokens(operationStr, 3);
private static ActionResource withService(String operationStr, Optional<ParserContext> context) {
String[] tokens = getTokens(operationStr, 3, context);
return new ActionResource(tokens[0], tokens[2], tokens[1]);
}

private static String[] getTokens(String operationStr, int expectedTokens) {
private static String[] getTokens(String operationStr, int expectedTokens, Optional<ParserContext> context) {
String[] tokens = operationStr.split(OPERATION_SEPARATOR);
if (tokens.length != expectedTokens) {
throw new IllegalArgumentException(String.format("%s should have just %d %s", operationStr, expectedTokens - 1, OPERATION_SEPARATOR));
String msg = String.format("%s should have just %d %s", operationStr, expectedTokens - 1, OPERATION_SEPARATOR);
context.ifPresentOrElse(c -> c.addValidationError(msg), () -> {
throw new IllegalArgumentException(msg);
});
}
return tokens;
}

public static ActionResource getActionResource(FunctionDefinition function) {
Function<String, ActionResource> factory = map.get(function.getType());
public static ActionResource getActionResource(FunctionDefinition function, Optional<ParserContext> context) {
BiFunction<String, Optional<ParserContext>, ActionResource> factory = map.get(function.getType());
if (factory == null) {
throw new UnsupportedOperationException(function.getType() + " does not support action resources");
String msg = function.getType() + " does not support action resources";
context.ifPresentOrElse(c -> c.addValidationError(msg), () -> {
throw new UnsupportedOperationException(msg);
});
}
String operation = function.getOperation();
if (operation == null) {
throw new IllegalArgumentException("operation string must not be null for function " + function.getName());
String msg = "operation string must not be null for function " + function.getName();
context.ifPresentOrElse(c -> c.addValidationError(msg), () -> {
throw new IllegalArgumentException(msg);
});
}
return factory.apply(operation);
return factory.apply(operation, context);
}

private ActionResourceFactory() {
Expand Down
4 changes: 2 additions & 2 deletions .../java/org/kie/kogito/serverless/workflow/parser/handlers/CompositeContextNodeHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ private MakeNodeResult processActionFilter(RuleFlowNodeContainerFactory<?, ?> em
return filterAndMergeNode(embeddedSubProcess, collectVar, fromExpr, resultExpr, toExpr, useData, shouldMerge,
(factory, inputVar, outputVar) -> addActionMetadata(getActionNode(factory, action.getSubFlowRef(), inputVar, outputVar), action));
} else {
throw new IllegalArgumentException("Action node " + action.getName() + " of state " + state.getName() + " does not have function or event defined");
return faultyNodeResult(embeddedSubProcess, "Action node " + action.getName() + " of state " + state.getName() + " does not have function or event defined");
}
}

Expand Down Expand Up @@ -199,7 +199,7 @@ private TimerNodeFactory<?> createTimerNode(RuleFlowNodeContainerFactory<?, ?> f
.findFirst()
.map(functionDef -> fromFunctionDefinition(embeddedSubProcess, functionDef, functionRef, varInfo))
.or(() -> fromPredefinedFunction(embeddedSubProcess, functionRef, varInfo))
.orElseThrow(() -> new IllegalArgumentException("Cannot find function " + functionName));
.orElseGet(() -> faultyNode(embeddedSubProcess, "Cannot find function " + functionName));
}

private Stream<FunctionDefinition> getFunctionDefStream() {
Expand Down
Loading
Loading