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

chore(cli): refactor notices.ts to not use configuration settings #32558

Merged
merged 3 commits into from
Dec 18, 2024
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
7 changes: 6 additions & 1 deletion packages/aws-cdk/lib/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,12 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n

const cmd = argv._[0];

const notices = Notices.create({ configuration, includeAcknowledged: cmd === 'notices' ? !argv.unacknowledged : false });
const notices = Notices.create({
context: configuration.context,
output: configuration.settings.get(['outdir']),
shouldDisplay: configuration.settings.get(['notices']),
includeAcknowledged: cmd === 'notices' ? !argv.unacknowledged : false,
});
await notices.refresh();

const sdkProvider = await SdkProvider.withAwsCliCompatibleDefaults({
Expand Down
57 changes: 26 additions & 31 deletions packages/aws-cdk/lib/notices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type { Environment } from '@aws-cdk/cx-api';
import * as fs from 'fs-extra';
import * as semver from 'semver';
import { debug, print, warning, error } from './logging';
import { Configuration } from './settings';
import { Context } from './settings';
import { loadTreeFromDir, some } from './tree';
import { flatMap } from './util';
import { cdkCacheDir } from './util/directories';
Expand All @@ -15,9 +15,9 @@ const CACHE_FILE_PATH = path.join(cdkCacheDir(), 'notices.json');

export interface NoticesProps {
/**
* A `Configuration` instance holding CDK context and settings.
* CDK context
*/
readonly configuration: Configuration;
readonly context: Context;

/**
* Include notices that have already been acknowledged.
Expand All @@ -26,6 +26,19 @@ export interface NoticesProps {
*/
readonly includeAcknowledged?: boolean;

/**
* Global CLI option for output directory for synthesized cloud assembly
*
* @default 'cdk.out'
*/
readonly output?: string;

/**
* Global CLI option for whether we show notices
*
* @default true
*/
readonly shouldDisplay?: boolean;
}

export interface NoticesPrintOptions {
Expand All @@ -36,7 +49,6 @@ export interface NoticesPrintOptions {
* @default false
*/
readonly showTotal?: boolean;

}

export interface NoticesRefreshOptions {
Expand All @@ -63,7 +75,6 @@ export interface NoticesFilterFilterOptions {
}

export class NoticesFilter {

public static filter(options: NoticesFilterFilterOptions): FilteredNotice[] {
return [
...this.findForCliVersion(options.data, options.cliVersion),
Expand Down Expand Up @@ -124,9 +135,7 @@ export class NoticesFilter {
function compareVersions(pattern: string, target: string | undefined): boolean {
return semver.satisfies(target ?? '', pattern);
}

});

}

private static findForBootstrapVersion(data: Notice[], bootstrappedEnvironments: BootstrappedEnvironment[]): FilteredNotice[] {
Expand Down Expand Up @@ -159,7 +168,6 @@ export class NoticesFilter {
filtered.addDynamicValue('ENVIRONMENTS', affected.map(s => s.environment.name).join(','));

return [filtered];

});
}

Expand All @@ -178,7 +186,6 @@ export class NoticesFilter {
}
});
}

}

/**
Expand All @@ -193,7 +200,6 @@ export interface BootstrappedEnvironment {
* Provides access to notices the CLI can display.
*/
export class Notices {

/**
* Create an instance. Note that this replaces the singleton.
*/
Expand All @@ -211,7 +217,9 @@ export class Notices {

private static _instance: Notices | undefined;

private readonly configuration: Configuration;
private readonly context: Context;
private readonly output: string;
private readonly shouldDisplay: boolean;
private readonly acknowledgedIssueNumbers: Set<Number>;
private readonly includeAcknowlegded: boolean;

Expand All @@ -221,9 +229,11 @@ export class Notices {
private readonly bootstrappedEnvironments: Map<string, BootstrappedEnvironment> = new Map();

private constructor(props: NoticesProps) {
this.configuration = props.configuration;
this.acknowledgedIssueNumbers = new Set(this.configuration.context.get('acknowledged-issue-numbers') ?? []);
this.context = props.context;
this.acknowledgedIssueNumbers = new Set(this.context.get('acknowledged-issue-numbers') ?? []);
this.includeAcknowlegded = props.includeAcknowledged ?? false;
this.output = props.output ?? 'cdk.out';
this.shouldDisplay = props.shouldDisplay ?? true;
}

/**
Expand All @@ -248,8 +258,7 @@ export class Notices {
* If context is configured to not display notices, this will no-op.
*/
public async refresh(options: NoticesRefreshOptions = {}) {

if (!this.shouldDisplay()) {
if (!this.shouldDisplay) {
return;
}

Expand All @@ -266,15 +275,14 @@ export class Notices {
* Display the relevant notices (unless context dictates we shouldn't).
*/
public display(options: NoticesPrintOptions = {}) {

if (!this.shouldDisplay()) {
if (!this.shouldDisplay) {
return;
}

const filteredNotices = NoticesFilter.filter({
data: Array.from(this.data),
cliVersion: versionNumber(),
outDir: this.configuration.settings.get(['output']) ?? 'cdk.out',
outDir: this.output,
bootstrappedEnvironments: Array.from(this.bootstrappedEnvironments.values()),
});

Expand Down Expand Up @@ -303,17 +311,7 @@ export class Notices {
print('');
print(`There are ${filteredNotices.length} unacknowledged notice(s).`);
}

}

/**
* Determine whether or not notices should be displayed based on the
* configuration provided at instantiation time.
*/
private shouldDisplay(): boolean {
return this.configuration.settings.get(['notices']) ?? true;
}

}

export interface Component {
Expand All @@ -335,7 +333,6 @@ export interface Notice {
* dynamic values as it has access to the dynamic matching data.
*/
export class FilteredNotice {

private readonly dynamicValues: { [key: string]: string } = {};

public constructor(public readonly notice: Notice) {}
Expand All @@ -353,7 +350,6 @@ export class FilteredNotice {
`\tAffected versions: ${componentsValue}`,
`\tMore information at: https://github.com/aws/aws-cdk/issues/${this.notice.issueNumber}`,
].join('\n\n') + '\n');

}

private formatOverview() {
Expand All @@ -372,7 +368,6 @@ export class FilteredNotice {
const pattern = new RegExp(Object.keys(this.dynamicValues).join('|'), 'g');
return input.replace(pattern, (matched) => this.dynamicValues[matched] ?? matched);
}

}

export interface NoticeDataSource {
Expand Down
4 changes: 2 additions & 2 deletions packages/aws-cdk/test/api/environment-resources.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { GetParameterCommand } from '@aws-sdk/client-ssm';
import { ToolkitInfo } from '../../lib/api';
import { EnvironmentResourcesRegistry } from '../../lib/api/environment-resources';
import { CachedDataSource, Notices, NoticesFilter } from '../../lib/notices';
import { Configuration } from '../../lib/settings';
import { Context } from '../../lib/settings';
import * as version from '../../lib/version';
import { MockSdk, mockBootstrapStack, mockSSMClient } from '../util/mock-sdk';
import { MockToolkitInfo } from '../util/mock-toolkitinfo';
Expand Down Expand Up @@ -98,7 +98,7 @@ describe('validateversion without bootstrap stack', () => {
jest.spyOn(version, 'versionNumber').mockImplementation(() => '1.0.0');

// THEN
const notices = Notices.create({ configuration: new Configuration() });
const notices = Notices.create({ context: new Context() });
await notices.refresh({ dataSource: { fetch: async () => [] } });
await expect(envResources().validateVersion(8, '/abc')).resolves.toBeUndefined();

Expand Down
Loading
Loading