Skip to content

Commit

Permalink
fix(sfn): stop replacing JsonPath.DISCARD with null (#24717)
Browse files Browse the repository at this point in the history
Follow-up to #24593. The `renderJsonPath` function is subsituting a literal `null` for `JsonPath.DISCARD`, which results in the key being dropped if the value is sent across a language boundary, which effectively changes semantics.

The `JsonPath.DISCARD` value is a `Token` that ultimately resolves to `null`, and it must be preserved as such so that it is safe to exchange across languages.

Thanks to @beck3905 for reporting & diagnosing this.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
RomainMuller authored Mar 21, 2023
1 parent f511000 commit 413b643
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 16 deletions.
3 changes: 1 addition & 2 deletions packages/@aws-cdk/aws-stepfunctions/lib/states/state.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Token } from '@aws-cdk/core';
import { IConstruct, Construct, Node } from 'constructs';
import { Condition } from '../condition';
import { FieldUtils, JsonPath } from '../fields';
import { FieldUtils } from '../fields';
import { StateGraph } from '../state-graph';
import { CatchProps, Errors, IChainable, INextable, RetryProps } from '../types';

Expand Down Expand Up @@ -578,7 +578,6 @@ export function renderList<T>(xs: T[], mapFn: (x: T) => any, sortFn?: (a: T, b:
*/
export function renderJsonPath(jsonPath?: string): undefined | null | string {
if (jsonPath === undefined) { return undefined; }
if (jsonPath === JsonPath.DISCARD) { return null; }

if (!Token.isUnresolved(jsonPath) && !jsonPath.startsWith('$')) {
throw new Error(`Expected JSON path to start with '$', got: ${jsonPath}`);
Expand Down
29 changes: 18 additions & 11 deletions packages/@aws-cdk/aws-stepfunctions/test/state.test.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,33 @@
import * as assert from '@aws-cdk/assertions';
import * as cdk from '@aws-cdk/core';
import { FakeTask } from './fake-task';
import { renderGraph } from './private/render-util';
import { JsonPath } from '../lib';
import { JsonPath, StateMachine } from '../lib';

test('JsonPath.DISCARD can be used to discard a state\'s output', () => {
const stack = new cdk.Stack();

// GIVEN
const app = new cdk.App();
const stack = new cdk.Stack(app, 'TestStack');
const task = new FakeTask(stack, 'my-state', {
inputPath: JsonPath.DISCARD,
outputPath: JsonPath.DISCARD,
resultPath: JsonPath.DISCARD,
});
new StateMachine(stack, 'state-machine', {
definition: task,
});

// WHEN
const definitionString = new assert.Capture();
assert.Template.fromStack(stack).hasResourceProperties('AWS::StepFunctions::StateMachine', {
DefinitionString: definitionString,
});

// THEN
const definition = JSON.parse(definitionString.asString());

expect(renderGraph(task)).toEqual({
StartAt: 'my-state',
expect(definition).toMatchObject({
States: {
'my-state': {
End: true,
Type: 'Task',
Resource: expect.any(String),
Parameters: expect.any(Object),
// The important bits:
InputPath: null,
OutputPath: null,
ResultPath: null,
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/core/lib/private/cloudformation-lang.ts
Original file line number Diff line number Diff line change
Expand Up @@ -452,4 +452,4 @@ class ScopedCache<O extends object, K, V> {
}
}

const stringifyCache = new ScopedCache<Stack, string, string>();
const stringifyCache = new ScopedCache<Stack, string, string>();
8 changes: 8 additions & 0 deletions packages/@aws-cdk/core/lib/private/resolve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,14 @@ export function resolve(obj: any, options: IResolveOptions): any {
return arr;
}

//
// literal null -- from JsonNull resolution, preserved as-is (semantically meaningful)
//

if (obj === null) {
return obj;
}

//
// tokens - invoke 'resolve' and continue to resolve recursively
//
Expand Down
10 changes: 8 additions & 2 deletions packages/@aws-cdk/core/lib/token.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { unresolved } from './private/encoding';
import { Intrinsic } from './private/intrinsic';
import { resolve } from './private/resolve';
import { TokenMap } from './private/token-map';
import { IResolvable, ITokenResolver } from './resolvable';
import { IResolvable, ITokenResolver, IResolveContext } from './resolvable';
import { TokenizedStringFragments } from './string-fragments';

/**
Expand Down Expand Up @@ -236,12 +236,18 @@ export class Tokenization {
* An object which serializes to the JSON `null` literal, and which can safely
* be passed across languages where `undefined` and `null` are not different.
*/
export class JsonNull {
export class JsonNull implements IResolvable {
/** The canonical instance of `JsonNull`. */
public static readonly INSTANCE = new JsonNull();

public readonly creationStack: string[] = [];

private constructor() { }

public resolve(_ctx: IResolveContext): any {
return null;
}

/** Obtains the JSON representation of this object (`null`) */
public toJSON(): any {
return null;
Expand Down

0 comments on commit 413b643

Please sign in to comment.