Skip to content

Commit 98a829e

Browse files
authored
Merge pull request #101 from lukonjun/improve-dsl-validation-81
improve validation
2 parents 8c416f2 + 2aed3b8 commit 98a829e

File tree

2 files changed

+77
-22
lines changed

2 files changed

+77
-22
lines changed

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

Lines changed: 74 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,15 @@
2121
import io.serverlessworkflow.api.Workflow;
2222
import io.serverlessworkflow.api.actions.Action;
2323
import io.serverlessworkflow.api.branches.Branch;
24+
import io.serverlessworkflow.api.error.Error;
2425
import io.serverlessworkflow.api.events.EventDefinition;
2526
import io.serverlessworkflow.api.events.OnEvents;
2627
import io.serverlessworkflow.api.functions.FunctionDefinition;
28+
import io.serverlessworkflow.api.interfaces.State;
2729
import io.serverlessworkflow.api.interfaces.WorkflowValidator;
30+
import io.serverlessworkflow.api.retry.RetryDefinition;
2831
import io.serverlessworkflow.api.states.*;
32+
import io.serverlessworkflow.api.switchconditions.DataCondition;
2933
import io.serverlessworkflow.api.switchconditions.EventCondition;
3034
import io.serverlessworkflow.api.validation.ValidationError;
3135
import io.serverlessworkflow.api.validation.WorkflowSchemaLoader;
@@ -86,10 +90,11 @@ public List<ValidationError> validate() {
8690
.forEach(m -> {
8791
if((!m.equals("#/functions: expected type: JSONObject, found: JSONArray") &&
8892
!m.equals("#/events: expected type: JSONObject, found: JSONArray") &&
89-
!m.equals("#/start: expected type: JSONObject, found: String"))) {
90-
addValidationError(m,
91-
ValidationError.SCHEMA_VALIDATION);
92-
}});
93+
!m.equals("#/start: expected type: JSONObject, found: String") &&
94+
!m.equals("#/retries: expected type: JSONObject, found: JSONArray"))) {
95+
addValidationError(m,
96+
ValidationError.SCHEMA_VALIDATION);
97+
}});
9398

9499
}
95100
}
@@ -136,6 +141,21 @@ public List<ValidationError> validate() {
136141
ValidationError.WORKFLOW_VALIDATION);
137142
}
138143

