From 189bae3bac3a1b282400a6dc93935dcf6104930d Mon Sep 17 00:00:00 2001 From: Kaizen Conroy <36202692+kaizencc@users.noreply.github.com> Date: Fri, 20 Dec 2024 10:09:10 -0500 Subject: [PATCH] refactor(cli): docs, doctor, context commands do not use configuration directly anymore (#32578) This is a refactor that follows #32558. The effort is to ensure that the `configuration` object is not directly used by commands and instead, the relevant information is passed directly to the function. - doctor: no options needed - docs: 1 option needed - context: in addition to a few options, the context command also saves *some* values to `cdk.context.json`. this requires some modifications to the `Context` object to support this. We move away from using the archaic API in `command-api.ts`. This seems to be an old effort to standardize CLI commands that was only used for doctor/docs/context. We have since evolved to use command-specific property bags. Since this is a refactor PR. I aim to change no functionality of the CLI. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- codecov.yml | 1 + packages/aws-cdk/lib/cli-arguments.ts | 2 +- packages/aws-cdk/lib/cli.ts | 23 ++-- packages/aws-cdk/lib/command-api.ts | 26 ---- packages/aws-cdk/lib/commands/context.ts | 64 ++++++++-- packages/aws-cdk/lib/commands/docs.ts | 15 ++- packages/aws-cdk/lib/commands/doctor.ts | 3 +- packages/aws-cdk/lib/config.ts | 4 +- .../lib/parse-command-line-arguments.ts | 2 + packages/aws-cdk/lib/settings.ts | 49 ++++++-- packages/aws-cdk/test/cdk-docs.test.ts | 12 +- packages/aws-cdk/test/cdk-doctor.test.ts | 5 +- packages/aws-cdk/test/cli-arguments.test.ts | 19 +++ .../test/commands/context-command.test.ts | 119 ++++++++++-------- packages/aws-cdk/test/context.test.ts | 62 +++++---- packages/aws-cdk/test/notices.test.ts | 8 +- packages/aws-cdk/test/settings.test.ts | 8 +- 17 files changed, 264 insertions(+), 158 deletions(-) delete mode 100644 packages/aws-cdk/lib/command-api.ts create mode 100644 packages/aws-cdk/test/cli-arguments.test.ts diff --git a/codecov.yml b/codecov.yml index bb41d9b4d440a..1ab1a7e41c8f6 100644 --- a/codecov.yml +++ b/codecov.yml @@ -34,3 +34,4 @@ component_management: # https://docs.codecov.com/docs/ignoring-paths ignore: - packages/aws-cdk/lib/parse-command-line-arguments.ts # this file is generated and some lines cannot be tested + - packages/aws-cdk/lib/cli.ts # we integ test this file diff --git a/packages/aws-cdk/lib/cli-arguments.ts b/packages/aws-cdk/lib/cli-arguments.ts index 4a475017ca8b9..4d14edc5bf60f 100644 --- a/packages/aws-cdk/lib/cli-arguments.ts +++ b/packages/aws-cdk/lib/cli-arguments.ts @@ -1224,7 +1224,7 @@ export interface ContextOptions { /** * Clear all context * - * @default - undefined + * @default - false */ readonly clear?: boolean; } diff --git a/packages/aws-cdk/lib/cli.ts b/packages/aws-cdk/lib/cli.ts index 418d2893df8d9..363d0889c4ec2 100644 --- a/packages/aws-cdk/lib/cli.ts +++ b/packages/aws-cdk/lib/cli.ts @@ -17,9 +17,9 @@ import { Deployments } from '../lib/api/deployments'; import { PluginHost } from '../lib/api/plugin'; import { ToolkitInfo } from '../lib/api/toolkit-info'; import { CdkToolkit, AssetBuildTime } from '../lib/cdk-toolkit'; -import { realHandler as context } from '../lib/commands/context'; -import { realHandler as docs } from '../lib/commands/docs'; -import { realHandler as doctor } from '../lib/commands/doctor'; +import { contextHandler as context } from '../lib/commands/context'; +import { docs } from '../lib/commands/docs'; +import { doctor } from '../lib/commands/doctor'; import { getMigrateScanType } from '../lib/commands/migrate'; import { cliInit, printAvailableTemplates } from '../lib/init'; import { data, debug, error, print, setCI, setLogLevel, LogLevel } from '../lib/logging'; @@ -37,7 +37,6 @@ if (!process.stdout.isTTY) { } export async function exec(args: string[], synthesizer?: Synthesizer): Promise { - const argv = await parseCommandLineArguments(args); // if one -v, log at a DEBUG level @@ -154,9 +153,6 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise { @@ -207,13 +202,19 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise void, but ours are actually - * (args) => Promise, so we deal with the asyncness by copying the actual - * handler object to `args.commandHandler` which will be executed an 'await'ed later on - * (instead of awaiting 'main'). - * - * Also adds exception handling so individual command handlers don't all have to do it. - */ - -/** - * The parts of the world that our command functions have access to - */ -export interface CommandOptions { - args: Arguments; - configuration: Configuration; - aws: SdkProvider; -} - -/** - * This is the type of the *real* async command handlers - */ -export type CommandHandler = (options: CommandOptions) => Promise; diff --git a/packages/aws-cdk/lib/commands/context.ts b/packages/aws-cdk/lib/commands/context.ts index dcf0f5b202589..b4ea3d4d356ec 100644 --- a/packages/aws-cdk/lib/commands/context.ts +++ b/packages/aws-cdk/lib/commands/context.ts @@ -1,27 +1,64 @@ import * as chalk from 'chalk'; import { minimatch } from 'minimatch'; import * as version from '../../lib/version'; -import { CommandOptions } from '../command-api'; import { print, error, warning } from '../logging'; import { Context, PROJECT_CONFIG, PROJECT_CONTEXT, USER_DEFAULTS } from '../settings'; import { renderTable } from '../util'; -export async function realHandler(options: CommandOptions): Promise { - const { configuration, args } = options; - if (args.clear) { - configuration.context.clear(); - await configuration.saveContext(); +/** + * Options for the context command + */ +export interface ContextOptions { + /** + * The context object sourced from all context locations + */ + context: Context; + + /** + * The context key (or its index) to reset + * + * @default undefined + */ + reset?: string; + + /** + * Ignore missing key error + * + * @default false + */ + force?: boolean; + + /** + * Clear all context + * + * @default false + */ + clear?: boolean; + + /** + * Use JSON output instead of YAML when templates are printed to STDOUT + * + * @default false + */ + json?: boolean; +} + +export async function contextHandler(options: ContextOptions): Promise { + if (options.clear) { + options.context.clear(); + await options.context.save(PROJECT_CONTEXT); print('All context values cleared.'); - } else if (args.reset) { - invalidateContext(configuration.context, args.reset as string, args.force as boolean); - await configuration.saveContext(); + } else if (options.reset) { + invalidateContext(options.context, options.reset, options.force ?? false); + await options.context.save(PROJECT_CONTEXT); } else { // List -- support '--json' flag - if (args.json) { - const contextValues = configuration.context.all; + if (options.json) { + /* istanbul ignore next */ + const contextValues = options.context.all; process.stdout.write(JSON.stringify(contextValues, undefined, 2)); } else { - listContext(configuration.context); + listContext(options.context); } } await version.displayVersionMessage(); @@ -105,6 +142,7 @@ function invalidateContext(context: Context, key: string, force: boolean) { throw new Error(`No context value matching key: ${key}`); } } + function printUnset(unset: string[]) { if (unset.length === 0) return; print('The following matched context values reset. They will be refreshed on next synthesis'); @@ -112,6 +150,7 @@ function printUnset(unset: string[]) { print(' %s', match); }); } + function printReadonly(readonly: string[]) { if (readonly.length === 0) return; warning('The following matched context values could not be reset through the CLI'); @@ -121,6 +160,7 @@ function printReadonly(readonly: string[]) { print(''); print('This usually means they are configured in %s or %s', chalk.blue(PROJECT_CONFIG), chalk.blue(USER_DEFAULTS)); } + function keysByExpression(context: Context, expression: string) { return context.keys.filter(minimatch.filter(expression)); } diff --git a/packages/aws-cdk/lib/commands/docs.ts b/packages/aws-cdk/lib/commands/docs.ts index 85c06c51e2a85..7baa89e639b6d 100644 --- a/packages/aws-cdk/lib/commands/docs.ts +++ b/packages/aws-cdk/lib/commands/docs.ts @@ -1,16 +1,25 @@ import * as childProcess from 'child_process'; import * as chalk from 'chalk'; import { debug, print, warning } from '../../lib/logging'; -import { CommandOptions } from '../command-api'; export const command = 'docs'; export const describe = 'Opens the reference documentation in a browser'; export const aliases = ['doc']; -export async function realHandler(options: CommandOptions): Promise { +/** + * Options for the docs command + */ +export interface DocsOptions { + /** + * The command to use to open the browser + */ + browser: string; +} + +export async function docs(options: DocsOptions): Promise { const url = 'https://docs.aws.amazon.com/cdk/api/v2/'; print(chalk.green(url)); - const browserCommand = (options.args.browser as string).replace(/%u/g, url); + const browserCommand = (options.browser).replace(/%u/g, url); debug(`Opening documentation ${chalk.green(browserCommand)}`); return new Promise((resolve, _reject) => { childProcess.exec(browserCommand, (err, stdout, stderr) => { diff --git a/packages/aws-cdk/lib/commands/doctor.ts b/packages/aws-cdk/lib/commands/doctor.ts index 5ea4ddc6367eb..724a6cec23577 100644 --- a/packages/aws-cdk/lib/commands/doctor.ts +++ b/packages/aws-cdk/lib/commands/doctor.ts @@ -3,9 +3,8 @@ import * as cxapi from '@aws-cdk/cx-api'; import * as chalk from 'chalk'; import { print } from '../../lib/logging'; import * as version from '../../lib/version'; -import { CommandOptions } from '../command-api'; -export async function realHandler(_options: CommandOptions): Promise { +export async function doctor(): Promise { let exitStatus: number = 0; for (const verification of verifications) { if (!await verification()) { diff --git a/packages/aws-cdk/lib/config.ts b/packages/aws-cdk/lib/config.ts index d4c46936f990d..3ef2946f3f27b 100644 --- a/packages/aws-cdk/lib/config.ts +++ b/packages/aws-cdk/lib/config.ts @@ -373,9 +373,9 @@ export async function makeConfig(): Promise { context: { description: 'Manage cached context values', options: { - reset: { alias: 'e', desc: 'The context key (or its index) to reset', type: 'string', requiresArg: true }, + reset: { alias: 'e', desc: 'The context key (or its index) to reset', type: 'string', requiresArg: true, default: undefined }, force: { alias: 'f', desc: 'Ignore missing key error', type: 'boolean', default: false }, - clear: { desc: 'Clear all context', type: 'boolean' }, + clear: { desc: 'Clear all context', type: 'boolean', default: false }, }, }, docs: { diff --git a/packages/aws-cdk/lib/parse-command-line-arguments.ts b/packages/aws-cdk/lib/parse-command-line-arguments.ts index 2432fa23ba3c8..4cdd4f1023806 100644 --- a/packages/aws-cdk/lib/parse-command-line-arguments.ts +++ b/packages/aws-cdk/lib/parse-command-line-arguments.ts @@ -742,6 +742,7 @@ export function parseCommandLineArguments(args: Array): any { desc: 'The context key (or its index) to reset', type: 'string', requiresArg: true, + default: undefined, }) .option('force', { alias: 'f', @@ -752,6 +753,7 @@ export function parseCommandLineArguments(args: Array): any { .option('clear', { desc: 'Clear all context', type: 'boolean', + default: false, }) ) .command(['docs', 'doc'], 'Opens the reference documentation in a browser', (yargs: Argv) => diff --git a/packages/aws-cdk/lib/settings.ts b/packages/aws-cdk/lib/settings.ts index c0ba9221474e8..7e558a391d5ac 100644 --- a/packages/aws-cdk/lib/settings.ts +++ b/packages/aws-cdk/lib/settings.ts @@ -99,7 +99,7 @@ export class Configuration { return this._projectConfig; } - private get projectContext() { + public get projectContext() { if (!this._projectContext) { throw new Error('#load has not been called yet!'); } @@ -121,12 +121,12 @@ export class Configuration { } const contextSources = [ - this.commandLineContext, - this.projectConfig.subSettings([CONTEXT_KEY]).makeReadOnly(), - this.projectContext, + { bag: this.commandLineContext }, + { fileName: PROJECT_CONFIG, bag: this.projectConfig.subSettings([CONTEXT_KEY]).makeReadOnly() }, + { fileName: PROJECT_CONTEXT, bag: this.projectContext }, ]; if (readUserContext) { - contextSources.push(userConfig.subSettings([CONTEXT_KEY]).makeReadOnly()); + contextSources.push({ fileName: USER_DEFAULTS, bag: userConfig.subSettings([CONTEXT_KEY]).makeReadOnly() }); } this.context = new Context(...contextSources); @@ -166,6 +166,19 @@ async function loadAndLog(fileName: string): Promise { return ret; } +interface ContextBag { + /** + * The file name of the context. Will be used to potentially + * save new context back to the original file. + */ + fileName?: string; + + /** + * The context values. + */ + bag: Settings; +} + /** * Class that supports overlaying property bags * @@ -176,9 +189,11 @@ async function loadAndLog(fileName: string): Promise { */ export class Context { private readonly bags: Settings[]; + private readonly fileNames: (string|undefined)[]; - constructor(...bags: Settings[]) { - this.bags = bags.length > 0 ? bags : [new Settings()]; + constructor(...bags: ContextBag[]) { + this.bags = bags.length > 0 ? bags.map(b => b.bag) : [new Settings()]; + this.fileNames = bags.length > 0 ? bags.map(b => b.fileName) : ['default']; } public get keys(): string[] { @@ -227,6 +242,26 @@ export class Context { this.unset(key); } } + + /** + * Save a specific context file + */ + public async save(fileName: string): Promise { + const index = this.fileNames.indexOf(fileName); + + // File not found, don't do anything in this scenario + if (index === -1) { + return this; + } + + const bag = this.bags[index]; + if (bag.readOnly) { + throw new Error(`Context file ${fileName} is read only!`); + } + + await bag.save(fileName); + return this; + } } /** diff --git a/packages/aws-cdk/test/cdk-docs.test.ts b/packages/aws-cdk/test/cdk-docs.test.ts index ecc21794d9e04..34d1b7e97e4a8 100644 --- a/packages/aws-cdk/test/cdk-docs.test.ts +++ b/packages/aws-cdk/test/cdk-docs.test.ts @@ -1,23 +1,17 @@ import * as child_process from 'child_process'; import { mocked } from 'jest-mock'; -import { CommandHandler } from '../lib/command-api'; -import { realHandler } from '../lib/commands/docs'; -const argv = { - browser: 'echo %u', - commandHandler: undefined as (CommandHandler | undefined), -}; +import { docs } from '../lib/commands/docs'; // eslint-disable-next-line no-console console.log = jest.fn(); jest.mock('child_process'); describe('`cdk docs`', () => { - test('exits with 0 when everything is OK', async () => { const mockChildProcessExec: any = (_: string, cb: (err?: Error, stdout?: string, stderr?: string) => void) => cb(); mocked(child_process.exec).mockImplementation(mockChildProcessExec); - const result = await realHandler({ args: argv } as any); + const result = await docs({ browser: 'echo %u' }); expect(result).toBe(0); }); @@ -25,7 +19,7 @@ describe('`cdk docs`', () => { const mockChildProcessExec: any = (_: string, cb: (err: Error, stdout?: string, stderr?: string) => void) => cb(new Error('TEST')); mocked(child_process.exec).mockImplementation(mockChildProcessExec); - const result = await realHandler({ args: argv } as any); + const result = await docs({ browser: 'echo %u' }); expect(result).toBe(0); }); }); diff --git a/packages/aws-cdk/test/cdk-doctor.test.ts b/packages/aws-cdk/test/cdk-doctor.test.ts index 2b014e45f5521..9947ea92ed1b5 100644 --- a/packages/aws-cdk/test/cdk-doctor.test.ts +++ b/packages/aws-cdk/test/cdk-doctor.test.ts @@ -1,12 +1,11 @@ -import { realHandler } from '../lib/commands/doctor'; +import { doctor } from '../lib/commands/doctor'; // eslint-disable-next-line no-console console.log = jest.fn(); describe('`cdk doctor`', () => { test('exits with 0 when everything is OK', async () => { - const argv: any = {}; - const result = await realHandler({ args: argv } as any); + const result = await doctor(); expect(result).toBe(0); }); }); diff --git a/packages/aws-cdk/test/cli-arguments.test.ts b/packages/aws-cdk/test/cli-arguments.test.ts new file mode 100644 index 0000000000000..c1e4bcc56c1fe --- /dev/null +++ b/packages/aws-cdk/test/cli-arguments.test.ts @@ -0,0 +1,19 @@ +import { CliArguments } from '../lib/cli-arguments'; +import { Command } from '../lib/settings'; + +// CliArguments is not being used right now, so the testing suite is rather redundant. +// This file is meant to be populated when CliArguments is used. +test('cli arguments can be used as a type', async () => { + const argv: CliArguments = { + _: [Command.DEPLOY], + globalOptions: { + 'lookups': true, + 'ignore-errors': false, + 'json': false, + 'verbose': false, + }, + }; + + expect(argv._[0]).toBe('deploy'); + expect(argv.globalOptions?.lookups).toBeTruthy(); +}); diff --git a/packages/aws-cdk/test/commands/context-command.test.ts b/packages/aws-cdk/test/commands/context-command.test.ts index 71c24dda4e638..988076b7c1d1c 100644 --- a/packages/aws-cdk/test/commands/context-command.test.ts +++ b/packages/aws-cdk/test/commands/context-command.test.ts @@ -1,5 +1,5 @@ /* eslint-disable import/order */ -import { realHandler } from '../../lib/commands/context'; +import { contextHandler } from '../../lib/commands/context'; import { Configuration, Settings, Context } from '../../lib/settings'; describe('context --list', () => { @@ -13,10 +13,9 @@ describe('context --list', () => { }); // WHEN - await realHandler({ - configuration, - args: {}, - } as any); + await contextHandler({ + context: configuration.context, + }); }); }); @@ -33,10 +32,10 @@ describe('context --reset', () => { }); // WHEN - await realHandler({ - configuration, - args: { reset: 'foo' }, - } as any); + await contextHandler({ + context: configuration.context, + reset: 'foo', + }); // THEN expect(configuration.context.all).toEqual({ @@ -56,10 +55,10 @@ describe('context --reset', () => { }); // WHEN - await realHandler({ - configuration, - args: { reset: '1' }, - } as any); + await contextHandler({ + context: configuration.context, + reset: '1', + }); // THEN expect(configuration.context.all).toEqual({ @@ -81,10 +80,10 @@ describe('context --reset', () => { }); // WHEN - await realHandler({ - configuration, - args: { reset: 'match-*' }, - } as any); + await contextHandler({ + context: configuration.context, + reset: 'match-*', + }); // THEN expect(configuration.context.all).toEqual({ @@ -104,10 +103,10 @@ describe('context --reset', () => { }); // WHEN - await realHandler({ - configuration, - args: { reset: 'fo*' }, - } as any); + await contextHandler({ + context: configuration.context, + reset: 'fo*', + }); // THEN expect(configuration.context.all).toEqual({ @@ -122,14 +121,14 @@ describe('context --reset', () => { 'foo': 'bar', 'match-a': 'baz', }, true); - configuration.context = new Context(readOnlySettings, new Settings()); + configuration.context = new Context({ bag: readOnlySettings }, { bag: new Settings() }); configuration.context.set('match-b', 'quux'); // When - await expect(realHandler({ - configuration, - args: { reset: 'match-*' }, - } as any)); + await expect(contextHandler({ + context: configuration.context, + reset: 'match-*', + })); // Then expect(configuration.context.all).toEqual({ @@ -148,10 +147,10 @@ describe('context --reset', () => { }); // THEN - await expect(realHandler({ - configuration, - args: { reset: 'baz' }, - } as any)).rejects.toThrow(/No context value matching key/); + await expect(contextHandler({ + context: configuration.context, + reset: 'baz', + })).rejects.toThrow(/No context value matching key/); }); test('Doesn\'t throw when key not found and --force is set', async () => { @@ -164,10 +163,11 @@ describe('context --reset', () => { }); // THEN - await expect(realHandler({ - configuration, - args: { reset: 'baz', force: true }, - } as any)); + await expect(contextHandler({ + context: configuration.context, + reset: 'baz', + force: true, + })); }); test('throws when no key of index found', async () => { @@ -180,10 +180,10 @@ describe('context --reset', () => { }); // THEN - await expect(realHandler({ - configuration, - args: { reset: '2' }, - } as any)).rejects.toThrow(/No context key with number/); + await expect(contextHandler({ + context: configuration.context, + reset: '2', + })).rejects.toThrow(/No context key with number/); }); test('throws when resetting read-only values', async () => { @@ -192,17 +192,17 @@ describe('context --reset', () => { const readOnlySettings = new Settings({ foo: 'bar', }, true); - configuration.context = new Context(readOnlySettings); + configuration.context = new Context({ bag: readOnlySettings }); expect(configuration.context.all).toEqual({ foo: 'bar', }); // THEN - await expect(realHandler({ - configuration, - args: { reset: 'foo' }, - } as any)).rejects.toThrow(/Cannot reset readonly context value with key/); + await expect(contextHandler({ + context: configuration.context, + reset: 'foo', + })).rejects.toThrow(/Cannot reset readonly context value with key/); }); test('throws when no matches could be reset', async () => { @@ -213,7 +213,7 @@ describe('context --reset', () => { 'match-a': 'baz', 'match-b': 'quux', }, true); - configuration.context = new Context(readOnlySettings); + configuration.context = new Context({ bag: readOnlySettings }); expect(configuration.context.all).toEqual({ 'foo': 'bar', @@ -222,11 +222,32 @@ describe('context --reset', () => { }); // THEN - await expect(realHandler({ - configuration, - args: { reset: 'match-*' }, - } as any)).rejects.toThrow(/None of the matched context values could be reset/); + await expect(contextHandler({ + context: configuration.context, + reset: 'match-*', + })).rejects.toThrow(/None of the matched context values could be reset/); }); - }); +describe('context --clear', () => { + test('can clear all context keys', async () => { + // GIVEN + const configuration = new Configuration(); + configuration.context.set('foo', 'bar'); + configuration.context.set('baz', 'quux'); + + expect(configuration.context.all).toEqual({ + foo: 'bar', + baz: 'quux', + }); + + // WHEN + await contextHandler({ + context: configuration.context, + clear: true, + }); + + // THEN + expect(configuration.context.all).toEqual({}); + }); +}); diff --git a/packages/aws-cdk/test/context.test.ts b/packages/aws-cdk/test/context.test.ts index 95146d20e3501..5f2c766146089 100644 --- a/packages/aws-cdk/test/context.test.ts +++ b/packages/aws-cdk/test/context.test.ts @@ -2,7 +2,7 @@ import * as os from 'os'; import * as path from 'path'; import * as fs from 'fs-extra'; -import { Configuration, TRANSIENT_CONTEXT_KEY } from '../lib/settings'; +import { Configuration, Context, PROJECT_CONFIG, PROJECT_CONTEXT, Settings, TRANSIENT_CONTEXT_KEY } from '../lib/settings'; const state: { previousWorkingDir?: string; @@ -26,8 +26,8 @@ afterEach(async () => { test('load context from both files if available', async () => { // GIVEN - await fs.writeJSON('cdk.context.json', { foo: 'bar' }); - await fs.writeJSON('cdk.json', { context: { boo: 'far' } }); + await fs.writeJSON(PROJECT_CONTEXT, { foo: 'bar' }); + await fs.writeJSON(PROJECT_CONFIG, { context: { boo: 'far' } }); // WHEN const config = await new Configuration({ readUserContext: false }).load(); @@ -39,65 +39,65 @@ test('load context from both files if available', async () => { test('deleted context disappears from new file', async () => { // GIVEN - await fs.writeJSON('cdk.context.json', { foo: 'bar' }); - await fs.writeJSON('cdk.json', { context: { foo: 'bar' } }); + await fs.writeJSON(PROJECT_CONTEXT, { foo: 'bar' }); + await fs.writeJSON(PROJECT_CONFIG, { context: { foo: 'bar' } }); const config = await new Configuration({ readUserContext: false }).load(); // WHEN config.context.unset('foo'); - await config.saveContext(); + await config.context.save(PROJECT_CONTEXT); // THEN - expect(await fs.readJSON('cdk.context.json')).toEqual({}); - expect(await fs.readJSON('cdk.json')).toEqual({ context: { foo: 'bar' } }); + expect(await fs.readJSON(PROJECT_CONTEXT)).toEqual({}); + expect(await fs.readJSON(PROJECT_CONFIG)).toEqual({ context: { foo: 'bar' } }); }); test('clear deletes from new file', async () => { // GIVEN - await fs.writeJSON('cdk.context.json', { foo: 'bar' }); - await fs.writeJSON('cdk.json', { context: { boo: 'far' } }); + await fs.writeJSON(PROJECT_CONTEXT, { foo: 'bar' }); + await fs.writeJSON(PROJECT_CONFIG, { context: { boo: 'far' } }); const config = await new Configuration({ readUserContext: false }).load(); // WHEN config.context.clear(); - await config.saveContext(); + await config.context.save(PROJECT_CONTEXT); // THEN - expect(await fs.readJSON('cdk.context.json')).toEqual({}); - expect(await fs.readJSON('cdk.json')).toEqual({ context: { boo: 'far' } }); + expect(await fs.readJSON(PROJECT_CONTEXT)).toEqual({}); + expect(await fs.readJSON(PROJECT_CONFIG)).toEqual({ context: { boo: 'far' } }); }); test('context is preserved in the location from which it is read', async () => { // GIVEN - await fs.writeJSON('cdk.json', { context: { 'boo:boo': 'far' } }); + await fs.writeJSON(PROJECT_CONFIG, { context: { 'boo:boo': 'far' } }); const config = await new Configuration({ readUserContext: false }).load(); // WHEN expect(config.context.all).toEqual({ 'boo:boo': 'far' }); - await config.saveContext(); + await config.context.save(PROJECT_CONTEXT); // THEN - expect(await fs.readJSON('cdk.context.json')).toEqual({}); - expect(await fs.readJSON('cdk.json')).toEqual({ context: { 'boo:boo': 'far' } }); + expect(await fs.readJSON(PROJECT_CONTEXT)).toEqual({}); + expect(await fs.readJSON(PROJECT_CONFIG)).toEqual({ context: { 'boo:boo': 'far' } }); }); -test('surive no context in old file', async () => { +test('save no context in old file', async () => { // GIVEN - await fs.writeJSON('cdk.json', { }); - await fs.writeJSON('cdk.context.json', { boo: 'far' }); + await fs.writeJSON(PROJECT_CONFIG, {}); + await fs.writeJSON(PROJECT_CONTEXT, { boo: 'far' }); const config = await new Configuration({ readUserContext: false }).load(); // WHEN expect(config.context.all).toEqual({ boo: 'far' }); - await config.saveContext(); + await config.context.save(PROJECT_CONTEXT); // THEN - expect(await fs.readJSON('cdk.context.json')).toEqual({ boo: 'far' }); + expect(await fs.readJSON(PROJECT_CONTEXT)).toEqual({ boo: 'far' }); }); test('command line context is merged with stored context', async () => { // GIVEN - await fs.writeJSON('cdk.context.json', { boo: 'far' }); + await fs.writeJSON(PROJECT_CONTEXT, { boo: 'far' }); const config = await new Configuration({ readUserContext: false, commandLineArguments: { @@ -114,7 +114,7 @@ test('can save and load', async () => { // GIVEN const config1 = await new Configuration({ readUserContext: false }).load(); config1.context.set('some_key', 'some_value'); - await config1.saveContext(); + await config1.context.save(PROJECT_CONTEXT); expect(config1.context.get('some_key')).toEqual('some_value'); // WHEN @@ -128,7 +128,7 @@ test('transient values arent saved to disk', async () => { // GIVEN const config1 = await new Configuration({ readUserContext: false }).load(); config1.context.set('some_key', { [TRANSIENT_CONTEXT_KEY]: true, value: 'some_value' }); - await config1.saveContext(); + await config1.context.save(PROJECT_CONTEXT); expect(config1.context.get('some_key').value).toEqual('some_value'); // WHEN @@ -137,3 +137,15 @@ test('transient values arent saved to disk', async () => { // THEN expect(config2.context.get('some_key')).toEqual(undefined); }); + +test('cannot save readonly values', async () => { + // GIVEN + const settings = new Settings({ foo: 'bar', boo: 'far' }, true); + const context = new Context({ + fileName: PROJECT_CONFIG, + bag: settings, + }); + + // THEN + await expect(context.save(PROJECT_CONFIG)).rejects.toThrow(/Context file cdk.json is read only!/); +}); diff --git a/packages/aws-cdk/test/notices.test.ts b/packages/aws-cdk/test/notices.test.ts index 9c6f1d0786fb0..8a70067573251 100644 --- a/packages/aws-cdk/test/notices.test.ts +++ b/packages/aws-cdk/test/notices.test.ts @@ -608,7 +608,7 @@ describe(Notices, () => { // within the affected version range of both notices jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.126.0'); - const context = new Context(new Settings({ 'acknowledged-issue-numbers': [MULTIPLE_AFFECTED_VERSIONS_NOTICE.issueNumber] })); + const context = new Context({ bag: new Settings({ 'acknowledged-issue-numbers': [MULTIPLE_AFFECTED_VERSIONS_NOTICE.issueNumber] }) }); const notices = Notices.create({ context }); await notices.refresh({ @@ -626,7 +626,7 @@ describe(Notices, () => { // within the affected version range of both notices jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.126.0'); - const context = new Context(new Settings({ 'acknowledged-issue-numbers': [MULTIPLE_AFFECTED_VERSIONS_NOTICE.issueNumber] })); + const context = new Context({ bag: new Settings({ 'acknowledged-issue-numbers': [MULTIPLE_AFFECTED_VERSIONS_NOTICE.issueNumber] }) }); const notices = Notices.create({ context, includeAcknowledged: true }); await notices.refresh({ @@ -749,7 +749,7 @@ describe(Notices, () => { // within the affected version range of both notices jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.126.0'); - const context = new Context(new Settings({ 'acknowledged-issue-numbers': [MULTIPLE_AFFECTED_VERSIONS_NOTICE.issueNumber] })); + const context = new Context({ bag: new Settings({ 'acknowledged-issue-numbers': [MULTIPLE_AFFECTED_VERSIONS_NOTICE.issueNumber] }) }); const notices = Notices.create({ context }); await notices.refresh({ @@ -766,7 +766,7 @@ describe(Notices, () => { // within the affected version range of both notices jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.126.0'); - const context = new Context(new Settings({ 'acknowledged-issue-numbers': [MULTIPLE_AFFECTED_VERSIONS_NOTICE.issueNumber] })); + const context = new Context({ bag: new Settings({ 'acknowledged-issue-numbers': [MULTIPLE_AFFECTED_VERSIONS_NOTICE.issueNumber] }) }); const notices = Notices.create({ context, includeAcknowledged: true }); await notices.refresh({ dataSource: { fetch: async () => [BASIC_NOTICE, MULTIPLE_AFFECTED_VERSIONS_NOTICE] }, diff --git a/packages/aws-cdk/test/settings.test.ts b/packages/aws-cdk/test/settings.test.ts index fde5c1c74665c..7edc2e9b487a2 100644 --- a/packages/aws-cdk/test/settings.test.ts +++ b/packages/aws-cdk/test/settings.test.ts @@ -6,7 +6,7 @@ test('can delete values from Context object', () => { // GIVEN const settings1 = new Settings({ foo: 'bar' }); const settings2 = new Settings({ boo: 'baz' }); - const context = new Context(settings1, settings2); + const context = new Context({ bag: settings1 }, { bag: settings2 }); // WHEN context.unset('foo'); @@ -21,7 +21,7 @@ test('can set values in Context object', () => { // GIVEN const settings1 = new Settings(); const settings2 = new Settings(); - const context = new Context(settings1, settings2); + const context = new Context({ bag: settings1 }, { bag: settings2 }); // WHEN context.set('foo', 'bar'); @@ -36,7 +36,7 @@ test('can set values in Context object if first is immutable', () => { // GIVEN const settings1 = new Settings(); const settings2 = new Settings(); - const context = new Context(settings1.makeReadOnly(), settings2); + const context = new Context({ bag: settings1.makeReadOnly() }, { bag: settings2 }); // WHEN context.set('foo', 'bar'); @@ -51,7 +51,7 @@ test('can clear all values in all objects', () => { // GIVEN const settings1 = new Settings({ foo: 'bar' }); const settings2 = new Settings({ foo: 'snar', boo: 'gar' }); - const context = new Context(settings1, settings2); + const context = new Context({ bag: settings1 }, { bag: settings2 }); // WHEN context.clear();