Skip to content

Commit 201b8e3

Browse files
kaizenccTikiTDO
authored andcommitted
fix(assertions): hasResourceProperties is incompatible with Match.not and Match.absent (aws#16678)
Fixes aws#16626. This PR modifies `hasResourceProperties()` so that it accounts for the special case where `Properties` does not exist on the template. The following assertions previously behaved incorrectly when `Properties` were not in the template and are now fixed: ```ts template.fromStack(stack); // some template with no `Properties`. // assert that `Properties` does not exist in the template. Returns true. template.hasResourceProperties('Foo::Bar', Match.absent()); // assert that `baz` is not a `Property` of 'Foo::Bar'. Returns true. template.hasResourceProperties('Foo::Bar', { baz: Match.absent(), }; // assert that `baz` does not have a value of `qux` in the `Properties` object. Returns true. template.hasResourceProperties('Foo::Bar', Match.not({baz: 'qux'}); ``` It also moves `AbsentMatch` into the `private` folder so that it can be exposed internally as a special case for `hasResourceProperties()`. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 8d337b1 commit 201b8e3

File tree

5 files changed

+85
-22
lines changed

5 files changed

+85
-22
lines changed

Diff for: packages/@aws-cdk/assertions/lib/match.ts

+1-14
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { Matcher, MatchResult } from './matcher';
2+
import { AbsentMatch } from './private/matchers/absent';
23
import { getType } from './private/type';
34

45
/**
@@ -329,17 +330,3 @@ class AnyMatch extends Matcher {
329330
return result;
330331
}
331332
}
332-
333-
class AbsentMatch extends Matcher {
334-
constructor(public readonly name: string) {
335-
super();
336-
}
337-
338-
public test(actual: any): MatchResult {
339-
const result = new MatchResult(actual);
340-
if (actual !== undefined) {
341-
result.push(this, [], `Received ${actual}, but key should be absent`);
342-
}
343-
return result;
344-
}
345-
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import { Matcher, MatchResult } from '../../matcher';
2+
3+
export class AbsentMatch extends Matcher {
4+
constructor(public readonly name: string) {
5+
super();
6+
}
7+
8+
public test(actual: any): MatchResult {
9+
const result = new MatchResult(actual);
10+
if (actual !== undefined) {
11+
result.push(this, [], `Received ${actual}, but key should be absent`);
12+
}
13+
return result;
14+
}
15+
}

Diff for: packages/@aws-cdk/assertions/lib/private/resources.ts

+28-1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { Match, Matcher } from '..';
2+
import { AbsentMatch } from './matchers/absent';
13
import { formatFailure, matchSection } from './section';
24
import { Resource, Template } from './template';
35

@@ -15,7 +17,6 @@ export function findResources(template: Template, type: string, props: any = {})
1517
export function hasResource(template: Template, type: string, props: any): string | void {
1618
const section = template.Resources;
1719
const result = matchSection(filterType(section, type), props);
18-
1920
if (result.match) {
2021
return;
2122
}
@@ -30,13 +31,39 @@ export function hasResource(template: Template, type: string, props: any): strin
3031
].join('\n');
3132
}
3233

34+
export function hasResourceProperties(template: Template, type: string, props: any): string | void {
35+
// amended needs to be a deep copy to avoid modifying the template.
36+
let amended = JSON.parse(JSON.stringify(template));
37+
38+
// special case to exclude AbsentMatch because adding an empty Properties object will affect its evaluation.
39+
if (!Matcher.isMatcher(props) || !(props instanceof AbsentMatch)) {
40+
amended = addEmptyProperties(amended);
41+
}
42+
43+
return hasResource(amended, type, Match.objectLike({
44+
Properties: props,
45+
}));
46+
}
47+
3348
export function countResources(template: Template, type: string): number {
3449
const section = template.Resources;
3550
const types = filterType(section, type);
3651

3752
return Object.entries(types).length;
3853
}
3954

55+
function addEmptyProperties(template: Template): Template {
56+
let section = template.Resources;
57+
58+
Object.keys(section).map((key) => {
59+
if (!section[key].hasOwnProperty('Properties')) {
60+
section[key].Properties = {};
61+
}
62+
});
63+
64+
return template;
65+
}
66+
4067
function filterType(section: { [key: string]: Resource }, type: string): { [key: string]: Resource } {
4168
return Object.entries(section ?? {})
4269
.filter(([_, v]) => v.Type === type)

Diff for: packages/@aws-cdk/assertions/lib/template.ts

+5-4
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { Match } from './match';
33
import { Matcher } from './matcher';
44
import { findMappings, hasMapping } from './private/mappings';
55
import { findOutputs, hasOutput } from './private/outputs';
6-
import { countResources, findResources, hasResource } from './private/resources';
6+
import { countResources, findResources, hasResource, hasResourceProperties } from './private/resources';
77
import { Template as TemplateType } from './private/template';
88

99
/**
@@ -74,9 +74,10 @@ export class Template {
7474
* @param props the 'Properties' section of the resource as should be expected in the template.
7575
*/
7676
public hasResourceProperties(type: string, props: any): void {
77-
this.hasResource(type, Match.objectLike({
78-
Properties: Matcher.isMatcher(props) ? props : Match.objectLike(props),
79-
}));
77+
const matchError = hasResourceProperties(this.template, type, props);
78+
if (matchError) {
79+
throw new Error(matchError);
80+
}
8081
}
8182

8283
/**

Diff for: packages/@aws-cdk/assertions/test/template.test.ts

+36-3
Original file line numberDiff line numberDiff line change
@@ -270,33 +270,56 @@ describe('Template', () => {
270270
});
271271

272272
describe('hasResourceProperties', () => {
273-
test('absent', () => {
273+
test('exact match', () => {
274+
const stack = new Stack();
275+
new CfnResource(stack, 'Foo', {
276+
type: 'Foo::Bar',
277+
properties: { baz: 'qux' },
278+
});
279+
280+
const inspect = Template.fromStack(stack);
281+
inspect.hasResourceProperties('Foo::Bar', { baz: 'qux' });
282+
283+
expect(() => inspect.hasResourceProperties('Foo::Bar', { baz: 'waldo' }))
284+
.toThrow(/Expected waldo but received qux at \/Properties\/baz/);
285+
286+
expect(() => inspect.hasResourceProperties('Foo::Bar', { baz: 'qux', fred: 'waldo' }))
287+
.toThrow(/Missing key at \/Properties\/fred/);
288+
});
289+
290+
test('absent - with properties', () => {
274291
const stack = new Stack();
275292
new CfnResource(stack, 'Foo', {
276293
type: 'Foo::Bar',
277294
properties: { baz: 'qux' },
278295
});
279296

280297
const inspect = Template.fromStack(stack);
298+
281299
inspect.hasResourceProperties('Foo::Bar', {
282300
bar: Match.absent(),
283301
});
302+
284303
expect(() => inspect.hasResourceProperties('Foo::Bar', {
285304
baz: Match.absent(),
286305
})).toThrow(/key should be absent at \/Properties\/baz/);
287306
});
288307

289-
test('absent - no properties on template', () => {
308+
test('absent - no properties', () => {
290309
const stack = new Stack();
291310
new CfnResource(stack, 'Foo', {
292311
type: 'Foo::Bar',
293312
});
294313

295314
const inspect = Template.fromStack(stack);
315+
316+
expect(() => inspect.hasResourceProperties('Foo::Bar', { bar: Match.absent(), baz: 'qux' }))
317+
.toThrow(/Missing key at \/Properties\/baz/);
318+
296319
inspect.hasResourceProperties('Foo::Bar', Match.absent());
297320
});
298321

299-
test('not', () => {
322+
test('not - with properties', () => {
300323
const stack = new Stack();
301324
new CfnResource(stack, 'Foo', {
302325
type: 'Foo::Bar',
@@ -308,6 +331,16 @@ describe('Template', () => {
308331
baz: 'boo',
309332
}));
310333
});
334+
335+
test('not - no properties', () => {
336+
const stack = new Stack();
337+
new CfnResource(stack, 'Foo', {
338+
type: 'Foo::Bar',
339+
});
340+
341+
const inspect = Template.fromStack(stack);
342+
inspect.hasResourceProperties('Foo::Bar', Match.not({ baz: 'qux' }));
343+
});
311344
});
312345

313346
describe('getResources', () => {

0 commit comments

Comments
 (0)