144+
if (workflow.getStates() != null && !workflow.getStates().isEmpty()) {
145+
boolean existingStateWithStartProperty = false;
146+
String startProperty = workflow.getStart().getStateName();
147+
for (State s : workflow.getStates()) {
148+
if (s.getName().equals(startProperty)) {
149+
existingStateWithStartProperty = true;
150+
break;
151+
}
152+
}
153+
if (!existingStateWithStartProperty) {
154+
addValidationError("No state name found that matches the workflow start definition",
155+
ValidationError.WORKFLOW_VALIDATION);
156+
}
157+
}
158+
139159
Validation validation = new Validation();
140160
if (workflow.getStates() != null && !workflow.getStates().isEmpty()) {
141161
workflow.getStates().forEach(s -> {
@@ -150,12 +170,37 @@ public List<ValidationError> validate() {
150170
validation.addEndState();
151171
}
152172

153-
if (s instanceof OperationState) {
154-
OperationState operationState = (OperationState) s;
155-
if (operationState.getActions() == null || operationState.getActions().size() < 1) {
156-
addValidationError("Operation State has no actions defined",
173+
if (workflow.getRetries() != null){
174+
List<RetryDefinition> retryDefs = workflow.getRetries().getRetryDefs();
175+
if(s.getOnErrors() == null || s.getOnErrors().isEmpty()){
176+
addValidationError("No onErrors found for state" + s.getName() + " but retries is defined",
157177
ValidationError.WORKFLOW_VALIDATION);
178+
}else {
179+
for(Error e:s.getOnErrors()){
180+
if(e.getRetryRef() == null || e.getRetryRef().isEmpty()){
181+
addValidationError("No retryRef found for onErrors" + e.getError(),
182+
ValidationError.WORKFLOW_VALIDATION);
183+
}
184+
else {
185+
boolean validRetryDefinition = false;
186+
for(RetryDefinition rd:retryDefs){
187+
if(rd.getName().equals(e.getRetryRef())){
188+
validRetryDefinition = true;
189+
break;
190+
}
191+
}
192+
if(!validRetryDefinition) {
193+
addValidationError(e.getRetryRef() +" is not a valid retryRef",
194+
ValidationError.WORKFLOW_VALIDATION);
195+
}
196+
}
197+
}
158198
}
199+
}
200+
201+
202+
if (s instanceof OperationState) {
203+
OperationState operationState = (OperationState) s;
159204

160205
List<Action> actions = operationState.getActions();
161206
for (Action action : actions) {
@@ -203,10 +248,6 @@ public List<ValidationError> validate() {
203248
}
204249
List<OnEvents> eventsActionsList = eventState.getOnEvents();
205250
for (OnEvents onEvents : eventsActionsList) {
206-
if (onEvents.getActions() == null || onEvents.getActions().size() < 1) {
207-
addValidationError("Event State eventsActions has no actions",
208-
ValidationError.WORKFLOW_VALIDATION);
209-
}
210251

211252
List<String> eventRefs = onEvents.getEventRefs();
212253
if (eventRefs == null || eventRefs.size() < 1) {
@@ -243,6 +284,18 @@ public List<ValidationError> validate() {
243284
addValidationError("Switch state event condition eventRef does not reference a defined workflow event",
244285
ValidationError.WORKFLOW_VALIDATION);
245286
}
287+
if(ec.getEnd()!=null){
288+
validation.addEndState();
289+
}
290+
}
291+
}
292+
293+
if (switchState.getDataConditions() != null && switchState.getDataConditions().size() > 0) {
294+
List<DataCondition> dataConditions = switchState.getDataConditions();
295+
for (DataCondition dc : dataConditions) {
296+
if(dc.getEnd()!=null){
297+
validation.addEndState();
298+
}
246299
}
247300
}
248301
}
@@ -257,16 +310,17 @@ public List<ValidationError> validate() {
257310

258311
if (s instanceof ParallelState) {
259312
ParallelState parallelState = (ParallelState) s;
260-
if (parallelState.getBranches() == null || parallelState.getBranches().size() < 1) {
261-
addValidationError("Parallel state should have branches",
313+
314+
if (parallelState.getBranches() == null || parallelState.getBranches().size() < 2) {
315+
addValidationError("Parallel state should have at lest two branches",
262316
ValidationError.WORKFLOW_VALIDATION);
263317
}
264318

319+
265320
List<Branch> branches = parallelState.getBranches();
266321
for (Branch branch : branches) {
267-
if ((branch.getActions() == null || branch.getActions().size() < 1)
268-
&& (branch.getWorkflowId() == null || branch.getWorkflowId().length() < 1)) {
269-
addValidationError("Parallel state should define either actions or workflow id",
322+
if(branch.getWorkflowId() == null || branch.getWorkflowId().length() < 1) {
323+
addValidationError("Parallel state should define workflow id",
270324
ValidationError.WORKFLOW_VALIDATION);
271325
}
272326
}
@@ -282,7 +336,7 @@ public List<ValidationError> validate() {
282336

283337
if (s instanceof InjectState) {
284338
InjectState injectState = (InjectState) s;
285-
if (injectState.getData() == null) {
339+
if (injectState.getData() == null || injectState.getData().isEmpty()) {
286340
addValidationError("InjectState should have non-null data",
287341
ValidationError.WORKFLOW_VALIDATION);
288342
}
@@ -300,9 +354,8 @@ public List<ValidationError> validate() {
300354
ValidationError.WORKFLOW_VALIDATION);
301355
}
302356

303-
if ((forEachState.getActions() == null || forEachState.getActions().size() < 1)
304-
&& (forEachState.getWorkflowId() == null || forEachState.getWorkflowId().length() < 1)) {
305-
addValidationError("ForEach state should define either actions or workflow id",
357+
if (forEachState.getWorkflowId() == null || forEachState.getWorkflowId().length() < 1) {
358+
addValidationError("ForEach state should define a workflow id",
306359
ValidationError.WORKFLOW_VALIDATION);
307360
}
308361
}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,10 @@ public void testFromIncompleteWorkflow() {
7070
WorkflowValidator workflowValidator = new WorkflowValidatorImpl();
7171
List<ValidationError> validationErrors = workflowValidator.setWorkflow(workflow).validate();
7272
Assertions.assertNotNull(validationErrors);
73-
Assertions.assertEquals(1, validationErrors.size());
73+
Assertions.assertEquals(2, validationErrors.size());
7474

7575
Assertions.assertEquals("Workflow name should not be empty", validationErrors.get(0).getMessage());
76+
Assertions.assertEquals("No state name found that matches the workflow start definition", validationErrors.get(1).getMessage());
7677
}
7778

7879
@Test
@@ -131,6 +132,7 @@ public void testOperationStateNoFunctionRef() {
131132
" }\n" +
132133
"]\n" +
133134
"}").validate();
135+
134136
Assertions.assertNotNull(validationErrors);
135137
Assertions.assertEquals(1, validationErrors.size());
136138

0 commit comments

Comments
 (0)