fix(sfn): Task parameters property does nothing (aws#5408)

Make `Task`'s `parameters` property be respected, while at the same time making it very clear that it actually shouldn't be used at all via documentation. It got added in a previous commit but wasn't actually respected. We have the option between deprecating it (since it never really worked), or making it work but disrecommending it. Could still serve as a useful location to add overrides in cases where people want to use Tokens in place of enum-typed parameters, which today is not possible. Fixes aws#5267.
ed-at-work · Dec 17, 2019 · 079bd8f · 079bd8f
1 parent d03899f
commit 079bd8f
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 6 deletions.
diff --git a/packages/@aws-cdk/aws-stepfunctions/lib/states/task.ts b/packages/@aws-cdk/aws-stepfunctions/lib/states/task.ts
@@ -5,6 +5,7 @@ import { FieldUtils } from '../fields';
 import { StateGraph } from '../state-graph';
 import { IStepFunctionsTask, StepFunctionsTaskConfig } from '../step-functions-task';
 import { CatchProps, IChainable, INextable, RetryProps } from '../types';
+import { noEmptyObject } from '../util';
 import { StateType } from './private/state-type';
 import { renderJsonPath, State } from './state';
 
@@ -55,12 +56,26 @@ export interface TaskProps {
     readonly resultPath?: string;
 
     /**
-     * Parameters pass a collection of key-value pairs, either static values or JSONPath expressions that select from the input.
+     * Parameters to invoke the task with
+     *
+     * It is not recommended to use this field. The object that is passed in
+     * the `task` property will take care of returning the right values for the
+     * `Parameters` field in the Step Functions definition.
+     *
+     * The various classes that implement `IStepFunctionsTask` will take a
+     * properties which make sense for the task type. For example, for
+     * `InvokeFunction` the field that populates the `parameters` field will be
+     * called `payload`, and for the `PublishToTopic` the `parameters` field
+     * will be populated via a combination of the referenced topic, subject and
+     * message.
+     *
+     * If passed anyway, the keys in this map will override the parameters
+     * returned by the task object.
      *
      * @see
      * https://docs.aws.amazon.com/step-functions/latest/dg/input-output-inputpath-params.html#input-output-parameters
      *
-     * @default No parameters
+     * @default - Use the parameters implied by the `task` property
      */
     readonly parameters?: { [name: string]: any };
 
@@ -93,7 +108,12 @@ export class Task extends State implements INextable {
         super(scope, id, props);
 
         this.timeout = props.timeout;
-        this.taskProps = props.task.bind(this);
+        const taskProps = props.task.bind(this);
+
+        this.taskProps = {
+            ...taskProps,
+            parameters: noEmptyObject({ ...taskProps.parameters || {}, ...props.parameters || {} }),
+        };
         this.endStates = [this];
     }
 

diff --git a/packages/@aws-cdk/aws-stepfunctions/lib/step-functions-task.ts b/packages/@aws-cdk/aws-stepfunctions/lib/step-functions-task.ts
@@ -28,9 +28,10 @@ export interface StepFunctionsTaskConfig {
   /**
    * Parameters pass a collection of key-value pairs, either static values or JSONPath expressions that select from the input.
    *
-   * What is passed here will be merged with any default parameters
-   * configured by the `resource`. For example, a DynamoDB table target
-   * will
+   * The meaning of these parameters is task-dependent.
+   *
+   * Its values will be merged with the `parameters` property which is configured directly
+   * on the Task state.
    *
    * @see
    * https://docs.aws.amazon.com/step-functions/latest/dg/input-output-inputpath-params.html#input-output-parameters

diff --git a/packages/@aws-cdk/aws-stepfunctions/lib/util.ts b/packages/@aws-cdk/aws-stepfunctions/lib/util.ts
@@ -0,0 +1,4 @@
+export function noEmptyObject<A>(o: Record<string, A>): Record<string, A> | undefined {
+  if (Object.keys(o).length === 0) { return undefined; }
+  return o;
+}
diff --git a/packages/@aws-cdk/aws-stepfunctions/test/test.state-machine-resources.ts b/packages/@aws-cdk/aws-stepfunctions/test/test.state-machine-resources.ts
@@ -130,6 +130,48 @@ export = {
         test.done();
     },
 
+    'Task combines taskobject parameters with direct parameters'(test: Test) {
+        // GIVEN
+        const stack = new cdk.Stack();
+        const task = new stepfunctions.Task(stack, 'Task', {
+            inputPath: "$",
+            outputPath: "$.state",
+            task: {
+                bind: () => ({
+                    resourceArn: 'resource',
+                    parameters: {
+                        a: "aa",
+                    }
+                })
+            },
+            parameters: {
+                b: "bb"
+            }
+        });
+
+        // WHEN
+        const taskState = task.toStateJson();
+
+        // THEN
+        test.deepEqual(taskState, { End: true,
+            Retry: undefined,
+            Catch: undefined,
+            InputPath: '$',
+            Parameters:
+             { a: 'aa',
+               b: 'bb', },
+            OutputPath: '$.state',
+            Type: 'Task',
+            Comment: undefined,
+            Resource: 'resource',
+            ResultPath: undefined,
+            TimeoutSeconds: undefined,
+            HeartbeatSeconds: undefined
+        });
+
+        test.done();
+    },
+
     'Can grant start execution to a role'(test: Test) {
         // GIVEN
         const stack = new cdk.Stack();