Skip to content

Commit a1da772

Browse files
authoredDec 6, 2021
fix(stepfunctions): prefixes not appended to states in parallel branches (#17806)
There is an order mismatch issue when calling `Parallel.branch()`. `branch` used to immediately create the state graph of the branch which locks in the `stateIds` of the branch states. This fails to take into account the following scenario: ```ts class ParallelMachineFragment extends stepfunctions.StateMachineFragment { public readonly startState: stepfunctions.State; public readonly endStates: stepfunctions.INextable[]; constructor(scope: Construct, id: string) { super(scope, id); const step1 = new stepfunctions.Pass(this, 'Step 1'); const parallelState = new stepfunctions.Parallel(this, 'Parallel State'); const chain = parallelState.branch(step1); this.startState = parallelState; this.endStates = [chain]; } } class MyStack extends Stack { constructor(scope: Construct, id: string) { const fragment1 = new ParallelMachineFragment(this, 'Fragment 1').prefixStates(); const fragment2 = new ParallelMachineFragment(this, 'Fragment 2').prefixStates(); new stepfunctions.StateMachine(stack, 'State Machine', { definition: fragment1.next(fragment2), }); } } ``` Here, the branches in the `parallelState` do not get prefixes before they are evaluated, causing an error since they have the same `stateId`. The fix is to make the state graph creation late-bound; it now happens when we call `Parallel.bindToGraph()`. At this point, it will know of any prefixes. Fixes #17354. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 7f45de4 commit a1da772

File tree

3 files changed

+77
-4
lines changed

3 files changed

+77
-4
lines changed
 

‎packages/@aws-cdk/aws-stepfunctions/lib/states/parallel.ts

+15-2
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ export interface ParallelProps {
7171
*/
7272
export class Parallel extends State implements INextable {
7373
public readonly endStates: INextable[];
74+
private readonly _branches: IChainable[] = [];
7475

7576
constructor(scope: Construct, id: string, props: ParallelProps = {}) {
7677
super(scope, id, props);
@@ -112,11 +113,23 @@ export class Parallel extends State implements INextable {
112113
* Define one or more branches to run in parallel
113114
*/
114115
public branch(...branches: IChainable[]): Parallel {
115-
for (const branch of branches) {
116+
// Store branches for late-bound stategraph creation when we call bindToGraph.
117+
this._branches.push(...branches);
118+
return this;
119+
}
120+
121+
/**
122+
* Overwrites State.bindToGraph. Adds branches to
123+
* the Parallel state here so that any necessary
124+
* prefixes are appended first.
125+
*/
126+
public bindToGraph(graph: StateGraph) {
127+
for (const branch of this._branches) {
116128
const name = `Parallel '${this.stateId}' branch ${this.branches.length + 1}`;
117129
super.addBranch(new StateGraph(branch.startState, name));
118130
}
119-
return this;
131+
this._branches.splice(0, this._branches.length);
132+
return super.bindToGraph(graph);
120133
}
121134

122135
/**
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import { Match, Template } from '@aws-cdk/assertions';
2+
import * as cdk from '@aws-cdk/core';
3+
import { Construct } from 'constructs';
4+
import * as stepfunctions from '../lib';
5+
6+
describe('State Machine Fragment', () => {
7+
test('Prefix applied correctly on Fragments with Parallel states', () => {
8+
// GIVEN
9+
const stack = new cdk.Stack();
10+
11+
// WHEN
12+
const fragment1 = new ParallelMachineFragment(stack, 'Fragment 1').prefixStates();
13+
const fragment2 = new ParallelMachineFragment(stack, 'Fragment 2').prefixStates();
14+
15+
new stepfunctions.StateMachine(stack, 'State Machine', {
16+
definition: fragment1.next(fragment2),
17+
});
18+
19+
// THEN
20+
Template.fromStack(stack).hasResourceProperties('AWS::StepFunctions::StateMachine', {
21+
DefinitionString: Match.serializedJson({
22+
StartAt: 'Fragment 1: Parallel State',
23+
States: {
24+
'Fragment 1: Parallel State': Match.objectLike({
25+
Branches: [Match.objectLike({
26+
States: {
27+
'Fragment 1: Step 1': Match.anyValue(),
28+
},
29+
})],
30+
}),
31+
'Fragment 2: Parallel State': Match.objectLike({
32+
Branches: [Match.objectLike({
33+
States: {
34+
'Fragment 2: Step 1': Match.anyValue(),
35+
},
36+
})],
37+
}),
38+
},
39+
}),
40+
});
41+
});
42+
});
43+
44+
class ParallelMachineFragment extends stepfunctions.StateMachineFragment {
45+
public readonly startState: stepfunctions.State;
46+
public readonly endStates: stepfunctions.INextable[];
47+
48+
constructor(scope: Construct, id: string) {
49+
super(scope, id);
50+
51+
const step1 = new stepfunctions.Pass(this, 'Step 1');
52+
const parallelState = new stepfunctions.Parallel(this, 'Parallel State');
53+
const chain = parallelState.branch(step1);
54+
this.startState = parallelState;
55+
this.endStates = [chain];
56+
}
57+
}

‎packages/@aws-cdk/aws-stepfunctions/test/states-language.test.ts

+5-2
Original file line numberDiff line numberDiff line change
@@ -613,9 +613,12 @@ describe('States Language', () => {
613613
const state1 = new stepfunctions.Pass(stack, 'State1');
614614
const state2 = new stepfunctions.Pass(stack, 'State2');
615615

616-
expect(() => new stepfunctions.Parallel(stack, 'Parallel')
616+
const parallel = new stepfunctions.Parallel(stack, 'Parallel')
617617
.branch(state1.next(state2))
618-
.branch(state2)).toThrow();
618+
.branch(state2);
619+
620+
// WHEN
621+
expect(() => render(parallel)).toThrow();
619622
}),
620623

621624
describe('findReachableStates', () => {

0 commit comments

Comments
 (0)
Please sign in to comment.