From 6e6bdb576992a58da6ae065588222303d559d4b5 Mon Sep 17 00:00:00 2001 From: Ahmed Sobhi Date: Tue, 11 Jul 2023 10:22:32 +0100 Subject: [PATCH] address comments, add validations, refactor and rename --- .../aws-cloudwatch/lib/dashboard.ts | 8 +- .../aws-cloudwatch/lib/variable.ts | 88 ++++++++++++------- .../aws-cloudwatch/test/dashboard.test.ts | 38 +++++++- 3 files changed, 95 insertions(+), 39 deletions(-) diff --git a/packages/aws-cdk-lib/aws-cloudwatch/lib/dashboard.ts b/packages/aws-cdk-lib/aws-cloudwatch/lib/dashboard.ts index 494a44d54de83..64dda1e7e5db3 100644 --- a/packages/aws-cdk-lib/aws-cloudwatch/lib/dashboard.ts +++ b/packages/aws-cdk-lib/aws-cloudwatch/lib/dashboard.ts @@ -1,7 +1,7 @@ import { Construct } from 'constructs'; import { CfnDashboard } from './cloudwatch.generated'; import { Column, Row } from './layout'; -import { IVariable } from './variable'; +import { Variable } from './variable'; import { IWidget } from './widget'; import { Lazy, Resource, Stack, Token, Annotations, Duration } from '../../core'; @@ -87,7 +87,7 @@ export interface DashboardProps { * * @default - No variables */ - readonly variables?: IVariable[]; + readonly variables?: Variable[]; } /** @@ -111,7 +111,7 @@ export class Dashboard extends Resource { private readonly rows: IWidget[] = []; - private readonly variables: IVariable[] = []; + private readonly variables: Variable[] = []; constructor(scope: Construct, id: string, props: DashboardProps = {}) { super(scope, id, { @@ -193,7 +193,7 @@ export class Dashboard extends Resource { * * @see https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/cloudwatch_dashboard_variables.html */ - public addVariable(variable: IVariable) { + public addVariable(variable: Variable) { this.variables.push(variable); } } diff --git a/packages/aws-cdk-lib/aws-cloudwatch/lib/variable.ts b/packages/aws-cdk-lib/aws-cloudwatch/lib/variable.ts index 8cf96f53fb12e..a59d49dc2fb65 100644 --- a/packages/aws-cdk-lib/aws-cloudwatch/lib/variable.ts +++ b/packages/aws-cdk-lib/aws-cloudwatch/lib/variable.ts @@ -30,14 +30,14 @@ export enum VariableType { /** * A single dashboard variable */ -export interface IVariable { +export interface Variable { /** * Return the variable JSON for use in the dashboard */ toJson(): any; } -export interface IDashboardVariable { +export interface DashboardVariableOptions { /** * Type of the variable */ @@ -60,24 +60,30 @@ export interface IDashboardVariable { /** * Optional label in the toolbar + * + * @default - the variable's value */ readonly label?: string; /** * Optional default value + * + * @default - no default value is set */ - readonly defaultValue: any; + readonly defaultValue?: any; /** * Whether the variable is visible + * + * @default - true */ readonly visible?: boolean; } -export abstract class DashboardVariable implements IVariable { - private readonly baseProps: IDashboardVariable; +export abstract class DashboardVariable implements Variable { + private readonly baseProps: DashboardVariableOptions; - protected constructor(props: IDashboardVariable) { + protected constructor(props: DashboardVariableOptions) { this.baseProps = props; } @@ -94,76 +100,96 @@ export abstract class DashboardVariable implements IVariable { } } -export interface IVariableValue { +export interface VariableValue { /** * Optional label for the selected item * - * @default - value + * @default - the variable's value */ readonly label?: string; + /** * Value of the selected item */ readonly value: string; } -export interface IValueDashboardVariable extends IDashboardVariable { +/** + * Options for {@link ValueDashboardVariable} + */ +export interface ValueDashboardVariableOptions extends DashboardVariableOptions { /** - * List of custom values for the variable + * List of custom values for the variable. + * It is required for variables of types {@link VariableInputType.RADIO} and {@link VariableInputType.SELECT} * - * @default - No values + * @default - no values */ - readonly values?: IVariableValue[]; + readonly values?: VariableValue[]; } /** - * The variable populated from the custom values + * A dashboard variable supporting all {@link VariableInputType}. */ export class ValueDashboardVariable extends DashboardVariable { - private readonly props: IValueDashboardVariable; - - constructor(props: IValueDashboardVariable) { - super(props); - this.props = props; + private readonly options: ValueDashboardVariableOptions; + + constructor(options: ValueDashboardVariableOptions) { + super(options); + if (options.inputType != VariableInputType.INPUT && (options.values || []).length == 0) { + throw new Error(`Variable with input type ${options.inputType} requires values to be provided.`); + } + this.options = options; } toJson(): any { const base = super.toJson(); return { ...base, - values: this.props.values ? this.props.values.map(value => ({ label: value.label, value: value.value })) : undefined, + values: this.options.values ? this.options.values.map(value => ({ label: value.label, value: value.value })) : undefined, }; } } /** - * The variable populated from the metric search + * Options for {@link SearchDashboardVariable} */ -export interface ISearchDashboardVariable extends IDashboardVariable { +export interface SearchDashboardVariableOptions extends DashboardVariableOptions { /** - * Search expression + * A search expression that specifies a namespace, dimension name and a metric name. + * + * For example `{AWS/EC2,InstanceId} MetricName=\"CPUUtilization\"` */ readonly searchExpression: string; + /** - * Optional dimension name from the search + * The dimension name, that the search expression retrieves, whose values will be used to populate the values to choose from. + * + * For example `InstanceId` */ - readonly populateFrom?: string; + readonly populateFrom: string; } +/** + * A dashboard variable with inputType {@link VariableInputType.SELECT} or {@link VariableInputType.RADIO} that populates + * the list of choices from a specific dimension in the given search expression + */ export class SearchDashboardVariable extends DashboardVariable { - private readonly props: ISearchDashboardVariable; - - constructor(props: ISearchDashboardVariable) { - super(props); - this.props = props; + private readonly options: SearchDashboardVariableOptions; + + constructor(options: SearchDashboardVariableOptions) { + super(options); + if (options.inputType === VariableInputType.INPUT) { + throw new Error('Unsupported inputType INPUT. Please choose either SELECT or RADIO'); + } + this.options = options; } toJson(): any { const base = super.toJson(); return { ...base, - search: this.props.searchExpression, - populateFrom: this.props.populateFrom, + search: this.options.searchExpression, + populateFrom: this.options.populateFrom, }; } } \ No newline at end of file diff --git a/packages/aws-cdk-lib/aws-cloudwatch/test/dashboard.test.ts b/packages/aws-cdk-lib/aws-cloudwatch/test/dashboard.test.ts index 59c28a21a5070..58188f6deefb5 100644 --- a/packages/aws-cdk-lib/aws-cloudwatch/test/dashboard.test.ts +++ b/packages/aws-cdk-lib/aws-cloudwatch/test/dashboard.test.ts @@ -1,13 +1,16 @@ -import { Template, Annotations, Match } from '../../assertions'; -import { App, Duration, Stack } from '../../core'; +import {Annotations, Match, Template} from '../../assertions'; +import {App, Duration, Stack} from '../../core'; import { Dashboard, GraphWidget, + MathExpression, PeriodOverride, + SearchDashboardVariable, TextWidget, - MathExpression, TextWidgetBackground, - SearchDashboardVariable, VariableInputType, VariableType, ValueDashboardVariable, + ValueDashboardVariable, + VariableInputType, + VariableType, } from '../lib'; describe('Dashboard', () => { @@ -381,7 +384,34 @@ describe('Dashboard', () => { hasVariables(resources[key].Properties, [ { defaultValue: 'us-east-1', id: 'region3', inputType: 'radio', label: 'RegionRadio', property: 'region', type: 'property', values: [{ label: 'IAD', value: 'us-east-1' }, { label: 'DUB', value: 'us-west-2' }], visible: true }, ]); + }); + test('search dashboard variable fails if unsupported input inputType', () => { + expect(() => new SearchDashboardVariable({ + defaultValue: '__FIRST', + id: 'InstanceId', + label: 'Instance', + inputType: VariableInputType.INPUT, + type: VariableType.PROPERTY, + value: 'InstanceId', + searchExpression: '{AWS/EC2,InstanceId} MetricName=\"CPUUtilization\"', + populateFrom: 'InstanceId', + visible: true, + })).toThrow(/Unsupported inputType INPUT. Please choose either SELECT or RADIO/); + }); + + test('value dashboard variable fails if no values provided for select or radio inputType', () => { + [VariableInputType.SELECT, VariableInputType.RADIO].forEach(inputType => { + expect(() => new ValueDashboardVariable({ + inputType, + type: VariableType.PATTERN, + value: 'us-east-1', + id: 'region3', + label: 'RegionPatternWithValues', + defaultValue: 'us-east-1', + visible: true, + })).toThrow(`Variable with input type ${inputType} requires values to be provided.`); + }); }); });