Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core): generalization of dependencies #1583

Merged
merged 14 commits into from
Feb 4, 2019
Merged

feat(core): generalization of dependencies #1583

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
74cf1e7
feat(core): generalization of dependencies
Jan 20, 2019
aa100a0
Don't use Set in public API
Jan 21, 2019
4df8045
Interface should have readonly properties
Jan 22, 2019
3bda9c3
Fix dependency handling in APIGW
Jan 22, 2019
642a136
Merge remote-tracking branch 'origin/master' into huijbers/construct-…
Jan 22, 2019
b054224
Rename loadBalancerConstructs => loadBalancerDependencies
Jan 23, 2019
e296d3c
Simplify dependencies by not making them lazy
Jan 23, 2019
ec74b29
Move dependency resolution to Stack
Jan 23, 2019
0a27398
Update some more expectations
Jan 24, 2019
8787b06
Merge remote-tracking branch 'origin/master' into huijbers/construct-…
rix0rrr Feb 4, 2019
3874942
Introduce IDependable
rix0rrr Feb 4, 2019
f2de176
Use IDependable in case of bundled dependencies
rix0rrr Feb 4, 2019
9e1873e
Updating tests
rix0rrr Feb 4, 2019
5926bf6
Merge remote-tracking branch 'origin/master' into huijbers/construct-…
rix0rrr Feb 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions examples/cdk-examples-typescript/custom-resource/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ interface DemoResourceProps {
failCreate?: boolean;
}

class DemoResource extends cdk.Construct implements cdk.IDependable {
public readonly dependencyElements: cdk.IDependable[];
class DemoResource extends cdk.Construct {
public readonly response: string;

constructor(scope: cdk.Construct, id: string, props: DemoResourceProps) {
Expand All @@ -36,7 +35,6 @@ class DemoResource extends cdk.Construct implements cdk.IDependable {
});

this.response = resource.getAtt('Response').toString();
this.dependencyElements = [resource];
}
}

Expand Down Expand Up @@ -91,7 +89,7 @@ class FailAfterCreatingStack extends cdk.Stack {
});

// Make sure the rollback gets triggered only after the custom resource has been fully created.
bucket.addDependency(resource);
bucket.node.addDependency(resource);
}
}

