Split PluginAdapter into Dynamic and Static parts (#513)

In some cases (e.g. WeightConfig) we want to have information from the PluginAdapater before loading any data from the server. In other cases, we need to combine the PluginAdapater with actual data, e.g. so we can get the description of a GitHub node. To support this, we split the PluginAdapter into a Static and Dynamic component. The Dynamic component has data needed to give node descriptions, etc. Given a static adapter, you can get a promise to load the dynamic adapter. Given the dynamic adapter, you can immediately get the static adapter. (There's a parallel to NodeReference (static) and NodePorcelain (dynamic)). Test plan: Travis passes, as does manual testing of the frontend.
2018-07-23 15:37:14 -07:00 · 2018-07-23 15:37:14 -07:00 · 2a39844445
parent 3c14ef8a43
commit 2a39844445
6 changed files with 184 additions and 139 deletions
--- a/src/app/credExplorer/App.js
+++ b/src/app/credExplorer/App.js
@ -4,12 +4,12 @@ import React from "react";
 import {StyleSheet, css} from "aphrodite/no-important";

 import LocalStore from "./LocalStore";
-import {createPluginAdapter as createGithubAdapter} from "../../plugins/github/pluginAdapter";
-import {createPluginAdapter as createGitAdapter} from "../../plugins/git/pluginAdapter";
+import {StaticPluginAdapter as GithubAdapter} from "../../plugins/github/pluginAdapter";
+import {StaticPluginAdapter as GitAdapter} from "../../plugins/git/pluginAdapter";
 import {Graph} from "../../core/graph";
 import {pagerank} from "../../core/attribution/pagerank";
 import {PagerankTable} from "./PagerankTable";
-import type {PluginAdapter} from "../pluginAdapter";
+import type {DynamicPluginAdapter} from "../pluginAdapter";
 import {type EdgeEvaluator} from "../../core/attribution/pagerank";
 import {WeightConfig} from "./WeightConfig";
 import type {PagerankNodeDecomposition} from "../../core/attribution/pagerankNodeDecomposition";
@ -23,7 +23,7 @@ type State = {
  data: {|
    graphWithMetadata: ?{|
      +graph: Graph,
-      +adapters: $ReadOnlyArray<PluginAdapter>,
+      +adapters: $ReadOnlyArray<DynamicPluginAdapter>,
      +nodeCount: number,
      +edgeCount: number,
    |},
@ -146,17 +146,19 @@ export default class App extends React.Component<Props, State> {
      return;
    }

-    const githubPromise = createGithubAdapter(repoOwner, repoName).then(
-      (adapter) => {
+    const githubPromise = new GithubAdapter()
+      .load(repoOwner, repoName)
+      .then((adapter) => {
        const graph = adapter.graph();
        return {graph, adapter};
-      }
-    );
+      });

-    const gitPromise = createGitAdapter(repoOwner, repoName).then((adapter) => {
-      const graph = adapter.graph();
-      return {graph, adapter};
-    });
+    const gitPromise = new GitAdapter()
+      .load(repoOwner, repoName)
+      .then((adapter) => {
+        const graph = adapter.graph();
+        return {graph, adapter};
+      });

    Promise.all([gitPromise, githubPromise]).then((graphsAndAdapters) => {
      const graph = Graph.merge(graphsAndAdapters.map((x) => x.graph));
--- a/src/app/credExplorer/PagerankTable.js
+++ b/src/app/credExplorer/PagerankTable.js
@ -14,16 +14,16 @@ import type {
  ScoredContribution,
 } from "../../core/attribution/pagerankNodeDecomposition";
 import type {Contribution} from "../../core/attribution/graphToMarkovChain";
-import type {PluginAdapter} from "../pluginAdapter";
+import type {DynamicPluginAdapter} from "../pluginAdapter";
 import * as NullUtil from "../../util/null";

 // TODO: Factor this out and test it (#465)
 export function nodeDescription(
  address: NodeAddressT,
-  adapters: $ReadOnlyArray<PluginAdapter>
+  adapters: $ReadOnlyArray<DynamicPluginAdapter>
 ): string {
  const adapter = adapters.find((adapter) =>
-    NodeAddress.hasPrefix(address, adapter.nodePrefix())
+    NodeAddress.hasPrefix(address, adapter.static().nodePrefix())
  );
  if (adapter == null) {
    const result = NodeAddress.toString(address);
@ -43,10 +43,10 @@ export function nodeDescription(
 function edgeVerb(
  address: EdgeAddressT,
  direction: "FORWARD" | "BACKWARD",
-  adapters: $ReadOnlyArray<PluginAdapter>
+  adapters: $ReadOnlyArray<DynamicPluginAdapter>
 ): string {
  const adapter = adapters.find((adapter) =>
-    EdgeAddress.hasPrefix(address, adapter.edgePrefix())
+    EdgeAddress.hasPrefix(address, adapter.static().edgePrefix())
  );
  if (adapter == null) {
    const result = EdgeAddress.toString(address);
@ -55,6 +55,7 @@ function edgeVerb(
  }

  const edgeType = adapter
+    .static()
    .edgeTypes()
    .find(({prefix}) => EdgeAddress.hasPrefix(address, prefix));
  if (edgeType == null) {
@ -72,13 +73,13 @@ function scoreDisplay(probability: number) {

 type SharedProps = {|
  +pnd: PagerankNodeDecomposition,
-  +adapters: $ReadOnlyArray<PluginAdapter>,
+  +adapters: $ReadOnlyArray<DynamicPluginAdapter>,
  +maxEntriesPerList: number,
 |};

 type PagerankTableProps = {|
  +pnd: ?PagerankNodeDecomposition,
-  +adapters: ?$ReadOnlyArray<PluginAdapter>,
+  +adapters: ?$ReadOnlyArray<DynamicPluginAdapter>,
  +maxEntriesPerList: number,
 |};
 type PagerankTableState = {|topLevelFilter: NodeAddressT|};
@ -116,21 +117,24 @@ export class PagerankTable extends React.PureComponent<
      throw new Error("Impossible.");
    }

-    function optionGroup(adapter: PluginAdapter) {
+    function optionGroup(adapter: DynamicPluginAdapter) {
      const header = (
        <option
-          key={adapter.nodePrefix()}
-          value={adapter.nodePrefix()}
+          key={adapter.static().nodePrefix()}
+          value={adapter.static().nodePrefix()}
          style={{fontWeight: "bold"}}
        >
-          {adapter.name()}
+          {adapter.static().name()}
        </option>
      );
-      const entries = adapter.nodeTypes().map((type) => (
-        <option key={type.prefix} value={type.prefix}>
-          {"\u2003" + type.name}
-        </option>
-      ));
+      const entries = adapter
+        .static()
+        .nodeTypes()
+        .map((type) => (
+          <option key={type.prefix} value={type.prefix}>
+            {"\u2003" + type.name}
+          </option>
+        ));
      return [header, ...entries];
    }
    return (
@ -143,7 +147,9 @@ export class PagerankTable extends React.PureComponent<
          }}
        >
          <option value={NodeAddress.empty}>Show all</option>
-          {sortBy(adapters, (a) => a.name()).map(optionGroup)}
+          {sortBy(adapters, (a: DynamicPluginAdapter) => a.static().name()).map(
+            optionGroup
+          )}
        </select>
      </label>
    );
@ -362,7 +368,7 @@ export class ContributionRow extends React.PureComponent<

 export class ContributionView extends React.PureComponent<{|
  +contribution: Contribution,
-  +adapters: $ReadOnlyArray<PluginAdapter>,
+  +adapters: $ReadOnlyArray<DynamicPluginAdapter>,
 |}> {
  render() {
    const {contribution, adapters} = this.props;
--- a/src/app/credExplorer/PagerankTable.test.js
+++ b/src/app/credExplorer/PagerankTable.test.js
@ -56,71 +56,87 @@ function example() {

  const adapters = [
    {
-      name: () => "foo",
+      static: () => ({
+        name: () => "foo",
+        nodePrefix: () => NodeAddress.fromParts(["foo"]),
+        edgePrefix: () => EdgeAddress.fromParts(["foo"]),
+        nodeTypes: () => [
+          {name: "alpha", prefix: NodeAddress.fromParts(["foo", "a"])},
+          {name: "beta", prefix: NodeAddress.fromParts(["foo", "b"])},
+        ],
+        edgeTypes: () => [
+          {
+            prefix: EdgeAddress.fromParts(["foo"]),
+            forwardName: "foos",
+            backwardName: "is fooed by",
+          },
+        ],
+        load: (_unused_repoOwner, _unused_repoName) => {
+          throw new Error("unused");
+        },
+      }),
      graph: () => {
        throw new Error("unused");
      },
-      renderer: () => ({
-        edgeVerb: (_unused_e, direction) =>
-          direction === "FORWARD" ? "foos" : "is fooed by",
-      }),
      nodeDescription: (x) => `foo: ${NodeAddress.toString(x)}`,
-      nodePrefix: () => NodeAddress.fromParts(["foo"]),
-      edgePrefix: () => EdgeAddress.fromParts(["foo"]),
-      nodeTypes: () => [
-        {name: "alpha", prefix: NodeAddress.fromParts(["foo", "a"])},
-        {name: "beta", prefix: NodeAddress.fromParts(["foo", "b"])},
-      ],
-      edgeTypes: () => [
-        {
-          prefix: EdgeAddress.fromParts(["foo"]),
-          forwardName: "foos",
-          backwardName: "is fooed by",
-        },
-      ],
    },
    {
-      name: () => "bar",
+      static: () => ({
+        name: () => "bar",
+        nodePrefix: () => NodeAddress.fromParts(["bar"]),
+        edgePrefix: () => EdgeAddress.fromParts(["bar"]),
+        nodeTypes: () => [
+          {name: "alpha", prefix: NodeAddress.fromParts(["bar", "a"])},
+        ],
+        edgeTypes: () => [
+          {
+            prefix: EdgeAddress.fromParts(["bar"]),
+            forwardName: "bars",
+            backwardName: "is barred by",
+          },
+        ],
+        load: (_unused_repoOwner, _unused_repoName) => {
+          throw new Error("unused");
+        },
+      }),
      graph: () => {
        throw new Error("unused");
      },
      nodeDescription: (x) => `bar: ${NodeAddress.toString(x)}`,
-      nodePrefix: () => NodeAddress.fromParts(["bar"]),
-      edgePrefix: () => EdgeAddress.fromParts(["bar"]),
-      nodeTypes: () => [
-        {name: "alpha", prefix: NodeAddress.fromParts(["bar", "a"])},
-      ],
-      edgeTypes: () => [
-        {
-          prefix: EdgeAddress.fromParts(["bar"]),
-          forwardName: "bars",
-          backwardName: "is barred by",
-        },
-      ],
    },
    {
-      name: () => "xox",
+      static: () => ({
+        name: () => "xox",
+        nodePrefix: () => NodeAddress.fromParts(["xox"]),
+        edgePrefix: () => EdgeAddress.fromParts(["xox"]),
+        nodeTypes: () => [],
+        edgeTypes: () => [],
+        load: (_unused_repoOwner, _unused_repoName) => {
+          throw new Error("unused");
+        },
+      }),
      graph: () => {
        throw new Error("unused");
      },
      nodeDescription: (_unused_arg) => `xox node!`,
-      nodePrefix: () => NodeAddress.fromParts(["xox"]),
-      edgePrefix: () => EdgeAddress.fromParts(["xox"]),
-      nodeTypes: () => [],
-      edgeTypes: () => [],
    },
    {
-      name: () => "unused",
+      static: () => ({
+        nodePrefix: () => NodeAddress.fromParts(["unused"]),
+        edgePrefix: () => EdgeAddress.fromParts(["unused"]),
+        nodeTypes: () => [],
+        edgeTypes: () => [],
+        name: () => "unused",
+        load: (_unused_repoOwner, _unused_repoName) => {
+          throw new Error("unused");
+        },
+      }),
      graph: () => {
        throw new Error("unused");
      },
      nodeDescription: () => {
        throw new Error("Unused");
      },
-      nodePrefix: () => NodeAddress.fromParts(["unused"]),
-      edgePrefix: () => EdgeAddress.fromParts(["unused"]),
-      nodeTypes: () => [],
-      edgeTypes: () => [],
    },
  ];

--- a/src/app/pluginAdapter.js
+++ b/src/app/pluginAdapter.js
@ -2,19 +2,24 @@

 import type {Graph, NodeAddressT, EdgeAddressT} from "../core/graph";

-export interface PluginAdapter {
+export interface StaticPluginAdapter {
  name(): string;
-  graph(): Graph;
  nodePrefix(): NodeAddressT;
  edgePrefix(): EdgeAddressT;
  nodeTypes(): Array<{|
    +name: string,
    +prefix: NodeAddressT,
  |}>;
-  nodeDescription(NodeAddressT): string;
  edgeTypes(): Array<{|
    +forwardName: string,
    +backwardName: string,
    +prefix: EdgeAddressT,
  |}>;
+  load(repoOwner: string, repoName: string): Promise<DynamicPluginAdapter>;
+}
+
+export interface DynamicPluginAdapter {
+  graph(): Graph;
+  nodeDescription(NodeAddressT): string;
+  static (): StaticPluginAdapter;
 }
--- a/src/plugins/git/pluginAdapter.js
+++ b/src/plugins/git/pluginAdapter.js
@ -1,47 +1,23 @@
 // @flow
-import type {PluginAdapter as IPluginAdapter} from "../../app/pluginAdapter";
+import type {
+  StaticPluginAdapter as IStaticPluginAdapter,
+  DynamicPluginAdapter as IDynamicPluginAdapter,
+} from "../../app/pluginAdapter";
 import {Graph} from "../../core/graph";
 import * as N from "./nodes";
 import * as E from "./edges";
 import {description} from "./render";

-export async function createPluginAdapter(
-  repoOwner: string,
-  repoName: string
-): Promise<IPluginAdapter> {
-  const url = `/api/v1/data/data/${repoOwner}/${repoName}/git/graph.json`;
-  const response = await fetch(url);
-  if (!response.ok) {
-    return Promise.reject(response);
-  }
-  const json = await response.json();
-  const graph = Graph.fromJSON(json);
-  return new PluginAdapter(graph);
-}
-
-class PluginAdapter implements IPluginAdapter {
-  +_graph: Graph;
-  constructor(graph: Graph) {
-    this._graph = graph;
-  }
+export class StaticPluginAdapter implements IStaticPluginAdapter {
  name() {
    return "Git";
  }
-  graph() {
-    return this._graph;
-  }
  nodePrefix() {
    return N._Prefix.base;
  }
  edgePrefix() {
    return E._Prefix.base;
  }
-  nodeDescription(node) {
-    // This cast is unsound, and might throw at runtime, but won't have
-    // silent failures or cause problems down the road.
-    const address = N.fromRaw((node: any));
-    return description(address);
-  }
  nodeTypes() {
    return [
      {name: "Blob", prefix: N._Prefix.blob},
@ -79,4 +55,36 @@ class PluginAdapter implements IPluginAdapter {
      },
    ];
  }
+  async load(
+    repoOwner: string,
+    repoName: string
+  ): Promise<IDynamicPluginAdapter> {
+    const url = `/api/v1/data/data/${repoOwner}/${repoName}/git/graph.json`;
+    const response = await fetch(url);
+    if (!response.ok) {
+      return Promise.reject(response);
+    }
+    const json = await response.json();
+    const graph = Graph.fromJSON(json);
+    return new DynamicPluginAdapter(graph);
+  }
+}
+
+class DynamicPluginAdapter implements IDynamicPluginAdapter {
+  +_graph: Graph;
+  constructor(graph: Graph) {
+    this._graph = graph;
+  }
+  graph() {
+    return this._graph;
+  }
+  nodeDescription(node) {
+    // This cast is unsound, and might throw at runtime, but won't have
+    // silent failures or cause problems down the road.
+    const address = N.fromRaw((node: any));
+    return description(address);
+  }
+  static() {
+    return new StaticPluginAdapter();
+  }
 }
--- a/src/plugins/github/pluginAdapter.js
+++ b/src/plugins/github/pluginAdapter.js
@ -1,5 +1,8 @@
 // @flow
-import type {PluginAdapter as IPluginAdapter} from "../../app/pluginAdapter";
+import type {
+  StaticPluginAdapter as IStaticPluginAdapter,
+  DynamicPluginAdapter as IDynamicPluginAdapater,
+} from "../../app/pluginAdapter";
 import {type Graph, NodeAddress} from "../../core/graph";
 import {createGraph} from "./createGraph";
 import * as N from "./nodes";
@ -7,44 +10,10 @@ import * as E from "./edges";
 import {RelationalView} from "./relationalView";
 import {description} from "./render";

-export async function createPluginAdapter(
-  repoOwner: string,
-  repoName: string
-): Promise<IPluginAdapter> {
-  const url = `/api/v1/data/data/${repoOwner}/${repoName}/github/view.json`;
-  const response = await fetch(url);
-  if (!response.ok) {
-    return Promise.reject(response);
-  }
-  const json = await response.json();
-  const view = RelationalView.fromJSON(json);
-  const graph = createGraph(view);
-  return new PluginAdapter(view, graph);
-}
-
-class PluginAdapter implements IPluginAdapter {
-  +_view: RelationalView;
-  +_graph: Graph;
-  constructor(view: RelationalView, graph: Graph) {
-    this._view = view;
-    this._graph = graph;
-  }
+export class StaticPluginAdapter implements IStaticPluginAdapter {
  name() {
    return "GitHub";
  }
-  nodeDescription(node) {
-    // This cast is unsound, and might throw at runtime, but won't have
-    // silent failures or cause problems down the road.
-    const address = N.fromRaw((node: any));
-    const entity = this._view.entity(address);
-    if (entity == null) {
-      throw new Error(`unknown entity: ${NodeAddress.toString(node)}`);
-    }
-    return description(entity);
-  }
-  graph() {
-    return this._graph;
-  }
  nodePrefix() {
    return N._Prefix.base;
  }
@ -85,4 +54,43 @@ class PluginAdapter implements IPluginAdapter {
      },
    ];
  }
+  async load(
+    repoOwner: string,
+    repoName: string
+  ): Promise<IDynamicPluginAdapater> {
+    const url = `/api/v1/data/data/${repoOwner}/${repoName}/github/view.json`;
+    const response = await fetch(url);
+    if (!response.ok) {
+      return Promise.reject(response);
+    }
+    const json = await response.json();
+    const view = RelationalView.fromJSON(json);
+    const graph = createGraph(view);
+    return new DynamicPluginAdapter(view, graph);
+  }
+}
+
+class DynamicPluginAdapter implements IDynamicPluginAdapater {
+  +_view: RelationalView;
+  +_graph: Graph;
+  constructor(view: RelationalView, graph: Graph) {
+    this._view = view;
+    this._graph = graph;
+  }
+  nodeDescription(node) {
+    // This cast is unsound, and might throw at runtime, but won't have
+    // silent failures or cause problems down the road.
+    const address = N.fromRaw((node: any));
+    const entity = this._view.entity(address);
+    if (entity == null) {
+      throw new Error(`unknown entity: ${NodeAddress.toString(node)}`);
+    }
+    return description(entity);
+  }
+  graph() {
+    return this._graph;
+  }
+  static() {
+    return new StaticPluginAdapter();
+  }
 }