Repository: ChristopheBougere/asl-validator
Pull request #126 (2023-02-14 15:15:12)

feat: added check for wait duration properties
refactor: leverage common code between exclusive prop checks

massfords: @ChristopheBougere build fails w/ a commit message error. I had missed the merge of my other PR so perhaps it doesn't like the merge. Feel free to squash

I can look at the other issues I added later this week. I think the refactoring in this PR to have a common function for doing the exclusive prop check will make the other issues easier to close.

Patch:
diff --git a/src/checks/exactlyOneOfProperty.ts b/src/checks/exactlyOneOfProperty.ts
new file mode 100644
index 0000000..f4cd6cc
--- /dev/null
+++ b/src/checks/exactlyOneOfProperty.ts
@@ -0,0 +1,50 @@
+import {State, StateMachine, StateMachineError, StateMachineErrorCode} from "../types";
+import {getStates, getStatesContainer, StateEntry} from "./get-states";
+
+export const exactlyOneOfPropertyWorkflow = ({definition, filter, props, errorCode}: {
+    definition: StateMachine,
+    filter: (entry: StateEntry) => boolean
+    props: string[],
+    errorCode: StateMachineErrorCode
+}): StateMachineError[] => {
+    const errorMessages: StateMachineError[] = [];
+    getStatesContainer(definition).forEach((states) => {
+        getStates(states)
+            .filter(filter).forEach(({stateName, state}) => {
+            const error = exactlyOneOfProperty({
+                props,
+                state,
+                stateName,
+                errorCode
+            })
+            if (error) {
+                errorMessages.push(error)
+            }
+        })
+    })
+    return errorMessages;
+
+}
+
+export const exactlyOneOfProperty = ({props, state, stateName, errorCode}: {
+    props: string[],
+    state: State,
+    stateName: string,
+    errorCode: StateMachineErrorCode
+}): StateMachineError | null => {
+    const count = props.map((prop) => prop in state ? 1 : 0)
+        .map((val) => Number(val))
+        .reduce((prev, curr) => prev + curr, 0)
+    if (count !== 1) {
+        return {
+            "Error code": errorCode,
+            // Use of JSONPath within the error message is unnecessary
+            // since the state names are unique.
+            Message: `State "${stateName}" MUST contain exactly one of ${props.map((p) => {
+                return `"${p}"`
+            }).join(", ")}`,
+
+        }
+    }
+    return null
+}
diff --git a/src/checks/get-states.ts b/src/checks/get-states.ts
index 7459037..c27db78 100644
--- a/src/checks/get-states.ts
+++ b/src/checks/get-states.ts
@@ -8,7 +8,9 @@ export const getStatesContainer = (definition: StateMachine) : States[] => {
     })
 }
 
