chore: remove an additional "prepare" usage

packages/@aws-cdk/aws-apigateway/lib/deployment.ts

@@ -1,7 +1,7 @@
-import { CfnResource, Construct, Lazy, RemovalPolicy, Resource, Stack } from '@aws-cdk/core';
 import * as crypto from 'crypto';
+import { Construct, Lazy, RemovalPolicy, Resource } from '@aws-cdk/core';
 import { CfnDeployment } from './apigateway.generated';
-import { IRestApi, RestApi, SpecRestApi } from './restapi';
+import { IRestApi, RestApi, SpecRestApi, RestApiBase } from './restapi';
 
 export interface DeploymentProps  {
   /**
@@ -77,6 +77,10 @@ export class Deployment extends Resource {
 
     this.api = props.api;
     this.deploymentId = Lazy.stringValue({ produce: () => this.resource.ref });
+
+    if (props.api instanceof RestApiBase) {
+      props.api._attachDeployment(this);
+    }
   }
 
   /**
@@ -90,30 +94,6 @@ export class Deployment extends Resource {
   public addToLogicalId(data: any) {
     this.resource.addToLogicalId(data);
   }
-
-  /**
-   * Hook into synthesis before it occurs and make any final adjustments.
-   */
-  protected prepare() {
-    if (this.api instanceof RestApi) {
-      // Ignore IRestApi that are imported
-
-      /*
-       * https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-apigateway-deployment.html
-       * Quoting from CloudFormation's docs - "If you create an AWS::ApiGateway::RestApi resource and its methods (using AWS::ApiGateway::Method) in
-       * the same template as your deployment, the deployment must depend on the RestApi's methods. To create a dependency, add a DependsOn attribute
-       * to the deployment. If you don't, AWS CloudFormation creates the deployment right after it creates the RestApi resource that doesn't contain
-       * any methods, and AWS CloudFormation encounters the following error: The REST API doesn't contain any methods."
-       */
-
-      /*
-       * Adding a dependency between LatestDeployment and Method construct, using ConstructNode.addDependencies(), creates additional dependencies
-       * between AWS::ApiGateway::Deployment and the AWS::Lambda::Permission nodes (children under Method), causing cyclic dependency errors. Hence,
-       * falling back to declaring dependencies between the underlying CfnResources.
-       */
-      this.api.methods.map(m => m.node.defaultChild as CfnResource).forEach(m => this.resource.addDependsOn(m));
-    }
-  }
 }
 
 interface LatestDeploymentResourceProps {
@@ -122,9 +102,9 @@ interface LatestDeploymentResourceProps {
 }
 
 class LatestDeploymentResource extends CfnDeployment {
-  private hashComponents = new Array<any>();
-
-  private api: IRestApi;
+  private readonly hashComponents = new Array<any>();
+  private readonly originalLogicalId: string;
+  private readonly api: IRestApi;
 
   constructor(scope: Construct, id: string, props: LatestDeploymentResourceProps) {
     super(scope, id, {
@@ -133,31 +113,8 @@ class LatestDeploymentResource extends CfnDeployment {
     });
 
     this.api = props.restApi;
-
-    const originalLogicalId = Stack.of(this).getLogicalId(this);
-
-    this.overrideLogicalId(Lazy.stringValue({ produce: ctx => {
-      const hash = [ ...this.hashComponents ];
-
-      if (this.api instanceof RestApi || this.api instanceof SpecRestApi) { // Ignore IRestApi that are imported
-
-        // Add CfnRestApi to the logical id so a new deployment is triggered when any of its properties change.
-        const cfnRestApiCF = (this.api.node.defaultChild as any)._toCloudFormation();
-        hash.push(ctx.resolve(cfnRestApiCF));
-      }
-
-      let lid = originalLogicalId;
-
-      // if hash components were added to the deployment, we use them to calculate
-      // a logical ID for the deployment resource.
-      if (hash.length > 0) {
-        const md5 = crypto.createHash('md5');
-        hash.map(x => ctx.resolve(x)).forEach(c => md5.update(JSON.stringify(c)));
-        lid += md5.digest('hex');
-      }
-
-      return lid;
-    }}));
+    this.originalLogicalId = this.stack.getLogicalId(this);
+    this.overrideLogicalId(Lazy.stringValue({ produce: () => this.calculateLogicalId() }));
   }
 
   /**
@@ -173,4 +130,27 @@ class LatestDeploymentResource extends CfnDeployment {
 
     this.hashComponents.push(data);
   }
+
+  private calculateLogicalId() {
+    const hash = [ ...this.hashComponents ];
+
+    if (this.api instanceof RestApi || this.api instanceof SpecRestApi) { // Ignore IRestApi that are imported
+
+      // Add CfnRestApi to the logical id so a new deployment is triggered when any of its properties change.
+      const cfnRestApiCF = (this.api.node.defaultChild as any)._toCloudFormation();
+      hash.push(this.stack.resolve(cfnRestApiCF));
+    }
+
+    let lid = this.originalLogicalId;
+
+    // if hash components were added to the deployment, we use them to calculate
+    // a logical ID for the deployment resource.
+    if (hash.length > 0) {
+      const md5 = crypto.createHash('md5');
+      hash.map(x => this.stack.resolve(x)).forEach(c => md5.update(JSON.stringify(c)));
+      lid += md5.digest('hex');
+    }
+
+    return lid;
+  }
 }

packages/@aws-cdk/aws-apigateway/lib/restapi.ts

@@ -1,6 +1,6 @@
 import { IVpcEndpoint } from '@aws-cdk/aws-ec2';
 import * as iam from '@aws-cdk/aws-iam';
-import { CfnOutput, Construct, IResource as IResourceBase, Resource, Stack } from '@aws-cdk/core';
+import { CfnOutput, Construct, IResource as IResourceBase, Resource, Stack, CfnResource } from '@aws-cdk/core';
 import { ApiDefinition } from './api-definition';
 import { ApiKey, ApiKeyOptions, IApiKey } from './api-key';
 import { CfnAccount, CfnRestApi } from './apigateway.generated';
@@ -248,7 +248,6 @@ export interface SpecRestApiProps extends RestApiBaseProps {
  * Base implementation that are common to various implementations of IRestApi
  */
 export abstract class RestApiBase extends Resource implements IRestApi {
-
   /**
    * Checks if the given object is an instance of RestApiBase.
    * @internal
@@ -384,6 +383,15 @@ export abstract class RestApiBase extends Resource implements IRestApi {
     ignore(method);
   }
 
+  /**
+   * Associates a Deployment resource with this REST API.
+   *
+   * @internal
+   */
+  public _attachDeployment(deployment: Deployment) {
+    ignore(deployment);
+  }
+
   protected configureCloudWatchRole(apiResource: CfnRestApi) {
     const role = new iam.Role(this, 'CloudWatchRole', {
       assumedBy: new iam.ServicePrincipal('apigateway.amazonaws.com'),
@@ -550,6 +558,11 @@ export class RestApi extends RestApiBase {
    */
   public readonly methods = new Array<Method>();
 
+  /**
+   * This list of deployments bound to this RestApi
+   */
+  private readonly deployments = new Array<Deployment>();
+
   constructor(scope: Construct, id: string, props: RestApiProps = { }) {
     super(scope, id, props);
 
@@ -627,6 +640,29 @@ export class RestApi extends RestApiBase {
    */
   public _attachMethod(method: Method) {
     this.methods.push(method);
+
+    // add this method as a dependency to all deployments defined for this api
+    // when additional deployments are added, _attachDeployment is called and
+    // this method will be added there.
+    for (const dep of this.deployments) {
+      dep.node.addDependency(method.node.defaultChild as CfnResource);
+    }
+  }
+
+  /**
+   * Attaches a deployment to this REST API.
+   *
+   * @internal
+   */
+  public _attachDeployment(deployment: Deployment) {
+    this.deployments.push(deployment);
+
+    // add all methods that were already defined as dependencies of this deployment
+    // when additional methods are added, _attachMethod is called and it will be
+    // added as a dependency to this deployment.
+    for (const method of this.methods) {
+      deployment.node.addDependency(method.node.defaultChild as CfnResource);
+    }
   }
 
   /**

packages/@aws-cdk/aws-iam/lib/policy.ts

@@ -124,7 +124,18 @@ export class Policy extends Resource implements IPolicy {
         Lazy.stringValue({ produce: () => generatePolicyName(scope, resource.logicalId) }),
     });
 
-    const resource = new CfnPolicy(this, 'Resource', {
+    // this function returns `true` if the CFN resource should be included in
+    // the cloudformation template unless `force` is `true`, if the policy
+    // document is empty, the resource will not be included.
+    const self = this;
+
+    class CfnPolicyConditional extends CfnPolicy {
+      protected shouldSynthesize() {
+        return self.force || self.referenceTaken || (!self.document.isEmpty && self.isAttached);
+      }
+    }
+
+    const resource = new CfnPolicyConditional(this, 'Resource', {
       policyDocument: this.document,
       policyName: this.physicalName,
       roles: undefinedIfEmpty(() => this.roles.map(r => r.roleName)),

packages/@aws-cdk/core/lib/cfn-resource.ts

@@ -278,6 +278,10 @@ export class CfnResource extends CfnRefElement {
    * @internal
    */
   public _toCloudFormation(): object {
+    if (!this.shouldSynthesize()) {
+      return { };
+    }
+
     try {
       const ret = {
         Resources: {
@@ -362,6 +366,17 @@ export class CfnResource extends CfnRefElement {
   protected validateProperties(_properties: any) {
     // Nothing
   }
+
+  /**
+   * Can be overridden by subclasses to determine if this resource will be rendered
+   * into the cloudformation template.
+   *
+   * @returns `true` if the resource should be included or `false` is the resource
+   * should be omitted.
+   */
+  protected shouldSynthesize() {
+    return true;
+  }
 }
 
 export enum TagType {

packages/@aws-cdk/core/lib/nested-stack.ts

@@ -187,7 +187,7 @@ export class NestedStack extends Stack {
       return false;
     }
 
-    const cfn = JSON.stringify((this as any)._toCloudFormation());
+    const cfn = JSON.stringify(this._toCloudFormation());
     const templateHash = crypto.createHash('sha256').update(cfn).digest('hex');
 
     const templateLocation = this._parentStack.addFileAsset({

packages/@aws-cdk/core/test/test.resource.ts

@@ -707,6 +707,25 @@ export = {
 
     test.done();
   },
+
+  'subclasses can override "shouldSynthesize" to lazy-determine if the resource should be included'(test: Test) {
+    // GIVEN
+    class MyResource extends CfnResource {
+      protected shouldSynthesize() {
+        return false;
+      }
+    }
+
+    const stack = new Stack();
+
+    // WHEN
+    new MyResource(stack, 'R1', { type: 'Foo::R1' });
+    new CfnResource(stack, 'R2', { type: 'Foo::R2' });
+
+    // THEN - only R2 is synthesized
+    test.deepEqual(toCloudFormation(stack), { Resources: { R2: { Type: 'Foo::R2' } } });
+    test.done();
+  },
 };
 
 interface CounterProps {

packages/@aws-cdk/pipelines/lib/pipeline.ts

@@ -102,6 +102,8 @@ export class CdkPipeline extends Construct {
       pipeline: this._pipeline,
       projectName: maybeSuffix(props.pipelineName, '-publish'),
     });
+
+    this.node.applyAspect({ visit: () => this._assets.removeAssetsStageIfEmpty() });
   }
 
   /**
@@ -178,14 +180,6 @@ export class CdkPipeline extends Construct {
     return ret;
   }
 
-  protected onPrepare() {
-    super.onPrepare();
-
-    // TODO: Support this in a proper way in the upstream library. For now, we
-    // "un-add" the Assets stage if it turns out to be empty.
-    this._assets.removeAssetsStageIfEmpty();
-  }
-
   /**
    * Return all StackDeployActions in an ordered list
    */

packages/@aws-cdk/pipelines/lib/stage.ts

@@ -54,6 +54,8 @@ export class CdkStage extends Construct {
     this.pipelineStage = props.pipelineStage;
     this.cloudAssemblyArtifact = props.cloudAssemblyArtifact;
     this.host = props.host;
+
+    this.node.applyAspect({ visit: () => this.prepare() });
   }
 
   /**
@@ -175,7 +177,7 @@ export class CdkStage extends Construct {
    * after creation, nor is there a way to specify relative priorities, which
    * is a limitation that we should take away in the base library.
    */
-  protected prepare() {
+  private prepare() {
     // FIXME: Make sure this only gets run once. There seems to be an issue in the reconciliation
     // loop that may trigger this more than once if it throws an error somewhere, and the exception
     // that gets thrown here will then override the actual failure.