Skip to content

Commit 500c31b

Browse files
ricardozaniniricardozanini
committed
Fix #213 - Make a required property for Retry and verify RetryDef
Signed-off-by: Ricardo Zanini <[email protected]>
1 parent efd800b commit 500c31b

File tree

3 files changed

+138
-32
lines changed

3 files changed

+138
-32
lines changed

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

+4-4
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

+69-28
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
}
@@ -154,7 +168,7 @@ public List<ValidationError> validate() {
154168
ValidationError.WORKFLOW_VALIDATION);
155169
}
156170

157-
if (!haveFunctionDefinition(
171+
if (isMissingFunctionDefinition(
158172
action.getFunctionRef().getRefName(), functions)) {
159173
addValidationError(
160174
"Operation State action functionRef does not reference an existing workflow function definition",
@@ -164,25 +178,36 @@ public List<ValidationError> validate() {
164178

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

167-
if (!haveEventsDefinition(
181+
if (isMissingEventsDefinition(
168182
action.getEventRef().getTriggerEventRef(), events)) {
169183
addValidationError(
170184
"Operation State action trigger event def does not reference an existing workflow event definition",
171185
ValidationError.WORKFLOW_VALIDATION);
172186
}
173187

174-
if (!haveEventsDefinition(action.getEventRef().getResultEventRef(), events)) {
188+
if (isMissingEventsDefinition(
189+
action.getEventRef().getResultEventRef(), events)) {
175190
addValidationError(
176191
"Operation State action results event def does not reference an existing workflow event definition",
177192
ValidationError.WORKFLOW_VALIDATION);
178193
}
179194
}
195+
196+
if (action.getRetryRef() != null
197+
&& isMissingRetryDefinition(
198+
action.getRetryRef(), workflow.getRetries().getRetryDefs())) {
199+
addValidationError(
200+
String.format(
201+
"Operation State action '%s' retryRef does not reference an existing workflow retry definition",
202+
action.getName()),
203+
ValidationError.WORKFLOW_VALIDATION);
204+
}
180205
}
181206
}
182207

183208
if (s instanceof EventState) {
184209
EventState eventState = (EventState) s;
185-
if (eventState.getOnEvents() == null || eventState.getOnEvents().size() < 1) {
210+
if (eventState.getOnEvents() == null || eventState.getOnEvents().isEmpty()) {
186211
addValidationError(
187212
"Event State has no eventActions defined",
188213
ValidationError.WORKFLOW_VALIDATION);
@@ -191,13 +216,13 @@ public List<ValidationError> validate() {
191216
for (OnEvents onEvents : eventsActionsList) {
192217

193218
List<String> eventRefs = onEvents.getEventRefs();
194-
if (eventRefs == null || eventRefs.size() < 1) {
219+
if (eventRefs == null || eventRefs.isEmpty()) {
195220
addValidationError(
196221
"Event State eventsActions has no event refs",
197222
ValidationError.WORKFLOW_VALIDATION);
198223
} else {
199224
for (String eventRef : eventRefs) {
200-
if (!haveEventsDefinition(eventRef, events)) {
225+
if (isMissingEventsDefinition(eventRef, events)) {
201226
addValidationError(
202227
"Event State eventsActions eventRef does not match a declared workflow event definition",
203228
ValidationError.WORKFLOW_VALIDATION);
@@ -210,9 +235,9 @@ public List<ValidationError> validate() {
210235
if (s instanceof SwitchState) {
211236
SwitchState switchState = (SwitchState) s;
212237
if ((switchState.getDataConditions() == null
213-
|| switchState.getDataConditions().size() < 1)
238+
|| switchState.getDataConditions().isEmpty())
214239
&& (switchState.getEventConditions() == null
215-
|| switchState.getEventConditions().size() < 1)) {
240+
|| switchState.getEventConditions().isEmpty())) {
216241
addValidationError(
217242
"Switch state should define either data or event conditions",
218243
ValidationError.WORKFLOW_VALIDATION);
@@ -225,10 +250,10 @@ public List<ValidationError> validate() {
225250
}
226251

227252
if (switchState.getEventConditions() != null
228-
&& switchState.getEventConditions().size() > 0) {
253+
&& !switchState.getEventConditions().isEmpty()) {
229254
List<EventCondition> eventConditions = switchState.getEventConditions();
230255
for (EventCondition ec : eventConditions) {
231-
if (!haveEventsDefinition(ec.getEventRef(), events)) {
256+
if (isMissingEventsDefinition(ec.getEventRef(), events)) {
232257
addValidationError(
233258
"Switch state event condition eventRef does not reference a defined workflow event",
234259
ValidationError.WORKFLOW_VALIDATION);
@@ -240,7 +265,7 @@ public List<ValidationError> validate() {
240265
}
241266

242267
if (switchState.getDataConditions() != null
243-
&& switchState.getDataConditions().size() > 0) {
268+
&& !switchState.getDataConditions().isEmpty()) {
244269
List<DataCondition> dataConditions = switchState.getDataConditions();
245270
for (DataCondition dc : dataConditions) {
246271
if (dc.getEnd() != null) {
@@ -252,7 +277,7 @@ public List<ValidationError> validate() {
252277

253278
if (s instanceof SleepState) {
254279
SleepState sleepState = (SleepState) s;
255-
if (sleepState.getDuration() == null || sleepState.getDuration().length() < 1) {
280+
if (sleepState.getDuration() == null || sleepState.getDuration().isEmpty()) {
256281
addValidationError(
257282
"Sleep state should have a non-empty time delay",
258283
ValidationError.WORKFLOW_VALIDATION);
@@ -292,13 +317,13 @@ public List<ValidationError> validate() {
292317
if (s instanceof CallbackState) {
293318
CallbackState callbackState = (CallbackState) s;
294319

295-
if (!haveEventsDefinition(callbackState.getEventRef(), events)) {
320+
if (isMissingEventsDefinition(callbackState.getEventRef(), events)) {
296321
addValidationError(
297322
"CallbackState event ref does not reference a defined workflow event definition",
298323
ValidationError.WORKFLOW_VALIDATION);
299324
}
300325

301-
if (!haveFunctionDefinition(
326+
if (isMissingFunctionDefinition(
302327
callbackState.getAction().getFunctionRef().getRefName(), functions)) {
303328
addValidationError(
304329
"CallbackState action function ref does not reference a defined workflow function definition",
@@ -334,32 +359,48 @@ public WorkflowValidator reset() {
334359
return this;
335360
}
336361

337-
private boolean haveFunctionDefinition(String functionName, List<FunctionDefinition> functions) {
362+
private boolean isMissingFunctionDefinition(
363+
String functionName, List<FunctionDefinition> functions) {
338364
if (functions != null) {
339-
FunctionDefinition fun =
340-
functions.stream().filter(f -> f.getName().equals(functionName)).findFirst().orElse(null);
341-
342-
return fun == null ? false : true;
365+
return functions.stream()
366+
.filter(f -> f.getName().equals(functionName))
367+
.findFirst()
368+
.orElse(null)
369+
== null;
343370
} else {
344-
return false;
371+
return true;
345372
}
346373
}
347374

348-
private boolean haveEventsDefinition(String eventName, List<EventDefinition> events) {
375+
private boolean isMissingEventsDefinition(String eventName, List<EventDefinition> events) {
349376
if (eventName == null) {
350-
return true;
377+
return false;
351378
}
352379
if (events != null) {
353-
EventDefinition eve =
354-
events.stream().filter(e -> e.getName().equals(eventName)).findFirst().orElse(null);
355-
return eve == null ? false : true;
380+
return events.stream().filter(e -> e.getName().equals(eventName)).findFirst().orElse(null)
381+
== null;
356382
} else {
357-
return false;
383+
return true;
384+
}
385+
}
386+
387+
private boolean isMissingRetryDefinition(String retryName, List<RetryDefinition> retries) {
388+
if (retries != null) {
389+
return retries.stream()
390+
.filter(f -> f.getName() != null && f.getName().equals(retryName))
391+
.findFirst()
392+
.orElse(null)
393+
== null;
394+
} else {
395+
return true;
358396
}
359397
}
360398

361399
private static final Set<String> skipMessages =
362-
Set.of("$.start: string found, object expected", "$.functions: array found, object expected");
400+
Set.of(
401+
"$.start: string found, object expected",
402+
"$.functions: array found, object expected",
403+
"$.retries: array found, object expected");
363404

364405
private void addValidationError(String message, String type) {
365406
if (skipMessages.contains(message)) {

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

+65
Original file line numberDiff line numberDiff line change
@@ -333,4 +333,69 @@ void testEventCall() {
333333
.withEnd(new End())));
334334
Assertions.assertTrue(new WorkflowValidatorImpl().setWorkflow(workflow).validate().isEmpty());
335335
}
336+
337+
/**
338+
* @see <a href="https://github.com/serverlessworkflow/sdk-java/issues/213">Retry definition
339+
* validation doesn't work</a>
340+
*/
341+
@Test
342+
public void testValidateRetry() {
343+
WorkflowValidator workflowValidator = new WorkflowValidatorImpl();
344+
List<ValidationError> validationErrors =
345+
workflowValidator
346+
.setSource(
347+
"{\n"
348+
+ " \"id\": \"workflow_1\",\n"
349+
+ " \"name\": \"workflow_1\",\n"
350+
+ " \"description\": \"workflow_1\",\n"
351+
+ " \"version\": \"1.0\",\n"
352+
+ " \"specVersion\": \"0.8\",\n"
353+
+ " \"start\": \"Task1\",\n"
354+
+ " \"functions\": [\n"
355+
+ " {\n"
356+
+ " \"name\": \"increment\",\n"
357+
+ " \"type\": \"custom\",\n"
358+
+ " \"operation\": \"worker\"\n"
359+
+ " }\n"
360+
+ " ],\n"
361+
+ " \"retries\": [\n"
362+
+ " {\n"
363+
+ " \"maxAttempts\": 3\n"
364+
+ " },\n"
365+
+ " {\n"
366+
+ " \"name\": \"testRetry\" \n"
367+
+ " }\n"
368+
+ " ],\n"
369+
+ " \"states\": [\n"
370+
+ " {\n"
371+
+ " \"name\": \"Task1\",\n"
372+
+ " \"type\": \"operation\",\n"
373+
+ " \"actionMode\": \"sequential\",\n"
374+
+ " \"actions\": [\n"
375+
+ " {\n"
376+
+ " \"functionRef\": {\n"
377+
+ " \"refName\": \"increment\",\n"
378+
+ " \"arguments\": {\n"
379+
+ " \"input\": \"some text\"\n"
380+
+ " }\n"
381+
+ " },\n"
382+
+ " \"retryRef\": \"const\",\n"
383+
+ " \"actionDataFilter\": {\n"
384+
+ " \"toStateData\": \"${ .result }\"\n"
385+
+ " }\n"
386+
+ " }\n"
387+
+ " ],\n"
388+
+ " \"end\": true\n"
389+
+ " }\n"
390+
+ " ]\n"
391+
+ "}")
392+
.validate();
393+
394+
Assertions.assertNotNull(validationErrors);
395+
Assertions.assertEquals(2, validationErrors.size());
396+
Assertions.assertEquals("Retry name should not be empty", validationErrors.get(0).getMessage());
397+
Assertions.assertEquals(
398+
"Operation State action 'null' retryRef does not reference an existing workflow retry definition",
399+
validationErrors.get(1).getMessage());
400+
}
336401
}

0 commit comments

Comments
 (0)