-export const getStates = (states: States): Array<{stateName: string, state: State & {Type: string}}> => {
+export type StateEntry = {stateName: string, state: State & {Type: string}}
+
+export const getStates = (states: States): Array<StateEntry> => {
     return Object.keys(states).map((stateName) => {
         const state = states[stateName]
         return {stateName, state}
diff --git a/src/checks/terminalStateWithNext.ts b/src/checks/terminalStateWithNext.ts
index 4f52e16..853062a 100644
--- a/src/checks/terminalStateWithNext.ts
+++ b/src/checks/terminalStateWithNext.ts
@@ -1,5 +1,5 @@
-import {AslChecker, StateMachineError, StateMachineErrorCode} from "../types";
-import {getStates, getStatesContainer} from "./get-states";
+import {AslChecker, StateMachineErrorCode} from "../types";
+import {exactlyOneOfPropertyWorkflow} from "./exactlyOneOfProperty";
 
 // performs the check that non-Terminal states do not contain a `Next` field.
 // This replaces the following schema snippet:
@@ -9,22 +9,14 @@ import {getStates, getStatesContainer} from "./get-states";
 //     "required": ["End"]
 //   }],
 export const terminalStateWithNext: AslChecker = (definition) => {
-    const errorMessages: StateMachineError[] = [];
-    getStatesContainer(definition).forEach((states) => {
-        getStates(states).forEach(({stateName, state}) => {
-            // Terminal fields don't need this check
-            if (state.Type === 'Succeed' || state.Type === 'Fail' || state.Type === 'Choice') {
-                return
-            }
-            if ('End' in state && 'Next' in state) {
-                errorMessages.push({
-                    "Error code": StateMachineErrorCode.TerminalStateWithNextError,
-                    // Use of JSONPath within the error message is unnecessary
-                    // since the state names are unique.
-                    Message: `State "${stateName}" contains both "End" and "Next" fields.`,
-                })
-            }
-        })
+    return exactlyOneOfPropertyWorkflow({
+        props: ['End', 'Next'],
+        errorCode: StateMachineErrorCode.TerminalStateWithNextError,
+        definition,
+        filter: ({state}) => {
+            return state.Type !== 'Succeed' &&
+                state.Type !== 'Fail' &&
+                state.Type !== 'Choice'
+        }
     })
-    return errorMessages;
 }
diff --git a/src/checks/waitDuration.ts b/src/checks/waitDuration.ts
new file mode 100644
index 0000000..8179d6d
--- /dev/null
+++ b/src/checks/waitDuration.ts
@@ -0,0 +1,13 @@
+import {AslChecker, StateMachineErrorCode} from "../types";
+import {exactlyOneOfPropertyWorkflow} from "./exactlyOneOfProperty";
+
+export const waitDuration: AslChecker = (definition) => {
+    return exactlyOneOfPropertyWorkflow({
+        props: ["Seconds", "SecondsPath", "Timestamp", "TimestampPath"],
+        errorCode: StateMachineErrorCode.WaitDurationError,
+        definition,
+        filter: ({state}) => {
+            return state.Type === 'Wait'
+        }
+    })
+}
diff --git a/src/types.ts b/src/types.ts
index 254ece6..4e97789 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -13,6 +13,7 @@ export enum StateMachineErrorCode {
   MissingTransitionTarget = 'MISSING_TRANSITION_TARGET',
   SchemaValidationFailed = 'SCHEMA_VALIDATION_FAILED',
   TerminalStateWithNextError = 'TERMINAL_STATE_WITH_NEXT',
+  WaitDurationError = 'WAIT_DURATION',
 }
 export type StateMachineError = {
   'Error code': StateMachineErrorCode;
diff --git a/src/validator.ts b/src/validator.ts
index 852e439..e052c62 100644
--- a/src/validator.ts
+++ b/src/validator.ts
@@ -8,6 +8,7 @@ import {missingTerminalStateErrors} from './checks/missing-terminal-state-errors
 import {StateMachine, StateMachineError, ValidationOptions} from './types';
 import {mustNotHaveDuplicateFieldNamesAfterEvaluation} from "./checks/duplicate-payload-template-fields";
 import {terminalStateWithNext} from "./checks/terminalStateWithNext";
+import {waitDuration} from "./checks/waitDuration";
 
 const DefaultOptions: ValidationOptions = {
     checkPaths: true,
@@ -28,6 +29,7 @@ export = function validator(definition: StateMachine, opts?: ValidationOptions):
         errors.push(...missingTerminalStateErrors(definition, options));
         errors.push(...mustNotHaveDuplicateFieldNamesAfterEvaluation(definition, options))
         errors.push(...terminalStateWithNext(definition, options));
+        errors.push(...waitDuration(definition, options));
     }
 
     return {


Test patch:
diff --git a/src/__tests__/definitions/invalid-wait-duration.json b/src/__tests__/definitions/invalid-wait-duration.json
new file mode 100644
index 0000000..5587e36
--- /dev/null
+++ b/src/__tests__/definitions/invalid-wait-duration.json
@@ -0,0 +1,28 @@
+{
+  "Comment": "An example Wait with multiple duration definitions",
+  "StartAt": "FirstState",
+  "States": {
+    "FirstState": {
+      "Type": "Task",
+      "Resource": "arn:aws:lambda:region-1:1234567890:function:FUNCTION_NAME",
+      "Next": "wait_using_seconds"
+    },
+    "wait_using_seconds": {
+      "Type": "Wait",
+      "Seconds": 10,
+      "SecondsPath": "$.expiryseconds",
+      "Next": "wait_using_timestamp"
+    },
+    "wait_using_timestamp": {
+      "Type": "Wait",
+      "Timestamp": "2015-09-04T01:59:00Z",
+      "TimestampPath": "$.expirydate",
+      "Next": "FinalState"
+    },
+    "FinalState": {
+      "Type": "Task",
+      "Resource": "arn:aws:lambda:region-1:1234567890:function:FUNCTION_NAME",
+      "End": true
+    }
+  }
+}