From 8182bb340cdcadd3d46556cc03392117143a4a6f Mon Sep 17 00:00:00 2001 From: William Chargin Date: Tue, 29 May 2018 18:47:04 -0700 Subject: [PATCH] Add edges to the graph (#318) Summary: Based on code originally paired with @decentralion. Test Plan: Unit tests added. Running `yarn travis` is sufficient. wchargin-branch: v2-edges --- src/core2/graph.js | 127 ++++++++++++++-- src/core2/graph.test.js | 326 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 441 insertions(+), 12 deletions(-) diff --git a/src/core2/graph.js b/src/core2/graph.js index 54c00ef..6dfb3be 100644 --- a/src/core2/graph.js +++ b/src/core2/graph.js @@ -1,7 +1,7 @@ // @flow import deepEqual from "lodash.isequal"; -import type {Address, PluginType} from "./address"; +import type {Address} from "./address"; import {AddressMap} from "./address"; import type {Compatible} from "../util/compat"; @@ -81,17 +81,37 @@ export type Plugins = $ReadOnlyArray>; type MaybeNode = {|+address: Address, +node: Node | void|}; type Integer = number; +type IndexedEdge = {| + +address: Address, + +srcIndex: Integer, + +dstIndex: Integer, + +payload: any, +|}; + export class Graph { _plugins: Plugins; _pluginMap: PluginMap; - _nodeIndices: AddressMap<{|+address: Address, +index: Integer|}>; + + // Invariant: sizes of `_nodeIndices`, `_nodes`, `_outEdges`, and + // `_inEdges` are all equal. _nodes: MaybeNode[]; + _nodeIndices: AddressMap<{|+address: Address, +index: Integer|}>; + + // If `idx` is the index of a node `v`, then `_outEdges[idx]` is the + // list of `e.address` for all edges `e` whose source is `v`. + // Likewise, `_inEdges[idx]` has the addresses of all in-edges to `v`. + _outEdges: Address[][]; + _inEdges: Address[][]; + _edges: AddressMap; constructor(plugins: Plugins) { this._plugins = plugins.slice(); this._pluginMap = createPluginMap(this._plugins); this._nodes = []; this._nodeIndices = new AddressMap(); + this._outEdges = []; + this._inEdges = []; + this._edges = new AddressMap(); } ref(address: Address): NodeReference { @@ -135,9 +155,17 @@ export class Graph { } } - edge(address: Address): Edge { - const _ = address; - throw new Error("Graphv2 is not yet implemented"); + edge(address: Address): ?Edge { + const indexedEdge = this._edges.get(address); + if (!indexedEdge) { + return undefined; + } + return { + address: indexedEdge.address, + src: this._nodes[indexedEdge.srcIndex].address, + dst: this._nodes[indexedEdge.dstIndex].address, + payload: indexedEdge.payload, + }; } /** @@ -145,9 +173,19 @@ export class Graph { * * If filter is provided, it will return only edges with the requested type. */ - edges(filter?: PluginType): Iterator> { - const _ = filter; - throw new Error("Graphv2 is not yet implemented"); + *edges(options?: PluginFilter): Iterator> { + let edges = this._edges.getAll().map((indexedEdge) => ({ + address: indexedEdge.address, + src: this._nodes[indexedEdge.srcIndex].address, + dst: this._nodes[indexedEdge.dstIndex].address, + payload: indexedEdge.payload, + })); + const filter = addressFilterer(options); + for (const edge of edges) { + if (filter(edge.address)) { + yield edge; + } + } } _addNodeAddress(address: Address): Integer { @@ -158,6 +196,8 @@ export class Graph { const index = this._nodes.length; this._nodeIndices.add({address, index}); this._nodes.push({address, node: undefined}); + this._outEdges.push([]); + this._inEdges.push([]); return index; } } @@ -196,13 +236,65 @@ export class Graph { } addEdge(edge: Edge): this { - const _ = edge; - throw new Error("Graphv2 is not yet implemented"); + if (edge == null) { + throw new Error(`edge is ${String(edge)}`); + } + if (edge.address == null) { + throw new Error(`address is ${String(edge.address)}`); + } + if (edge.src == null) { + throw new Error(`src is ${String(edge.src)}`); + } + if (edge.dst == null) { + throw new Error(`dst is ${String(edge.dst)}`); + } + const srcIndex = this._addNodeAddress(edge.src); + const dstIndex = this._addNodeAddress(edge.dst); + const indexedEdge = { + address: edge.address, + srcIndex, + dstIndex, + payload: edge.payload, + }; + return this._addIndexedEdge(indexedEdge); + } + + _addIndexedEdge(indexedEdge: IndexedEdge): this { + const existingIndexedEdge = this._edges.get(indexedEdge.address); + if (existingIndexedEdge !== undefined) { + if (deepEqual(existingIndexedEdge, indexedEdge)) { + return this; + } else { + throw new Error( + `edge at address ${JSON.stringify( + indexedEdge.address + )} exists with distinct contents` + ); + } + } + this._edges.add(indexedEdge); + this._outEdges[indexedEdge.srcIndex].push(indexedEdge.address); + this._inEdges[indexedEdge.dstIndex].push(indexedEdge.address); + return this; } removeEdge(address: Address): this { - const _ = address; - throw new Error("Graphv2 is not yet implemented"); + // TODO(perf): This is linear in the degree of the endpoints of the + // edge. Consider storing in non-list form. + const indexedEdge = this._edges.get(address); + if (indexedEdge) { + this._edges.remove(address); + [ + this._outEdges[indexedEdge.srcIndex], + this._inEdges[indexedEdge.dstIndex], + ].forEach((edges) => { + const index = edges.findIndex((ea) => deepEqual(ea, address)); + if (index !== -1) { + edges.splice(index, 1); + } + }); + } + return this; } /** @@ -235,11 +327,22 @@ export class Graph { return false; } + const theseEdges = Array.from(this.edges()); + const thoseEdges = Array.from(that.edges()); + if (theseEdges.length !== thoseEdges.length) { + return false; + } + for (const node of theseNodes) { if (!deepEqual(node, that.node(node.address))) { return false; } } + for (const edge of theseEdges) { + if (!deepEqual(edge, that.edge(edge.address))) { + return false; + } + } return true; } diff --git a/src/core2/graph.test.js b/src/core2/graph.test.js index 1d1d1ea..6adb8ef 100644 --- a/src/core2/graph.test.js +++ b/src/core2/graph.test.js @@ -149,6 +149,158 @@ describe("graph", () => { const nodes = Array.from(g.nodes()); expect(nodes).toHaveLength(0); }); + it("does not include absent nodes with incident edges", () => { + const g = newGraph() + .addNode(barPayload()) + .addEdge({ + address: { + owner: {plugin: EXAMPLE_PLUGIN_NAME, type: "EDGE"}, + id: "edge", + }, + src: barPayload().address(), + dst: new BarPayload(2, "goodbye").address(), + payload: "I have a source, but no destination", + }); + expect(Array.from(g.nodes())).toHaveLength(1); + }); + }); + + describe("edge", () => { + const srcPayload = () => new BarPayload(1, "first"); + const dstPayload = () => new BarPayload(2, "second"); + const edge = ({id = "my-favorite-edge", payload = 12} = {}) => ({ + address: {owner: {plugin: EXAMPLE_PLUGIN_NAME, type: "EDGE"}, id}, + src: srcPayload().address(), + dst: dstPayload().address(), + payload, + }); + + it("returns a normal edge", () => { + expect( + newGraph() + .addNode(srcPayload()) + .addNode(dstPayload()) + .addEdge(edge()) + .edge(edge().address) + ).toEqual(edge()); + }); + + it("returns a dangling edge", () => { + expect( + newGraph() + .addEdge(edge()) + .edge(edge().address) + ).toEqual(edge()); + }); + + it("returns `undefined` for an absent edge", () => { + expect(newGraph().edge(edge().address)).toBe(undefined); + }); + + it("throws for null or undefined address", () => { + expect(() => { + newGraph().edge((null: any)); + }).toThrow("null"); + expect(() => { + newGraph().edge((undefined: any)); + }).toThrow("undefined"); + }); + }); + + describe("edges", () => { + const srcPayload = () => new BarPayload(1, "first"); + const dstPayload = () => new BarPayload(2, "second"); + const edge = () => ({ + address: {owner: {plugin: EXAMPLE_PLUGIN_NAME, type: "EDGE"}, id: "e"}, + src: srcPayload().address(), + dst: dstPayload().address(), + payload: 12, + }); + + it("returns an empty iterator for an empty graph", () => { + expect(Array.from(newGraph().edges())).toEqual([]); + }); + + it("includes a normal edge in the graph", () => { + expect( + Array.from( + newGraph() + .addNode(srcPayload()) + .addNode(dstPayload()) + .addEdge(edge()) + .edges() + ) + ).toEqual([edge()]); + }); + + it("includes a dangling edge in the graph", () => { + expect( + Array.from( + newGraph() + .addEdge(edge()) + .edges() + ) + ).toEqual([edge()]); + }); + + it("supports filtering by plugin", () => { + expect( + Array.from( + newGraph() + .addEdge(edge()) + .edges({plugin: "SOMEONE_ELSE"}) + ) + ).toHaveLength(0); + expect( + Array.from( + newGraph() + .addEdge(edge()) + .edges({plugin: EXAMPLE_PLUGIN_NAME}) + ) + ).toHaveLength(1); + }); + + it("omits removed edges", () => { + expect( + Array.from( + newGraph() + .addEdge(edge()) + .removeEdge(edge().address) + .edges() + ) + ).toHaveLength(0); + }); + + it("supports filtering by plugin and type", () => { + expect( + Array.from( + newGraph() + .addEdge(edge()) + .edges({plugin: "SOMEONE_ELSE", type: "SOMETHING"}) + ) + ).toHaveLength(0); + expect( + Array.from( + newGraph() + .addEdge(edge()) + .edges({plugin: EXAMPLE_PLUGIN_NAME, type: "BOUNDARY"}) + ) + ).toHaveLength(0); + expect( + Array.from( + newGraph() + .addEdge(edge()) + .edges({plugin: EXAMPLE_PLUGIN_NAME, type: "EDGE"}) + ) + ).toHaveLength(1); + }); + + it("complains if you filter by only type", () => { + // $ExpectFlowError + expect(() => Array.from(newGraph().nodes({type: "FOO"}))).toThrowError( + "must filter by plugin" + ); + }); }); describe("addNode", () => { @@ -192,7 +344,165 @@ describe("graph", () => { }); }); + describe("addEdge", () => { + const srcPayload = () => new BarPayload(1, "first"); + const dstPayload = () => new BarPayload(2, "second"); + const edge = ({id = "my-favorite-edge", payload = 12} = {}) => ({ + address: {owner: {plugin: EXAMPLE_PLUGIN_NAME, type: "EDGE"}, id}, + src: srcPayload().address(), + dst: dstPayload().address(), + payload, + }); + + it("adds an edge between two existing nodes", () => { + expect( + Array.from( + newGraph() + .addNode(srcPayload()) + .addNode(dstPayload()) + .addEdge(edge()) + .edges() + ) + ).toEqual([edge()]); + }); + + it("is idempotent", () => { + expect( + Array.from( + newGraph() + .addNode(srcPayload()) + .addNode(dstPayload()) + .addEdge(edge()) + .addEdge(edge()) + .edges() + ) + ).toEqual([edge()]); + }); + + it("throws an error for a payload conflict at a given address", () => { + const e1 = edge({id: "my-edge", payload: "uh"}); + const e2 = edge({id: "my-edge", payload: "oh"}); + const g = newGraph() + .addNode(srcPayload()) + .addNode(dstPayload()) + .addEdge(e1); + expect(() => { + g.addEdge(e2); + }).toThrow("exists with distinct contents"); + }); + + it("adds an edge whose `src` is not present", () => { + expect( + Array.from( + newGraph() + .addNode(dstPayload()) + .addEdge(edge()) + .edges() + ) + ).toEqual([edge()]); + }); + + it("adds an edge whose `dst` is not present", () => { + expect( + Array.from( + newGraph() + .addNode(srcPayload()) + .addEdge(edge()) + .edges() + ) + ).toEqual([edge()]); + }); + + it("adds an edge whose `src` and `dst` are not present", () => { + expect( + Array.from( + newGraph() + .addEdge(edge()) + .edges() + ) + ).toEqual([edge()]); + }); + + it("adds a loop", () => { + const e = {...edge(), dst: srcPayload().address()}; + expect( + Array.from( + newGraph() + .addNode(srcPayload()) + .addEdge(e) + .edges() + ) + ).toEqual([e]); + }); + + it("throws for null or undefined edge", () => { + expect(() => { + newGraph().addEdge((null: any)); + }).toThrow("null"); + expect(() => { + newGraph().addEdge((undefined: any)); + }).toThrow("undefined"); + }); + + it("throws for null or undefined address", () => { + const e = (address: any) => ({...edge(), address}); + expect(() => { + newGraph().addEdge(e(null)); + }).toThrow("null"); + expect(() => { + newGraph().addEdge(e(undefined)); + }).toThrow("undefined"); + }); + + it("throws for null or undefined src", () => { + const e = (src: any) => ({...edge(), src}); + expect(() => { + newGraph().addEdge(e(null)); + }).toThrow("null"); + expect(() => { + newGraph().addEdge(e(undefined)); + }).toThrow("undefined"); + }); + + it("throws for null or undefined dst", () => { + const e = (dst: any) => ({...edge(), dst}); + expect(() => { + newGraph().addEdge(e(null)); + }).toThrow("null"); + expect(() => { + newGraph().addEdge(e(undefined)); + }).toThrow("undefined"); + }); + }); + + describe("removeEdge", () => { + it("removes a nonexistent edge without error", () => { + newGraph().removeEdge({ + owner: {plugin: EXAMPLE_PLUGIN_NAME, type: "EDGE"}, + id: "nope", + }); + }); + + it("throws for null or undefined address", () => { + expect(() => { + newGraph().removeEdge((null: any)); + }).toThrow("null"); + expect(() => { + newGraph().removeEdge((undefined: any)); + }).toThrow("undefined"); + }); + }); + describe("equals", () => { + const srcPayload = () => new BarPayload(1, "first"); + const dstPayload = () => new BarPayload(2, "second"); + const edge = (payload) => ({ + address: {owner: {plugin: EXAMPLE_PLUGIN_NAME, type: "EDGE"}, id: "e"}, + src: srcPayload().address(), + dst: dstPayload().address(), + payload, + }); + it("empty graphs are equal", () => { expect(newGraph().equals(newGraph())).toBe(true); }); @@ -211,12 +521,28 @@ describe("graph", () => { const g1 = newGraph().addNode(new BarPayload(1, "there")); expect(g0.equals(g1)).toBe(false); }); + it("graphs with different edges are not equal", () => { + const g0 = newGraph(); + const g1 = newGraph().addEdge(edge("hello")); + expect(g0.equals(g1)).toBe(false); + }); + it("graphs with different edges at same address are not equal", () => { + const g0 = newGraph().addEdge(edge("hello")); + const g1 = newGraph().addEdge(edge("there")); + expect(g0.equals(g1)).toBe(false); + }); it("adding and removing a node doesn't change equality", () => { const g = newGraph() .addNode(new FooPayload()) .removeNode(new FooPayload().address()); expect(g.equals(newGraph())).toBe(true); }); + it("adding and removing an edge doesn't change equality", () => { + const g = newGraph() + .addEdge(edge("hello")) + .removeEdge(edge("hello").address); + expect(g.equals(newGraph())).toBe(true); + }); }); });