`credExplorer/App` stores weights, not evaluator (#792)

This commit refactors `credExplorer/App` so that instead of storing an `EdgeEvaulator` in its state, it stores `WeightedTypes` instead. This has a few benefits: - It's trivial to generate the right default value for `WeightedTypes`, so we no longer allow the variable to be nullable in the state. This simplifies logic, removes an error case, and means that we don't require the `WeightConfig` to mount before the app is usable. - `WeightedTypes` are serializable and can be tested for equality, so they are a better-behaved piece of state - We put off the information-destroying transformation as long as possible - In the future, I think we may want to move the weights/types concept into core, at which point the `WeightedTypes` will directly be consumed by the `core/attribution` module. Test plan: Unit tests are pretty thorough; to be safe, I tested the UI myself.
2018-09-06 15:29:38 -07:00 · 2018-09-06 15:29:38 -07:00 · ad5ea761ea
parent 417265a4d4
commit ad5ea761ea
5 changed files with 52 additions and 65 deletions
--- a/src/app/credExplorer/App.js
+++ b/src/app/credExplorer/App.js
@ -2,15 +2,18 @@

 import React from "react";

-import * as NullUtil from "../../util/null";
 import type {Assets} from "../assets";
 import type {LocalStore} from "../localStore";
 import CheckedLocalStore from "../checkedLocalStore";
 import BrowserLocalStore from "../browserLocalStore";

-import {type EdgeEvaluator} from "../../core/attribution/pagerank";
+import {defaultStaticAdapters} from "../adapters/defaultPlugins";
 import {PagerankTable} from "./pagerankTable/Table";
 import {WeightConfig} from "./WeightConfig";
+import {
+  type WeightedTypes,
+  defaultWeightsForAdapterSet,
+} from "./weights/weights";
 import RepositorySelect from "./RepositorySelect";
 import {_Prefix as GithubPrefix} from "../../plugins/github/nodes";
 import {
@ -20,7 +23,6 @@ import {
  uninitializedState,
 } from "./state";
 import {StaticAdapterSet} from "../adapters/adapterSet";
-import {defaultStaticAdapters} from "../adapters/defaultPlugins";

 export default class AppPage extends React.Component<{|+assets: Assets|}> {
  static _LOCAL_STORE = new CheckedLocalStore(
@ -49,7 +51,7 @@ type Props = {|
 |};
 type State = {|
  appState: AppState,
-  edgeEvaluator: ?EdgeEvaluator,
+  weightedTypes: WeightedTypes,
 |};

 export function createApp(
@ -65,7 +67,7 @@ export function createApp(
      super(props);
      this.state = {
        appState: uninitializedState(),
-        edgeEvaluator: null,
+        weightedTypes: defaultWeightsForAdapterSet(props.adapters),
      };
      this.stateTransitionMachine = createSTM(
        () => this.state.appState,
@ -115,14 +117,13 @@ export function createApp(
          <button
            disabled={
              appState.type === "UNINITIALIZED" ||
-              appState.loading === "LOADING" ||
-              this.state.edgeEvaluator == null
+              appState.loading === "LOADING"
            }
            onClick={() =>
              this.stateTransitionMachine.loadGraphAndRunPagerank(
                this.props.assets,
                this.props.adapters,
-                NullUtil.get(this.state.edgeEvaluator),
+                this.state.weightedTypes,
                GithubPrefix.user
              )
            }
@ -130,7 +131,7 @@ export function createApp(
            Analyze cred
          </button>
          <WeightConfig
-            onChange={(edgeEvaluator) => this.setState({edgeEvaluator})}
+            onChange={(weightedTypes) => this.setState({weightedTypes})}
            adapters={this.props.adapters}
          />
          <LoadingIndicator appState={this.state.appState} />
--- a/src/app/credExplorer/App.test.js
+++ b/src/app/credExplorer/App.test.js
@ -8,6 +8,8 @@ import {makeRepo} from "../../core/repo";
 import {Assets} from "../assets";
 import testLocalStore from "../testLocalStore";
 import {DynamicAdapterSet, StaticAdapterSet} from "../adapters/adapterSet";
+import {FactorioStaticAdapter} from "../adapters/demoAdapters";
+import {defaultWeightsForAdapter} from "./weights/weights";

 import RepositorySelect from "./RepositorySelect";
 import {PagerankTable} from "./pagerankTable/Table";
@ -59,12 +61,6 @@ describe("app/credExplorer/App", () => {
    };
  }

-  function createEvaluator() {
-    return function(_unused_edge) {
-      return {toWeight: 1, froWeight: 1};
-    };
-  }
-
  const emptyAdapters = new DynamicAdapterSet(new StaticAdapterSet([]), []);
  const exampleStates = {
    uninitialized: uninitializedState,
@ -135,9 +131,10 @@ describe("app/credExplorer/App", () => {
        const {el, setState} = example();
        setState(stateFn());
        const wc = el.find(WeightConfig);
-        const ee = createEvaluator();
-        wc.props().onChange(ee);
-        expect(el.state().edgeEvaluator).toBe(ee);
+        const wt = defaultWeightsForAdapter(new FactorioStaticAdapter());
+        wc.props().onChange(wt);
+        expect(el.state().weightedTypes).toBe(wt);
+        expect(wc.props().adapters).toBe(el.instance().props.adapters);
      });
    }

@ -146,8 +143,6 @@ describe("app/credExplorer/App", () => {
      it(`has a ${adjective} analyze cred button`, () => {
        const {el, loadGraphAndRunPagerank, setState} = example();
        setState(stateFn());
-        const edgeEvaluator = createEvaluator();
-        el.setState({edgeEvaluator});
        el.update();
        const button = el.findWhere(
          (b) => b.text() === "Analyze cred" && b.is("button")
@ -161,22 +156,11 @@ describe("app/credExplorer/App", () => {
          expect(loadGraphAndRunPagerank).toBeCalledWith(
            el.instance().props.assets,
            el.instance().props.adapters,
-            edgeEvaluator,
+            el.instance().state.weightedTypes,
            GithubPrefix.user
          );
        }
      });
-      if (!disabled) {
-        it("...unless the EdgeEvaluator is not available", () => {
-          const {el, setState} = example();
-          setState(stateFn());
-          el.update();
-          const button = el.findWhere(
-            (b) => b.text() === "Analyze cred" && b.is("button")
-          );
-          expect(button.props().disabled).toBe(true);
-        });
-      }
    }

    function testPagerankTable(stateFn, present: boolean) {
--- a/src/app/credExplorer/WeightConfig.js
+++ b/src/app/credExplorer/WeightConfig.js
@ -3,8 +3,6 @@
 import React from "react";
 import * as NullUtil from "../../util/null";

-import {type EdgeEvaluator} from "../../core/attribution/pagerank";
-import {weightsToEdgeEvaluator} from "./weights/weightsToEdgeEvaluator";
 import type {StaticPluginAdapter} from "../adapters/pluginAdapter";
 import type {StaticAdapterSet} from "../adapters/adapterSet";
 import {
@ -17,7 +15,7 @@ import {FALLBACK_NAME} from "../adapters/fallbackAdapter";

 type Props = {|
  +adapters: StaticAdapterSet,
-  +onChange: (EdgeEvaluator) => void,
+  +onChange: (WeightedTypes) => void,
 |};

 type State = {
@ -94,7 +92,6 @@ export class WeightConfig extends React.Component<Props, State> {
          )
        )
    );
-    const edgeEvaluator = weightsToEdgeEvaluator(weights);
-    this.props.onChange(edgeEvaluator);
+    this.props.onChange(weights);
  }
 }
--- a/src/app/credExplorer/state.js
+++ b/src/app/credExplorer/state.js
@ -13,6 +13,8 @@ import {
 } from "../../core/attribution/pagerank";

 import {StaticAdapterSet, DynamicAdapterSet} from "../adapters/adapterSet";
+import type {WeightedTypes} from "./weights/weights";
+import {weightsToEdgeEvaluator} from "./weights/weightsToEdgeEvaluator";

 /*
  This models the UI states of the credExplorer/App as a state machine.
@ -71,11 +73,11 @@ export function uninitializedState(): AppState {
 export interface StateTransitionMachineInterface {
  +setRepo: (Repo) => void;
  +loadGraph: (Assets, StaticAdapterSet) => Promise<boolean>;
-  +runPagerank: (EdgeEvaluator, NodeAddressT) => Promise<void>;
+  +runPagerank: (WeightedTypes, NodeAddressT) => Promise<void>;
  +loadGraphAndRunPagerank: (
    Assets,
    StaticAdapterSet,
-    EdgeEvaluator,
+    WeightedTypes,
    NodeAddressT
  ) => Promise<void>;
 }
@ -165,7 +167,7 @@ export class StateTransitionMachine implements StateTransitionMachineInterface {
  }

  async runPagerank(
-    edgeEvaluator: EdgeEvaluator,
+    weightedTypes: WeightedTypes,
    totalScoreNodePrefix: NodeAddressT
  ) {
    const state = this.getState();
@ -186,7 +188,7 @@ export class StateTransitionMachine implements StateTransitionMachineInterface {
    try {
      const pagerankNodeDecomposition = await this.pagerank(
        graph,
-        edgeEvaluator,
+        weightsToEdgeEvaluator(weightedTypes),
        {
          verbose: true,
          totalScoreNodePrefix: totalScoreNodePrefix,
@ -215,7 +217,7 @@ export class StateTransitionMachine implements StateTransitionMachineInterface {
  async loadGraphAndRunPagerank(
    assets: Assets,
    adapters: StaticAdapterSet,
-    edgeEvaluator: EdgeEvaluator,
+    weightedTypes: WeightedTypes,
    totalScoreNodePrefix: NodeAddressT
  ) {
    const state = this.getState();
@ -227,12 +229,12 @@ export class StateTransitionMachine implements StateTransitionMachineInterface {
      case "READY_TO_LOAD_GRAPH":
        const loadedGraph = await this.loadGraph(assets, adapters);
        if (loadedGraph) {
-          await this.runPagerank(edgeEvaluator, totalScoreNodePrefix);
+          await this.runPagerank(weightedTypes, totalScoreNodePrefix);
        }
        break;
      case "READY_TO_RUN_PAGERANK":
      case "PAGERANK_EVALUATED":
-        await this.runPagerank(edgeEvaluator, totalScoreNodePrefix);
+        await this.runPagerank(weightedTypes, totalScoreNodePrefix);
        break;
      default:
        throw new Error((type: empty));
--- a/src/app/credExplorer/state.test.js
+++ b/src/app/credExplorer/state.test.js
@ -11,11 +11,16 @@ import {Graph, NodeAddress} from "../../core/graph";
 import {Assets} from "../assets";
 import {makeRepo, type Repo} from "../../core/repo";
 import {type EdgeEvaluator} from "../../core/attribution/pagerank";
+import {
+  type WeightedTypes,
+  defaultWeightsForAdapterSet,
+} from "./weights/weights";
 import {StaticAdapterSet, DynamicAdapterSet} from "../adapters/adapterSet";
 import type {
  PagerankNodeDecomposition,
  PagerankOptions,
 } from "../../core/attribution/pagerank";
+import {staticAdapterSet} from "../adapters/demoAdapters";

 describe("app/credExplorer/state", () => {
  function example(startingState: AppState) {
@ -66,8 +71,8 @@ describe("app/credExplorer/state", () => {
      loading: "NOT_LOADING",
    };
  }
-  function edgeEvaluator(): EdgeEvaluator {
-    return (_unused_Edge) => ({toWeight: 3, froWeight: 4});
+  function weightedTypes(): WeightedTypes {
+    return defaultWeightsForAdapterSet(staticAdapterSet());
  }
  function graphWithAdapters(): GraphWithAdapters {
    return {
@ -224,7 +229,7 @@ describe("app/credExplorer/state", () => {
      for (const b of badStates) {
        const {stm} = example(b);
        await expect(
-          stm.runPagerank(edgeEvaluator(), NodeAddress.empty)
+          stm.runPagerank(weightedTypes(), NodeAddress.empty)
        ).rejects.toThrow("incorrect state");
      }
    });
@ -234,7 +239,7 @@ describe("app/credExplorer/state", () => {
        const {stm, getState, pagerankMock} = example(g);
        const pnd = pagerankNodeDecomposition();
        pagerankMock.mockResolvedValue(pnd);
-        await stm.runPagerank(edgeEvaluator(), NodeAddress.empty);
+        await stm.runPagerank(weightedTypes(), NodeAddress.empty);
        const state = getState();
        if (state.type !== "PAGERANK_EVALUATED") {
          throw new Error("Impossible");
@ -246,16 +251,14 @@ describe("app/credExplorer/state", () => {
    it("immediately sets loading status", () => {
      const {getState, stm} = example(readyToRunPagerank());
      expect(loading(getState())).toBe("NOT_LOADING");
-      stm.runPagerank(edgeEvaluator(), NodeAddress.empty);
+      stm.runPagerank(weightedTypes(), NodeAddress.empty);
      expect(loading(getState())).toBe("LOADING");
    });
    it("calls pagerank with the totalScoreNodePrefix option", async () => {
      const {pagerankMock, stm} = example(readyToRunPagerank());
      const foo = NodeAddress.fromParts(["foo"]);
-      const ee = edgeEvaluator();
-      await stm.runPagerank(ee, foo);
+      await stm.runPagerank(weightedTypes(), foo);
      const args = pagerankMock.mock.calls[0];
-      expect(args[1]).toBe(ee);
      expect(args[2].totalScoreNodePrefix).toBe(foo);
    });
    it("does not transition if a repo change happens first", async () => {
@ -268,7 +271,7 @@ describe("app/credExplorer/state", () => {
            resolve(graphWithAdapters());
          })
      );
-      await stm.runPagerank(edgeEvaluator(), NodeAddress.empty);
+      await stm.runPagerank(weightedTypes(), NodeAddress.empty);
      const state = getState();
      expect(loading(state)).toBe("NOT_LOADING");
      expect(state.type).toBe("READY_TO_LOAD_GRAPH");
@ -280,7 +283,7 @@ describe("app/credExplorer/state", () => {
      // $ExpectFlowError
      console.error = jest.fn();
      pagerankMock.mockRejectedValue(error);
-      await stm.runPagerank(edgeEvaluator(), NodeAddress.empty);
+      await stm.runPagerank(weightedTypes(), NodeAddress.empty);
      const state = getState();
      expect(loading(state)).toBe("FAILED");
      expect(state.type).toBe("READY_TO_RUN_PAGERANK");
@ -296,7 +299,7 @@ describe("app/credExplorer/state", () => {
        stm.loadGraphAndRunPagerank(
          new Assets("gateway"),
          new StaticAdapterSet([]),
-          edgeEvaluator(),
+          weightedTypes(),
          NodeAddress.empty
        )
      ).rejects.toThrow("incorrect state");
@ -309,12 +312,12 @@ describe("app/credExplorer/state", () => {
      const assets = new Assets("/gateway/");
      const adapters = new StaticAdapterSet([]);
      const prefix = NodeAddress.fromParts(["bar"]);
-      const ee = edgeEvaluator();
-      await stm.loadGraphAndRunPagerank(assets, adapters, ee, prefix);
+      const wt = weightedTypes();
+      await stm.loadGraphAndRunPagerank(assets, adapters, wt, prefix);
      expect(stm.loadGraph).toHaveBeenCalledTimes(1);
      expect(stm.loadGraph).toHaveBeenCalledWith(assets, adapters);
      expect(stm.runPagerank).toHaveBeenCalledTimes(1);
-      expect(stm.runPagerank).toHaveBeenCalledWith(ee, prefix);
+      expect(stm.runPagerank).toHaveBeenCalledWith(wt, prefix);
    });
    it("does not run pagerank if loadGraph did not succeed", async () => {
      const {stm} = example(readyToLoadGraph());
@ -327,7 +330,7 @@ describe("app/credExplorer/state", () => {
      await stm.loadGraphAndRunPagerank(
        assets,
        adapters,
-        edgeEvaluator(),
+        weightedTypes(),
        prefix
      );
      expect(stm.loadGraph).toHaveBeenCalledTimes(1);
@ -338,32 +341,32 @@ describe("app/credExplorer/state", () => {
      (stm: any).loadGraph = jest.fn();
      (stm: any).runPagerank = jest.fn();
      const prefix = NodeAddress.fromParts(["bar"]);
-      const ee = edgeEvaluator();
+      const wt = weightedTypes();
      await stm.loadGraphAndRunPagerank(
        new Assets("/gateway/"),
        new StaticAdapterSet([]),
-        ee,
+        wt,
        prefix
      );
      expect(stm.loadGraph).toHaveBeenCalledTimes(0);
      expect(stm.runPagerank).toHaveBeenCalledTimes(1);
-      expect(stm.runPagerank).toHaveBeenCalledWith(ee, prefix);
+      expect(stm.runPagerank).toHaveBeenCalledWith(wt, prefix);
    });
    it("when PAGERANK_EVALUATED, runs pagerank", async () => {
      const {stm} = example(pagerankEvaluated());
      (stm: any).loadGraph = jest.fn();
      (stm: any).runPagerank = jest.fn();
      const prefix = NodeAddress.fromParts(["bar"]);
-      const ee = edgeEvaluator();
+      const wt = weightedTypes();
      await stm.loadGraphAndRunPagerank(
        new Assets("/gateway/"),
        new StaticAdapterSet([]),
-        ee,
+        wt,
        prefix
      );
      expect(stm.loadGraph).toHaveBeenCalledTimes(0);
      expect(stm.runPagerank).toHaveBeenCalledTimes(1);
-      expect(stm.runPagerank).toHaveBeenCalledWith(ee, prefix);
+      expect(stm.runPagerank).toHaveBeenCalledWith(wt, prefix);
    });
  });
 });