Skip to content

Commit 4fd0002

Browse files
authored
revert(core): add source tracing for L1 construct property mutations (#37415)
Reverts #37285 Closes #37407
1 parent a92105c commit 4fd0002

File tree

11 files changed

+34
-346
lines changed

11 files changed

+34
-346
lines changed

packages/aws-cdk-lib/aws-sns/test/sns.test.ts

Lines changed: 0 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,10 @@
1-
import * as fs from 'node:fs';
2-
import * as path from 'node:path';
31
import { Template } from '../../assertions';
4-
import { AssertionError } from '../../assertions/lib/private/error';
52
import * as notifications from '../../aws-codestarnotifications';
63
import * as iam from '../../aws-iam';
74
import { ServicePrincipal } from '../../aws-iam';
85
import * as kms from '../../aws-kms';
96
import { CfnKey } from '../../aws-kms';
107
import * as cdk from '../../core';
11-
import { Stage } from '../../core';
128
import * as sns from '../lib';
139
import { TopicGrants } from '../lib';
1410

@@ -1086,67 +1082,4 @@ describe('Topic', () => {
10861082
).toThrow('`fifoThroughputScope` can only be set for FIFO SNS topics.');
10871083
});
10881084
});
1089-
1090-
/*
1091-
This is a representative test suite for source tracing.
1092-
What we are asserting here about CfnTopic applies to all L1 constructs.
1093-
*/
1094-
describe('Source tracing', () => {
1095-
test('Metadata contains propertyAssignment and stack trace with CDK_DEBUG=1', () => {
1096-
try {
1097-
process.env.CDK_DEBUG = '1';
1098-
const stack = new cdk.Stack();
1099-
1100-
const topic = new sns.CfnTopic(stack, 'MyTopic', {
1101-
topicName: 'topicName',
1102-
});
1103-
1104-
topic.displayName = 'something';
1105-
const lineWherePropertyWasSet = getLineNumber() - 1; // the one before this one
1106-
1107-
const asm = synth(stack);
1108-
const metadata = JSON.parse(fs.readFileSync(path.join(asm.directory, 'Default.metadata.json'), 'utf8'));
1109-
const propertyAssignmentEntry = metadata['/Default/MyTopic'].find((e: any) => e.type === 'aws:cdk:propertyAssignment');
1110-
1111-
expect(propertyAssignmentEntry).toBeDefined();
1112-
expect(propertyAssignmentEntry.data.propertyName).toEqual('DisplayName');
1113-
expect(propertyAssignmentEntry.data.stackTrace.some(
1114-
(t: string) => t.includes(`${__filename}:${lineWherePropertyWasSet}`)),
1115-
).toBe(true);
1116-
} finally {
1117-
delete process.env.CDK_DEBUG;
1118-
}
1119-
});
1120-
1121-
test('Metadata does not contain propertyAssignment by default', () => {
1122-
const stack = new cdk.Stack();
1123-
1124-
const topic = new sns.CfnTopic(stack, 'MyTopic', {
1125-
topicName: 'topicName',
1126-
});
1127-
1128-
topic.displayName = 'something';
1129-
1130-
const asm = synth(stack);
1131-
const metadata = JSON.parse(fs.readFileSync(path.join(asm.directory, 'Default.metadata.json'), 'utf8'));
1132-
const propertyAssignmentEntry = metadata['/Default/MyTopic'].find((e: any) => e.type === 'aws:cdk:propertyAssignment');
1133-
1134-
expect(propertyAssignmentEntry).toBeUndefined();
1135-
});
1136-
});
11371085
});
1138-
1139-
function synth(stack: cdk.Stack) {
1140-
const stage = Stage.of(stack);
1141-
if (!Stage.isStage(stage)) {
1142-
throw new AssertionError('unexpected: all stacks must be part of a Stage or an App');
1143-
}
1144-
1145-
return stage.synth();
1146-
}
1147-
1148-
function getLineNumber(): number {
1149-
const err = new Error();
1150-
const line = err.stack?.split('\n')[2]?.match(/:(\d+):\d+\)?$/)?.[1];
1151-
return Number(line);
1152-
}

