From 1e791782d5677f31063c42495487ccd8d6310bfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dandelion=20Man=C3=A9?= Date: Sat, 17 Mar 2018 00:28:17 -0700 Subject: [PATCH] Allow redundant adds to the Graph (#79) Graph.addNode and Graph.addEdge now allow adding the same node or edge multiple times, provided that the duplicate adds are trying to insert identical content. This came up while prototyping the GitHub plugin; rather than create myriad subgraphs and merge them, I found it convenient to construct a single graph and iteratively add nodes. Since the same node may be discovered multiple times (most notably user identities), there was a need for a "conservative add" abstraction that adds a node if it doesn't exist yet, but errors only if multiple adds conflict. Since this behavior is generic and highly conservative, it seemed appropriate to include in the graph class itself. Test Plan: The unit tests have been updated to include the new behavior. --- src/backend/graph.js | 30 ++++++++++++++++++++++-------- src/backend/graph.test.js | 22 +++++++++++++++++----- 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/src/backend/graph.js b/src/backend/graph.js index 16873e0..351b2a9 100644 --- a/src/backend/graph.js +++ b/src/backend/graph.js @@ -70,10 +70,17 @@ export class Graph { if (node == null) { throw new Error(`node is ${String(node)}`); } - if (this.getNode(node.address) !== undefined) { - throw new Error( - `node at address ${JSON.stringify(node.address)} already exists` - ); + const existingNode = this.getNode(node.address); + if (existingNode !== undefined) { + if (deepEqual(existingNode, node)) { + return this; + } else { + throw new Error( + `node at address ${JSON.stringify( + node.address + )} exists with distinct contents` + ); + } } this._nodes.add(node); this._outEdges.add({address: node.address, edges: []}); @@ -85,10 +92,17 @@ export class Graph { if (edge == null) { throw new Error(`edge is ${String(edge)}`); } - if (this.getEdge(edge.address) !== undefined) { - throw new Error( - `edge at address ${JSON.stringify(edge.address)} already exists` - ); + const existingEdge = this.getEdge(edge.address); + if (existingEdge !== undefined) { + if (deepEqual(existingEdge, edge)) { + return this; + } else { + throw new Error( + `edge at address ${JSON.stringify( + edge.address + )} exists with distinct contents` + ); + } } if (this.getNode(edge.src) === undefined) { throw new Error(`source ${JSON.stringify(edge.src)} does not exist`); diff --git a/src/backend/graph.test.js b/src/backend/graph.test.js index 0af5ea0..3224300 100644 --- a/src/backend/graph.test.js +++ b/src/backend/graph.test.js @@ -192,24 +192,36 @@ describe("graph", () => { }); describe("creating nodes and edges", () => { - it("forbids adding a node with existing address", () => { + it("forbids adding a node with existing address and different contents", () => { expect(() => demoData.simpleMealGraph().addNode({ address: demoData.crabNode().address, payload: {anotherCrab: true}, }) - ).toThrow(/already exists/); + ).toThrow(/exists with distinct contents/); }); - it("forbids adding an edge with existing address", () => { + it("adding a node redundantly is a no-op", () => { + const simple1 = demoData.simpleMealGraph(); + const simple2 = demoData.simpleMealGraph().addNode(demoData.heroNode()); + expect(simple1.equals(simple2)).toBe(true); + }); + + it("forbids adding an edge with existing address and different contents", () => { expect(() => demoData.simpleMealGraph().addEdge({ address: demoData.cookEdge().address, src: demoData.crabNode().address, dst: demoData.crabNode().address, - payload: {}, + payload: {isDifferent: true}, }) - ).toThrow(/already exists/); + ).toThrow(/exists with distinct contents/); + }); + + it("adding an edge redundantly is a no-op", () => { + const simple1 = demoData.simpleMealGraph(); + const simple2 = demoData.simpleMealGraph().addEdge(demoData.cookEdge()); + expect(simple1.equals(simple2)).toBe(true); }); it("allows creating self-loops", () => {