identity plugin provides a unified IdentitySpec (#1603)

This commit contains a slight refactor to the identity plugin so that it
provides a unified `IdentitySpec` type which wraps the list of
Identities with the metadata (currently a discourse server url) needed
to interpret those identities. This makes the API slightly nicer to use.

Test plan: Simple refactor; `yarn test` is sufficient.

src/api/load.js

@@ -91,10 +91,13 @@ export async function load(
   const pluginGraphs = await Promise.all(pluginGraphPromises);
   let graph = Graph.merge(pluginGraphs);
   const {identities, discourseServer} = project;
-  if (identities.length) {
+  const identitySpec = {
-    const serverUrl =
+    identities,
-      discourseServer == null ? null : discourseServer.serverUrl;
+    discourseServerUrl:
-    const contractions = nodeContractions(identities, serverUrl);
+      discourseServer == null ? null : discourseServer.serverUrl,
+  };
+  if (identitySpec.identities.length) {
+    const contractions = nodeContractions(identitySpec);
     // Only apply contractions if identities have been specified, since it involves
     // a full Graph copy
     graph = graph.contractNodes(contractions);

src/api/load.test.js

@@ -218,8 +218,9 @@ describe("api/load", () => {
     );
     const graphFile = path.join(projectDirectory, "graph.json");
     const graphJSON = JSON.parse(await fs.readFile(graphFile));
+    const identitySpec = {identities: [identity], discourseServerUrl};
     const identityGraph = combinedGraph().contractNodes(
-      nodeContractions([identity], discourseServerUrl)
+      nodeContractions(identitySpec)
     );
     const expectedJSON = identityGraph.toJSON();
     expect(graphJSON).toEqual(expectedJSON);

src/plugins/identity/contractIdentities.js

@@ -4,7 +4,7 @@ import * as Weights from "../../core/weights";
 import {type WeightedGraph as WeightedGraphT} from "../../core/weightedGraph";
 import {type NodeContraction, NodeAddress} from "../../core/graph";
 import {nodeContractions} from "./nodeContractions";
-import {type Identity} from "./identity";
+import {type IdentitySpec} from "./identity";
 
 /**
  * Applies nodeContractions to a WeightedGraph.
@@ -60,8 +60,7 @@ export function _contractWeightedGraph(
  */
 export function contractIdentities(
   wg: WeightedGraphT,
-  identities: $ReadOnlyArray<Identity>,
+  identitySpec: IdentitySpec
-  discourseUrl: string | null
 ): WeightedGraphT {
-  return _contractWeightedGraph(wg, nodeContractions(identities, discourseUrl));
+  return _contractWeightedGraph(wg, nodeContractions(identitySpec));
 }

src/plugins/identity/identity.js

@@ -19,6 +19,17 @@ export type Identity = {|
   +aliases: $ReadOnlyArray<Alias>,
 |};
 
+/**
+ * Fully specifies all Identity information.
+ *
+ * The discourseServerurl is needed if any Discourse aliases are present
+ * in the included identities.
+ */
+export type IdentitySpec = {|
+  +identities: $ReadOnlyArray<Identity>,
+  +discourseServerUrl: string | null,
+|};
+
 /**
  * Create a new node representing an identity.
  */

src/plugins/identity/nodeContractions.js

@@ -1,7 +1,7 @@
 // @flow
 
 import {type NodeContraction} from "../../core/graph";
-import {type Identity, identityNode} from "./identity";
+import {type Identity, identityNode, type IdentitySpec} from "./identity";
 import {resolveAlias} from "./alias";
 
 /**
@@ -20,10 +20,7 @@ import {resolveAlias} from "./alias";
  * refactor this method so it no longer takes a Discourse server url as a
  * special argument.
  */
-export function nodeContractions(
+export function nodeContractions(spec: IdentitySpec): NodeContraction[] {
-  identities: $ReadOnlyArray<Identity>,
-  discourseUrl: string | null
-): NodeContraction[] {
   function errorOnDuplicate(xs: $ReadOnlyArray<string>, kind: string) {
     const s = new Set();
     for (const x of xs) {
@@ -33,11 +30,12 @@ export function nodeContractions(
       s.add(x);
     }
   }
+  const {identities, discourseServerUrl} = spec;
   const usernames = identities.map((x) => x.username);
   errorOnDuplicate(usernames, "username");
   const aliases = [].concat(...identities.map((x) => x.aliases));
   errorOnDuplicate(aliases, "alias");
-  return identities.map((i) => _contraction(i, discourseUrl));
+  return identities.map((i) => _contraction(i, discourseServerUrl));
 }
 
 /**

src/plugins/identity/nodeContractions.test.js

@@ -41,27 +41,27 @@ describe("src/plugins/identity/nodeContractions", () => {
         {username: "foo", aliases: ["github/foo", "github/bar"]},
         {username: "foo", aliases: []},
       ];
-      expect(() => nodeContractions(identities, null)).toThrowError(
+      expect(() =>
-        "Duplicate username"
+        nodeContractions({identities, discourseServerUrl: null})
-      );
+      ).toThrowError("Duplicate username");
     });
     it("errors if any alias is duplicated", () => {
       const identities = [
         {username: "foo", aliases: ["github/foo", "github/bar"]},
         {username: "bar", aliases: ["github/foo"]},
       ];
-      expect(() => nodeContractions(identities, null)).toThrowError(
+      expect(() =>
-        "Duplicate alias"
+        nodeContractions({identities, discourseServerUrl: null})
-      );
+      ).toThrowError("Duplicate alias");
     });
     it("produces a contraction for each identity", () => {
       const identities = [
         {username: "foo", aliases: ["discourse/foo"]},
         {username: "bar", aliases: ["github/bar"]},
       ];
-      const url = "https://example.com";
+      const spec = {identities, discourseServerUrl: "https://example.com"};
-      expect(nodeContractions(identities, url)).toEqual(
+      expect(nodeContractions(spec)).toEqual(
-        identities.map((i) => _contraction(i, url))
+        identities.map((i) => _contraction(i, spec.discourseServerUrl))
       );
     });
   });