diff --git a/packages/@aws-cdk/core/lib/aspect.ts b/packages/@aws-cdk/core/lib/aspect.ts index 17534e2139f0f..3bca67a12aa6d 100644 --- a/packages/@aws-cdk/core/lib/aspect.ts +++ b/packages/@aws-cdk/core/lib/aspect.ts @@ -1,5 +1,7 @@ import { IConstruct } from './construct-compat'; +const ASPECTS_SYMBOL = Symbol('cdk-aspects'); + /** * Represents an Aspect */ @@ -9,3 +11,48 @@ export interface IAspect { */ visit(node: IConstruct): void; } + +/** + * Aspects can be applied to CDK tree scopes and can operate on the tree before + * synthesis. + */ +export class Aspects { + + /** + * Returns the `Aspects` object associated with a construct scope. + * @param scope The scope for which these aspects will apply. + */ + public static of(scope: IConstruct): Aspects { + let aspects = (scope as any)[ASPECTS_SYMBOL]; + if (!aspects) { + aspects = new Aspects(scope); + + Object.defineProperty(scope, ASPECTS_SYMBOL, { + value: aspects, + configurable: false, + enumerable: false, + }); + } + return aspects; + } + + // TODO(2.0): private readonly _aspects = new Array(); + private constructor(private readonly scope: IConstruct) { } + + /** + * Adds an aspect to apply this scope before synthesis. + * @param aspect The aspect to add. + */ + public add(aspect: IAspect) { + // TODO(2.0): this._aspects.push(aspect); + this.scope.node._actualNode.applyAspect(aspect); + } + + /** + * The list of aspects which were directly applied on this scope. + */ + public get aspects(): IAspect[] { + // TODO(2.0): return [ ...this._aspects ]; + return [ ...(this.scope.node._actualNode as any)._aspects ]; // clone + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/core/lib/construct-compat.ts b/packages/@aws-cdk/core/lib/construct-compat.ts index e689d3a837619..315fa8a38232f 100644 --- a/packages/@aws-cdk/core/lib/construct-compat.ts +++ b/packages/@aws-cdk/core/lib/construct-compat.ts @@ -13,7 +13,7 @@ import * as cxschema from '@aws-cdk/cloud-assembly-schema'; import * as cxapi from '@aws-cdk/cx-api'; import * as constructs from 'constructs'; -import { IAspect } from './aspect'; +import { IAspect, Aspects } from './aspect'; import { IDependable } from './dependency'; import { Token } from './token'; @@ -267,7 +267,13 @@ export class ConstructNode { */ public readonly _actualNode: constructs.Node; + /** + * The Construct class that hosts this API. + */ + private readonly host: Construct; + constructor(host: Construct, scope: IConstruct, id: string) { + this.host = host; this._actualNode = new constructs.Node(host, scope, id); // store a back reference on _actualNode so we can our ConstructNode from it @@ -433,9 +439,12 @@ export class ConstructNode { } /** - * Applies the aspect to this Constructs node + * DEPRECATED: Applies the aspect to this Constructs node + * + * @deprecated This API is going to be removed in the next major version of + * the AWS CDK. Please use `Aspects.of(scope).add()` instead. */ - public applyAspect(aspect: IAspect): void { this._actualNode.applyAspect(aspect); } + public applyAspect(aspect: IAspect): void { Aspects.of(this.host).add(aspect); } /** * All parent scopes of this construct. diff --git a/packages/@aws-cdk/core/lib/private/synthesis.ts b/packages/@aws-cdk/core/lib/private/synthesis.ts index 296ba171efde9..77f0bacb7004a 100644 --- a/packages/@aws-cdk/core/lib/private/synthesis.ts +++ b/packages/@aws-cdk/core/lib/private/synthesis.ts @@ -1,5 +1,6 @@ import * as cxapi from '@aws-cdk/cx-api'; import * as constructs from 'constructs'; +import { Aspects, IAspect } from '../aspect'; import { Construct, IConstruct, SynthesisOptions, ValidationError } from '../construct-compat'; import { Stack } from '../stack'; import { Stage, StageSynthesisOptions } from '../stage'; @@ -60,26 +61,35 @@ function synthNestedAssemblies(root: IConstruct, options: StageSynthesisOptions) * twice for the same construct. */ function invokeAspects(root: IConstruct) { + const invokedByPath: { [nodePath: string]: IAspect[] } = { }; + let nestedAspectWarning = false; recurse(root, []); function recurse(construct: IConstruct, inheritedAspects: constructs.IAspect[]) { - // hackery to be able to access some private members with strong types (yack!) - const node: NodeWithAspectPrivatesHangingOut = construct.node._actualNode as any; - - const allAspectsHere = [...inheritedAspects ?? [], ...node._aspects]; - const nodeAspectsCount = node._aspects.length; + const node = construct.node; + const aspects = Aspects.of(construct); + const allAspectsHere = [...inheritedAspects ?? [], ...aspects.aspects]; + const nodeAspectsCount = aspects.aspects.length; for (const aspect of allAspectsHere) { - if (node.invokedAspects.includes(aspect)) { continue; } + let invoked = invokedByPath[node.path]; + if (!invoked) { + invoked = invokedByPath[node.path] = []; + } + + if (invoked.includes(aspect)) { continue; } aspect.visit(construct); + // if an aspect was added to the node while invoking another aspect it will not be invoked, emit a warning // the `nestedAspectWarning` flag is used to prevent the warning from being emitted for every child - if (!nestedAspectWarning && nodeAspectsCount !== node._aspects.length) { + if (!nestedAspectWarning && nodeAspectsCount !== aspects.aspects.length) { construct.node.addWarning('We detected an Aspect was added via another Aspect, and will not be applied'); nestedAspectWarning = true; } - node.invokedAspects.push(aspect); + + // mark as invoked for this node + invoked.push(aspect); } for (const child of construct.node.children) { @@ -180,13 +190,3 @@ interface IProtectedConstructMethods extends IConstruct { */ onPrepare(): void; } - -/** - * The constructs Node type, but with some aspects-related fields public. - * - * Hackery! - */ -type NodeWithAspectPrivatesHangingOut = Omit & { - readonly invokedAspects: constructs.IAspect[]; - readonly _aspects: constructs.IAspect[]; -}; \ No newline at end of file diff --git a/packages/@aws-cdk/core/lib/tag-aspect.ts b/packages/@aws-cdk/core/lib/tag-aspect.ts index a0c8445cd84a8..3c8fd7b01a6b8 100644 --- a/packages/@aws-cdk/core/lib/tag-aspect.ts +++ b/packages/@aws-cdk/core/lib/tag-aspect.ts @@ -1,5 +1,5 @@ // import * as cxapi from '@aws-cdk/cx-api'; -import { IAspect } from './aspect'; +import { IAspect, Aspects } from './aspect'; import { Construct, IConstruct } from './construct-compat'; import { ITaggable, TagManager } from './tag-manager'; @@ -86,17 +86,21 @@ abstract class TagBase implements IAspect { export class Tag extends TagBase { /** - * add tags to the node of a construct and all its the taggable children + * DEPRECATED: add tags to the node of a construct and all its the taggable children + * + * @deprecated use `Tags.of(scope).add()` */ public static add(scope: Construct, key: string, value: string, props: TagProps = {}) { - scope.node.applyAspect(new Tag(key, value, props)); + Tags.of(scope).add(key, value, props); } /** - * remove tags to the node of a construct and all its the taggable children + * DEPRECATED: remove tags to the node of a construct and all its the taggable children + * + * @deprecated use `Tags.of(scope).remove()` */ public static remove(scope: Construct, key: string, props: TagProps = {}) { - scope.node.applyAspect(new RemoveTag(key, props)); + Tags.of(scope).remove(key, props); } /** @@ -126,6 +130,35 @@ export class Tag extends TagBase { } } +/** + * Manages AWS tags for all resources within a construct scope. + */ +export class Tags { + /** + * Returns the tags API for this scope. + * @param scope The scope + */ + public static of(scope: IConstruct): Tags { + return new Tags(scope); + } + + private constructor(private readonly scope: IConstruct) { } + + /** + * add tags to the node of a construct and all its the taggable children + */ + public add(key: string, value: string, props: TagProps = {}) { + Aspects.of(this.scope).add(new Tag(key, value, props)); + } + + /** + * remove tags to the node of a construct and all its the taggable children + */ + public remove(key: string, props: TagProps = {}) { + Aspects.of(this.scope).add(new RemoveTag(key, props)); + } +} + /** * The RemoveTag Aspect will handle removing tags from this node and children */ diff --git a/packages/@aws-cdk/core/test/test.aspect.ts b/packages/@aws-cdk/core/test/test.aspect.ts index 5b13b2a547b5b..90294a86055a7 100644 --- a/packages/@aws-cdk/core/test/test.aspect.ts +++ b/packages/@aws-cdk/core/test/test.aspect.ts @@ -1,7 +1,7 @@ import * as cxschema from '@aws-cdk/cloud-assembly-schema'; import { Test } from 'nodeunit'; import { App } from '../lib'; -import { IAspect } from '../lib/aspect'; +import { IAspect, Aspects } from '../lib/aspect'; import { Construct, IConstruct } from '../lib/construct-compat'; class MyConstruct extends Construct { @@ -29,7 +29,7 @@ export = { 'Aspects are invoked only once'(test: Test) { const app = new App(); const root = new MyConstruct(app, 'MyConstruct'); - root.node.applyAspect(new VisitOnce()); + Aspects.of(root).add(new VisitOnce()); app.synth(); test.deepEqual(root.visitCounter, 1); app.synth(); @@ -41,9 +41,9 @@ export = { const app = new App(); const root = new MyConstruct(app, 'MyConstruct'); const child = new MyConstruct(root, 'ChildConstruct'); - root.node.applyAspect({ + Aspects.of(root).add({ visit(construct: IConstruct) { - construct.node.applyAspect({ + Aspects.of(construct).add({ visit(inner: IConstruct) { inner.node.addMetadata('test', 'would-be-ignored'); }, @@ -62,7 +62,7 @@ export = { const app = new App(); const root = new MyConstruct(app, 'Construct'); const child = new MyConstruct(root, 'ChildConstruct'); - root.node.applyAspect(new MyAspect()); + Aspects.of(root).add(new MyAspect()); app.synth(); test.deepEqual(root.node.metadata[0].type, 'foo'); test.deepEqual(root.node.metadata[0].data, 'bar'); diff --git a/packages/@aws-cdk/core/test/test.stack.ts b/packages/@aws-cdk/core/test/test.stack.ts index c6a3be31af013..4eb7ab164b29c 100644 --- a/packages/@aws-cdk/core/test/test.stack.ts +++ b/packages/@aws-cdk/core/test/test.stack.ts @@ -2,7 +2,7 @@ import * as cxapi from '@aws-cdk/cx-api'; import { Test } from 'nodeunit'; import { App, CfnCondition, CfnInclude, CfnOutput, CfnParameter, - CfnResource, Construct, Lazy, ScopedAws, Stack, Tag, validateString, ISynthesisSession } from '../lib'; + CfnResource, Construct, Lazy, ScopedAws, Stack, validateString, ISynthesisSession, Tags } from '../lib'; import { Intrinsic } from '../lib/private/intrinsic'; import { resolveReferences } from '../lib/private/refs'; import { PostResolveToken } from '../lib/util'; @@ -840,7 +840,7 @@ export = { const stack2 = new Stack(stack1, 'stack2'); // WHEN - Tag.add(app, 'foo', 'bar'); + Tags.of(app).add('foo', 'bar'); // THEN const asm = app.synth(); diff --git a/packages/@aws-cdk/core/test/test.stage.ts b/packages/@aws-cdk/core/test/test.stage.ts index 4f5e5f02d0542..d93b8d2c08c38 100644 --- a/packages/@aws-cdk/core/test/test.stage.ts +++ b/packages/@aws-cdk/core/test/test.stage.ts @@ -1,6 +1,6 @@ import * as cxapi from '@aws-cdk/cx-api'; import { Test } from 'nodeunit'; -import { App, CfnResource, Construct, IAspect, IConstruct, Stack, Stage } from '../lib'; +import { App, CfnResource, Construct, IAspect, IConstruct, Stack, Stage, Aspects } from '../lib'; export = { 'Stack inherits unspecified part of the env from Stage'(test: Test) { @@ -148,7 +148,7 @@ export = { // WHEN const aspect = new TouchingAspect(); - stack.node.applyAspect(aspect); + Aspects.of(stack).add(aspect); // THEN app.synth(); @@ -168,7 +168,7 @@ export = { // WHEN const aspect = new TouchingAspect(); - app.node.applyAspect(aspect); + Aspects.of(app).add(aspect); // THEN app.synth(); diff --git a/packages/@aws-cdk/core/test/test.tag-aspect.ts b/packages/@aws-cdk/core/test/test.tag-aspect.ts index cd533bd537e67..ed21a5930b5b4 100644 --- a/packages/@aws-cdk/core/test/test.tag-aspect.ts +++ b/packages/@aws-cdk/core/test/test.tag-aspect.ts @@ -1,5 +1,5 @@ import { Test } from 'nodeunit'; -import { CfnResource, CfnResourceProps, Construct, RemoveTag, Stack, Tag, TagManager, TagType } from '../lib'; +import { CfnResource, CfnResourceProps, Construct, RemoveTag, Stack, Tag, TagManager, TagType, Aspects, Tags } from '../lib'; import { synthesize } from '../lib/private/synthesis'; class TaggableResource extends CfnResource { @@ -54,7 +54,7 @@ export = { const map = new MapTaggableResource(res, 'MapFakeResource', { type: 'AWS::Fake::Thing', }); - res.node.applyAspect(new Tag('foo', 'bar')); + Aspects.of(res).add(new Tag('foo', 'bar')); synthesize(root); @@ -72,10 +72,10 @@ export = { const res2 = new TaggableResource(res, 'FakeResource', { type: 'AWS::Fake::Thing', }); - res.node.applyAspect(new Tag('foo', 'bar')); - res.node.applyAspect(new Tag('foo', 'foobar')); - res.node.applyAspect(new Tag('foo', 'baz')); - res2.node.applyAspect(new Tag('foo', 'good')); + Aspects.of(res).add(new Tag('foo', 'bar')); + Aspects.of(res).add(new Tag('foo', 'foobar')); + Aspects.of(res).add(new Tag('foo', 'baz')); + Aspects.of(res2).add(new Tag('foo', 'good')); synthesize(root); test.deepEqual(res.tags.renderTags(), [{key: 'foo', value: 'baz'}]); test.deepEqual(res2.tags.renderTags(), [{key: 'foo', value: 'good'}]); @@ -96,10 +96,10 @@ export = { const map = new MapTaggableResource(res, 'MapFakeResource', { type: 'AWS::Fake::Thing', }); - root.node.applyAspect(new Tag('root', 'was here')); - res.node.applyAspect(new Tag('first', 'there is only 1')); - res.node.applyAspect(new RemoveTag('root')); - res.node.applyAspect(new RemoveTag('doesnotexist')); + Aspects.of(root).add(new Tag('root', 'was here')); + Aspects.of(res).add(new Tag('first', 'there is only 1')); + Aspects.of(res).add(new RemoveTag('root')); + Aspects.of(res).add(new RemoveTag('doesnotexist')); synthesize(root); test.deepEqual(res.tags.renderTags(), [{key: 'first', value: 'there is only 1'}]); @@ -123,10 +123,10 @@ export = { const map = new MapTaggableResource(res, 'MapFakeResource', { type: 'AWS::Fake::Thing', }); - Tag.add(root, 'root', 'was here'); - Tag.add(res, 'first', 'there is only 1'); - Tag.remove(res, 'root'); - Tag.remove(res, 'doesnotexist'); + Tags.of(root).add('root', 'was here'); + Tags.of(res).add('first', 'there is only 1'); + Tags.of(res).remove('root'); + Tags.of(res).remove('doesnotexist'); synthesize(root); @@ -142,7 +142,7 @@ export = { type: 'AWS::Fake::Thing', }); - res.node.applyAspect(new Tag('foo', 'bar')); + Aspects.of(res).add(new Tag('foo', 'bar')); synthesize(root); test.deepEqual(res.tags.renderTags(), [{key: 'foo', value: 'bar'}]); synthesize(root); @@ -159,8 +159,8 @@ export = { const res2 = new TaggableResource(res, 'FakeResource', { type: 'AWS::Fake::Thing', }); - res.node.applyAspect(new RemoveTag('key')); - res2.node.applyAspect(new Tag('key', 'value')); + Aspects.of(res).add(new RemoveTag('key')); + Aspects.of(res2).add(new Tag('key', 'value')); synthesize(root); test.deepEqual(res.tags.renderTags(), undefined); test.deepEqual(res2.tags.renderTags(), undefined); @@ -174,8 +174,8 @@ export = { const res2 = new TaggableResource(res, 'FakeResource', { type: 'AWS::Fake::Thing', }); - res.node.applyAspect(new RemoveTag('key', {priority: 0})); - res2.node.applyAspect(new Tag('key', 'value')); + Aspects.of(res).add(new RemoveTag('key', {priority: 0})); + Aspects.of(res2).add(new Tag('key', 'value')); synthesize(root); test.deepEqual(res.tags.renderTags(), undefined); test.deepEqual(res2.tags.renderTags(), [{key: 'key', value: 'value'}]); @@ -218,7 +218,7 @@ export = { ], }, }); - aspectBranch.node.applyAspect(new Tag('aspects', 'rule')); + Aspects.of(aspectBranch).add(new Tag('aspects', 'rule')); synthesize(root); test.deepEqual(aspectBranch.testProperties().tags, [{key: 'aspects', value: 'rule'}, {key: 'cfn', value: 'is cool'}]); test.deepEqual(asgResource.testProperties().tags, [