Repository: ChristopheBougere/asl-validator
Pull request #127 (2023-02-15 02:47:54)

feat: add check for task heartbeat and timeout props
refactor: support at most one for prop check rule


Patch:
diff --git a/src/checks/exactlyOneOfProperty.ts b/src/checks/checkProperty.ts
similarity index 51%
rename from src/checks/exactlyOneOfProperty.ts
rename to src/checks/checkProperty.ts
index f4cd6cc..d08c8f9 100644
--- a/src/checks/exactlyOneOfProperty.ts
+++ b/src/checks/checkProperty.ts
@@ -1,17 +1,30 @@
 import {State, StateMachine, StateMachineError, StateMachineErrorCode} from "../types";
 import {getStates, getStatesContainer, StateEntry} from "./get-states";
 
-export const exactlyOneOfPropertyWorkflow = ({definition, filter, props, errorCode}: {
+type StatePropChecker = (args: {
+    props: string[],
+    state: State,
+    stateName: string,
+    errorCode: StateMachineErrorCode
+}) => StateMachineError | null
+
+export type StatePropCheckerType = 'exactlyOne' | 'atMostOne'
+
+export const checkPropertyWorkflow = ({definition, filter, props, errorCode, propCheck}: {
     definition: StateMachine,
     filter: (entry: StateEntry) => boolean
     props: string[],
+    propCheck: StatePropCheckerType
     errorCode: StateMachineErrorCode
 }): StateMachineError[] => {
+
+    const propChecker : StatePropChecker = propCheck === 'exactlyOne' ? checkProperty : atMostOneOfProperty
+
     const errorMessages: StateMachineError[] = [];
     getStatesContainer(definition).forEach((states) => {
         getStates(states)
             .filter(filter).forEach(({stateName, state}) => {
-            const error = exactlyOneOfProperty({
+            const error = propChecker({
                 props,
                 state,
                 stateName,
@@ -23,18 +36,19 @@ export const exactlyOneOfPropertyWorkflow = ({definition, filter, props, errorCo
         })
     })
     return errorMessages;
-
 }
 
-export const exactlyOneOfProperty = ({props, state, stateName, errorCode}: {
+const getPropertyCount = ({props, state}: {
     props: string[],
     state: State,
-    stateName: string,
-    errorCode: StateMachineErrorCode
-}): StateMachineError | null => {
-    const count = props.map((prop) => prop in state ? 1 : 0)
+}) : number => {
+    return props.map((prop) => prop in state ? 1 : 0)
         .map((val) => Number(val))
         .reduce((prev, curr) => prev + curr, 0)
+}
+
+const checkProperty : StatePropChecker = ({props, state, stateName, errorCode})  => {
+    const count = getPropertyCount({state, props})
     if (count !== 1) {
         return {
             "Error code": errorCode,
@@ -48,3 +62,19 @@ export const exactlyOneOfProperty = ({props, state, stateName, errorCode}: {
     }
     return null
 }
+
+const atMostOneOfProperty : StatePropChecker = ({props, state, stateName, errorCode})  => {
+    const count = getPropertyCount({state, props})
+    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 at most one of ${props.map((p) => {
+                return `"${p}"`
+            }).join(", ")}`,
+
+        }
+    }
+    return null
+}
diff --git a/src/checks/taskChecks.ts b/src/checks/taskChecks.ts
new file mode 100644
index 0000000..ff9d7fe
--- /dev/null
+++ b/src/checks/taskChecks.ts
@@ -0,0 +1,26 @@
+import {AslChecker, StateMachineErrorCode} from "../types";
+import {checkPropertyWorkflow} from "./checkProperty";
+
+export const taskChecks: AslChecker = (definition) => {
+    return [
+        ...checkPropertyWorkflow({
+            definition,
+            filter: ({state}) => {
+                return state.Type === 'Task'
+            },
+            propCheck: 'atMostOne',
+            props: ["TimeoutSeconds", "TimeoutSecondsPath"],
+            errorCode: StateMachineErrorCode.TaskTimeoutError,
+        }),
+
+        ...checkPropertyWorkflow({
+            definition,
+            filter: ({state}) => {
+                return state.Type === 'Task'
+            },
+            propCheck: 'atMostOne',
+            props: ["HeartbeatSeconds", "HeartbeatSecondsPath"],
+            errorCode: StateMachineErrorCode.TaskHeartbeatError,
+        })
+    ]
+}
diff --git a/src/checks/terminalStateWithNext.ts b/src/checks/terminalStateWithNext.ts
index 853062a..7293bf1 100644
--- a/src/checks/terminalStateWithNext.ts
+++ b/src/checks/terminalStateWithNext.ts
@@ -1,5 +1,5 @@
 import {AslChecker, StateMachineErrorCode} from "../types";
-import {exactlyOneOfPropertyWorkflow} from "./exactlyOneOfProperty";
+import {checkPropertyWorkflow} from "./checkProperty";
 
 // performs the check that non-Terminal states do not contain a `Next` field.
 // This replaces the following schema snippet:
@@ -9,7 +9,7 @@ import {exactlyOneOfPropertyWorkflow} from "./exactlyOneOfProperty";
 //     "required": ["End"]
 //   }],
 export const terminalStateWithNext: AslChecker = (definition) => {
-    return exactlyOneOfPropertyWorkflow({
+    return checkPropertyWorkflow({
         props: ['End', 'Next'],
         errorCode: StateMachineErrorCode.TerminalStateWithNextError,
         definition,
@@ -17,6 +17,7 @@ export const terminalStateWithNext: AslChecker = (definition) => {
             return state.Type !== 'Succeed' &&
                 state.Type !== 'Fail' &&
                 state.Type !== 'Choice'
-        }
+        },
+        propCheck: 'exactlyOne'
     })
 }
diff --git a/src/checks/waitDuration.ts b/src/checks/waitDuration.ts
index 8179d6d..730ed55 100644
--- a/src/checks/waitDuration.ts
+++ b/src/checks/waitDuration.ts
@@ -1,13 +1,14 @@
 import {AslChecker, StateMachineErrorCode} from "../types";
-import {exactlyOneOfPropertyWorkflow} from "./exactlyOneOfProperty";
+import {checkPropertyWorkflow} from "./checkProperty";
 
 export const waitDuration: AslChecker = (definition) => {
-    return exactlyOneOfPropertyWorkflow({
+    return checkPropertyWorkflow({
         props: ["Seconds", "SecondsPath", "Timestamp", "TimestampPath"],
         errorCode: StateMachineErrorCode.WaitDurationError,
         definition,
         filter: ({state}) => {
             return state.Type === 'Wait'
-        }
+        },
+        propCheck: 'exactlyOne'
     })
 }
diff --git a/src/types.ts b/src/types.ts
index 4e97789..26377a5 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -14,6 +14,8 @@ export enum StateMachineErrorCode {
   SchemaValidationFailed = 'SCHEMA_VALIDATION_FAILED',
   TerminalStateWithNextError = 'TERMINAL_STATE_WITH_NEXT',
   WaitDurationError = 'WAIT_DURATION',
+  TaskTimeoutError = 'TASK_TIMEOUT',
+  TaskHeartbeatError = 'TASK_HEARTBEAT',
 }
 export type StateMachineError = {
   'Error code': StateMachineErrorCode;
diff --git a/src/validator.ts b/src/validator.ts
index e052c62..cc863c2 100644
--- a/src/validator.ts
+++ b/src/validator.ts
@@ -9,6 +9,7 @@ import {StateMachine, StateMachineError, ValidationOptions} from './types';
 import {mustNotHaveDuplicateFieldNamesAfterEvaluation} from "./checks/duplicate-payload-template-fields";
 import {terminalStateWithNext} from "./checks/terminalStateWithNext";
 import {waitDuration} from "./checks/waitDuration";
+import {taskChecks} from "./checks/taskChecks";
 
 const DefaultOptions: ValidationOptions = {
     checkPaths: true,
@@ -30,6 +31,7 @@ export = function validator(definition: StateMachine, opts?: ValidationOptions):
         errors.push(...mustNotHaveDuplicateFieldNamesAfterEvaluation(definition, options))
         errors.push(...terminalStateWithNext(definition, options));
         errors.push(...waitDuration(definition, options));
+        errors.push(...taskChecks(definition, options));
     }
 
     return {


Test patch:
diff --git a/src/__tests__/definitions/invalid-task-heartbeat.json b/src/__tests__/definitions/invalid-task-heartbeat.json
new file mode 100644
index 0000000..acbe0e2
--- /dev/null
+++ b/src/__tests__/definitions/invalid-task-heartbeat.json
@@ -0,0 +1,20 @@
+{
+  "Comment": "Task contains both HeartbeatSeconds and HeartbeatSecondsPath",
+  "StartAt": "X",
+  "States": {
+    "X": {
+      "Type": "Task",
+      "Resource": "arn:aws:swf:us-east-1:123456789012:task:X",
+      "Parameters": {
+        "flagged": true,
+        "parts": {
+          "first.$": "$.vals[0]",
+          "last3.$": "$.vals[3:]"
+        }
+      },
+      "HeartbeatSeconds": 20,
+      "HeartbeatSecondsPath": "$.foo",
+      "End": true
+    }
+  }
+}
diff --git a/src/__tests__/definitions/invalid-task-timout.json b/src/__tests__/definitions/invalid-task-timout.json
new file mode 100644
index 0000000..7dca148
--- /dev/null
+++ b/src/__tests__/definitions/invalid-task-timout.json
@@ -0,0 +1,20 @@
+{
+  "Comment": "Task contains both TimeoutSeconds and TimeoutSecondsPath",
+  "StartAt": "X",
+  "States": {
+    "X": {
+      "Type": "Task",
+      "Resource": "arn:aws:swf:us-east-1:123456789012:task:X",
+      "Parameters": {
+        "flagged": true,
+        "parts": {
+          "first.$": "$.vals[0]",
+          "last3.$": "$.vals[3:]"
+        }
+      },
+      "TimeoutSeconds": 20,
+      "TimeoutSecondsPath": "$.foo",
+      "End": true
+    }
+  }
+}