From fbd9d8b43563e96703e71c97f00503524439e5ef Mon Sep 17 00:00:00 2001 From: Sneha Solanki Date: Mon, 15 Feb 2021 22:03:12 +0100 Subject: [PATCH] fix(core/events/appscaling): allow schedule in application autoscaling and events to consider token in duration (#9413) feat(events,applicationautoscaling): schedule to check for minutes/hours/days, fixed cr comments, Fix for Issue: #9413 feat(events,applicationautoscaling): schedule fixed cr comments --- .../lib/schedule.ts | 7 ++++ .../test/test.cron.ts | 27 ++++++++++-- packages/@aws-cdk/aws-events/README.md | 1 + packages/@aws-cdk/aws-events/lib/schedule.ts | 7 ++++ .../@aws-cdk/aws-events/test/test.rule.ts | 33 +++++++++++++++ .../@aws-cdk/aws-events/test/test.schedule.ts | 42 +++++++++++++++++-- packages/@aws-cdk/core/lib/duration.ts | 24 ++++++++++- packages/@aws-cdk/core/test/duration.test.ts | 26 ++++++++++++ 8 files changed, 160 insertions(+), 7 deletions(-) diff --git a/packages/@aws-cdk/aws-applicationautoscaling/lib/schedule.ts b/packages/@aws-cdk/aws-applicationautoscaling/lib/schedule.ts index c576e17453557..cdeb00be9a426 100644 --- a/packages/@aws-cdk/aws-applicationautoscaling/lib/schedule.ts +++ b/packages/@aws-cdk/aws-applicationautoscaling/lib/schedule.ts @@ -17,6 +17,13 @@ export abstract class Schedule { * Construct a schedule from an interval and a time unit */ public static rate(duration: Duration): Schedule { + const validDurationUnit = ['minute', 'minutes', 'hour', 'hours', 'day', 'days']; + if (!validDurationUnit.includes(duration.unitLabel())) { + throw new Error("Allowed unit for scheduling is: 'minute', 'minutes', 'hour', 'hours', 'day' or 'days'"); + } + if (duration.isUnresolved()) { + return new LiteralSchedule(`rate(${duration.formatTokenToNumber()})`); + } if (duration.toSeconds() === 0) { throw new Error('Duration cannot be 0'); } diff --git a/packages/@aws-cdk/aws-applicationautoscaling/test/test.cron.ts b/packages/@aws-cdk/aws-applicationautoscaling/test/test.cron.ts index 6af9c525c9bf9..ca0fb69812b10 100644 --- a/packages/@aws-cdk/aws-applicationautoscaling/test/test.cron.ts +++ b/packages/@aws-cdk/aws-applicationautoscaling/test/test.cron.ts @@ -1,4 +1,4 @@ -import { Duration } from '@aws-cdk/core'; +import { Duration, Stack, Lazy } from '@aws-cdk/core'; import { Test } from 'nodeunit'; import * as appscaling from '../lib'; @@ -15,8 +15,15 @@ export = { 'rate must be whole number of minutes'(test: Test) { test.throws(() => { - appscaling.Schedule.rate(Duration.seconds(12345)); - }, /'12345 seconds' cannot be converted into a whole number of minutes/); + appscaling.Schedule.rate(Duration.minutes(0.13456)); + }, /'0.13456 minutes' cannot be converted into a whole number of seconds/); + test.done(); + }, + + 'rate must not be in seconds'(test: Test) { + test.throws(() => { + appscaling.Schedule.rate(Duration.seconds(1)); + }, /Allowed unit for scheduling is: 'minute', 'minutes', 'hour', 'hours', 'day' or 'days'/); test.done(); }, @@ -26,4 +33,18 @@ export = { }, /Duration cannot be 0/); test.done(); }, + + 'rate can be token'(test: Test) { + const stack = new Stack(); + const lazyDuration = Duration.minutes(Lazy.number({ produce: () => 5 })); + const rate = appscaling.Schedule.rate(lazyDuration); + test.equal('rate(5 minutes)', stack.resolve(rate).expressionString); + test.done(); + }, + + 'rate can be in allowed type hours'(test: Test) { + test.equal('rate(1 hour)', appscaling.Schedule.rate(Duration.hours(1)) + .expressionString); + test.done(); + }, }; diff --git a/packages/@aws-cdk/aws-events/README.md b/packages/@aws-cdk/aws-events/README.md index 565b0537d091d..465e1fb73e1b5 100644 --- a/packages/@aws-cdk/aws-events/README.md +++ b/packages/@aws-cdk/aws-events/README.md @@ -83,6 +83,7 @@ onCommitRule.addTarget(new targets.SnsTopic(topic, { ## Scheduling You can configure a Rule to run on a schedule (cron or rate). +Rate must be specified in minutes, hours or days. The following example runs a task every day at 4am: diff --git a/packages/@aws-cdk/aws-events/lib/schedule.ts b/packages/@aws-cdk/aws-events/lib/schedule.ts index 8c2c04481c0d2..c12afde30aa01 100644 --- a/packages/@aws-cdk/aws-events/lib/schedule.ts +++ b/packages/@aws-cdk/aws-events/lib/schedule.ts @@ -17,6 +17,13 @@ export abstract class Schedule { * Construct a schedule from an interval and a time unit */ public static rate(duration: Duration): Schedule { + const validDurationUnit = ['minute', 'minutes', 'hour', 'hours', 'day', 'days']; + if (validDurationUnit.indexOf(duration.unitLabel()) === -1) { + throw new Error('Allowed unit for scheduling is: \'minute\', \'minutes\', \'hour\', \'hours\', \'day\', \'days\''); + } + if (duration.isUnresolved()) { + return new LiteralSchedule(`rate(${duration.formatTokenToNumber()})`); + } if (duration.toSeconds() === 0) { throw new Error('Duration cannot be 0'); } diff --git a/packages/@aws-cdk/aws-events/test/test.rule.ts b/packages/@aws-cdk/aws-events/test/test.rule.ts index 80cf91948f5fa..72fe340242924 100644 --- a/packages/@aws-cdk/aws-events/test/test.rule.ts +++ b/packages/@aws-cdk/aws-events/test/test.rule.ts @@ -49,6 +49,39 @@ export = { test.done(); }, + 'get rate as token'(test: Test) { + const app = new cdk.App(); + const stack = new cdk.Stack(app, 'MyScheduledStack'); + const lazyDuration = cdk.Duration.minutes(cdk.Lazy.number({ produce: () => 5 })); + + new Rule(stack, 'MyScheduledRule', { + ruleName: 'rateInMinutes', + schedule: Schedule.rate(lazyDuration), + }); + + // THEN + expect(stack).to(haveResourceLike('AWS::Events::Rule', { + 'Name': 'rateInMinutes', + 'ScheduleExpression': 'rate(5 minutes)', + })); + + test.done(); + }, + + 'Seconds is not an allowed value for Schedule rate'(test: Test) { + const lazyDuration = cdk.Duration.seconds(cdk.Lazy.number({ produce: () => 5 })); + test.throws(() => Schedule.rate(lazyDuration), /Allowed unit for scheduling is: 'minute', 'minutes', 'hour', 'hours', 'day', 'days'/); + test.done(); + }, + + 'Millis is not an allowed value for Schedule rate'(test: Test) { + const lazyDuration = cdk.Duration.millis(cdk.Lazy.number({ produce: () => 5 })); + + // THEN + test.throws(() => Schedule.rate(lazyDuration), /Allowed unit for scheduling is: 'minute', 'minutes', 'hour', 'hours', 'day', 'days'/); + test.done(); + }, + 'rule with physical name'(test: Test) { // GIVEN const stack = new cdk.Stack(); diff --git a/packages/@aws-cdk/aws-events/test/test.schedule.ts b/packages/@aws-cdk/aws-events/test/test.schedule.ts index d2e045199360f..e9c24a51c92b2 100644 --- a/packages/@aws-cdk/aws-events/test/test.schedule.ts +++ b/packages/@aws-cdk/aws-events/test/test.schedule.ts @@ -1,4 +1,4 @@ -import { Duration } from '@aws-cdk/core'; +import { Duration, Stack, Lazy } from '@aws-cdk/core'; import { Test } from 'nodeunit'; import * as events from '../lib'; @@ -33,8 +33,15 @@ export = { 'rate must be whole number of minutes'(test: Test) { test.throws(() => { - events.Schedule.rate(Duration.seconds(12345)); - }, /'12345 seconds' cannot be converted into a whole number of minutes/); + events.Schedule.rate(Duration.minutes(0.13456)); + }, /'0.13456 minutes' cannot be converted into a whole number of seconds/); + test.done(); + }, + + 'rate must be whole number'(test: Test) { + test.throws(() => { + events.Schedule.rate(Duration.minutes(1/8)); + }, /'0.125 minutes' cannot be converted into a whole number of seconds/); test.done(); }, @@ -44,4 +51,33 @@ export = { }, /Duration cannot be 0/); test.done(); }, + + 'rate can be from a token'(test: Test) { + const stack = new Stack(); + const lazyDuration = Duration.minutes(Lazy.number({ produce: () => 5 })); + const rate = events.Schedule.rate(lazyDuration); + test.equal('rate(5 minutes)', stack.resolve(rate).expressionString); + test.done(); + }, + + 'rate can be in minutes'(test: Test) { + test.equal('rate(10 minutes)', + events.Schedule.rate(Duration.minutes(10)) + .expressionString); + test.done(); + }, + + 'rate can be in days'(test: Test) { + test.equal('rate(10 days)', + events.Schedule.rate(Duration.days(10)) + .expressionString); + test.done(); + }, + + 'rate can be in hours'(test: Test) { + test.equal('rate(10 hours)', + events.Schedule.rate(Duration.hours(10)) + .expressionString); + test.done(); + }, }; diff --git a/packages/@aws-cdk/core/lib/duration.ts b/packages/@aws-cdk/core/lib/duration.ts index 098744af2d0b8..b1c675d3ce722 100644 --- a/packages/@aws-cdk/core/lib/duration.ts +++ b/packages/@aws-cdk/core/lib/duration.ts @@ -1,4 +1,4 @@ -import { Token } from './token'; +import { Token, Tokenization } from './token'; /** * Represents a length of time. @@ -252,6 +252,28 @@ export class Duration { } return ret; } + + /** + * Checks if duration is a token or a resolvable object + */ + public isUnresolved() { + return Token.isUnresolved(this.amount); + } + + /** + * Returns unit of the duration + */ + public unitLabel() { + return this.unit.label; + } + + /** + * Returns stringified number of duration + */ + public formatTokenToNumber(): string { + const number = Tokenization.stringifyNumber(this.amount); + return `${number} ${this.unit.label}`; + } } /** diff --git a/packages/@aws-cdk/core/test/duration.test.ts b/packages/@aws-cdk/core/test/duration.test.ts index 70b7cb786e344..75b92c76924c2 100644 --- a/packages/@aws-cdk/core/test/duration.test.ts +++ b/packages/@aws-cdk/core/test/duration.test.ts @@ -168,6 +168,32 @@ nodeunitShim({ test.done(); }, + + 'get unit label from duration'(test: Test) { + test.equal(Duration.minutes(Lazy.number({ produce: () => 10 })).unitLabel(), 'minutes'); + test.equal(Duration.minutes(62).unitLabel(), 'minutes'); + test.equal(Duration.seconds(10).unitLabel(), 'seconds'); + test.equal(Duration.millis(1).unitLabel(), 'millis'); + test.equal(Duration.hours(1000).unitLabel(), 'hours'); + test.equal(Duration.days(2).unitLabel(), 'days'); + test.done(); + }, + + 'format number token to number'(test: Test) { + const stack = new Stack(); + const lazyDuration = Duration.minutes(Lazy.number({ produce: () => 10 })); + test.equal(stack.resolve(lazyDuration.formatTokenToNumber()), '10 minutes'); + test.equal(Duration.hours(10).formatTokenToNumber(), '10 hours'); + test.equal(Duration.days(5).formatTokenToNumber(), '5 days'); + test.done(); + }, + + 'duration is unresolved'(test: Test) { + const lazyDuration = Duration.minutes(Lazy.number({ produce: () => 10 })); + test.equal(lazyDuration.isUnresolved(), true); + test.equal(Duration.hours(10).isUnresolved(), false); + test.done(); + }, }); function floatEqual(test: Test, actual: number, expected: number) {