Skip to content

Commit ee02617

Browse files
ricardozaniniricardozanini
authored
Merge pull request #361 from ricardozanini/issue-213
Fix #213 - Make a required property for Retry and verify RetryDef
2 parents 14345dc + fea2703 commit ee02617

File tree

3 files changed

+124
-32
lines changed

3 files changed

+124
-32
lines changed

‎utils/src/main/java/io/serverlessworkflow/utils/WorkflowUtils.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -584,7 +584,7 @@ public static JsonNode mergeNodes(JsonNode mainNode, JsonNode updateNode) {
584584
if (mainNode instanceof ObjectNode) {
585585
// Overwrite field
586586
JsonNode value = updateNode.get(fieldName);
587-
((ObjectNode) mainNode).put(fieldName, value);
587+
((ObjectNode) mainNode).set(fieldName, value);
588588
}
589589
}
590590
}
@@ -601,7 +601,7 @@ public static JsonNode mergeNodes(JsonNode mainNode, JsonNode updateNode) {
601601
* @return original, main node with field added
602602
*/
603603
public static JsonNode addNode(JsonNode mainNode, JsonNode toAddNode, String fieldName) {
604-
((ObjectNode) mainNode).put(fieldName, toAddNode);
604+
((ObjectNode) mainNode).set(fieldName, toAddNode);
605605
return mainNode;
606606
}
607607

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

@@ -628,7 +628,7 @@ public static JsonNode addArray(JsonNode mainNode, ArrayNode toAddArray, String
628628
*/
629629
public static JsonNode addFieldValue(JsonNode mainNode, Object toAddValue, String fieldName) {
630630
ObjectMapper mapper = new ObjectMapper();
631-
((ObjectNode) mainNode).put(fieldName, mapper.valueToTree(toAddValue));
631+
((ObjectNode) mainNode).set(fieldName, mapper.valueToTree(toAddValue));
632632
return mainNode;
633633
}
634634

‎validation/src/main/java/io/serverlessworkflow/validation/WorkflowValidatorImpl.java‎

Lines changed: 55 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import io.serverlessworkflow.api.functions.FunctionDefinition;
2626
import io.serverlessworkflow.api.interfaces.State;
2727
import io.serverlessworkflow.api.interfaces.WorkflowValidator;
28+
import io.serverlessworkflow.api.retry.RetryDefinition;
2829
import io.serverlessworkflow.api.states.*;
2930
import io.serverlessworkflow.api.switchconditions.DataCondition;
3031
import io.serverlessworkflow.api.switchconditions.EventCondition;
@@ -78,7 +79,7 @@ public List<ValidationError> validate() {
7879

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

105+
if (workflow.getRetries() != null && workflow.getRetries().getRetryDefs() != null) {
106+
workflow
107+
.getRetries()
108+
.getRetryDefs()
109+
.forEach(
110+
r -> {
111+
if (r.getName() == null || r.getName().isEmpty()) {
112+
addValidationError(
113+
"Retry name should not be empty", ValidationError.WORKFLOW_VALIDATION);
114+
}
115+
});
116+
}
117+
104118
if (workflow.getStates() == null || workflow.getStates().isEmpty()) {
105119
addValidationError("No states found", ValidationError.WORKFLOW_VALIDATION);
106120
}
@@ -149,7 +163,7 @@ public List<ValidationError> validate() {
149163

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

160174
List<String> eventRefs = onEvents.getEventRefs();
161-
if (eventRefs == null || eventRefs.size() < 1) {
175+
if (eventRefs == null || eventRefs.isEmpty()) {
162176
addValidationError(
163177
"Event State eventsActions has no event refs",
164178
ValidationError.WORKFLOW_VALIDATION);
165179
} else {
166180
for (String eventRef : eventRefs) {
167-
if (!haveEventsDefinition(eventRef, events)) {
181+
if (isMissingEventsDefinition(eventRef, events)) {
168182
addValidationError(
169183
"Event State eventsActions eventRef does not match a declared workflow event definition",
170184
ValidationError.WORKFLOW_VALIDATION);
@@ -177,9 +191,9 @@ public List<ValidationError> validate() {
177191
if (s instanceof SwitchState) {
178192
SwitchState switchState = (SwitchState) s;
179193
if ((switchState.getDataConditions() == null
180-
|| switchState.getDataConditions().size() < 1)
194+
|| switchState.getDataConditions().isEmpty())
181195
&& (switchState.getEventConditions() == null
182-
|| switchState.getEventConditions().size() < 1)) {
196+
|| switchState.getEventConditions().isEmpty())) {
183197
addValidationError(
184198
"Switch state should define either data or event conditions",
185199
ValidationError.WORKFLOW_VALIDATION);
@@ -192,10 +206,10 @@ public List<ValidationError> validate() {
192206
}
193207

194208
if (switchState.getEventConditions() != null
195-
&& switchState.getEventConditions().size() > 0) {
209+
&& !switchState.getEventConditions().isEmpty()) {
196210
List<EventCondition> eventConditions = switchState.getEventConditions();
197211
for (EventCondition ec : eventConditions) {
198-
if (!haveEventsDefinition(ec.getEventRef(), events)) {
212+
if (isMissingEventsDefinition(ec.getEventRef(), events)) {
199213
addValidationError(
200214
"Switch state event condition eventRef does not reference a defined workflow event",
201215
ValidationError.WORKFLOW_VALIDATION);
@@ -207,7 +221,7 @@ public List<ValidationError> validate() {
207221
}
208222

209223
if (switchState.getDataConditions() != null
210-
&& switchState.getDataConditions().size() > 0) {
224+
&& !switchState.getDataConditions().isEmpty()) {
211225
List<DataCondition> dataConditions = switchState.getDataConditions();
212226
for (DataCondition dc : dataConditions) {
213227
if (dc.getEnd() != null) {
@@ -219,7 +233,7 @@ public List<ValidationError> validate() {
219233

220234
if (s instanceof SleepState) {
221235
SleepState sleepState = (SleepState) s;
222-
if (sleepState.getDuration() == null || sleepState.getDuration().length() < 1) {
236+
if (sleepState.getDuration() == null || sleepState.getDuration().isEmpty()) {
223237
addValidationError(
224238
"Sleep state should have a non-empty time delay",
225239
ValidationError.WORKFLOW_VALIDATION);
@@ -260,13 +274,13 @@ public List<ValidationError> validate() {
260274
if (s instanceof CallbackState) {
261275
CallbackState callbackState = (CallbackState) s;
262276

263-
if (!haveEventsDefinition(callbackState.getEventRef(), events)) {
277+
if (isMissingEventsDefinition(callbackState.getEventRef(), events)) {
264278
addValidationError(
265279
"CallbackState event ref does not reference a defined workflow event definition",
266280
ValidationError.WORKFLOW_VALIDATION);
267281
}
268282

269-
if (!haveFunctionDefinition(
283+
if (isMissingFunctionDefinition(
270284
callbackState.getAction().getFunctionRef().getRefName(), functions)) {
271285
addValidationError(
272286
"CallbackState action function ref does not reference a defined workflow function definition",
@@ -316,7 +330,7 @@ private void checkActionsDefinition(
316330
ValidationError.WORKFLOW_VALIDATION);
317331
}
318332

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

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

330-
if (!haveEventsDefinition(action.getEventRef().getTriggerEventRef(), events)) {
344+
if (isMissingEventsDefinition(action.getEventRef().getTriggerEventRef(), events)) {
331345
addValidationError(
332346
String.format(
333347
"State action '%s' trigger event def does not reference an existing workflow event definition",
334348
action.getName()),
335349
ValidationError.WORKFLOW_VALIDATION);
336350
}
337351

338-
if (!haveEventsDefinition(action.getEventRef().getResultEventRef(), events)) {
352+
if (isMissingEventsDefinition(action.getEventRef().getResultEventRef(), events)) {
339353
addValidationError(
340354
String.format(
341355
"State action '%s' results event def does not reference an existing workflow event definition",
342356
action.getName()),
343357
ValidationError.WORKFLOW_VALIDATION);
344358
}
345359
}
360+
361+
if (action.getRetryRef() != null
362+
&& isMissingRetryDefinition(action.getRetryRef(), workflow.getRetries().getRetryDefs())) {
363+
addValidationError(
364+
String.format(
365+
"Operation State action '%s' retryRef does not reference an existing workflow retry definition",
366+
action.getName()),
367+
ValidationError.WORKFLOW_VALIDATION);
368+
}
346369
}
347370
}
348371

349-
private boolean haveFunctionDefinition(String functionName, List<FunctionDefinition> functions) {
372+
private boolean isMissingFunctionDefinition(
373+
String functionName, List<FunctionDefinition> functions) {
350374
if (functions != null) {
351-
FunctionDefinition fun =
352-
functions.stream().filter(f -> f.getName().equals(functionName)).findFirst().orElse(null);
353-
354-
return fun == null ? false : true;
375+
return !functions.stream().anyMatch(f -> f.getName().equals(functionName));
355376
} else {
356-
return false;
377+
return true;
357378
}
358379
}
359380

360-
private boolean haveEventsDefinition(String eventName, List<EventDefinition> events) {
381+
private boolean isMissingEventsDefinition(String eventName, List<EventDefinition> events) {
361382
if (eventName == null) {
362-
return true;
383+
return false;
363384
}
364385
if (events != null) {
365-
EventDefinition eve =
366-
events.stream().filter(e -> e.getName().equals(eventName)).findFirst().orElse(null);
367-
return eve == null ? false : true;
386+
return !events.stream().anyMatch(e -> e.getName().equals(eventName));
368387
} else {
369-
return false;
388+
return true;
370389
}
371390
}
372391

392+
private boolean isMissingRetryDefinition(String retryName, List<RetryDefinition> retries) {
393+
return retries == null
394+
|| !retries.stream().anyMatch(f -> f.getName() != null && f.getName().equals(retryName));
395+
}
396+
373397
private static final Set<String> skipMessages =
374-
Set.of("$.start: string found, object expected", "$.functions: array found, object expected");
398+
Set.of(
399+
"$.start: string found, object expected",
400+
"$.functions: array found, object expected",
401+
"$.retries: array found, object expected");
375402

376403
private void addValidationError(String message, String type) {
377404
if (skipMessages.contains(message)) {

‎validation/src/test/java/io/serverlessworkflow/validation/test/WorkflowValidationTest.java‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,4 +367,69 @@ void testActionDefForEach() {
367367
"State action 'callFn' functionRef does not reference an existing workflow function definition",
368368
validationErrors.get(0).getMessage());
369369
}
370+
371+
/**
372+
* @see <a href="https://github.com/serverlessworkflow/sdk-java/issues/213">Retry definition
373+
* validation doesn't work</a>
374+
*/
375+
@Test
376+
public void testValidateRetry() {
377+
WorkflowValidator workflowValidator = new WorkflowValidatorImpl();
378+
List<ValidationError> validationErrors =
379+
workflowValidator
380+
.setSource(
381+
"{\n"
382+
+ " \"id\": \"workflow_1\",\n"
383+
+ " \"name\": \"workflow_1\",\n"
384+
+ " \"description\": \"workflow_1\",\n"
385+
+ " \"version\": \"1.0\",\n"
386+
+ " \"specVersion\": \"0.8\",\n"
387+
+ " \"start\": \"Task1\",\n"
388+
+ " \"functions\": [\n"
389+
+ " {\n"
390+
+ " \"name\": \"increment\",\n"
391+
+ " \"type\": \"custom\",\n"
392+
+ " \"operation\": \"worker\"\n"
393+
+ " }\n"
394+
+ " ],\n"
395+
+ " \"retries\": [\n"
396+
+ " {\n"
397+
+ " \"maxAttempts\": 3\n"
398+
+ " },\n"
399+
+ " {\n"
400+
+ " \"name\": \"testRetry\" \n"
401+
+ " }\n"
402+
+ " ],\n"
403+
+ " \"states\": [\n"
404+
+ " {\n"
405+
+ " \"name\": \"Task1\",\n"
406+
+ " \"type\": \"operation\",\n"
407+
+ " \"actionMode\": \"sequential\",\n"
408+
+ " \"actions\": [\n"
409+
+ " {\n"
410+
+ " \"functionRef\": {\n"
411+
+ " \"refName\": \"increment\",\n"
412+
+ " \"arguments\": {\n"
413+
+ " \"input\": \"some text\"\n"
414+
+ " }\n"
415+
+ " },\n"
416+
+ " \"retryRef\": \"const\",\n"
417+
+ " \"actionDataFilter\": {\n"
418+
+ " \"toStateData\": \"${ .result }\"\n"
419+
+ " }\n"
420+
+ " }\n"
421+
+ " ],\n"
422+
+ " \"end\": true\n"
423+
+ " }\n"
424+
+ " ]\n"
425+
+ "}")
426+
.validate();
427+
428+
Assertions.assertNotNull(validationErrors);
429+
Assertions.assertEquals(2, validationErrors.size());
430+
Assertions.assertEquals("Retry name should not be empty", validationErrors.get(0).getMessage());
431+
Assertions.assertEquals(
432+
"Operation State action 'null' retryRef does not reference an existing workflow retry definition",
433+
validationErrors.get(1).getMessage());
434+
}
370435
}

0 commit comments

Comments
 (0)