Skip to content

Commit

Permalink
fix(core): toJsonString() does not deal correctly with list tokens
Browse files Browse the repository at this point in the history
For reasons explained in the previous PR, list tokens need to be run
through a custom resource. This custom resource gets added during token
resolution, but because that is run multiple times the side effect
of adding the custom resource gets executed multiple times.

Add a cache to make sure the second execution returns the first answer
as the first one.

Not a fantastic solution, but given the constraints of Lazies, I'm not
sure we can do any better.

Fixes #14088.
  • Loading branch information
rix0rrr committed Apr 13, 2021
1 parent 1a30272 commit 263ce31
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 5 deletions.
36 changes: 34 additions & 2 deletions packages/@aws-cdk/core/lib/private/cloudformation-lang.ts
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,15 @@ function tokenAwareStringify(root: any, space: number, ctx: IResolveContext) {
//
// Therefore, if we encounter lists we need to defer to a custom resource to handle
// them properly at deploy time.
pushIntrinsic(CfnUtils.stringify(Stack.of(ctx.scope), `CdkJsonStringify${stringifyCounter++}`, intrinsic));
const stack = Stack.of(ctx.scope);

// Because this will be called twice (once during `prepare`, once during `resolve`),
// we need to make sure to be idempotent, so use a cache.
const stringifyResponse = stringifyCache.obtain(stack, JSON.stringify(intrinsic), () =>
CfnUtils.stringify(stack, `CdkJsonStringify${stringifyCounter++}`, intrinsic),
);

pushIntrinsic(stringifyResponse);
return;

case ResolutionTypeHint.NUMBER:
Expand Down Expand Up @@ -418,4 +426,28 @@ function quoteString(s: string) {
return s.substring(1, s.length - 1);
}

let stringifyCounter = 1;
let stringifyCounter = 1;

/**
* A cache scoped to object instances, that's maintained externally to the object instances
*/
class ScopedCache<O extends object, K, V> {
private cache = new WeakMap<O, Map<K, V>>();

public obtain(object: O, key: K, init: () => V): V {
let kvMap = this.cache.get(object);
if (!kvMap) {
kvMap = new Map();
this.cache.set(object, kvMap);
}

let ret = kvMap.get(key);
if (ret === undefined) {
ret = init();
kvMap.set(key, ret);
}
return ret;
}
}

const stringifyCache = new ScopedCache<Stack, string, string>();
18 changes: 15 additions & 3 deletions packages/@aws-cdk/core/test/cloudformation-json.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { App, Aws, CfnOutput, Fn, IPostProcessor, IResolvable, IResolveContext, Lazy, Stack, Token } from '../lib';
import { App, Aws, CfnOutput, CfnResource, Fn, IPostProcessor, IResolvable, IResolveContext, Lazy, Stack, Token } from '../lib';
import { Intrinsic } from '../lib/private/intrinsic';
import { evaluateCFN } from './evaluate-cfn';

Expand Down Expand Up @@ -102,10 +102,22 @@ describe('tokens that return literals', () => {
const someList = Token.asList(new Intrinsic({ Ref: 'Thing' }));

// WHEN
expect(stack.resolve(stack.toJsonString({ someList }))).toEqual({
new CfnResource(stack, 'Resource', {
type: 'AWS::Banana',
properties: {
someJson: stack.toJsonString({ someList }),
},
});

const asm = app.synth();
const template = asm.getStackByName(stack.stackName).template;
const stringifyLogicalId = Object.keys(template.Resources).filter(id => id.startsWith('CdkJsonStringify'))[0];
expect(stringifyLogicalId).toBeDefined();

expect(template.Resources.Resource.Properties.someJson).toEqual({
'Fn::Join': ['', [
'{"someList":',
{ 'Fn::GetAtt': [expect.stringContaining('CdkJsonStringify'), 'Value'] },
{ 'Fn::GetAtt': [stringifyLogicalId, 'Value'] },
'}',
]],
});
Expand Down

0 comments on commit 263ce31

Please sign in to comment.