Skip to content

Commit 5d62331

Browse files
author
Niranjan Jayakar
authored
fix(events,applicationautoscaling): specifying a schedule rate in seconds results in an error (#13689)
A previous change - b1449a1 - introduced a validation that should have been applied only when Duration is specified as a token. Instead, it was applied for non token values as well. Adjusting the validation so it only applies to when the duration is a token. fixes #13566 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 2445b68 commit 5d62331

File tree

5 files changed

+33
-13
lines changed

5 files changed

+33
-13
lines changed

packages/@aws-cdk/aws-applicationautoscaling/lib/schedule.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@ export abstract class Schedule {
1717
* Construct a schedule from an interval and a time unit
1818
*/
1919
public static rate(duration: Duration): Schedule {
20-
const validDurationUnit = ['minute', 'minutes', 'hour', 'hours', 'day', 'days'];
21-
if (!validDurationUnit.includes(duration.unitLabel())) {
22-
throw new Error("Allowed unit for scheduling is: 'minute', 'minutes', 'hour', 'hours', 'day' or 'days'");
23-
}
2420
if (duration.isUnresolved()) {
21+
const validDurationUnit = ['minute', 'minutes', 'hour', 'hours', 'day', 'days'];
22+
if (!validDurationUnit.includes(duration.unitLabel())) {
23+
throw new Error("Allowed units for scheduling are: 'minute', 'minutes', 'hour', 'hours', 'day' or 'days'");
24+
}
2525
return new LiteralSchedule(`rate(${duration.formatTokenToNumber()})`);
2626
}
2727
if (duration.toSeconds() === 0) {

packages/@aws-cdk/aws-applicationautoscaling/test/test.cron.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,16 @@ export = {
2020
test.done();
2121
},
2222

23-
'rate must not be in seconds'(test: Test) {
23+
'rate can be in seconds'(test: Test) {
24+
const duration = appscaling.Schedule.rate(Duration.seconds(120));
25+
test.equal('rate(2 minutes)', duration.expressionString);
26+
test.done();
27+
},
28+
29+
'rate must not be in seconds when specified as a token'(test: Test) {
2430
test.throws(() => {
25-
appscaling.Schedule.rate(Duration.seconds(1));
26-
}, /Allowed unit for scheduling is: 'minute', 'minutes', 'hour', 'hours', 'day' or 'days'/);
31+
appscaling.Schedule.rate(Duration.seconds(Lazy.number({ produce: () => 5 })));
32+
}, /Allowed units for scheduling/);
2733
test.done();
2834
},
2935

packages/@aws-cdk/aws-events/lib/schedule.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@ export abstract class Schedule {
1717
* Construct a schedule from an interval and a time unit
1818
*/
1919
public static rate(duration: Duration): Schedule {
20-
const validDurationUnit = ['minute', 'minutes', 'hour', 'hours', 'day', 'days'];
21-
if (validDurationUnit.indexOf(duration.unitLabel()) === -1) {
22-
throw new Error('Allowed unit for scheduling is: \'minute\', \'minutes\', \'hour\', \'hours\', \'day\', \'days\'');
23-
}
2420
if (duration.isUnresolved()) {
21+
const validDurationUnit = ['minute', 'minutes', 'hour', 'hours', 'day', 'days'];
22+
if (validDurationUnit.indexOf(duration.unitLabel()) === -1) {
23+
throw new Error("Allowed units for scheduling are: 'minute', 'minutes', 'hour', 'hours', 'day', 'days'");
24+
}
2525
return new LiteralSchedule(`rate(${duration.formatTokenToNumber()})`);
2626
}
2727
if (duration.toSeconds() === 0) {

packages/@aws-cdk/aws-events/test/test.rule.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,15 +70,15 @@ export = {
7070

7171
'Seconds is not an allowed value for Schedule rate'(test: Test) {
7272
const lazyDuration = cdk.Duration.seconds(cdk.Lazy.number({ produce: () => 5 }));
73-
test.throws(() => Schedule.rate(lazyDuration), /Allowed unit for scheduling is: 'minute', 'minutes', 'hour', 'hours', 'day', 'days'/);
73+
test.throws(() => Schedule.rate(lazyDuration), /Allowed units for scheduling/i);
7474
test.done();
7575
},
7676

7777
'Millis is not an allowed value for Schedule rate'(test: Test) {
7878
const lazyDuration = cdk.Duration.millis(cdk.Lazy.number({ produce: () => 5 }));
7979

8080
// THEN
81-
test.throws(() => Schedule.rate(lazyDuration), /Allowed unit for scheduling is: 'minute', 'minutes', 'hour', 'hours', 'day', 'days'/);
81+
test.throws(() => Schedule.rate(lazyDuration), /Allowed units for scheduling/i);
8282
test.done();
8383
},
8484

packages/@aws-cdk/aws-events/test/test.schedule.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,4 +80,18 @@ export = {
8080
.expressionString);
8181
test.done();
8282
},
83+
84+
'rate can be in seconds'(test: Test) {
85+
test.equal('rate(2 minutes)',
86+
events.Schedule.rate(Duration.seconds(120))
87+
.expressionString);
88+
test.done();
89+
},
90+
91+
'rate must not be in seconds when specified as a token'(test: Test) {
92+
test.throws(() => {
93+
events.Schedule.rate(Duration.seconds(Lazy.number({ produce: () => 5 })));
94+
}, /Allowed units for scheduling/);
95+
test.done();
96+
},
8397
};

0 commit comments

Comments
 (0)