Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(aws-stepfunctions, aws-stepfunctions-tasks): missing suffix in fi… #2939

Merged
merged 5 commits into from
Jun 21, 2019
Merged

fix(aws-stepfunctions, aws-stepfunctions-tasks): missing suffix in fi… #2939

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a75dc02
fix(aws-stepfunctions, aws-stepfunctions-tasks): missing suffix in fi…
wqzoww Jun 19, 2019
dcf972b
Move renderObject() to consuming class so it can't be forgotten
rix0rrr Jun 20, 2019
c754c52
Merge branch 'master' into master
rix0rrr Jun 20, 2019
9d529e3
Merge remote-tracking branch 'origin/master' into pr/wqzoww/2939
rix0rrr Jun 20, 2019
125a259
feat(aws-stepfunctions-tasks) complete supporting parameters for lambda
wqzoww Jun 21, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-stepfunctions-tasks/test/integ.invoke-function.expected.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@
{
"Ref": "CallbackHandler4434C38D"
},
"\",\"Payload\":{\"token\":\"$$.Task.Token\"}},\"Type\":\"Task\",\"Resource\":\"arn:aws:states:::lambda:invoke.waitForTaskToken\",\"ResultPath\":\"$.status\"},\"Job Complete?\":{\"Type\":\"Choice\",\"Choices\":[{\"Variable\":\"$.status\",\"StringEquals\":\"FAILED\",\"Next\":\"Job Failed\"},{\"Variable\":\"$.status\",\"StringEquals\":\"SUCCEEDED\",\"Next\":\"Final step\"}]},\"Job Failed\":{\"Type\":\"Fail\",\"Error\":\"DescribeJob returned FAILED\",\"Cause\":\"AWS Batch Job Failed\"},\"Final step\":{\"Type\":\"Pass\",\"End\":true}},\"TimeoutSeconds\":30}"
"\",\"Payload\":{\"token.$\":\"$$.Task.Token\"}},\"Type\":\"Task\",\"Resource\":\"arn:aws:states:::lambda:invoke.waitForTaskToken\",\"ResultPath\":\"$.status\"},\"Job Complete?\":{\"Type\":\"Choice\",\"Choices\":[{\"Variable\":\"$.status\",\"StringEquals\":\"FAILED\",\"Next\":\"Job Failed\"},{\"Variable\":\"$.status\",\"StringEquals\":\"SUCCEEDED\",\"Next\":\"Final step\"}]},\"Job Failed\":{\"Type\":\"Fail\",\"Error\":\"DescribeJob returned FAILED\",\"Cause\":\"AWS Batch Job Failed\"},\"Final step\":{\"Type\":\"Pass\",\"End\":true}},\"TimeoutSeconds\":30}"
]
]
},
Expand Down
41 changes: 3 additions & 38 deletions packages/@aws-cdk/aws-stepfunctions-tasks/test/invoke-function.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ beforeEach(() => {
});
});

