refactor(toolkit-lib): compute each resource's property hash once per refactor

packages/@aws-cdk/toolkit-lib/lib/api/refactoring/context.ts

@@ -4,7 +4,7 @@ import { EnvironmentPlaceholders } from '@aws-cdk/cloud-assembly-api';
 import type { StackDefinition } from '@aws-sdk/client-cloudformation';
 import type { CloudFormationStack } from './cloudformation';
 import { ResourceLocation, ResourceMapping } from './cloudformation';
-import type { GraphDirection } from './digest';
+import type { GraphDirection, PropertyHashCache } from './digest';
 import { computeResourceDigests } from './digest';
 import { ToolkitError } from '../../toolkit/toolkit-error';
 import { equalSets, setDiff } from '../../util/sets';
@@ -44,8 +44,10 @@ export class RefactoringContext {
 
   constructor(props: RefactoringContextOptions) {
     this.environment = props.environment;
-    const moves = resourceMoves(props.deployedStacks, props.localStacks, 'direct', props.ignoreModifications);
-    const additionalOverrides = structuralOverrides(props.deployedStacks, props.localStacks);
+    // Both passes below hash the same resources; share the property hashes.
+    const propertyHashes: PropertyHashCache = new Map();
+    const moves = resourceMoves(props.deployedStacks, props.localStacks, 'direct', props.ignoreModifications, propertyHashes);
+    const additionalOverrides = structuralOverrides(props.deployedStacks, props.localStacks, propertyHashes);
     const overrides = (props.overrides ?? []).concat(additionalOverrides);
     const [nonAmbiguousMoves, ambiguousMoves] = partitionByAmbiguity(overrides, moves);
     this.ambiguousMoves = ambiguousMoves;
@@ -186,8 +188,11 @@ export class RefactoringContext {
  * opposite graph, we can use them as a set of overrides to disambiguate the original moves.
  *
  */
-function structuralOverrides(deployedStacks: CloudFormationStack[], localStacks: CloudFormationStack[]): ResourceMapping[] {
-  const moves = resourceMoves(deployedStacks, localStacks, 'opposite', true);
+function structuralOverrides(
+  deployedStacks: CloudFormationStack[],
+  localStacks: CloudFormationStack[],
+  propertyHashes?: PropertyHashCache): ResourceMapping[] {
+  const moves = resourceMoves(deployedStacks, localStacks, 'opposite', true, propertyHashes);
   const [nonAmbiguousMoves] = partitionByAmbiguity([], moves);
   return resourceMappings(nonAmbiguousMoves);
 }
@@ -196,9 +201,10 @@ function resourceMoves(
   before: CloudFormationStack[],
   after: CloudFormationStack[],
   direction: GraphDirection = 'direct',
-  ignoreModifications: boolean = false): ResourceMove[] {
-  const digestsBefore = resourceDigests(before, direction);
-  const digestsAfter = resourceDigests(after, direction);
+  ignoreModifications: boolean = false,
+  propertyHashes?: PropertyHashCache): ResourceMove[] {
+  const digestsBefore = resourceDigests(before, direction, propertyHashes);
+  const digestsAfter = resourceDigests(after, direction, propertyHashes);
 
   if (!(ignoreModifications || isomorphic(digestsBefore, digestsAfter))) {
     const message = ['A refactor operation cannot add, remove or update resources. Only resource moves and renames are allowed.'];
@@ -307,14 +313,17 @@ function zip(
 /**
  * Computes a list of pairs [digest, location] for each resource in the stack.
  */
-function resourceDigests(stacks: CloudFormationStack[], direction: GraphDirection): Record<string, ResourceLocation[]> {
+function resourceDigests(
+  stacks: CloudFormationStack[],
+  direction: GraphDirection,
+  propertyHashes?: PropertyHashCache): Record<string, ResourceLocation[]> {
   // index stacks by name
   const stacksByName = new Map<string, CloudFormationStack>();
   for (const stack of stacks) {
     stacksByName.set(stack.stackName, stack);
   }
 
-  const digests = computeResourceDigests(stacks, direction);
+  const digests = computeResourceDigests(stacks, direction, propertyHashes);
 
   return groupByKey(
     Object.entries(digests).map(([loc, digest]) => {

packages/@aws-cdk/toolkit-lib/lib/api/refactoring/digest.ts

@@ -23,7 +23,23 @@ export type GraphDirection =
  * CloudFormation template form a directed acyclic graph, this function is
  * well-defined.
  */
-export function computeResourceDigests(stacks: CloudFormationStack[], direction: GraphDirection = 'direct'): Record<string, string> {
+/**
+ * Caches the part of a resource's digest that does not depend on the direction
+ * of the resource graph: the hash of its own (reference-stripped) properties.
+ *
+ * A single refactor operation computes digests four times — for the deployed
+ * and the local stacks, in both graph directions — and that property hash is
+ * the same in all of them. Pass the same cache to each call to compute it once
+ * per resource. Keys are the resource objects themselves, so a cache must not
+ * outlive the templates it was built from.
+ */
+export type PropertyHashCache = Map<CloudFormationResource, string>;
+
+export function computeResourceDigests(
+  stacks: CloudFormationStack[],
+  direction: GraphDirection = 'direct',
+  propertyHashes: PropertyHashCache = new Map(),
+): Record<string, string> {
   const exports: { [p: string]: { stackName: string; value: any } } = Object.fromEntries(
     stacks.flatMap((s) =>
       Object.values(s.template.Outputs ?? {})
@@ -47,19 +63,24 @@ export function computeResourceDigests(stacks: CloudFormationStack[], direction:
     ? ResourceGraph.fromStacks(stacks)
     : ResourceGraph.fromStacks(stacks).opposite();
 
-  return computeDigestsInTopologicalOrder(graph, resources, exports);
+  return computeDigestsInTopologicalOrder(graph, resources, exports, propertyHashes);
 }
 
 function computeDigestsInTopologicalOrder(
   graph: ResourceGraph,
   resources: Record<string, CloudFormationResource>,
-  exports: Record<string, { stackName: string; value: any }>): Record<string, string> {
+  exports: Record<string, { stackName: string; value: any }>,
+  propertyHashes: PropertyHashCache): Record<string, string> {
   const nodes = graph.sortedNodes.filter(n => resources[n] != null);
   const result: Record<string, string> = {};
   for (const id of nodes) {
     const resource = resources[id];
     const depDigests = Array.from(graph.outNeighbors(id)).map((d) => result[d]);
-    const propertiesHash = hashObject(stripReferences(stripConstructPath(resource), exports));
+    let propertiesHash = propertyHashes.get(resource);
+    if (propertiesHash == null) {
+      propertiesHash = hashObject(stripReferences(stripConstructPath(resource), exports));
+      propertyHashes.set(resource, propertiesHash);
+    }
     const toHash = resource.Type + propertiesHash + depDigests.join('');
     result[id] = crypto.createHash('sha256').update(toHash).digest('hex');
   }

packages/@aws-cdk/toolkit-lib/test/api/refactoring/refactoring.test.ts

@@ -392,6 +392,35 @@ describe(computeResourceDigests, () => {
     expect(result['Stack1.Q1']).toBe(result['Stack1.Q2']);
   });
 
+  test('a shared property hash cache does not change the digests it produces', () => {
+    const template = {
+      Resources: {
+        Bucket: {
+          Type: 'AWS::S3::Bucket',
+          Properties: { Prop: 'my-bucket' },
+          Metadata: { 'aws:cdk:path': 'Stack/Bucket/Resource' },
+        },
+        Topic: {
+          Type: 'AWS::SNS::Topic',
+          DependsOn: 'Bucket',
+          Properties: { DisplayName: 'my-topic', Sub: { Ref: 'Bucket' } },
+        },
+      },
+    };
+    const stacks = makeStacks([template]);
+
+    for (const direction of ['direct', 'opposite'] as const) {
+      const uncached = computeResourceDigests(stacks, direction);
+
+      // The same cache is reused across both directions, as RefactoringContext does
+      const cache = new Map();
+      computeResourceDigests(stacks, direction === 'direct' ? 'opposite' : 'direct', cache);
+      const cached = computeResourceDigests(stacks, direction, cache);
+
+      expect(cached).toEqual(uncached);
+    }
+  });
+
   test('different physical IDs lead to different digests', () => {
     mockLoadResourceModel.mockReturnValue({
       primaryIdentifier: ['FooName'],