fix(core): Durations in the expected unit are not tested for integer-…

…ness (#20742)

----
Converting from a type to itself would not check the integral flag and simply return the current duration amount rather than throwing an error if the amount converted to was fractional.

Commands such as `toSeconds()` or `toMinutes()` had the same behavior since they leverage `convert()`.

`Duration.minutes(0.5).toMinutes();` previously would not throw an error, but `Duration.seconds(30).toMinutes();` would. 
`Duration.minutes(0.5).toMinutes();` will now throw an error while `Duration.minutes(0.5).toMinutes({ integral: false});` will not.

fix(@aws-cdk/aws-events/schedule.ts): `rate()` now correctly catches if a duration is defined in fractions of a minute. The root cause was the use of `Duration.toMinutes()` in `rate()`. Now all rates defined with durations must be in whole minutes.

docs: this limitation on rates is highlighted more clearly in the documentation for aws-events/schedule.ts and aws-events/rule.ts. 

This closes #17814.

### All Submissions:

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

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/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/main/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*

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/);
+});

packages/@aws-cdk/aws-cloudfront/test/origin.test.ts

@@ -13,18 +13,27 @@ 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,
     });
   }).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(() => {

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', () => {

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;

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');
     }
 

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/);
+  });
+});

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})`);
   }

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);
-}
+}