From 81b7002ce814efde15b1af011bbd4e85f45d2634 Mon Sep 17 00:00:00 2001
From: Brian Litwin <brian.n.litwin@gmail.com>
Date: Sun, 17 Feb 2019 15:24:10 -0500
Subject: [PATCH] Add optional node prefix filter to pagerankGraph (#1090)

Continuing work on #1020.
Adding an optional parameter to `nodes()` which enables optional
node prefix filtering.

Test plan:

@decentralion suggested on Discord that the tests should verify:
1) the parameter was passed to `_graph` correctly
2) the augmentation logic was applied correctly

The tests I added are identical to the tests in `graph.test`, except
that they verify that the result of `pagerankGraph` matches that of
`graph`. On one hand, this creates a dependence on `graph`,
as these tests don't verify that the filter works correctly, only that
graph has applied the filter and returned the iterator.
However, my prevailing thought is that it isn't `pagerankGraph's` responsibility
to test the behavior of `graph`, and so testing the exact filter results
of `pagerankGraph` like we do in `graph.test` isn't the best strategy, and
testing that `pagerankGraph`'s results equal `graph`'s results is a better strategy.

The tests also check that a `score` is provided alongside each `node` in the iterator,
to minimally satisfy @decentralion's second spec.

yarn test passes.
---
 src/core/pagerankGraph.js      | 12 ++++---
 src/core/pagerankGraph.test.js | 59 +++++++++++++++++++++++++++++++++-
 2 files changed, 65 insertions(+), 6 deletions(-)

diff --git a/src/core/pagerankGraph.js b/src/core/pagerankGraph.js
index cdea6ee..6ab8720 100644
--- a/src/core/pagerankGraph.js
+++ b/src/core/pagerankGraph.js
@@ -186,8 +186,8 @@ export class PagerankGraph {
     return this._syntheticLoopWeight;
   }
 
-  *_nodesIterator(): Iterator<ScoredNode> {
-    for (const node of this._graph.nodes()) {
+  *_nodesIterator(options?: {|+prefix: NodeAddressT|}): Iterator<ScoredNode> {
+    for (const node of this._graph.nodes(options)) {
       const score = NullUtil.get(this._scores.get(node));
       yield {node, score};
     }
@@ -196,11 +196,13 @@ export class PagerankGraph {
   /**
    * Provides node and score for every node in the underlying graph.
    *
-   * TODO(#1020): Allow optional filtering, as in Graph.nodes.
+   * Optionally, provide a node prefix to return an iterator containing
+   * only node/score objects whose nodes match the provided node prefix.
+   * See Graph.nodes and Address.hasPrefix for details.
    */
-  nodes(): Iterator<ScoredNode> {
+  nodes(options?: {|+prefix: NodeAddressT|}): Iterator<ScoredNode> {
     this._verifyGraphNotModified();
-    return this._nodesIterator();
+    return this._nodesIterator(options);
   }
 
   /**
diff --git a/src/core/pagerankGraph.test.js b/src/core/pagerankGraph.test.js
index 2ab5c62..5385388 100644
--- a/src/core/pagerankGraph.test.js
+++ b/src/core/pagerankGraph.test.js
@@ -1,7 +1,13 @@
 // @flow
 
 import sortBy from "lodash.sortby";
-import {Graph, NodeAddress, EdgeAddress, type Edge} from "./graph";
+import {
+  Graph,
+  NodeAddress,
+  EdgeAddress,
+  type NodeAddressT,
+  type Edge,
+} from "./graph";
 import {PagerankGraph} from "./pagerankGraph";
 import {advancedGraph} from "./graphTestUtil";
 import * as NullUtil from "../util/null";
@@ -63,6 +69,57 @@ describe("core/pagerankGraph", () => {
     });
   });
 
+  describe("node prefix filter matches graph filter", () => {
+    const n1 = NodeAddress.empty;
+    const n2 = NodeAddress.fromParts(["foo"]);
+    const n3 = NodeAddress.fromParts(["foo", "bar"]);
+    const n4 = NodeAddress.fromParts(["zod", "bar"]);
+    const g = () =>
+      new Graph()
+        .addNode(n1)
+        .addNode(n2)
+        .addNode(n3)
+        .addNode(n4);
+    const pg = () => new PagerankGraph(g(), defaultEvaluator);
+
+    function expectPagerankGraphToEqualGraph(
+      options: {|+prefix: NodeAddressT|} | void
+    ) {
+      const pagerankGraphNodes = Array.from(pg().nodes(options)).sort();
+      const graphNodes = Array.from(g().nodes(options)).sort();
+
+      pagerankGraphNodes.forEach(
+        (pgNode, i) =>
+          expect(pgNode.node).toEqual(graphNodes[i]) &&
+          expect(pgNode.score).toBe(0.25)
+      );
+    }
+
+    it("with no options object", () => {
+      expectPagerankGraphToEqualGraph(undefined);
+    });
+
+    it("with prefix filter", () => {
+      expectPagerankGraphToEqualGraph({prefix: n2});
+    });
+
+    it("with empty prefix", () => {
+      expectPagerankGraphToEqualGraph({prefix: NodeAddress.empty});
+    });
+
+    it("with prefix that matches nothing", () => {
+      expectPagerankGraphToEqualGraph({prefix: NodeAddress.fromParts(["2"])});
+    });
+  });
+
+  describe("node prefix filter", () => {
+    it("requires a prefix when options are specified", () => {
+      const pg = new PagerankGraph(nonEmptyGraph(), defaultEvaluator);
+      // $ExpectFlowError
+      expect(() => Array.from(pg.nodes({}))).toThrow("prefix");
+    });
+  });
+
   describe("edge/edges", () => {
     it("edges returns the same edges as are in the graph", () => {
       const g = advancedGraph().graph1();