test('Lambda function can be used in a Task', () => {
test('Invoke lambda with function ARN', () => {
// WHEN
const task = new sfn.Task(stack, 'Task', { task: new tasks.InvokeFunction(fn) });
new sfn.StateMachine(stack, 'SM', {
Expand All @@ -39,7 +39,7 @@ test('Lambda function payload ends up in Parameters', () => {
definition: new sfn.Task(stack, 'Task', {
task: new tasks.InvokeFunction(fn, {
payload: {
foo: 'bar'
foo: sfn.Data.stringAt('$.bar')
}
})
})
Expand All @@ -48,45 +48,10 @@ test('Lambda function payload ends up in Parameters', () => {
expect(stack).toHaveResource('AWS::StepFunctions::StateMachine', {
DefinitionString: {
"Fn::Join": ["", [
"{\"StartAt\":\"Task\",\"States\":{\"Task\":{\"End\":true,\"Parameters\":{\"foo\":\"bar\"},\"Type\":\"Task\",\"Resource\":\"",
"{\"StartAt\":\"Task\",\"States\":{\"Task\":{\"End\":true,\"Parameters\":{\"foo.$\":\"$.bar\"},\"Type\":\"Task\",\"Resource\":\"",
{ "Fn::GetAtt": ["Fn9270CBC0", "Arn"] },
"\"}}}"
]]
},
});
});

test('Lambda function can be used in a Task with Task Token', () => {
const task = new sfn.Task(stack, 'Task', {
task: new tasks.RunLambdaTask(fn, {
waitForTaskToken: true,
payload: {
token: sfn.Context.taskToken
}
})
});
new sfn.StateMachine(stack, 'SM', {
definition: task
});

// THEN
expect(stack).toHaveResource('AWS::StepFunctions::StateMachine', {
DefinitionString: {
"Fn::Join": ["", [
"{\"StartAt\":\"Task\",\"States\":{\"Task\":{\"End\":true,\"Parameters\":{\"FunctionName\":\"",
{ Ref: "Fn9270CBC0" },
"\",\"Payload\":{\"token\":\"$$.Task.Token\"}},\"Type\":\"Task\",\"Resource\":\"arn:aws:states:::lambda:invoke.waitForTaskToken\"}}}"
]]
},
});
});

test('Task throws if waitForTaskToken is supplied but task token is not included', () => {
expect(() => {
new sfn.Task(stack, 'Task', {
task: new tasks.RunLambdaTask(fn, {
waitForTaskToken: true
})
});
}).toThrow(/Task Token is missing in payload/i);
});
73 changes: 73 additions & 0 deletions packages/@aws-cdk/aws-stepfunctions-tasks/test/run-lambda-task.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import '@aws-cdk/assert/jest';
import lambda = require('@aws-cdk/aws-lambda');
import sfn = require('@aws-cdk/aws-stepfunctions');
import { Stack } from '@aws-cdk/cdk';
import tasks = require('../lib');

let stack: Stack;
let fn: lambda.Function;
beforeEach(() => {
stack = new Stack();
fn = new lambda.Function(stack, 'Fn', {
code: lambda.Code.inline('hello'),
handler: 'index.hello',
runtime: lambda.Runtime.Python27,
});
});

test('Invoke lambda with default magic ARN', () => {
const task = new sfn.Task(stack, 'Task', {
task: new tasks.RunLambdaTask(fn, {
payload: {
foo: 'bar'
}
})
});
new sfn.StateMachine(stack, 'SM', {
definition: task
});

expect(stack).toHaveResource('AWS::StepFunctions::StateMachine', {
DefinitionString: {
"Fn::Join": ["", [
"{\"StartAt\":\"Task\",\"States\":{\"Task\":{\"End\":true,\"Parameters\":{\"FunctionName\":\"",
{ Ref: "Fn9270CBC0" },
"\",\"Payload\":{\"foo\":\"bar\"}},\"Type\":\"Task\",\"Resource\":\"arn:aws:states:::lambda:invoke\"}}}"
]]
},
});
});

test('Lambda function can be used in a Task with Task Token', () => {
const task = new sfn.Task(stack, 'Task', {
task: new tasks.RunLambdaTask(fn, {
waitForTaskToken: true,
payload: {
token: sfn.Context.taskToken
}
})
});
new sfn.StateMachine(stack, 'SM', {
definition: task
});

expect(stack).toHaveResource('AWS::StepFunctions::StateMachine', {
DefinitionString: {
"Fn::Join": ["", [
"{\"StartAt\":\"Task\",\"States\":{\"Task\":{\"End\":true,\"Parameters\":{\"FunctionName\":\"",
{ Ref: "Fn9270CBC0" },
"\",\"Payload\":{\"token.$\":\"$$.Task.Token\"}},\"Type\":\"Task\",\"Resource\":\"arn:aws:states:::lambda:invoke.waitForTaskToken\"}}}"
]]
},
});
});

test('Task throws if waitForTaskToken is supplied but task token is not included', () => {
expect(() => {
new sfn.Task(stack, 'Task', {
task: new tasks.RunLambdaTask(fn, {
waitForTaskToken: true
})
});
}).toThrow(/Task Token is missing in payload/i);
});
21 changes: 18 additions & 3 deletions packages/@aws-cdk/aws-stepfunctions/lib/json-path.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ export function renderObject(obj: object | undefined): object | undefined {
return recurseObject(obj, {
handleString: renderString,
handleList: renderStringList,
handleNumber: renderNumber
handleNumber: renderNumber,
handleBoolean: renderBoolean,
});
}

Expand All @@ -63,6 +64,10 @@ export function findReferencedPaths(obj: object | undefined): Set<string> {
const path = jsonPathNumber(x);
if (path !== undefined) { found.add(path); }
return {};
},

handleBoolean(_key: string, _x: boolean) {
return {};
}
});

Expand All @@ -73,6 +78,7 @@ interface FieldHandlers {
handleString(key: string, x: string): {[key: string]: string};
handleList(key: string, x: string[]): {[key: string]: string[] | string };
handleNumber(key: string, x: number): {[key: string]: number | string};
handleBoolean(key: string, x: boolean): {[key: string]: boolean};
}

export function recurseObject(obj: object | undefined, handlers: FieldHandlers): object | undefined {
Expand All @@ -86,6 +92,8 @@ export function recurseObject(obj: object | undefined, handlers: FieldHandlers):
Object.assign(ret, handlers.handleNumber(key, value));
} else if (Array.isArray(value)) {
Object.assign(ret, recurseArray(key, value, handlers));
} else if (typeof value === 'boolean') {
Object.assign(ret, handlers.handleBoolean(key, value));
} else if (value === null || value === undefined) {
// Nothing
} else if (typeof value === 'object') {
Expand Down Expand Up @@ -144,7 +152,7 @@ function renderString(key: string, value: string): {[key: string]: string} {
}

/**
* Render a parameter string
* Render a parameter string list
*
* If the string value starts with '$.', render it as a path string, otherwise as a direct string.
*/
Expand All @@ -158,7 +166,7 @@ function renderStringList(key: string, value: string[]): {[key: string]: string[
}

/**
* Render a parameter string
* Render a parameter number
*
* If the string value starts with '$.', render it as a path string, otherwise as a direct string.
*/
Expand All @@ -171,6 +179,13 @@ function renderNumber(key: string, value: number): {[key: string]: number | stri
}
}

/**
* Render a parameter boolean
*/
function renderBoolean(key: string, value: boolean): {[key: string]: boolean} {
return { [key]: value };
}

/**
* If the indicated string is an encoded JSON path, return the path
*
Expand Down
3 changes: 2 additions & 1 deletion packages/@aws-cdk/aws-stepfunctions/lib/state-graph.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import iam = require('@aws-cdk/aws-iam');
import { FieldUtils } from "./fields";
import { State } from "./states/state";

/**
Expand Down Expand Up @@ -101,7 +102,7 @@ export class StateGraph {
public toGraphJson(): object {
const states: any = {};
for (const state of this.allStates) {
states[state.stateId] = state.toStateJson();
states[state.stateId] = FieldUtils.renderObject(state.toStateJson());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this should be the responsibility of the individual tasks. It only makes sense in the Parameters field, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right. I forgot the reference path only applies for Parameters field, rather than the entire block of state.

}

return {
Expand Down
8 changes: 7 additions & 1 deletion packages/@aws-cdk/aws-stepfunctions/test/test.fields.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import { Context, Data, FieldUtils } from "../lib";
export = {
'deep replace correctly handles fields in arrays'(test: Test) {
test.deepEqual(FieldUtils.renderObject({
unknown: undefined,
bool: true,
literal: 'literal',
field: Data.stringAt('$.stringField'),
listField: Data.listAt('$.listField'),
Expand All @@ -14,6 +16,7 @@ export = {
}
]
}), {
'bool': true,
'literal': 'literal',
'field.$': '$.stringField',
'listField.$': '$.listField',
Expand All @@ -33,17 +36,20 @@ export = {
str: Context.stringAt('$$.Execution.StartTime'),
count: Context.numberAt('$$.State.RetryCount'),
token: Context.taskToken,
entire: Context.entireContext
}), {
'str.$': '$$.Execution.StartTime',
'count.$': '$$.State.RetryCount',
'token.$': '$$.Task.Token'
'token.$': '$$.Task.Token',
'entire.$': '$$'
});

test.done();
},

'find all referenced paths'(test: Test) {
test.deepEqual(FieldUtils.findReferencedPaths({
bool: false,
literal: 'literal',
field: Data.stringAt('$.stringField'),
listField: Data.listAt('$.listField'),
Expand Down