diff --git a/packages/@aws-cdk/aws-cloudfront-origins/test/http-origin.test.ts b/packages/@aws-cdk/aws-cloudfront-origins/test/http-origin.test.ts index f1109a7f8c7db..626f9f0cbe9ed 100644 --- a/packages/@aws-cdk/aws-cloudfront-origins/test/http-origin.test.ts +++ b/packages/@aws-cdk/aws-cloudfront-origins/test/http-origin.test.ts @@ -68,11 +68,9 @@ test('renders an example with all available props', () => { test.each([ Duration.seconds(0), - Duration.seconds(0.5), - Duration.seconds(60.5), Duration.seconds(181), Duration.minutes(5), -])('validates readTimeout is an integer between 1 and 180 seconds', (readTimeout) => { +])('validates readTimeout is an integer between 1 and 180 seconds - out of bounds', (readTimeout) => { expect(() => { new HttpOrigin('www.example.com', { readTimeout, @@ -81,15 +79,35 @@ test.each([ }); test.each([ - Duration.seconds(0), Duration.seconds(0.5), Duration.seconds(60.5), +])('validates readTimeout is an integer between 1 and 180 seconds - not an int', (readTimeout) => { + expect(() => { + new HttpOrigin('www.example.com', { + readTimeout, + }); + }).toThrow(/must be a whole number of/); +}); + +test.each([ + Duration.seconds(0), Duration.seconds(181), Duration.minutes(5), -])('validates keepaliveTimeout is an integer between 1 and 180 seconds', (keepaliveTimeout) => { +])('validates keepaliveTimeout is an integer between 1 and 180 seconds - out of bounds', (keepaliveTimeout) => { expect(() => { new HttpOrigin('www.example.com', { keepaliveTimeout, }); }).toThrow(`keepaliveTimeout: Must be an int between 1 and 180 seconds (inclusive); received ${keepaliveTimeout.toSeconds()}.`); }); + +test.each([ + Duration.seconds(0.5), + Duration.seconds(60.5), +])('validates keepaliveTimeout is an integer between 1 and 180 seconds - not an int', (keepaliveTimeout) => { + expect(() => { + new HttpOrigin('www.example.com', { + keepaliveTimeout, + }); + }).toThrow(/must be a whole number of/); +}); diff --git a/packages/@aws-cdk/aws-cloudfront/test/origin.test.ts b/packages/@aws-cdk/aws-cloudfront/test/origin.test.ts index 418bac1f3d275..38829cd484ee3 100644 --- a/packages/@aws-cdk/aws-cloudfront/test/origin.test.ts +++ b/packages/@aws-cdk/aws-cloudfront/test/origin.test.ts @@ -13,11 +13,9 @@ beforeEach(() => { test.each([ Duration.seconds(0), - Duration.seconds(0.5), - Duration.seconds(10.5), Duration.seconds(11), Duration.minutes(5), -])('validates connectionTimeout is an int between 1 and 10 seconds', (connectionTimeout) => { +])('validates connectionTimeout is an int between 1 and 10 seconds - out of bounds', (connectionTimeout) => { expect(() => { new TestOrigin('www.example.com', { connectionTimeout, @@ -25,6 +23,17 @@ test.each([ }).toThrow(`connectionTimeout: Must be an int between 1 and 10 seconds (inclusive); received ${connectionTimeout.toSeconds()}.`); }); +test.each([ + Duration.seconds(0.5), + Duration.seconds(10.5), +])('validates connectionTimeout is an int between 1 and 10 seconds - not an int', (connectionTimeout) => { + expect(() => { + new TestOrigin('www.example.com', { + connectionTimeout, + }); + }).toThrow(/must be a whole number of/); +}); + test.each([-0.5, 0.5, 1.5, 4]) ('validates connectionAttempts is an int between 1 and 3', (connectionAttempts) => { expect(() => { diff --git a/packages/@aws-cdk/aws-cloudfront/test/web-distribution.test.ts b/packages/@aws-cdk/aws-cloudfront/test/web-distribution.test.ts index 21e98fc311572..7062635c11f1b 100644 --- a/packages/@aws-cdk/aws-cloudfront/test/web-distribution.test.ts +++ b/packages/@aws-cdk/aws-cloudfront/test/web-distribution.test.ts @@ -1702,7 +1702,7 @@ added the ellipsis so a user would know there was more to r...`, customOriginSource: { domainName: 'myorigin.com' }, }], }); - }).toThrow(/connectionTimeout: You can specify a number of seconds between 1 and 10 \(inclusive\)./); + }).toThrow(/must be a whole number of/); }); test('connectionTimeout < 1', () => { diff --git a/packages/@aws-cdk/aws-events/lib/rule.ts b/packages/@aws-cdk/aws-events/lib/rule.ts index 18e7f6417aa40..5ef09a21dc69d 100644 --- a/packages/@aws-cdk/aws-events/lib/rule.ts +++ b/packages/@aws-cdk/aws-events/lib/rule.ts @@ -37,30 +37,32 @@ export interface RuleProps { /** * The schedule or rate (frequency) that determines when EventBridge - * runs the rule. For more information, see Schedule Expression Syntax for + * runs the rule. + * + * You must specify this property, the `eventPattern` property, or both. + * + * For more information, see Schedule Expression Syntax for * Rules in the Amazon EventBridge User Guide. * * @see https://docs.aws.amazon.com/eventbridge/latest/userguide/scheduled-events.html * - * You must specify this property, the `eventPattern` property, or both. - * * @default - None. */ readonly schedule?: Schedule; /** * Describes which events EventBridge routes to the specified target. - * These routed events are matched events. For more information, see Events - * and Event Patterns in the Amazon EventBridge User Guide. - * - * @see - * https://docs.aws.amazon.com/eventbridge/latest/userguide/eventbridge-and-event-patterns.html + * These routed events are matched events. * * You must specify this property (either via props or via * `addEventPattern`), the `scheduleExpression` property, or both. The * method `addEventPattern` can be used to add filter values to the event * pattern. * + * For more information, see Events and Event Patterns in the Amazon EventBridge User Guide. + * + * @see https://docs.aws.amazon.com/eventbridge/latest/userguide/eventbridge-and-event-patterns.html + * * @default - None. */ readonly eventPattern?: EventPattern; diff --git a/packages/@aws-cdk/aws-events/lib/schedule.ts b/packages/@aws-cdk/aws-events/lib/schedule.ts index acdb1595a244f..49ca801472d7c 100644 --- a/packages/@aws-cdk/aws-events/lib/schedule.ts +++ b/packages/@aws-cdk/aws-events/lib/schedule.ts @@ -3,6 +3,10 @@ import { Construct } from 'constructs'; /** * Schedule for scheduled event rules + * + * Note that rates cannot be defined in fractions of minutes. + * + * @see https://docs.aws.amazon.com/eventbridge/latest/userguide/scheduled-events.html */ export abstract class Schedule { /** @@ -16,6 +20,8 @@ export abstract class Schedule { /** * Construct a schedule from an interval and a time unit + * + * Rates may be defined with any unit of time, but when converted into minutes, the duration must be a positive whole number of minutes. */ public static rate(duration: Duration): Schedule { if (duration.isUnresolved()) { @@ -25,7 +31,7 @@ export abstract class Schedule { } return new LiteralSchedule(`rate(${duration.formatTokenToNumber()})`); } - if (duration.toSeconds() === 0) { + if (duration.toMinutes() === 0) { throw new Error('Duration cannot be 0'); } diff --git a/packages/@aws-cdk/aws-events/test/schedule.test.ts b/packages/@aws-cdk/aws-events/test/schedule.test.ts index 9dea322c62d0a..ff1eac5417a85 100644 --- a/packages/@aws-cdk/aws-events/test/schedule.test.ts +++ b/packages/@aws-cdk/aws-events/test/schedule.test.ts @@ -27,24 +27,18 @@ describe('schedule', () => { }).expressionString); }); - test('rate must be whole number of minutes', () => { - expect(() => { - events.Schedule.rate(Duration.minutes(0.13456)); - }).toThrow(/'0.13456 minutes' cannot be converted into a whole number of seconds/); - }); - - test('rate must be whole number', () => { - expect(() => { - events.Schedule.rate(Duration.minutes(1/8)); - }).toThrow(/'0.125 minutes' cannot be converted into a whole number of seconds/); - }); - test('rate cannot be 0', () => { expect(() => { events.Schedule.rate(Duration.days(0)); }).toThrow(/Duration cannot be 0/); }); + test('rate cannot be negative', () => { + expect(() => { + events.Schedule.rate(Duration.minutes(-2)); + }).toThrow(/Duration amounts cannot be negative/); + }); + test('rate can be from a token', () => { const stack = new Stack(); const lazyDuration = Duration.minutes(Lazy.number({ produce: () => 5 })); @@ -82,3 +76,41 @@ describe('schedule', () => { }).toThrow(/Allowed units for scheduling/); }); }); + +describe('fractional minutes checks', () => { + test('rate cannot be a fractional amount of minutes (defined with seconds)', () => { + expect(() => { + events.Schedule.rate(Duration.seconds(150)); + }).toThrow(/cannot be converted into a whole number of/); + }); + + test('rate cannot be a fractional amount of minutes (defined with minutes)', () => { + expect(()=> { + events.Schedule.rate(Duration.minutes(5/3)); + }).toThrow(/must be a whole number of/); + }); + + test('rate cannot be a fractional amount of minutes (defined with hours)', () => { + expect(()=> { + events.Schedule.rate(Duration.hours(1.03)); + }).toThrow(/cannot be converted into a whole number of/); + }); + + test('rate cannot be less than 1 minute (defined with seconds)', () => { + expect(() => { + events.Schedule.rate(Duration.seconds(30)); + }).toThrow(/'30 seconds' cannot be converted into a whole number of minutes./); + }); + + test('rate cannot be less than 1 minute (defined with minutes as fractions)', () => { + expect(() => { + events.Schedule.rate(Duration.minutes(1/2)); + }).toThrow(/must be a whole number of/); + }); + + test('rate cannot be less than 1 minute (defined with minutes as decimals)', () => { + expect(() => { + events.Schedule.rate(Duration.minutes(0.25)); + }).toThrow(/must be a whole number of/); + }); +}); \ No newline at end of file diff --git a/packages/@aws-cdk/core/lib/duration.ts b/packages/@aws-cdk/core/lib/duration.ts index 29cafc4f8b189..db5f9a73716ad 100644 --- a/packages/@aws-cdk/core/lib/duration.ts +++ b/packages/@aws-cdk/core/lib/duration.ts @@ -313,8 +313,12 @@ class TimeUnit { } function convert(amount: number, fromUnit: TimeUnit, toUnit: TimeUnit, { integral = true }: TimeConversionOptions) { - if (fromUnit.inMillis === toUnit.inMillis) { return amount; } - + if (fromUnit.inMillis === toUnit.inMillis) { + if (integral && !Token.isUnresolved(amount) && !Number.isInteger(amount)) { + throw new Error(`${amount} must be a whole number of ${toUnit}.`); + } + return amount; + } if (Token.isUnresolved(amount)) { throw new Error(`Duration must be specified as 'Duration.${toUnit}()' here since its value comes from a token and cannot be converted (got Duration.${fromUnit})`); } diff --git a/packages/@aws-cdk/core/test/duration.test.ts b/packages/@aws-cdk/core/test/duration.test.ts index 466369be3f4f8..c1144c53c43c2 100644 --- a/packages/@aws-cdk/core/test/duration.test.ts +++ b/packages/@aws-cdk/core/test/duration.test.ts @@ -183,9 +183,57 @@ describe('duration', () => { }); }); +describe('integral flag checks', () => { + test('convert fractional minutes to minutes', () => { + expect(() => { + Duration.minutes(0.5).toMinutes(); + }).toThrow(/must be a whole number of/); + }); + + test('convert fractional minutes to minutes - integral: false', () => { + expect(Duration.minutes(5.5).toMinutes({ integral: false })).toEqual(5.5); + }); + + test('convert whole minutes to minutes', () => { + expect(Duration.minutes(5).toMinutes()).toEqual(5); + }); + + test('convert fractional minutes to fractional seconds', () => { + expect(() => { + Duration.minutes(9/8).toSeconds(); + }).toThrow(/cannot be converted into a whole number of/); + }); + + test('convert fractional minutes to fractional seconds - integral: false', () => { + expect(Duration.minutes(9/8).toSeconds({ integral: false })).toEqual(67.5); + }); + + test('convert fractional minutes to whole seconds', () => { + expect(Duration.minutes(5/4).toSeconds({ integral: false })).toEqual(75); + }); + + test('convert whole minutes to whole seconds', () => { + expect(Duration.minutes(10).toSeconds({ integral: false })).toEqual(600); + }); + + test('convert seconds to fractional minutes', () => { + expect(() => { + Duration.seconds(45).toMinutes(); + }).toThrow(/cannot be converted into a whole number of/); + }); + + test('convert seconds to fractional minutes - integral: false', () => { + expect(Duration.seconds(45).toMinutes({ integral: false })).toEqual(0.75); + }); + + test('convert seconds to whole minutes', () => { + expect(Duration.seconds(120).toMinutes()).toEqual(2); + }); +}); + function floatEqual(actual: number, expected: number) { expect( // Floats are subject to rounding errors up to Number.ESPILON actual >= expected - Number.EPSILON && actual <= expected + Number.EPSILON, ).toEqual(true); -} +} \ No newline at end of file