Use payload-JSON equality for `Node`s (#322)

Summary:
A `Node` includes a ref and a payload. We should say that two nodes at
the same address are equal iff their payloads are equal, and two
payloads at the same address are equal iff their JSON contents are
equal. Importantly, `Node` equality does not depend on the `ref`’s
internal structure: this structure includes a reference to the enclosing
graph, and so using this notion of node equality caused `equals` to
incorrectly return `false` when the inputs were two logically equal
graphs with different internal structure. (It also caused `equals` to be
very slow, performing a deep equality check on the graphs for every
node.)

Test Plan:
A unit test has been strengthened. It fails before this patch, and
passes afterward.

wchargin-branch: v2-node-payload-json-equality
This commit is contained in:
William Chargin 2018-05-31 13:22:37 -07:00 committed by GitHub
parent 5dcf32560d
commit 6e442c2f0c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 10 additions and 4 deletions

View File

@ -343,8 +343,12 @@ export class Graph {
return false;
}
for (const node of theseNodes) {
if (!deepEqual(node, that.node(node.address))) {
for (const thisNode of theseNodes) {
const thatNode = that.node(thisNode.address);
if (thatNode == null) {
return false;
}
if (!deepEqual(thisNode.payload.toJSON(), thatNode.payload.toJSON())) {
return false;
}
}

View File

@ -767,10 +767,12 @@ describe("graph", () => {
expect(g0.equals(g1)).toBe(false);
});
it("adding and removing a node doesn't change equality", () => {
const g = newGraph()
const g = newGraph().addNode(new BarPayload(1, "along for the ride"));
const h = newGraph()
.addNode(new BarPayload(1, "along for the ride"))
.addNode(new FooPayload())
.removeNode(new FooPayload().address());
expect(g.equals(newGraph())).toBe(true);
expect(g.equals(h)).toBe(true);
});
it("adding and removing an edge doesn't change equality", () => {
const g = newGraph()