Expand Down
2 changes: 1 addition & 1 deletion examples/cdk-examples-typescript/resource-overrides/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class ResourceOverridesExample extends cdk.Stack {
// This is how to specify resource options such as dependencies, metadata, update policy
//

bucketResource.addDependency(otherBucket.node.findChild('Resource') as cdk.Resource);
bucketResource.node.addDependency(otherBucket.node.findChild('Resource') as cdk.Resource);
bucketResource.options.metadata = { MetadataKey: 'MetadataValue' };
bucketResource.options.updatePolicy = {
autoScalingRollingUpdate: {
Expand Down
20 changes: 4 additions & 16 deletions packages/@aws-cdk/aws-apigateway/lib/deployment.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,18 +51,13 @@ export interface DeploymentProps {
* Furthermore, since a deployment does not reference any of the REST API
* resources and methods, CloudFormation will likely provision it before these
* resources are created, which means that it will represent a "half-baked"
* model. Use the `addDependency(dep)` method to circumvent that. This is done
* model. Use the `node.addDependency(dep)` method to circumvent that. This is done
* automatically for the `restApi.latestDeployment` deployment.
*/
export class Deployment extends cdk.Construct implements cdk.IDependable {
export class Deployment extends cdk.Construct {
public readonly deploymentId: string;
public readonly api: IRestApi;

/**
* Allows taking a dependency on this construct.
*/
public readonly dependencyElements = new Array<cdk.IDependable>();

private readonly resource: LatestDeploymentResource;

constructor(scope: cdk.Construct, id: string, props: DeploymentProps) {
Expand All @@ -79,15 +74,6 @@ export class Deployment extends cdk.Construct implements cdk.IDependable {

this.api = props.api;
this.deploymentId = new cdk.Token(() => this.resource.deploymentId).toString();
this.dependencyElements.push(this.resource);
}

/**
* Adds a dependency for this deployment. Should be called by all resources and methods
* so they are provisioned before this Deployment.
*/
public addDependency(dep: cdk.IDependable) {
this.resource.addDependency(dep);
}

/**
Expand Down Expand Up @@ -182,5 +168,7 @@ class LatestDeploymentResource extends CfnDeployment {

this.lazyLogicalId = this.originalLogicalId + md5.digest("hex");
}

super.prepare();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my note about maybe moving resource dependency calculation to the stack. If you decide to leave Rhiannon here, then add a wee comment on the super call to hint why it Ian needed

}
}
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-apigateway/lib/method.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export class Method extends cdk.Construct {

const deployment = props.resource.resourceApi.latestDeployment;
if (deployment) {
deployment.addDependency(resource);
deployment.node.addDependency(resource);
deployment.addToLogicalId({ method: methodProps });
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-apigateway/lib/resource.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ export class Resource extends cdk.Construct implements IRestApiResource {

const deployment = props.parent.resourceApi.latestDeployment;
if (deployment) {
deployment.addDependency(resource);
deployment.node.addDependency(resource);
deployment.addToLogicalId({ resource: resourceProps });
}

Expand Down
17 changes: 2 additions & 15 deletions packages/@aws-cdk/aws-apigateway/lib/restapi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ export interface RestApiProps extends ResourceOptions {
* By default, the API will automatically be deployed and accessible from a
* public endpoint.
*/
export class RestApi extends cdk.Construct implements cdk.IDependable, IRestApi {
export class RestApi extends cdk.Construct implements IRestApi {
/**
* Imports an existing REST API resource.
* @param parent Parent construct
Expand All @@ -177,11 +177,6 @@ export class RestApi extends cdk.Construct implements cdk.IDependable, IRestApi
*/
public latestDeployment?: Deployment;

/**
* Allows taking a dependency on this construct.
*/
public readonly dependencyElements = new Array<cdk.IDependable>();

/**
* API Gateway stage that points to the latest deployment (if defined).
*
Expand Down Expand Up @@ -226,14 +221,6 @@ export class RestApi extends cdk.Construct implements cdk.IDependable, IRestApi
this.configureCloudWatchRole(resource);
}

this.dependencyElements.push(resource);
if (this.latestDeployment) {
this.dependencyElements.push(this.latestDeployment);
}
if (this.deploymentStage) {
this.dependencyElements.push(this.deploymentStage);
}

// configure the "root" resource
this.root = {
node: this.node,
Expand Down Expand Up @@ -372,7 +359,7 @@ export class RestApi extends cdk.Construct implements cdk.IDependable, IRestApi
cloudWatchRoleArn: role.roleArn
});

resource.addDependency(apiResource);
resource.node.addDependency(apiResource);
}
}

Expand Down
4 changes: 1 addition & 3 deletions packages/@aws-cdk/aws-apigateway/lib/stage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,8 @@ export interface MethodDeploymentOptions {
cacheDataEncrypted?: boolean;
}

export class Stage extends cdk.Construct implements cdk.IDependable {
export class Stage extends cdk.Construct {
public readonly stageName: string;
public readonly dependencyElements = new Array<cdk.IDependable>();

private readonly restApi: IRestApi;

Expand Down Expand Up @@ -170,7 +169,6 @@ export class Stage extends cdk.Construct implements cdk.IDependable {

this.stageName = resource.ref;
this.restApi = props.deployment.api;
this.dependencyElements.push(resource);
}

/**
Expand Down
8 changes: 5 additions & 3 deletions packages/@aws-cdk/aws-apigateway/test/integ.restapi.books.expected.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -530,8 +530,8 @@
"booksapibooksGETA776447A",
"booksapibooksPOSTF6C6559D",
"booksapibooksbookid5264BCA2",
"booksapibooksbookidGETCCE21986",
"booksapibooksbookidDELETE214F4059"
"booksapibooksbookidDELETE214F4059",
"booksapibooksbookidGETCCE21986"
]
},
"booksapiDeploymentStageprod55D8E03E": {
Expand Down Expand Up @@ -837,7 +837,9 @@
"Ref": "AWS::Region"
},
".",
{ "Ref": "AWS::URLSuffix" },
{
"Ref": "AWS::URLSuffix"
},
"/",
{
"Ref": "booksapiDeploymentStageprod55D8E03E"
Expand Down
4 changes: 3 additions & 1 deletion packages/@aws-cdk/aws-apigateway/test/integ.restapi.defaults.expected.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,9 @@
"Ref": "AWS::Region"
},
".",
{ "Ref": "AWS::URLSuffix" },
{
"Ref": "AWS::URLSuffix"
},
"/",
{
"Ref": "myapiDeploymentStageprod298F01AF"
Expand Down
14 changes: 8 additions & 6 deletions packages/@aws-cdk/aws-apigateway/test/integ.restapi.expected.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@
},
"DependsOn": [
"myapiv113487378",
"myapiv1toysA55FCBC4",
"myapiv1toysGET7348114D",
"myapiv1toysPOST55128058",
"myapiv1toysPUT59AFBBC2",
"myapiv1appliances507FEFF4",
"myapiv1appliancesGET8FE872EC",
"myapiv1books1D4BE6C1",
"myapiv1booksGETC6B996D0",
"myapiv1booksPOST53E2832E"
"myapiv1booksPOST53E2832E",
"myapiv1toysA55FCBC4",
"myapiv1toysGET7348114D",
"myapiv1toysPOST55128058",
"myapiv1toysPUT59AFBBC2"
],
"DeletionPolicy": "Retain"
},
Expand Down Expand Up @@ -602,7 +602,9 @@
"Ref": "AWS::Region"
},
".",
{ "Ref": "AWS::URLSuffix" },
{
"Ref": "AWS::URLSuffix"
},
"/",
{
"Ref": "myapiDeploymentStagebeta96434BEB"
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-apigateway/test/test.deployment.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ export = {
const dep = new cdk.Resource(stack, 'MyResource', { type: 'foo' });

// WHEN
deployment.addDependency(dep);
deployment.node.addDependency(dep);

expect(stack).to(haveResource('AWS::ApiGateway::Deployment', {
DependsOn: [ "MyResource" ],
Expand Down
Loading