Skip to content

Commit

Permalink
fix(stepfunctions): map property maxConcurrency is not token-aware (#…
Browse files Browse the repository at this point in the history
…20279)

Allows specifying `JsonPath.numberAt()` in `maxConcurrency` without synth-time errors and fixes #20152 

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
kaizencc authored May 11, 2022
1 parent abc1f57 commit 14be764
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 1 deletion.
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

0 comments on commit 14be764

Please sign in to comment.