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.

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 #213 - Make a required property for Retry and verify RetryDef #361

Merged
merged 3 commits into from
May 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,7 @@ public static JsonNode mergeNodes(JsonNode mainNode, JsonNode updateNode) {
if (mainNode instanceof ObjectNode) {
// Overwrite field
JsonNode value = updateNode.get(fieldName);
((ObjectNode) mainNode).put(fieldName, value);
((ObjectNode) mainNode).set(fieldName, value);
}
}
}
Expand All @@ -601,7 +601,7 @@ public static JsonNode mergeNodes(JsonNode mainNode, JsonNode updateNode) {
* @return original, main node with field added
*/
public static JsonNode addNode(JsonNode mainNode, JsonNode toAddNode, String fieldName) {
((ObjectNode) mainNode).put(fieldName, toAddNode);
((ObjectNode) mainNode).set(fieldName, toAddNode);
return mainNode;
}

Expand All @@ -614,7 +614,7 @@ public static JsonNode addNode(JsonNode mainNode, JsonNode toAddNode, String fie
* @return original, main node with array added
*/
public static JsonNode addArray(JsonNode mainNode, ArrayNode toAddArray, String arrayName) {
((ObjectNode) mainNode).put(arrayName, toAddArray);
((ObjectNode) mainNode).set(arrayName, toAddArray);
return mainNode;
}

Expand All @@ -628,7 +628,7 @@ public static JsonNode addArray(JsonNode mainNode, ArrayNode toAddArray, String
*/
public static JsonNode addFieldValue(JsonNode mainNode, Object toAddValue, String fieldName) {
ObjectMapper mapper = new ObjectMapper();
((ObjectNode) mainNode).put(fieldName, mapper.valueToTree(toAddValue));
((ObjectNode) mainNode).set(fieldName, mapper.valueToTree(toAddValue));
return mainNode;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import io.serverlessworkflow.api.functions.FunctionDefinition;
import io.serverlessworkflow.api.interfaces.State;
import io.serverlessworkflow.api.interfaces.WorkflowValidator;
import io.serverlessworkflow.api.retry.RetryDefinition;
import io.serverlessworkflow.api.states.*;
import io.serverlessworkflow.api.switchconditions.DataCondition;
import io.serverlessworkflow.api.switchconditions.EventCondition;
Expand Down Expand Up @@ -78,7 +79,7 @@ public List<ValidationError> validate() {

// if there are schema validation errors
// there is no point of doing the workflow validation
if (validationErrors.size() > 0) {
if (!validationErrors.isEmpty()) {
return validationErrors;
} else if (workflow == null) {
workflow = Workflow.fromSource(source);
Expand All @@ -101,6 +102,19 @@ public List<ValidationError> validate() {
"Workflow version should not be empty", ValidationError.WORKFLOW_VALIDATION);
}

if (workflow.getRetries() != null && workflow.getRetries().getRetryDefs() != null) {
workflow
.getRetries()
.getRetryDefs()
.forEach(
r -> {
if (r.getName() == null || r.getName().isEmpty()) {
addValidationError(
"Retry name should not be empty", ValidationError.WORKFLOW_VALIDATION);
}
});
}

if (workflow.getStates() == null || workflow.getStates().isEmpty()) {
addValidationError("No states found", ValidationError.WORKFLOW_VALIDATION);
}
Expand Down Expand Up @@ -149,7 +163,7 @@ public List<ValidationError> validate() {

if (s instanceof EventState) {
EventState eventState = (EventState) s;
if (eventState.getOnEvents() == null || eventState.getOnEvents().size() < 1) {
if (eventState.getOnEvents() == null || eventState.getOnEvents().isEmpty()) {
addValidationError(
"Event State has no eventActions defined",
ValidationError.WORKFLOW_VALIDATION);
Expand All @@ -158,13 +172,13 @@ public List<ValidationError> validate() {
for (OnEvents onEvents : eventsActionsList) {

List<String> eventRefs = onEvents.getEventRefs();
if (eventRefs == null || eventRefs.size() < 1) {
if (eventRefs == null || eventRefs.isEmpty()) {
addValidationError(
"Event State eventsActions has no event refs",
ValidationError.WORKFLOW_VALIDATION);
} else {
for (String eventRef : eventRefs) {
if (!haveEventsDefinition(eventRef, events)) {
if (isMissingEventsDefinition(eventRef, events)) {
addValidationError(
"Event State eventsActions eventRef does not match a declared workflow event definition",
ValidationError.WORKFLOW_VALIDATION);
Expand All @@ -177,9 +191,9 @@ public List<ValidationError> validate() {
if (s instanceof SwitchState) {
SwitchState switchState = (SwitchState) s;
if ((switchState.getDataConditions() == null
|| switchState.getDataConditions().size() < 1)
|| switchState.getDataConditions().isEmpty())
&& (switchState.getEventConditions() == null
|| switchState.getEventConditions().size() < 1)) {
|| switchState.getEventConditions().isEmpty())) {
addValidationError(
"Switch state should define either data or event conditions",
ValidationError.WORKFLOW_VALIDATION);
Expand All @@ -192,10 +206,10 @@ public List<ValidationError> validate() {
}

if (switchState.getEventConditions() != null
&& switchState.getEventConditions().size() > 0) {
&& !switchState.getEventConditions().isEmpty()) {
List<EventCondition> eventConditions = switchState.getEventConditions();
for (EventCondition ec : eventConditions) {
if (!haveEventsDefinition(ec.getEventRef(), events)) {
if (isMissingEventsDefinition(ec.getEventRef(), events)) {
addValidationError(
"Switch state event condition eventRef does not reference a defined workflow event",
ValidationError.WORKFLOW_VALIDATION);
Expand All @@ -207,7 +221,7 @@ public List<ValidationError> validate() {
}

if (switchState.getDataConditions() != null
&& switchState.getDataConditions().size() > 0) {
&& !switchState.getDataConditions().isEmpty()) {
List<DataCondition> dataConditions = switchState.getDataConditions();
for (DataCondition dc : dataConditions) {
if (dc.getEnd() != null) {
Expand All @@ -219,7 +233,7 @@ public List<ValidationError> validate() {

if (s instanceof SleepState) {
SleepState sleepState = (SleepState) s;
if (sleepState.getDuration() == null || sleepState.getDuration().length() < 1) {
if (sleepState.getDuration() == null || sleepState.getDuration().isEmpty()) {
addValidationError(
"Sleep state should have a non-empty time delay",
ValidationError.WORKFLOW_VALIDATION);
Expand Down Expand Up @@ -260,13 +274,13 @@ public List<ValidationError> validate() {
if (s instanceof CallbackState) {
CallbackState callbackState = (CallbackState) s;

if (!haveEventsDefinition(callbackState.getEventRef(), events)) {
if (isMissingEventsDefinition(callbackState.getEventRef(), events)) {
addValidationError(
"CallbackState event ref does not reference a defined workflow event definition",
ValidationError.WORKFLOW_VALIDATION);
}

if (!haveFunctionDefinition(
if (isMissingFunctionDefinition(
callbackState.getAction().getFunctionRef().getRefName(), functions)) {
addValidationError(
"CallbackState action function ref does not reference a defined workflow function definition",
Expand Down Expand Up @@ -316,7 +330,7 @@ private void checkActionsDefinition(
ValidationError.WORKFLOW_VALIDATION);
}

if (!haveFunctionDefinition(action.getFunctionRef().getRefName(), functions)) {
if (isMissingFunctionDefinition(action.getFunctionRef().getRefName(), functions)) {
addValidationError(
String.format(
"State action '%s' functionRef does not reference an existing workflow function definition",
Expand All @@ -327,51 +341,64 @@ private void checkActionsDefinition(

if (action.getEventRef() != null) {

if (!haveEventsDefinition(action.getEventRef().getTriggerEventRef(), events)) {
if (isMissingEventsDefinition(action.getEventRef().getTriggerEventRef(), events)) {
addValidationError(
String.format(
"State action '%s' trigger event def does not reference an existing workflow event definition",
action.getName()),
ValidationError.WORKFLOW_VALIDATION);
}

if (!haveEventsDefinition(action.getEventRef().getResultEventRef(), events)) {
if (isMissingEventsDefinition(action.getEventRef().getResultEventRef(), events)) {
addValidationError(
String.format(
"State action '%s' results event def does not reference an existing workflow event definition",
action.getName()),
ValidationError.WORKFLOW_VALIDATION);
}
}

if (action.getRetryRef() != null
&& isMissingRetryDefinition(action.getRetryRef(), workflow.getRetries().getRetryDefs())) {
addValidationError(
String.format(
"Operation State action '%s' retryRef does not reference an existing workflow retry definition",
action.getName()),
ValidationError.WORKFLOW_VALIDATION);
}
}
}

private boolean haveFunctionDefinition(String functionName, List<FunctionDefinition> functions) {
private boolean isMissingFunctionDefinition(
String functionName, List<FunctionDefinition> functions) {
if (functions != null) {
FunctionDefinition fun =
functions.stream().filter(f -> f.getName().equals(functionName)).findFirst().orElse(null);

return fun == null ? false : true;
return !functions.stream().anyMatch(f -> f.getName().equals(functionName));
} else {
return false;
return true;
}
}

private boolean haveEventsDefinition(String eventName, List<EventDefinition> events) {
private boolean isMissingEventsDefinition(String eventName, List<EventDefinition> events) {
if (eventName == null) {
return true;
return false;
}
if (events != null) {
EventDefinition eve =
events.stream().filter(e -> e.getName().equals(eventName)).findFirst().orElse(null);
return eve == null ? false : true;
return !events.stream().anyMatch(e -> e.getName().equals(eventName));
} else {
return false;
return true;
}
}

private boolean isMissingRetryDefinition(String retryName, List<RetryDefinition> retries) {
return retries == null
|| !retries.stream().anyMatch(f -> f.getName() != null && f.getName().equals(retryName));
}

private static final Set<String> skipMessages =
Set.of("$.start: string found, object expected", "$.functions: array found, object expected");
Set.of(
"$.start: string found, object expected",
"$.functions: array found, object expected",
"$.retries: array found, object expected");

private void addValidationError(String message, String type) {
if (skipMessages.contains(message)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -367,4 +367,69 @@ void testActionDefForEach() {
"State action 'callFn' functionRef does not reference an existing workflow function definition",
validationErrors.get(0).getMessage());
}

/**
* @see <a href="https://github.com/serverlessworkflow/sdk-java/issues/213">Retry definition
* validation doesn't work</a>
*/
@Test
public void testValidateRetry() {
WorkflowValidator workflowValidator = new WorkflowValidatorImpl();
List<ValidationError> validationErrors =
workflowValidator
.setSource(
"{\n"
+ " \"id\": \"workflow_1\",\n"
+ " \"name\": \"workflow_1\",\n"
+ " \"description\": \"workflow_1\",\n"
+ " \"version\": \"1.0\",\n"
+ " \"specVersion\": \"0.8\",\n"
+ " \"start\": \"Task1\",\n"
+ " \"functions\": [\n"
+ " {\n"
+ " \"name\": \"increment\",\n"
+ " \"type\": \"custom\",\n"
+ " \"operation\": \"worker\"\n"
+ " }\n"
+ " ],\n"
+ " \"retries\": [\n"
+ " {\n"
+ " \"maxAttempts\": 3\n"
+ " },\n"
+ " {\n"
+ " \"name\": \"testRetry\" \n"
+ " }\n"
+ " ],\n"
+ " \"states\": [\n"
+ " {\n"
+ " \"name\": \"Task1\",\n"
+ " \"type\": \"operation\",\n"
+ " \"actionMode\": \"sequential\",\n"
+ " \"actions\": [\n"
+ " {\n"
+ " \"functionRef\": {\n"
+ " \"refName\": \"increment\",\n"
+ " \"arguments\": {\n"
+ " \"input\": \"some text\"\n"
+ " }\n"
+ " },\n"
+ " \"retryRef\": \"const\",\n"
+ " \"actionDataFilter\": {\n"
+ " \"toStateData\": \"${ .result }\"\n"
+ " }\n"
+ " }\n"
+ " ],\n"
+ " \"end\": true\n"
+ " }\n"
+ " ]\n"
+ "}")
.validate();

Assertions.assertNotNull(validationErrors);
Assertions.assertEquals(2, validationErrors.size());
Assertions.assertEquals("Retry name should not be empty", validationErrors.get(0).getMessage());
Assertions.assertEquals(
"Operation State action 'null' retryRef does not reference an existing workflow retry definition",
validationErrors.get(1).getMessage());
}
}
Loading