PR Feedback

1. Dropped the free-floating toSeconds function 2. Added test for unresolved tokens in constructor 3. Made .toString() return a breaking token.
aws · RomainMuller · Jun 20, 2019 · Jun 12, 2019 · Jun 13, 2019 · Jun 14, 2019
commit d1ef198769875e00271c42752a2502b0cb9528e8
diff --git a/packages/@aws-cdk/aws-apigateway/lib/stage.ts b/packages/@aws-cdk/aws-apigateway/lib/stage.ts
@@ -256,7 +256,7 @@ export class Stage extends Resource {
       return {
         httpMethod, resourcePath,
         cacheDataEncrypted: options.cacheDataEncrypted,
-        cacheTtlInSeconds: toSeconds(options.cacheTtl),
+        cacheTtlInSeconds: options.cacheTtl && options.cacheTtl.toSeconds(),
         cachingEnabled: options.cachingEnabled,
         dataTraceEnabled: options.dataTraceEnabled,
         loggingLevel: options.loggingLevel,

diff --git a/packages/@aws-cdk/aws-applicationautoscaling/lib/step-scaling-action.ts b/packages/@aws-cdk/aws-applicationautoscaling/lib/step-scaling-action.ts
@@ -86,7 +86,7 @@ export class StepScalingAction extends cdk.Construct {
       scalingTargetId: props.scalingTarget.scalableTargetId,
       stepScalingPolicyConfiguration: {
         adjustmentType: props.adjustmentType,
-        cooldown: cdk.toSeconds(props.cooldown),
+        cooldown: props.cooldown && props.cooldown.toSeconds(),
         minAdjustmentMagnitude: props.minAdjustmentMagnitude,
         metricAggregationType: props.metricAggregationType,
         stepAdjustments: cdk.Lazy.anyValue({ produce: () => this.adjustments }),

diff --git a/packages/@aws-cdk/aws-applicationautoscaling/lib/target-tracking-scaling-policy.ts b/packages/@aws-cdk/aws-applicationautoscaling/lib/target-tracking-scaling-policy.ts
@@ -128,8 +128,8 @@ export class TargetTrackingScalingPolicy extends cdk.Construct {
           predefinedMetricType: props.predefinedMetric,
           resourceLabel: props.resourceLabel,
         } : undefined,
-        scaleInCooldown: cdk.toSeconds(props.scaleInCooldown),
-        scaleOutCooldown: cdk.toSeconds(props.scaleOutCooldown),
+        scaleInCooldown: props.scaleInCooldown && props.scaleInCooldown.toSeconds(),
+        scaleOutCooldown: props.scaleOutCooldown && props.scaleOutCooldown.toSeconds(),
         targetValue: props.targetValue
       }
     });

diff --git a/packages/@aws-cdk/aws-autoscaling/lib/lifecycle-hook.ts b/packages/@aws-cdk/aws-autoscaling/lib/lifecycle-hook.ts
@@ -103,7 +103,7 @@ export class LifecycleHook extends Resource implements ILifecycleHook {
     const resource = new CfnLifecycleHook(this, 'Resource', {
       autoScalingGroupName: props.autoScalingGroup.autoScalingGroupName,
       defaultResult: props.defaultResult,
-      heartbeatTimeout: toSeconds(props.heartbeatTimeout),
+      heartbeatTimeout: props.heartbeatTimeout && props.heartbeatTimeout.toSeconds(),
       lifecycleHookName: props.lifecycleHookName,
       lifecycleTransition: props.lifecycleTransition,
       notificationMetadata: props.notificationMetadata,

diff --git a/packages/@aws-cdk/aws-autoscaling/lib/step-scaling-action.ts b/packages/@aws-cdk/aws-autoscaling/lib/step-scaling-action.ts
@@ -74,7 +74,7 @@ export class StepScalingAction extends cdk.Construct {
       policyType: 'StepScaling',
       autoScalingGroupName: props.autoScalingGroup.autoScalingGroupName,
       cooldown: props.cooldown && props.cooldown.toSeconds().toString(),
-      estimatedInstanceWarmup: cdk.toSeconds(props.estimatedInstanceWarmup),
+      estimatedInstanceWarmup: props.estimatedInstanceWarmup && props.estimatedInstanceWarmup.toSeconds(),
       adjustmentType: props.adjustmentType,
       minAdjustmentMagnitude: props.minAdjustmentMagnitude,
       metricAggregationType: props.metricAggregationType,

diff --git a/packages/@aws-cdk/aws-autoscaling/lib/target-tracking-scaling-policy.ts b/packages/@aws-cdk/aws-autoscaling/lib/target-tracking-scaling-policy.ts
@@ -123,7 +123,7 @@ export class TargetTrackingScalingPolicy extends cdk.Construct {
       policyType: 'TargetTrackingScaling',
       autoScalingGroupName: props.autoScalingGroup.autoScalingGroupName,
       cooldown: props.cooldown && props.cooldown.toSeconds().toString(),
-      estimatedInstanceWarmup: cdk.toSeconds(props.estimatedInstanceWarmup),
+      estimatedInstanceWarmup: props.estimatedInstanceWarmup && props.estimatedInstanceWarmup.toSeconds(),
       targetTrackingConfiguration: {
         customizedMetricSpecification: renderCustomMetric(props.customMetric),
         disableScaleIn: props.disableScaleIn,

diff --git a/packages/@aws-cdk/aws-cloudfront/lib/web_distribution.ts b/packages/@aws-cdk/aws-cloudfront/lib/web_distribution.ts
@@ -606,8 +606,8 @@ export class CloudFrontWebDistribution extends cdk.Construct implements IDistrib
           ? {
             httpPort: originConfig.customOriginSource.httpPort || 80,
             httpsPort: originConfig.customOriginSource.httpsPort || 443,
-            originKeepaliveTimeout: cdk.toSeconds(originConfig.customOriginSource.originKeepaliveTimeout) || 5,
-            originReadTimeout: cdk.toSeconds(originConfig.customOriginSource.originReadTimeout) || 30,
+            originKeepaliveTimeout: originConfig.customOriginSource.originKeepaliveTimeout && originConfig.customOriginSource.originKeepaliveTimeout.toSeconds() || 5,
+            originReadTimeout: originConfig.customOriginSource.originReadTimeout && originConfig.customOriginSource.originReadTimeout.toSeconds() || 30,
             originProtocolPolicy: originConfig.customOriginSource.originProtocolPolicy || OriginProtocolPolicy.HttpsOnly,
             originSslProtocols: originConfig.customOriginSource.allowedOriginSSLVersions || [OriginSslPolicy.TLSv1_2]
           }
@@ -704,10 +704,10 @@ export class CloudFrontWebDistribution extends cdk.Construct implements IDistrib
       allowedMethods: this.METHOD_LOOKUP_MAP[input.allowedMethods || CloudFrontAllowedMethods.GET_HEAD],
       cachedMethods: this.METHOD_LOOKUP_MAP[input.cachedMethods || CloudFrontAllowedCachedMethods.GET_HEAD],
       compress: input.compress,
-      defaultTtl: cdk.toSeconds(input.defaultTtl),
+      defaultTtl: input.defaultTtl && input.defaultTtl.toSeconds(),
       forwardedValues: input.forwardedValues || { queryString: false, cookies: { forward: "none" } },
-      maxTtl: cdk.toSeconds(input.maxTtl),
-      minTtl: cdk.toSeconds(input.minTtl),
+      maxTtl: input.maxTtl && input.maxTtl.toSeconds(),
+      minTtl: input.minTtl && input.minTtl.toSeconds(),
       trustedSigners: input.trustedSigners,
       targetOriginId: input.targetOriginId,
       viewerProtocolPolicy: protoPolicy || ViewerProtocolPolicy.RedirectToHTTPS,

diff --git a/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts b/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts
@@ -150,7 +150,7 @@ export abstract class BaseService extends Resource
         maximumPercent: props.maximumPercent || 200,
         minimumHealthyPercent: props.minimumHealthyPercent === undefined ? 50 : props.minimumHealthyPercent
       },
-      healthCheckGracePeriodSeconds: toSeconds(props.healthCheckGracePeriod),
+      healthCheckGracePeriodSeconds: props.healthCheckGracePeriod && props.healthCheckGracePeriod.toSeconds(),
       /* role: never specified, supplanted by Service Linked Role */
       networkConfiguration: Lazy.anyValue({ produce: () => this.networkConfiguration }),
       serviceRegistries: Lazy.anyValue({ produce: () => this.serviceRegistries }),

diff --git a/packages/@aws-cdk/aws-ecs/lib/container-definition.ts b/packages/@aws-cdk/aws-ecs/lib/container-definition.ts
@@ -479,7 +479,7 @@ function renderHealthCheck(hc: HealthCheck): CfnTaskDefinition.HealthCheckProper
     command: getHealthCheckCommand(hc),
     interval: hc.interval != null ? hc.interval.toSeconds() : 30,
     retries: hc.retries !== undefined ? hc.retries : 3,
-    startPeriod: cdk.toSeconds(hc.startPeriod),
+    startPeriod: hc.startPeriod && hc.startPeriod.toSeconds(),
     timeout: hc.timeout !== undefined ? hc.timeout.toSeconds() : 5,
   };
 }

diff --git a/packages/@aws-cdk/aws-iam/lib/role.ts b/packages/@aws-cdk/aws-iam/lib/role.ts
@@ -196,7 +196,7 @@ export class Role extends Resource implements IRole {
     this.assumeRolePolicy = createAssumeRolePolicy(props.assumedBy, props.externalId);
     this.managedPolicyArns = props.managedPolicyArns || [ ];
 
-    const maxSessionDuration = toSeconds(props.maxSessionDuration);
+    const maxSessionDuration = props.maxSessionDuration && props.maxSessionDuration.toSeconds();
     validateMaxSessionDuration(maxSessionDuration);
 
     const role = new CfnRole(this, 'Resource', {

diff --git a/packages/@aws-cdk/aws-lambda/lib/function.ts b/packages/@aws-cdk/aws-lambda/lib/function.ts
@@ -427,7 +427,7 @@ export class Function extends FunctionBase {
       code: Lazy.anyValue({ produce: () => props.code._toJSON(resource) }),
       layers: Lazy.listValue({ produce: () => this.layers.map(layer => layer.layerVersionArn) }, { omitEmpty: true }),
       handler: props.handler,
-      timeout: toSeconds(props.timeout),
+      timeout: props.timeout && props.timeout.toSeconds(),
       runtime: props.runtime.name,
       role: this.role.roleArn,
       environment: Lazy.anyValue({ produce: () => this.renderEnvironment() }),

diff --git a/packages/@aws-cdk/aws-rds/lib/instance.ts b/packages/@aws-cdk/aws-rds/lib/instance.ts
@@ -518,7 +518,7 @@ abstract class DatabaseInstanceNew extends DatabaseInstanceBase implements IData
       enableIamDatabaseAuthentication: props.iamAuthentication,
       enablePerformanceInsights: props.enablePerformanceInsights,
       iops,
-      monitoringInterval: toSeconds(props.monitoringInterval),
+      monitoringInterval: props.monitoringInterval && props.monitoringInterval.toSeconds(),
       monitoringRoleArn: monitoringRole && monitoringRole.roleArn,
       multiAz: props.multiAz,
       optionGroupName: props.optionGroup && props.optionGroup.optionGroupName,

diff --git a/packages/@aws-cdk/aws-sqs/lib/queue.ts b/packages/@aws-cdk/aws-sqs/lib/queue.ts
@@ -248,11 +248,11 @@ export class Queue extends QueueBase {
       ...this.determineFifoProps(props),
       ...encryptionProps,
       redrivePolicy,
-      delaySeconds: toSeconds(props.deliveryDelay),
+      delaySeconds: props.deliveryDelay && props.deliveryDelay.toSeconds(),
       maximumMessageSize: props.maxMessageSizeBytes,
-      messageRetentionPeriod: toSeconds(props.retentionPeriod),
-      receiveMessageWaitTimeSeconds: toSeconds(props.receiveMessageWaitTime),
-      visibilityTimeout: toSeconds(props.visibilityTimeout),
+      messageRetentionPeriod: props.retentionPeriod && props.retentionPeriod.toSeconds(),
+      receiveMessageWaitTimeSeconds: props.receiveMessageWaitTime && props.receiveMessageWaitTime.toSeconds(),
+      visibilityTimeout: props.visibilityTimeout && props.visibilityTimeout.toSeconds(),
     });
     this.encryptionMasterKey = encryptionMasterKey;
     this.queueArn = queue.queueArn;

diff --git a/packages/@aws-cdk/aws-sqs/lib/validate-props.ts b/packages/@aws-cdk/aws-sqs/lib/validate-props.ts
@@ -2,11 +2,11 @@ import { toSeconds } from '@aws-cdk/cdk';
 import { QueueProps } from './index';
 
 export function validateProps(props: QueueProps) {
-  validateRange('delivery delay', toSeconds(props.deliveryDelay), 0, 900, 'seconds');
+  validateRange('delivery delay', props.deliveryDelay && props.deliveryDelay.toSeconds(), 0, 900, 'seconds');
   validateRange('maximum message size', props.maxMessageSizeBytes, 1_024, 262_144, 'bytes');
-  validateRange('message retention period', toSeconds(props.retentionPeriod), 60, 1_209_600, 'seconds');
-  validateRange('receive wait time', toSeconds(props.receiveMessageWaitTime), 0, 20, 'seconds');
-  validateRange('visibility timeout', toSeconds(props.visibilityTimeout), 0, 43_200, 'seconds');
+  validateRange('message retention period', props.retentionPeriod && props.retentionPeriod.toSeconds(), 60, 1_209_600, 'seconds');
+  validateRange('receive wait time', props.receiveMessageWaitTime && props.receiveMessageWaitTime.toSeconds(), 0, 20, 'seconds');
+  validateRange('visibility timeout', props.visibilityTimeout && props.visibilityTimeout.toSeconds(), 0, 43_200, 'seconds');
   validateRange('dead letter target maximum receive count', props.deadLetterQueue && props.deadLetterQueue.maxReceiveCount, 1, +Infinity);
 }
 

diff --git a/packages/@aws-cdk/aws-stepfunctions-tasks/lib/send-to-queue.ts b/packages/@aws-cdk/aws-stepfunctions-tasks/lib/send-to-queue.ts
@@ -60,7 +60,7 @@ export class SendToQueue implements sfn.IStepFunctionsTask {
         QueueUrl: this.queue.queueUrl,
         ...sfn.FieldUtils.renderObject({
           MessageBody: this.props.messageBody.value,
-          DelaySeconds: toSeconds(this.props.delay),
+          DelaySeconds: this.props.delay && this.props.delay.toSeconds(),
           MessageDeduplicationId: this.props.messageDeduplicationId,
           MessageGroupId: this.props.messageGroupId,
         })

diff --git a/packages/@aws-cdk/aws-stepfunctions/lib/states/state.ts b/packages/@aws-cdk/aws-stepfunctions/lib/states/state.ts
@@ -437,7 +437,7 @@ interface CatchTransition {
 function renderRetry(retry: RetryProps) {
     return {
         ErrorEquals: retry.errors,
-        IntervalSeconds: cdk.toSeconds(retry.interval),
+        IntervalSeconds: retry.interval && retry.interval.toSeconds(),
         MaxAttempts: retry.maxAttempts,
         BackoffRate: retry.backoffRate
     };

diff --git a/packages/@aws-cdk/aws-stepfunctions/lib/states/task.ts b/packages/@aws-cdk/aws-stepfunctions/lib/states/task.ts
@@ -128,8 +128,8 @@ export class Task extends State implements INextable {
             Resource: this.taskProps.resourceArn,
             Parameters: this.taskProps.parameters,
             ResultPath: renderJsonPath(this.resultPath),
-            TimeoutSeconds: cdk.toSeconds(this.timeout),
-            HeartbeatSeconds: cdk.toSeconds(this.taskProps.heartbeat),
+            TimeoutSeconds: this.timeout && this.timeout.toSeconds(),
+            HeartbeatSeconds: this.taskProps.heartbeat && this.taskProps.heartbeat.toSeconds(),
         };
     }
 

diff --git a/packages/@aws-cdk/cdk/lib/duration.ts b/packages/@aws-cdk/cdk/lib/duration.ts
@@ -1,3 +1,5 @@
+import { Token } from "./token";
+
 /**
  * Represents a length of time.
  */
@@ -64,6 +66,9 @@ export class Duration {
   private readonly unit: TimeUnit;
 
   private constructor(amount: number, unit: TimeUnit) {
+    if (Token.isUnresolved(amount)) {
+      throw new Error(`Duration amounts cannot be unresolved tokens. Received ${amount}`);
+    }
     if (amount < 0) {
       throw new Error(`Duration amounts cannot be negative. Received: ${amount}`);
     }
@@ -113,7 +118,12 @@ export class Duration {
   }
 
   public toString(): string {
-    return `${this.amount} ${this.unit}`;
+    return Token.asString(
+      () => {
+        throw new Error(`Duration.toString() was used, but .toSeconds, .toMinutes or .toDays should have been called instead`);
+      },
+      { displayHint: `${this.amount} ${this.unit.label}` }
+    );
   }
 
   private fractionDuration(symbol: string, modulus: number, next: (amount: number) => Duration): string {
@@ -141,17 +151,6 @@ export interface TimeConversionOptions {
   readonly integral?: boolean;
 }
 
-/**
- * Helpful syntax sugar for turning an optional `Duration` into a count of seconds.
- *
- * @param duration an optional duration to convert into an amount of sunctions.
- *
- * @return `duration && duration.toSeconds()`.
- */
-export function toSeconds(duration: Duration | undefined): number | undefined {
-  return duration && duration.toSeconds();
-}
-
 class TimeUnit {
   public static readonly Seconds = new TimeUnit('seconds', 1);
   public static readonly Minutes = new TimeUnit('minutes', 60);

diff --git a/packages/@aws-cdk/cdk/test/test.duration.ts b/packages/@aws-cdk/cdk/test/test.duration.ts
@@ -1,5 +1,5 @@
 import nodeunit = require('nodeunit');
-import { Duration } from '../lib';
+import { Duration, Stack, Token } from '../lib';
 
 export = nodeunit.testCase({
   'negative amount'(test: nodeunit.Test) {
@@ -8,6 +8,12 @@ export = nodeunit.testCase({
     test.done();
   },
 
+  'unresolved amount'(test: nodeunit.Test) {
+    test.throws(new Stack().resolve(Duration.seconds(Token.asNumber(1337))));
+
+    test.done();
+  },
+
   'Duration in seconds'(test: nodeunit.Test) {
     const duration = Duration.seconds(300);