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.

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

fix(core): hide dependencyRoots from public API #2668

Merged
merged 4 commits into from
May 30, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 13 additions & 11 deletions packages/@aws-cdk/aws-ec2/lib/vpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1302,23 +1302,25 @@ function describeSelection(placement: SubnetSelection): string {
class CompositeDependable implements IDependable {
private readonly dependables = new Array<IDependable>();

constructor() {
const self = this;
cdk.DependableTrait.implement(this, {
get dependencyRoots() {
const ret = [];
for (const dep of self.dependables) {
ret.push(...cdk.DependableTrait.get(dep).dependencyRoots);
}
return ret;
}
});
}

/**
* Add a construct to the dependency roots
*/
public add(dep: IDependable) {
this.dependables.push(dep);
}

/**
* Retrieve the current set of dependency roots
*/
public get dependencyRoots(): IConstruct[] {
const ret = [];
for (const dep of this.dependables) {
ret.push(...dep.dependencyRoots);
}
return ret;
}
}

/**
Expand Down
1 change: 0 additions & 1 deletion packages/@aws-cdk/aws-lambda/lib/function-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,6 @@ export abstract class FunctionBase extends Resource implements IFunction {
action: 'lambda:InvokeFunction',
});
},
dependencyRoots: [],
node: this.node,
},
});
Expand Down
18 changes: 8 additions & 10 deletions packages/@aws-cdk/cdk/lib/construct.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import cxapi = require('@aws-cdk/cx-api');
import { IAspect } from './aspect';
import { CLOUDFORMATION_TOKEN_RESOLVER, CloudFormationLang } from './cloudformation-lang';
import { IDependable } from './dependency';
import { DependableTrait, IDependable } from './dependency';
import { resolve } from './resolve';
import { Token } from './token';
import { makeUniqueId } from './uniqueid';
Expand Down Expand Up @@ -521,7 +521,7 @@ export class ConstructNode {

for (const source of this.findAll()) {
for (const dependable of source.node.dependencies) {
for (const target of dependable.dependencyRoots) {
for (const target of DependableTrait.get(dependable).dependencyRoots) {
let foundTargets = found.get(source);
if (!foundTargets) { found.set(source, foundTargets = new Set()); }

Expand Down Expand Up @@ -578,14 +578,6 @@ export class Construct implements IConstruct {
*/
public readonly node: ConstructNode;

/**
* The set of constructs that form the root of this dependable
*
* All resources under all returned constructs are included in the ordering
* dependency.
*/
public readonly dependencyRoots: IConstruct[] = [this];

/**
* Creates a new construct node.
*
Expand All @@ -596,6 +588,12 @@ export class Construct implements IConstruct {
*/
constructor(scope: Construct, id: string) {
this.node = new ConstructNode(this, scope, id);

// Implement IDependable privately
const self = this;
Copy link
Contributor

Choose a reason for hiding this comment

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

put a wee comment explaining what this does

DependableTrait.implement(this, {
get dependencyRoots() { return [self]; },
});
}

/**
Expand Down
73 changes: 62 additions & 11 deletions packages/@aws-cdk/cdk/lib/dependency.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,18 @@
import { IConstruct } from "./construct";

/**
* A set of constructs that can be depended upon
* Trait marker for classes that can be depended upon
*
* The presence of this interface indicates that an object has
* an `IDependableTrait` implementation.
*
* This interface can be used to take an (ordering) dependency on a set of
* constructs. An ordering dependency implies that the resources represented by
* those constructs are deployed before the resources depending ON them are
* deployed.
*/
export interface IDependable {
/**
* The set of constructs that form the root of this dependable
*
* All resources under all returned constructs are included in the ordering
* dependency.
*/
readonly dependencyRoots: IConstruct[];
// Empty, this interface is a trait marker
}

/**
Expand All @@ -27,17 +24,71 @@ export interface IDependable {
export class ConcreteDependable implements IDependable {
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is this used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh nice. for some reason I thought it is just introduced now.

private readonly _dependencyRoots = new Array<IConstruct>();

constructor() {
const self = this;
DependableTrait.implement(this, {
get dependencyRoots() { return self._dependencyRoots; },
});
}

/**
* Add a construct to the dependency roots
*/
public add(construct: IConstruct) {
this._dependencyRoots.push(construct);
}
}

/**
* Trait for IDependable
*
* Traits are interfaces that are privately implemented by objects. Instead of
* showing up in the public interface of a class, they need to be queried
* explicitly. This is used to implement certain framework features that are
* not intended to be used by Construct consumers, and so should be hidden
* from accidental use.
*
* @example
*
* // Usage
* const roots = DependableTrait.get(construct).dependencyRoots;
*
* // Definition
* DependableTrait.implement(construct, {
* get dependencyRoots() { return []; }
* });
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Example on how this is used

export abstract class DependableTrait {
/**
* Retrieve the current set of dependency roots
* Register `instance` to have the given DependableTrait
*
* Should be called in the class constructor.
*/
public get dependencyRoots(): IConstruct[] {
return this._dependencyRoots;
public static implement(instance: IDependable, trait: DependableTrait) {
// I would also like to reference classes (to cut down on the list of objects
// we need to manage), but we can't do that either since jsii doesn't have the
// concept of a class reference.
DependableTrait.traitMap.set(instance, trait);
}

/**
* Return the matching DependableTrait for the given class instance.
*/
public static get(instance: IDependable): DependableTrait {
const ret = DependableTrait.traitMap.get(instance);
if (!ret) {
throw new Error(`${instance} does not implement DependableTrait`);
}
return ret;
}

private static traitMap = new WeakMap<IDependable, DependableTrait>();

/**
* The set of constructs that form the root of this dependable
*
* All resources under all returned constructs are included in the ordering
* dependency.
*/
public abstract readonly dependencyRoots: IConstruct[];
}