packages/aws-cdk-lib/core/lib/stack-trace.ts

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import type { Node } from 'constructs';
21
import { debugModeEnabled } from './debug';
32

43
/**
@@ -191,25 +190,3 @@ interface CallSite {
191190
fileName: string;
192191
sourceLocation: string;
193192
}
194-
195-
/**
196-
* Records a metadata entry on a construct node to trace a property assignment.
197-
*
198-
* When debug mode is enabled (via the `CDK_DEBUG` environment variable),
199-
* this attaches `aws:cdk:propertyAssignment` metadata to the given node,
200-
* including a stack trace pointing back to the caller. This is useful for
201-
* diagnosing where a particular property value was set during synthesis.
202-
*
203-
* This is a no-op when debug mode is not enabled.
204-
*
205-
* @param node the construct node to attach the metadata to.
206-
* @param propertyName the name of the property being assigned.
207-
*/
208-
export function traceProperty(node: Node, propertyName: string) {
209-
if (debugModeEnabled()) {
210-
node.addMetadata('aws:cdk:propertyAssignment', {
211-
propertyName,
212-
stackTrace: captureStackTrace(traceProperty),
213-
});
214-
}
215-
}

tools/@aws-cdk/spec2cdk/lib/cdk/cdk.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ export class CdkCore extends ExternalModule {
3636
public readonly unionMapper = makeCallableExpr(this, 'unionMapper');
3737
public readonly requireProperty = makeCallableExpr(this, 'requireProperty');
3838
public readonly isResolvableObject = makeCallableExpr(this, 'isResolvableObject');
39-
public readonly traceProperty = makeCallableExpr(this, 'traceProperty');
4039
public readonly mapArrayInPlace = makeCallableExpr(this, 'mapArrayInPlace');
4140

4241
public readonly ValidationResult = $T(Type.fromName(this, 'ValidationResult'));

tools/@aws-cdk/spec2cdk/lib/cdk/resource-class.ts

Lines changed: 3 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -217,31 +217,7 @@ export class ResourceClass extends ClassType implements Referenceable {
217217
}
218218

219219
for (const prop of this.decider.classProperties) {
220-
const spec = prop.propertySpec;
221-
if (spec.immutable) {
222-
this.addProperty(spec);
223-
} else {
224-
// For mutable properties, generate getter and setter
225-
const backingFieldName = `_${spec.name}`;
226-
this.addProperty({
227-
name: backingFieldName,
228-
type: spec.type,
229-
optional: spec.optional,
230-
visibility: MemberVisibility.Private,
231-
docs: spec.docs,
232-
});
233-
this.addProperty({
234-
name: spec.name,
235-
type: spec.type,
236-
optional: spec.optional,
237-
docs: spec.docs,
238-
getterBody: Block.with(stmt.ret($this[backingFieldName])),
239-
setterBody: (value: Expression) => Block.with(
240-
CDK_CORE.traceProperty($this.node, expr.lit(prop.cfnName)),
241-
stmt.assign($this[backingFieldName], value),
242-
),
243-
});
244-
}
220+
this.addProperty(prop.propertySpec);
245221
}
246222

247223
// Copy properties onto class and props type
@@ -769,8 +745,8 @@ export class ResourceClass extends ClassType implements Referenceable {
769745

770746
init.addBody(
771747
// Props
772-
...this.decider.classProperties.map(({ propertySpec: { name, immutable }, initializer }) =>
773-
stmt.assign($this[immutable ? name : `_${name}`], initializer(props)),
748+
...this.decider.classProperties.map(({ propertySpec: { name }, initializer }) =>
749+
stmt.assign($this[name], initializer(props)),
774750
),
775751
);
776752

tools/@aws-cdk/spec2cdk/lib/cdk/resource-decider.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,6 @@ export class ResourceDecider {
104104
immutable: false,
105105
docs: this.defaultClassPropDocs(cfnName, prop),
106106
},
107-
cfnName,
108107
initializer: resolverResult.resolver,
109108
cfnValueToRender: { [resolverResult.name]: $this[resolverResult.name] },
110109
});
@@ -167,7 +166,6 @@ export class ResourceDecider {
167166
summary: 'Tag Manager which manages the tags for this resource',
168167
},
169168
},
170-
cfnName,
171169
initializer: (props: Expression) =>
172170
new CDK_CORE.TagManager(
173171
this.tagManagerVariant(variant),
@@ -186,7 +184,6 @@ export class ResourceDecider {
186184
optional: true, // Tags are never required
187185
docs: this.defaultClassPropDocs(cfnName, prop),
188186
},
189-
cfnName,
190187
initializer: (props: Expression) => $E(props)[originalName],
191188
cfnValueToRender: {}, // Gets rendered as part of the TagManager above
192189
},
@@ -224,7 +221,6 @@ export class ResourceDecider {
224221
summary: 'Tag Manager which manages the tags for this resource',
225222
},
226223
},
227-
cfnName,
228224
initializer: (_: Expression) =>
229225
new CDK_CORE.TagManager(
230226
this.tagManagerVariant(variant),
@@ -243,7 +239,6 @@ export class ResourceDecider {
243239
optional: true, // Tags are never required
244240
docs: this.defaultClassPropDocs(cfnName, prop),
245241
},
246-
cfnName,
247242
initializer: (props: Expression) => $E(props)[originalName],
248243
cfnValueToRender: {}, // Gets rendered as part of the TagManager above
249244
},
@@ -369,9 +364,6 @@ export interface PropsProperty {
369364
export interface ClassProperty {
370365
readonly propertySpec: PropertySpec;
371366

372-
/** The original CloudFormation property name */
373-
readonly cfnName: string;
374-
375367
/** Given the name of the props value, produce the member value */
376368
readonly initializer: (props: Expression) => Expression;
377369

tools/@aws-cdk/spec2cdk/test/__snapshots__/cfn-prop-mixins.test.ts.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

33
exports[`L1 property mixin for a standard-issue resource 1`] = `
44
"/* eslint-disable prettier/prettier, @stylistic/max-len */

tools/@aws-cdk/spec2cdk/test/__snapshots__/fake-services.test.ts.snap

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

33
exports[`can codegen deprecated service 1`] = `
44
"/* eslint-disable prettier/prettier, @stylistic/max-len */
@@ -54,7 +54,7 @@ export class CfnResource extends cdk.CfnResource implements cdk.IInspectable, IR
5454
/**
5555
* The identifier of the resource.
5656
*/
57-
private _id?: string;
57+
public id?: string;
5858
5959
/**
6060
* Create a new \`AWS::Some::Resource\`.
@@ -69,7 +69,7 @@ export class CfnResource extends cdk.CfnResource implements cdk.IInspectable, IR
6969
properties: props
7070
});
7171
72-
this._id = props.id;
72+
this.id = props.id;
7373
}
7474
7575
public get resourceRef(): ResourceReference {
@@ -78,20 +78,6 @@ export class CfnResource extends cdk.CfnResource implements cdk.IInspectable, IR
7878
};
7979
}
8080
81-
/**
82-
* The identifier of the resource.
83-
*/
84-
public get id(): string | undefined {
85-
return this._id;
86-
}
87-
/**
88-
* The identifier of the resource.
89-
*/
90-
public set id(value: string | undefined) {
91-
cdk.traceProperty(this.node, "Id");
92-
this._id = value;
93-
}
94-
9581
protected get cfnProperties(): Record<string, any> {
9682
return {
9783
id: this.id

0 commit comments

Comments
 (0)