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.

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

fix(stepfunctions): map property maxConcurrency is not token-aware #20279

Merged
merged 2 commits into from
May 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion packages/@aws-cdk/aws-stepfunctions/lib/states/map.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Token } from '@aws-cdk/core';
import { Construct } from 'constructs';
import { Chain } from '../chain';
import { FieldUtils } from '../fields';
Expand Down Expand Up @@ -190,7 +191,7 @@ export class Map extends State implements INextable {
errors.push('Map state must have a non-empty iterator');
}

if (this.maxConcurrency !== undefined && !isPositiveInteger(this.maxConcurrency)) {
if (this.maxConcurrency !== undefined && !Token.isUnresolved(this.maxConcurrency) && !isPositiveInteger(this.maxConcurrency)) {
errors.push('maxConcurrency has to be a positive integer');
}

Expand Down
26 changes: 26 additions & 0 deletions packages/@aws-cdk/aws-stepfunctions/test/map.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ describe('Map State', () => {
},
});
}),

test('State Machine With Map State and ResultSelector', () => {
// GIVEN
const stack = new cdk.Stack();
Expand Down Expand Up @@ -84,6 +85,7 @@ describe('Map State', () => {
},
});
}),

test('synth is successful', () => {
const app = createAppWithMap((stack) => {
const map = new stepfunctions.Map(stack, 'Map State', {
Expand All @@ -96,6 +98,7 @@ describe('Map State', () => {

app.synth();
}),

test('fails in synthesis if iterator is missing', () => {
const app = createAppWithMap((stack) => {
const map = new stepfunctions.Map(stack, 'Map State', {
Expand All @@ -108,6 +111,7 @@ describe('Map State', () => {

expect(() => app.synth()).toThrow(/Map state must have a non-empty iterator/);
}),

test('fails in synthesis when maxConcurrency is a float', () => {
const app = createAppWithMap((stack) => {
const map = new stepfunctions.Map(stack, 'Map State', {
Expand All @@ -121,6 +125,7 @@ describe('Map State', () => {

expect(() => app.synth()).toThrow(/maxConcurrency has to be a positive integer/);
}),

test('fails in synthesis when maxConcurrency is a negative integer', () => {
const app = createAppWithMap((stack) => {
const map = new stepfunctions.Map(stack, 'Map State', {
Expand All @@ -134,6 +139,7 @@ describe('Map State', () => {

expect(() => app.synth()).toThrow(/maxConcurrency has to be a positive integer/);
}),

test('fails in synthesis when maxConcurrency is too big to be an integer', () => {
const app = createAppWithMap((stack) => {
const map = new stepfunctions.Map(stack, 'Map State', {
Expand All @@ -147,22 +153,42 @@ describe('Map State', () => {

expect(() => app.synth()).toThrow(/maxConcurrency has to be a positive integer/);
}),

test('does not fail synthesis when maxConcurrency is a jsonPath', () => {
const app = createAppWithMap((stack) => {
const map = new stepfunctions.Map(stack, 'Map State', {
maxConcurrency: stepfunctions.JsonPath.numberAt('$.maxConcurrency'),
itemsPath: stepfunctions.JsonPath.stringAt('$.inputForMap'),
});
map.iterator(new stepfunctions.Pass(stack, 'Pass State'));

return map;
});

expect(() => app.synth()).not.toThrow();
});

test('isPositiveInteger is false with negative number', () => {
expect(stepfunctions.isPositiveInteger(-1)).toEqual(false);
}),

test('isPositiveInteger is false with decimal number', () => {
expect(stepfunctions.isPositiveInteger(1.2)).toEqual(false);
}),

test('isPositiveInteger is false with a value greater than safe integer', () => {
const valueToTest = Number.MAX_SAFE_INTEGER + 1;
expect(stepfunctions.isPositiveInteger(valueToTest)).toEqual(false);
}),

test('isPositiveInteger is true with 0', () => {
expect(stepfunctions.isPositiveInteger(0)).toEqual(true);
}),

test('isPositiveInteger is true with 10', () => {
expect(stepfunctions.isPositiveInteger(10)).toEqual(true);
}),

test('isPositiveInteger is true with max integer value', () => {
expect(stepfunctions.isPositiveInteger(Number.MAX_SAFE_INTEGER)).toEqual(true);
});
Expand Down