Skip to content

Commit

Permalink
Fix for absent children prop in case of empty tree (#11128) (#11134)
Browse files Browse the repository at this point in the history
  • Loading branch information
jatgarg authored Jul 15, 2022
1 parent 2518461 commit 2a0ec73
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 4 deletions.
4 changes: 3 additions & 1 deletion packages/drivers/odsp-driver/src/compactSnapshotParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,11 @@ function readTreeSection(node: NodeCore) {
if (records.value !== undefined) {
assertBlobCoreInstance(records.value, "Blob value should be BlobCore");
snapshotTree.blobs[path] = records.value.toString();
} else {
} else if (records.children !== undefined) {
assertNodeCoreInstance(records.children, "Trees should be of type NodeCore");
snapshotTree.trees[path] = readTreeSection(records.children);
} else {
snapshotTree.trees[path] = { blobs: {}, commits: {}, trees: {} };
}
if (records.unreferenced !== undefined) {
assertBoolInstance(records.unreferenced, "Unreferenced flag should be bool");
Expand Down
9 changes: 6 additions & 3 deletions packages/drivers/odsp-driver/src/compactSnapshotWriter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,12 @@ function writeTreeSectionCore(treesNode: NodeCore, snapshotTree: ISnapshotTree)
if (snapshotTree.unreferenced) {
addBoolProperty(treeNode, "unreferenced", snapshotTree.unreferenced);
}
treeNode.addString("children", true);
const childNode = treeNode.addNode("list");
writeTreeSectionCore(childNode, value);
// Only write children prop if either blobs or trees are present.
if (Object.keys(value.blobs).length > 0 || Object.keys(value.trees).length > 0) {
treeNode.addString("children", true);
const childNode = treeNode.addNode("list");
writeTreeSectionCore(childNode, value);
}
}

if (snapshotTree.blobs) {
Expand Down
114 changes: 114 additions & 0 deletions packages/drivers/odsp-driver/src/test/jsonSnapshotFormatTests.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
/*!
* Copyright (c) Microsoft Corporation and contributors. All rights reserved.
* Licensed under the MIT License.
*/

/* eslint-disable max-len */

import { strict as assert } from "assert";
import { IOdspSnapshot } from "../contracts";
import { convertOdspSnapshotToSnapshotTreeAndBlobs } from "../odspSnapshotParser";

const snapshotTree: IOdspSnapshot = {
id: "bBzkVAgAHAAAA",
trees: [
{
id: "bBzkVAgAHAAAA",
sequenceNumber: 1,
entries: [
{
path: ".protocol",
type: "tree",
},
{
id: "bARA4itsHfCA5XZQaYhmASYpj",
path: ".protocol/quorumMembers",
type: "blob",
},
{
path: ".app",
type: "tree",
},
{
path: ".app/.channels",
type: "tree",
},
{
path: ".app/.channels/23c54bd8-ef53-42fa-a898-413de4c6f0f2",
type: "tree",
unreferenced: true,
},
{
id: "bARDoHhrwJMLoGao2yx8mD7nz",
path: ".app/.channels/23c54bd8-ef53-42fa-a898-413de4c6f0f2/.attributes",
type: "blob",
},
{
path: ".app/.channels/23c54bd8-ef53-42fa-a898-413de4c6f0f2/d65a4af3-0bf8-4052-8442-a898651ad9b8",
type: "tree",
},
],
},
],
blobs: [
{
id: "bARA4itsHfCA5XZQaYhmASYpj",
content: " \n",
size: 4,
encoding: undefined,
},
{
id: "bARDoHhrwJMLoGao2yx8mD7nz",
content: "KR \n",
size: 5,
encoding: undefined,
},
],
ops: [
{
sequenceNumber: 2,
op: {
clientId: "38777331-8149-4d15-b734-cd8110295ab6",
clientSequenceNumber: 2,
contents: null,
metadata: {},
minimumSequenceNumber: 136505,
referenceSequenceNumber: 136505,
sequenceNumber: 2,
term: 1,
timestamp: 1657840275913,
type: "op",
},
},
{
sequenceNumber: 3,
op: {
clientId: "38777331-8149-4d15-b734-cd8110295ab6",
clientSequenceNumber: -1,
contents: null,
minimumSequenceNumber: 136505,
referenceSequenceNumber: -1,
sequenceNumber: 3,
term: 1,
timestamp: 1657840275922,
type: "join",
},
},
],
};

describe("JSON Snapshot Format Conversion Tests", () => {
it("Conversion test", async () => {
const result = convertOdspSnapshotToSnapshotTreeAndBlobs(snapshotTree);
assert(result.sequenceNumber === 1, "Seq number should match");
assert(result.latestSequenceNumber === 3, "Latest sequence number should match");
assert(result.snapshotTree.id = snapshotTree.id, "Snapshot id should match");
assert(result.ops.length === 2, "2 ops should be there");
assert(result.blobs.size === 2, "2 blobs should be there");
assert(Object.keys(result.snapshotTree.trees).length === 2, "2 trees should be there");
const shouldBeEmptyTree = result.snapshotTree.trees[".app"]?.trees[".channels"]
?.trees["23c54bd8-ef53-42fa-a898-413de4c6f0f2"]?.trees["d65a4af3-0bf8-4052-8442-a898651ad9b8"];
const emptyTree = { blobs: {}, trees: {}, commits: {}, unreferenced: undefined };
assert.deepStrictEqual(shouldBeEmptyTree, emptyTree, "Tree should have no blobs and trees");
});
});

0 comments on commit 2a0ec73

Please sign in to comment.