(assertions): Capture captures value from last resource with matching type, not matched props

[poWer4aiX]

What is the problem?

I used Template.findResources() to filter for one resource in a stack, by type and value of the Name tag. For one property I used a Capture(). My expectation was that the captured value will be the one of the resource found, which was not the case. Instead, it was the value from the resource in the stack matching just the given type.

Reproduction Steps

test('capture', () => {
	const capture = new Capture();
	const resources = {
		Resources: {
			res1: { Type: 'res', Name: 'a', Prop: 1 },
			res2: { Type: 'res', Name: 'b', Prop: 2 }
		}
	}
	const template = Template.fromJSON(resources)
	const res = template.findResources('res', {
		Name: 'a', Prop: capture
	})
	// filtered resource is the correct one.
	console.log(JSON.stringify(res, null, 4))
	// fails, as the value (2) of the last 'Prop' from all resources is
	// captured, not the one of the last matching resource (1).
	expect(capture.asNumber()).toEqual(1);
})

What did you expect to happen?

The captured value should be the value from the last resource being filtered (i.e. by type and optional attributes). In the example code given above this should be 1 instead of 2.

What actually happened?

The captured value is the one of the attribute from the last resource of the resource type, independent if this one matched or not.

CDK CLI Version

1.128.0

Framework Version

1.128.0

Node.js Version

14.17.4

OS

Ubuntu 20.04, running in an WSL on Windows 10

Language

Typescript

Language Version

3.9.7

Other information

No response

[nija-at]

@poWer4aiX -

This is interesting. The Capture class was never intended to be used as part of findXxx() APIs, only with hasXxx() APIs.
But at the same time, it was never explicitly forbidden.

Can you switch to using the hasXxx() APIs and see if that fixes your problem?

[poWer4aiX]

@nija-at As the way how the template is being processed does not differ between the findXxx() and hasXxx() APIs I used the Capture...

The same issue exists as well when using hasXxx() instead of findXxx() (same processing). I have already extended the tests in tests/template.test.ts not only for findXxx(), but also for hasXxx() (around line 271):

    test('captures values from matched resources', () => {
      const stack = new Stack();
      new CfnResource(stack, 'res1', { type: 'res', properties: { Name: 'foo', fred: 'waldo' } });
      new CfnResource(stack, 'res2', { type: 'res', properties: { Name: 'bar', fred: 'fred' } });

      const inspect = Template.fromStack(stack);
      const capture = new Capture();
      inspect.hasResource('res', { Properties: { Name: 'foo', fred: capture } });
      expect(capture.asString()).toEqual('waldo');
    });

You can simply disable my fix in lib/private/section.ts (around line 29), by adding a false &&:

if (false && Object.keys(section).length > Object.keys(matching).length) {

Running a test with yarn test will fail now, as the capture is being applied against the non matching second resource as well, resulting in capturing the value from the non matching resource:

$ yarn test
yarn run v1.22.15
$ cdk-test
PASS test/capture.test.js (5.494 s)
PASS test/match.test.js (5.728 s)
PASS test/private/section.test.js
FAIL test/template.test.js (6.958 s)
  ● Template › hasResource › captures values from matched resources

    expect(received).toEqual(expected) // deep equality

    Expected: "waldo"
    Received: "fred"

      277 |       const capture = new Capture();
      278 |       inspect.hasResource('res', { Properties: { Name: 'foo', fred: capture } });
    > 279 |       expect(capture.asString()).toEqual('waldo');
          |                                  ^
      280 |     });
      281 |   });
      282 |

      at Object.<anonymous> (test/template.test.ts:279:34)

Well my fix is quiet simple as it tests the matcher again, but on the reduced / filtered / matched subset of resources.

[nija-at]

Got it. Thanks for the update @poWer4aiX

[nija-at]

Originally posted by @nija-at in #17012 (review)

First, we need to decide what is the correct behaviour if Capture is used when there are multiple matching entries. There are two options -

(1) return all of them in a series such as

captured.asString(); // "Foo"
captured.next().asString(); // "Bar"
...

or
(2) throw an error in such a case.

In either case, the solution should be in the Capture class to hold state of all values that has flown through it, and finally decide how to handle them.

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.