From adcd8c31c4a3c5d453fea931b32d40534763daa5 Mon Sep 17 00:00:00 2001
From: Akshay Gupta <Dolvic@users.noreply.github.com>
Date: Tue, 22 Jun 2021 10:31:29 -0700
Subject: [PATCH] fix(core): `1 hour` renders as `60 minutes` (#15125)

When asked to convert full hours to human strings, due to floating point errors, they get rendered as minutes. 1 hour gets converted to 60 minutes. 2 hours converts to 1 hour 60 minutes.

This request fixes that by first multiplying `amount` with the `fromUnit.inMillis` before dividing by `toUnit.inMillis`. This ensures that if the amount is big enough, it gets divided properly.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
---
 .../@aws-cdk/aws-cognito/test/user-pool-client.test.ts | 10 +++++-----
 packages/@aws-cdk/core/lib/duration.ts                 |  3 +--
 packages/@aws-cdk/core/test/duration.test.ts           |  7 ++++++-
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/packages/@aws-cdk/aws-cognito/test/user-pool-client.test.ts b/packages/@aws-cdk/aws-cognito/test/user-pool-client.test.ts
index 084ea563cb477..22055828bebe5 100644
--- a/packages/@aws-cdk/aws-cognito/test/user-pool-client.test.ts
+++ b/packages/@aws-cdk/aws-cognito/test/user-pool-client.test.ts
@@ -670,7 +670,7 @@ describe('User Pool Client', () => {
           accessTokenValidity: validity,
           refreshTokenValidity: Duration.hours(1),
         });
-      }).toThrow(`accessTokenValidity: Must be a duration between 5 minutes and 60 minutes (inclusive); received ${validity.toHumanString()}.`);
+      }).toThrow(`accessTokenValidity: Must be a duration between 5 minutes and 1 hour (inclusive); received ${validity.toHumanString()}.`);
     });
 
     test.each([
@@ -686,7 +686,7 @@ describe('User Pool Client', () => {
           idTokenValidity: validity,
           refreshTokenValidity: Duration.hours(1),
         });
-      }).toThrow(`idTokenValidity: Must be a duration between 5 minutes and 60 minutes (inclusive); received ${validity.toHumanString()}.`);
+      }).toThrow(`idTokenValidity: Must be a duration between 5 minutes and 1 hour (inclusive); received ${validity.toHumanString()}.`);
     });
 
     test.each([
@@ -694,7 +694,7 @@ describe('User Pool Client', () => {
       Duration.minutes(59),
       Duration.days(10 * 365).plus(Duration.minutes(1)),
       Duration.days(10 * 365 + 1),
-    ])('validates refreshTokenValidity is a duration between 60 minutes and 10 years', (validity) => {
+    ])('validates refreshTokenValidity is a duration between 1 hour and 10 years', (validity) => {
       const stack = new Stack();
       const pool = new UserPool(stack, 'Pool');
       expect(() => {
@@ -702,7 +702,7 @@ describe('User Pool Client', () => {
           userPoolClientName: 'Client1',
           refreshTokenValidity: validity,
         });
-      }).toThrow(`refreshTokenValidity: Must be a duration between 60 minutes and 3650 days (inclusive); received ${validity.toHumanString()}.`);
+      }).toThrow(`refreshTokenValidity: Must be a duration between 1 hour and 3650 days (inclusive); received ${validity.toHumanString()}.`);
     });
 
     test.each([
@@ -758,7 +758,7 @@ describe('User Pool Client', () => {
       Duration.minutes(120),
       Duration.days(365),
       Duration.days(10 * 365),
-    ])('validates refreshTokenValidity is a duration between 60 minutes and 10 years (valid)', (validity) => {
+    ])('validates refreshTokenValidity is a duration between 1 hour and 10 years (valid)', (validity) => {
       const stack = new Stack();
       const pool = new UserPool(stack, 'Pool');
 
diff --git a/packages/@aws-cdk/core/lib/duration.ts b/packages/@aws-cdk/core/lib/duration.ts
index b1c675d3ce722..0061e7928e900 100644
--- a/packages/@aws-cdk/core/lib/duration.ts
+++ b/packages/@aws-cdk/core/lib/duration.ts
@@ -309,12 +309,11 @@ class TimeUnit {
 
 function convert(amount: number, fromUnit: TimeUnit, toUnit: TimeUnit, { integral = true }: TimeConversionOptions) {
   if (fromUnit.inMillis === toUnit.inMillis) { return amount; }
-  const multiplier = fromUnit.inMillis / toUnit.inMillis;
 
   if (Token.isUnresolved(amount)) {
     throw new Error(`Unable to perform time unit conversion on un-resolved token ${amount}.`);
   }
-  const value = amount * multiplier;
+  const value = (amount * fromUnit.inMillis) / toUnit.inMillis;
   if (!Number.isInteger(value) && integral) {
     throw new Error(`'${amount} ${fromUnit}' cannot be converted into a whole number of ${toUnit}.`);
   }
diff --git a/packages/@aws-cdk/core/test/duration.test.ts b/packages/@aws-cdk/core/test/duration.test.ts
index 75b92c76924c2..5b04827d72f07 100644
--- a/packages/@aws-cdk/core/test/duration.test.ts
+++ b/packages/@aws-cdk/core/test/duration.test.ts
@@ -147,8 +147,13 @@ nodeunitShim({
     test.equal(Duration.minutes(0).toHumanString(), '0 minutes');
     test.equal(Duration.minutes(Lazy.number({ produce: () => 5 })).toHumanString(), '<token> minutes');
 
-    test.equal(Duration.minutes(10).toHumanString(), '10 minutes');
+    test.equal(Duration.days(1).toHumanString(), '1 day');
+    test.equal(Duration.hours(1).toHumanString(), '1 hour');
     test.equal(Duration.minutes(1).toHumanString(), '1 minute');
+    test.equal(Duration.seconds(1).toHumanString(), '1 second');
+    test.equal(Duration.millis(1).toHumanString(), '1 milli');
+
+    test.equal(Duration.minutes(10).toHumanString(), '10 minutes');
 
     test.equal(Duration.minutes(62).toHumanString(), '1 hour 2 minutes');