diff --git a/packages/aws-cdk/lib/api/cxapp/exec.ts b/packages/aws-cdk/lib/api/cxapp/exec.ts index 32dea945ca5e0..5d830301dfd1d 100644 --- a/packages/aws-cdk/lib/api/cxapp/exec.ts +++ b/packages/aws-cdk/lib/api/cxapp/exec.ts @@ -12,7 +12,7 @@ import { SDK } from '../util/sdk'; export async function execProgram(aws: SDK, config: Configuration): Promise { const env: { [key: string]: string } = { }; - const context = config.context.everything(); + const context = config.context.all; await populateDefaultEnvironmentIfNeeded(aws, context); let pathMetadata: boolean = config.settings.get(['pathMetadata']); diff --git a/packages/aws-cdk/lib/commands/context.ts b/packages/aws-cdk/lib/commands/context.ts index 429e1360f5357..4cdd2f3440dee 100644 --- a/packages/aws-cdk/lib/commands/context.ts +++ b/packages/aws-cdk/lib/commands/context.ts @@ -27,7 +27,7 @@ export function handler(args: yargs.Arguments) { export async function realHandler(options: CommandOptions): Promise { const { configuration, args } = options; - const contextValues = configuration.context.everything(); + const contextValues = configuration.context.all; if (args.clear) { configuration.context.clear(); diff --git a/packages/aws-cdk/lib/settings.ts b/packages/aws-cdk/lib/settings.ts index 0b7e1e6f9295e..434b1a96f8370 100644 --- a/packages/aws-cdk/lib/settings.ts +++ b/packages/aws-cdk/lib/settings.ts @@ -18,10 +18,11 @@ const CONTEXT_KEY = 'context'; */ export class Configuration { public settings = new Settings(); - public context = new Context(new Settings(), [], new Settings()); + public context = new Context(); private readonly defaultConfig = new Settings({ versionReporting: true, pathMetadata: true }); private readonly commandLineArguments: Settings; + private readonly commandLineContext: Settings; private projectConfig: Settings; private projectContext: Settings; private loaded = false; @@ -30,6 +31,7 @@ export class Configuration { this.commandLineArguments = commandLineArguments ? Settings.fromCommandLineArguments(commandLineArguments) : new Settings(); + this.commandLineContext = this.commandLineArguments.subSettings([CONTEXT_KEY]).makeReadOnly(); } /** @@ -40,7 +42,12 @@ export class Configuration { this.projectConfig = await loadAndLog(PROJECT_CONFIG); this.projectContext = await loadAndLog(PROJECT_CONTEXT); - this.context = new Context(this.projectConfig, [CONTEXT_KEY], this.projectContext); + await this.migrateLegacyContext(); + + this.context = new Context( + this.commandLineContext, + this.projectConfig.subSettings([CONTEXT_KEY]).makeReadOnly(), + this.projectContext); // Build settings from what's left this.settings = this.defaultConfig @@ -55,66 +62,103 @@ export class Configuration { } /** - * Save the project config + * Save the project context */ public async saveContext(): Promise { - if (!this.loaded) { return this; } + if (!this.loaded) { return this; } // Avoid overwriting files with nothing - if (this.context.modifiedBottom) { - await this.projectConfig.save(PROJECT_CONFIG); - } await this.projectContext.save(PROJECT_CONTEXT); return this; } + + /** + * Migrate context from the 'context' field in the projectConfig object to the dedicated object + * + * Only migrate context whose key contains a ':', to migrate only context generated + * by context providers. + */ + private async migrateLegacyContext() { + const legacyContext = this.projectConfig.get([CONTEXT_KEY]); + if (legacyContext === undefined) { return; } + + const toMigrate = Object.keys(legacyContext).filter(k => k.indexOf(':') > -1); + if (toMigrate.length === 0) { return; } + + for (const key of toMigrate) { + this.projectContext.set([key], legacyContext[key]); + this.projectConfig.unset([CONTEXT_KEY, key]); + } + + // If the source object is empty now, completely remove it + if (Object.keys(this.projectConfig.get([CONTEXT_KEY])).length === 0) { + this.projectConfig.unset([CONTEXT_KEY]); + } + + // Save back + await this.projectConfig.save(PROJECT_CONFIG); + await this.projectContext.save(PROJECT_CONTEXT); + } } async function loadAndLog(fileName: string): Promise { const ret = new Settings(); await ret.load(fileName); if (!ret.empty) { - debug(fileName + ':', JSON.stringify(ret.get([]), undefined, 2)); + debug(fileName + ':', JSON.stringify(ret.all, undefined, 2)); } return ret; } /** - * Class that supports overlaying 2 property bags + * Class that supports overlaying property bags * - * Writes go to the topmost property bag, but if any writes collide between - * them the value will be deleted from the underlying property bag. + * Reads come from the first property bag that can has the given key, + * writes go to the first property bag that is not readonly. A write + * will remove the value from all property bags after the first + * writable one. */ export class Context { - public modifiedBottom = false; + private readonly bags: Settings[]; - constructor(private readonly bottom: Settings, private readonly bottomPrefixPath: string[], private readonly top: Settings) { + constructor(...bags: Settings[]) { + this.bags = bags.length > 0 ? bags : [new Settings()]; } public get keys(): string[] { - return Object.keys(this.everything()); + return Object.keys(this.all); } public has(key: string) { return this.keys.indexOf(key) > -1; } - public everything(): {[key: string]: any} { - const b = this.bottom.get(this.bottomPrefixPath) || {}; - const t = this.top.get([]) || {}; - return Object.assign(b, t); + public get all(): {[key: string]: any} { + let ret = new Settings(); + + // In reverse order so keys to the left overwrite keys to the right of them + for (const bag of [...this.bags].reverse()) { + ret = ret.merge(bag); + } + + return ret.all; } public get(key: string): any { - let x = this.top.get([key]); - if (x === undefined) { x = this.bottom.get(this.bottomPrefixPath.concat([key])); } - return x; + for (const bag of this.bags) { + const v = bag.get([key]); + if (v !== undefined) { return v; } + } + return undefined; } public set(key: string, value: any) { - this.top.set([key], value); - if (this.bottom.get(this.bottomPrefixPath.concat([key])) !== undefined) { - this.bottom.unset(this.bottomPrefixPath.concat([key])); - this.modifiedBottom = true; + for (const bag of this.bags) { + if (bag.readOnly) { continue; } + + // All bags past the first one have the value erased + bag.set([key], value); + value = undefined; } } @@ -177,7 +221,7 @@ export class Settings { return ret; } - constructor(private settings: SettingsMap = {}, private readOnly = false) {} + constructor(private settings: SettingsMap = {}, public readonly readOnly = false) {} public async load(fileName: string): Promise { if (this.readOnly) { @@ -205,14 +249,20 @@ export class Settings { return this; } + public get all(): any { + return this.get([]); + } + public merge(other: Settings): Settings { return new Settings(util.deepMerge(this.settings, other.settings)); } + public subSettings(keyPrefix: string[]) { + return new Settings(this.get(keyPrefix) || {}, false); + } + public makeReadOnly(): Settings { - const ret = this.clone(); - ret.readOnly = true; - return ret; + return new Settings(this.settings, true); } public clear() { @@ -247,10 +297,6 @@ export class Settings { this.set(path, undefined); } - public clone() { - return new Settings({ ...this.settings }); - } - private prohibitContextKey(key: string, fileName: string) { if (!this.settings.context) { return; } if (key in this.settings.context) { diff --git a/packages/aws-cdk/test/commands/test.context-command.ts b/packages/aws-cdk/test/commands/test.context-command.ts index 1e0ef1a4e12ba..8905f67abe3e2 100644 --- a/packages/aws-cdk/test/commands/test.context-command.ts +++ b/packages/aws-cdk/test/commands/test.context-command.ts @@ -9,6 +9,11 @@ export = { configuration.context.set('foo', 'bar'); configuration.context.set('baz', 'quux'); + test.deepEqual(configuration.context.all, { + foo: 'bar', + baz: 'quux' + }); + // WHEN await realHandler({ configuration, @@ -16,7 +21,7 @@ export = { } as any); // THEN - test.deepEqual(configuration.context.everything(), { + test.deepEqual(configuration.context.all, { baz: 'quux' }); diff --git a/packages/aws-cdk/test/test.context.ts b/packages/aws-cdk/test/test.context.ts index 062e12395801a..71f576efb2900 100644 --- a/packages/aws-cdk/test/test.context.ts +++ b/packages/aws-cdk/test/test.context.ts @@ -43,10 +43,10 @@ export = { test.done(); }, - async 'new context always goes to dedicated file, other context stays in source file'(test: Test) { + async 'context with colons gets migrated to new file'(test: Test) { // GIVEN await fs.writeJSON('cdk.context.json', { foo: 'bar' }); - await fs.writeJSON('cdk.json', { context: { boo: 'far' } }); + await fs.writeJSON('cdk.json', { context: { 'boo': 'far', 'boo:boo': 'far:far' } }); const config = await new Configuration().load(); // WHEN @@ -54,30 +54,13 @@ export = { await config.saveContext(); // THEN - test.deepEqual(await fs.readJSON('cdk.context.json'), { foo: 'bar', baz: 'quux' }); - test.deepEqual(await fs.readJSON('cdk.json'), { context: { boo: 'far' } }); + test.deepEqual(await fs.readJSON('cdk.context.json'), { 'foo': 'bar', 'boo:boo': 'far:far', 'baz': 'quux' }); + test.deepEqual(await fs.readJSON('cdk.json'), { context: { boo: 'far'} }); test.done(); }, - async 'overwritten always goes to dedicated file, removed from source file'(test: Test) { - // GIVEN - await fs.writeJSON('cdk.context.json', { foo: 'bar' }); - await fs.writeJSON('cdk.json', { context: { foo: 'bar' } }); - const config = await new Configuration().load(); - - // WHEN - config.context.set('foo', 'boom'); - await config.saveContext(); - - // THEN - test.deepEqual(await fs.readJSON('cdk.context.json'), { foo: 'boom' }); - test.deepEqual(await fs.readJSON('cdk.json'), { context: {} }); - - test.done(); - }, - - async 'deleted context disappears from both files'(test: Test) { + async 'deleted context disappears from new file'(test: Test) { // GIVEN await fs.writeJSON('cdk.context.json', { foo: 'bar' }); await fs.writeJSON('cdk.json', { context: { foo: 'bar' } }); @@ -89,12 +72,12 @@ export = { // THEN test.deepEqual(await fs.readJSON('cdk.context.json'), {}); - test.deepEqual(await fs.readJSON('cdk.json'), { context: {} }); + test.deepEqual(await fs.readJSON('cdk.json'), { context: { foo: 'bar' }}); test.done(); }, - async 'clear deletes from both files'(test: Test) { + async 'clear deletes from new file'(test: Test) { // GIVEN await fs.writeJSON('cdk.context.json', { foo: 'bar' }); await fs.writeJSON('cdk.json', { context: { boo: 'far' } }); @@ -106,23 +89,23 @@ export = { // THEN test.deepEqual(await fs.readJSON('cdk.context.json'), {}); - test.deepEqual(await fs.readJSON('cdk.json'), { context: {} }); + test.deepEqual(await fs.readJSON('cdk.json'), { context: { boo: 'far' } }); test.done(); }, async 'surive missing new file'(test: Test) { // GIVEN - await fs.writeJSON('cdk.json', { context: { boo: 'far' } }); + await fs.writeJSON('cdk.json', { context: { 'boo:boo' : 'far' } }); const config = await new Configuration().load(); // WHEN - test.deepEqual(config.context.everything(), { boo: 'far' }); + test.deepEqual(config.context.all, { 'boo:boo' : 'far' }); await config.saveContext(); // THEN - test.deepEqual(await fs.readJSON('cdk.context.json'), {}); - test.deepEqual(await fs.readJSON('cdk.json'), { context: { boo: 'far' } }); + test.deepEqual(await fs.readJSON('cdk.context.json'), { 'boo:boo' : 'far' }); + test.deepEqual(await fs.readJSON('cdk.json'), {}); test.done(); }, @@ -134,7 +117,7 @@ export = { const config = await new Configuration().load(); // WHEN - test.deepEqual(config.context.everything(), { boo: 'far' }); + test.deepEqual(config.context.all, { boo: 'far' }); await config.saveContext(); // THEN @@ -142,4 +125,15 @@ export = { test.done(); }, + + async 'command line context is merged with stored context'(test: Test) { + // GIVEN + await fs.writeJSON('cdk.context.json', { boo: 'far' }); + const config = await new Configuration({ context: ['foo=bar'] } as any).load(); + + // WHEN + test.deepEqual(config.context.all, { foo: 'bar', boo: 'far' }); + + test.done(); + }, }; \ No newline at end of file diff --git a/packages/aws-cdk/test/test.settings.ts b/packages/aws-cdk/test/test.settings.ts new file mode 100644 index 0000000000000..0e86a46f62b04 --- /dev/null +++ b/packages/aws-cdk/test/test.settings.ts @@ -0,0 +1,72 @@ +import { Test } from 'nodeunit'; +import { Context, Settings } from '../lib/settings'; + +export = { + 'can delete values from Context object'(test: Test) { + // GIVEN + const settings1 = new Settings({ foo: 'bar' }); + const settings2 = new Settings({ boo: 'baz' }); + const context = new Context(settings1, settings2); + + // WHEN + context.unset('foo'); + + // THEN + test.deepEqual(context.all, { boo: 'baz' }); + test.deepEqual(settings1.all, {}); + test.deepEqual(settings2.all, { boo: 'baz' }); + + test.done(); + }, + + 'can set values in Context object'(test: Test) { + // GIVEN + const settings1 = new Settings(); + const settings2 = new Settings(); + const context = new Context(settings1, settings2); + + // WHEN + context.set('foo', 'bar'); + + // THEN + test.deepEqual(context.all, { foo: 'bar' }); + test.deepEqual(settings1.all, { foo: 'bar' }); + test.deepEqual(settings2.all, {}); + + test.done(); + }, + + 'can set values in Context object if first is immutable'(test: Test) { + // GIVEN + const settings1 = new Settings(); + const settings2 = new Settings(); + const context = new Context(settings1.makeReadOnly(), settings2); + + // WHEN + context.set('foo', 'bar'); + + // THEN + test.deepEqual(context.all, { foo: 'bar' }); + test.deepEqual(settings1.all, { }); + test.deepEqual(settings2.all, { foo: 'bar' }); + + test.done(); + }, + + 'can clear all values in all objects'(test: Test) { + // GIVEN + const settings1 = new Settings({ foo: 'bar' }); + const settings2 = new Settings({ foo: 'snar', boo: 'gar' }); + const context = new Context(settings1, settings2); + + // WHEN + context.clear(); + + // THEN + test.deepEqual(context.all, {}); + test.deepEqual(settings1.all, { }); + test.deepEqual(settings2.all, {}); + + test.done(); + }, +}; \ No newline at end of file