Skip to content

Commit

Permalink
[v2int/3.4] Fix a bug where child summarizer node's paths are incorre…
Browse files Browse the repository at this point in the history
…ctly set to parent's paths (#15433)

Porting #15222 to `release/v2int/3.4` branch.

When creating pending summaries for child summarizer node, use the
child's latest summary's path.

Co-authored-by: Navin Agarwal <[email protected]>
  • Loading branch information
markfields and agarwal-navin authored May 3, 2023
1 parent 9af3c07 commit 55d95bf
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class SummaryNodeWithGC extends SummaryNode {
* - Adds trackState param to summarize. If trackState is false, it bypasses the SummarizerNode and calls
* directly into summarizeInternal method.
*/
class SummarizerNodeWithGC extends SummarizerNode implements IRootSummarizerNodeWithGC {
export class SummarizerNodeWithGC extends SummarizerNode implements IRootSummarizerNodeWithGC {
// Tracks the work-in-progress used routes during summary.
private wipSerializedUsedRoutes: string | undefined;

Expand Down Expand Up @@ -527,8 +527,8 @@ class SummarizerNodeWithGC extends SummarizerNode implements IRootSummarizerNode
JSON.stringify(newSerializedRoutes),
{
referenceSequenceNumber: value.referenceSequenceNumber,
basePath: value.basePath,
localPath: value.localPath,
basePath: child.latestSummary.basePath,
localPath: child.latestSummary.localPath,
},
);
child.addPendingSummary(key, newLatestSummaryNode);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { MockLogger, TelemetryNullLogger } from "@fluidframework/telemetry-utils
import {
createRootSummarizerNodeWithGC,
IRootSummarizerNodeWithGC,
SummarizerNodeWithGC,
// eslint-disable-next-line import/no-internal-modules
} from "../summarizerNode/summarizerNodeWithGc";
import { mergeStats } from "../summaryUtils";
Expand Down Expand Up @@ -419,6 +420,13 @@ describe("SummarizerNodeWithGC Tests", () => {
);
assert(result.latestSummaryUpdated === true, "should update");
assert(result.wasSummaryTracked === true, "should be tracked");
const leafNodePath = `${ids[0]}/${ids[1]}/${ids[2]}`;
const leafNodeLatestSummary = (leafNode as SummarizerNodeWithGC).latestSummary;
assert.strictEqual(
leafNodeLatestSummary?.fullPath.toString(),
leafNodePath,
"The child node's latest summary path is incorrect",
);
});

it("Should add GC pending summary node created after parent node was summarized with empty used routes", async () => {
Expand Down Expand Up @@ -462,6 +470,13 @@ describe("SummarizerNodeWithGC Tests", () => {
);
assert(result.latestSummaryUpdated === true, "should update");
assert(result.wasSummaryTracked === true, "should be tracked");
const leafNodePath = `${ids[0]}/${ids[1]}/${ids[2]}`;
const leafNodeLatestSummary = (leafNode as SummarizerNodeWithGC).latestSummary;
assert.strictEqual(
leafNodeLatestSummary?.fullPath.toString(),
leafNodePath,
"The child node's latest summary path is incorrect",
);
});
});
});

0 comments on commit 55d95bf

Please sign in to comment.