api: add support for loading Discourse servers (#1325)

This commit modifies the `Project` type so that it allows settings for a
Discourse server, and ensures that `api/load` will appropriately load
the server in question, and include it in the output graph.

Putting the full Discourse declaration directly into the Project type is
an unsustainable development practice—in general, adding plugins should
not require changing core data types. However, at the moment I'm punting
on polishing the plugin system, in favor of adding the Discourse plugin
quickly, so I just put it into Project alongside the repo ids.

In the future, I expect to refactor the plugins around a much cleaner
interface; it's just not a priority as yet. (Tracking: #1120.)

This commit also makes the GitHub token optional in `api/load`, since
now it's very plausible that a user will want to only load a Discourse
server, and therefore not require a GitHub token.

As of this commit, it's still impossible to load Discourse projects, as
the CLI always sets a null Discourse server; and in any case, the
frontend would not properly display the project in question, as any
Discourse types would get filtered out.

Test plan: Mocking unit tests have been added to `api/load.test.js` to
ensure that the Discourse graph is loaded and merged correctly.

src/api/load.js

@@ -4,6 +4,7 @@ import fs from "fs-extra";
 import path from "path";
 
 import {TaskReporter} from "../util/taskReporter";
+import {Graph} from "../core/graph";
 import {loadGraph} from "../plugins/github/loadGraph";
 import {
   type TimelineCredParameters,
@@ -14,12 +15,15 @@ import {DEFAULT_CRED_CONFIG} from "../plugins/defaultCredConfig";
 
 import {type Project} from "../core/project";
 import {setupProjectDirectory} from "../core/project_io";
+import {loadDiscourse} from "../plugins/discourse/loadDiscourse";
+import * as NullUtil from "../util/null";
 
 export type LoadOptions = {|
   +project: Project,
   +params: TimelineCredParameters,
   +sourcecredDirectory: string,
-  +githubToken: string,
+  +githubToken: string | null,
+  +discourseKey: string | null,
 |};
 
 /**
@@ -46,13 +50,48 @@ export async function load(
   const cacheDirectory = path.join(sourcecredDirectory, "cache");
   await fs.mkdirp(cacheDirectory);
 
-  // future: support loading more plugins, and merging their graphs
-  const githubOptions = {
-    repoIds: project.repoIds,
-    token: githubToken,
-    cacheDirectory,
-  };
-  const graph = await loadGraph(githubOptions, taskReporter);
+  function discourseGraph(): ?Promise<Graph> {
+    const discourseServer = project.discourseServer;
+    if (discourseServer != null) {
+      const {serverUrl, apiUsername} = discourseServer;
+      if (options.discourseKey == null) {
+        throw new Error("Tried to load Discourse, but no Discourse key set");
+      }
+      const discourseOptions = {
+        fetchOptions: {apiKey: options.discourseKey, serverUrl, apiUsername},
+        cacheDirectory,
+      };
+      return loadDiscourse(discourseOptions, taskReporter);
+    }
+  }
+
+  function githubGraph(): ?Promise<Graph> {
+    if (project.repoIds.length) {
+      if (githubToken == null) {
+        throw new Error("Tried to load GitHub, but no GitHub token set.");
+      }
+      const githubOptions = {
+        repoIds: project.repoIds,
+        token: githubToken,
+        cacheDirectory,
+      };
+
+      return loadGraph(githubOptions, taskReporter);
+    }
+  }
+
+  // For each plugin that wants to provide a Graph, get a Promise for the
+  // graph. That way we can request them in parallel, via Promise.all, rather
+  // than blocking on the plugins sequentially.
+  // Since plugins often perform rate-limited IO, this may be a big performance
+  // improvement.
+  const pluginGraphPromises: Promise<Graph>[] = NullUtil.filter([
+    discourseGraph(),
+    githubGraph(),
+  ]);
+
+  const pluginGraphs = await Promise.all(pluginGraphPromises);
+  const graph = Graph.merge(pluginGraphs);
 
   const projectDirectory = await setupProjectDirectory(
     project,

src/api/load.test.js

@@ -6,6 +6,7 @@ import path from "path";
 import fs from "fs-extra";
 
 import type {Options as LoadGraphOptions} from "../plugins/github/loadGraph";
+import type {Options as LoadDiscourseOptions} from "../plugins/discourse/loadDiscourse";
 import type {Project} from "../core/project";
 import {
   directoryForProjectId,
@@ -14,7 +15,8 @@ import {
 } from "../core/project_io";
 import {makeRepoId} from "../core/repoId";
 import {defaultWeights} from "../analysis/weights";
-import {NodeAddress} from "../core/graph";
+import {NodeAddress, Graph} from "../core/graph";
+import {node} from "../core/graphTestUtil";
 import {TestTaskReporter} from "../util/taskReporter";
 import {load, type LoadOptions} from "./load";
 import {DEFAULT_CRED_CONFIG} from "../plugins/defaultCredConfig";
@@ -25,6 +27,11 @@ jest.mock("../plugins/github/loadGraph", () => ({
 }));
 const loadGraph: JestMockFn = (require("../plugins/github/loadGraph")
   .loadGraph: any);
+jest.mock("../plugins/discourse/loadDiscourse", () => ({
+  loadDiscourse: jest.fn(),
+}));
+const loadDiscourse: JestMockFn = (require("../plugins/discourse/loadDiscourse")
+  .loadDiscourse: any);
 
 jest.mock("../analysis/timeline/timelineCred", () => ({
   TimelineCred: {compute: jest.fn()},
@@ -36,17 +43,30 @@ describe("api/load", () => {
   const fakeTimelineCred = deepFreeze({
     toJSON: () => ({is: "fake-timeline-cred"}),
   });
-  const fakeGraph = deepFreeze({toJSON: () => ({is: "fake-graph"})});
+  const githubSentinel = node("github-sentinel");
+  const githubGraph = () => new Graph().addNode(githubSentinel);
+  const discourseSentinel = node("discourse-sentinel");
+  const discourseGraph = () => new Graph().addNode(discourseSentinel);
+  const combinedGraph = () => Graph.merge([githubGraph(), discourseGraph()]);
   beforeEach(() => {
     jest.clearAllMocks();
-    loadGraph.mockResolvedValue(fakeGraph);
+    loadGraph.mockResolvedValue(githubGraph());
+    loadDiscourse.mockResolvedValue(discourseGraph());
     timelineCredCompute.mockResolvedValue(fakeTimelineCred);
   });
-  const project: Project = deepFreeze({
+  const discourseServerUrl = "https://example.com";
+  const discourseApiUsername = "credbot";
+  const project: Project = {
     id: "foo",
     repoIds: [makeRepoId("foo", "bar")],
-  });
+    discourseServer: {
+      serverUrl: discourseServerUrl,
+      apiUsername: discourseApiUsername,
+    },
+  };
+  deepFreeze(project);
   const githubToken = "EXAMPLE_TOKEN";
+  const discourseKey = "EXAMPLE_KEY";
   const weights = defaultWeights();
   // Tweaks the weights so that we can ensure we aren't overriding with default weights
   weights.nodeManualWeights.set(NodeAddress.empty, 33);
@@ -60,6 +80,7 @@ describe("api/load", () => {
       githubToken,
       params,
       project,
+      discourseKey,
     };
     return {options, taskReporter, sourcecredDirectory};
   };
@@ -86,7 +107,22 @@ describe("api/load", () => {
     );
   });
 
-  it("saves the resultant graph to disk", async () => {
+  it("calls loadDiscourse with the right options", async () => {
+    const {options, taskReporter, sourcecredDirectory} = example();
+    await load(options, taskReporter);
+    const cacheDirectory = path.join(sourcecredDirectory, "cache");
+    const expectedOptions: LoadDiscourseOptions = {
+      fetchOptions: {
+        apiUsername: discourseApiUsername,
+        apiKey: discourseKey,
+        serverUrl: discourseServerUrl,
+      },
+      cacheDirectory,
+    };
+    expect(loadDiscourse).toHaveBeenCalledWith(expectedOptions, taskReporter);
+  });
+
+  it("saves a merged graph to disk", async () => {
     const {options, taskReporter, sourcecredDirectory} = example();
     await load(options, taskReporter);
     const projectDirectory = directoryForProjectId(
@@ -95,17 +131,21 @@ describe("api/load", () => {
     );
     const graphFile = path.join(projectDirectory, "graph.json");
     const graphJSON = JSON.parse(await fs.readFile(graphFile));
-    expect(graphJSON).toEqual(fakeGraph.toJSON());
+    const expectedJSON = combinedGraph().toJSON();
+    expect(graphJSON).toEqual(expectedJSON);
   });
 
   it("calls TimelineCred.compute with the right graph and options", async () => {
     const {options, taskReporter} = example();
     await load(options, taskReporter);
     expect(timelineCredCompute).toHaveBeenCalledWith(
-      fakeGraph,
+      expect.anything(),
       params,
       DEFAULT_CRED_CONFIG
     );
+    expect(timelineCredCompute.mock.calls[0][0].equals(combinedGraph())).toBe(
+      true
+    );
   });
 
   it("saves the resultant cred.json to disk", async () => {
@@ -131,4 +171,54 @@ describe("api/load", () => {
       {type: "FINISH", taskId: "load-foo"},
     ]);
   });
+
+  it("errors if a discourse server is provided without a discourse key", () => {
+    const {options, taskReporter} = example();
+    const optionsWithoutKey = {...options, discourseKey: null};
+    expect.assertions(1);
+    return load(optionsWithoutKey, taskReporter).catch((e) =>
+      expect(e.message).toMatch("no Discourse key")
+    );
+  });
+
+  it("errors if GitHub repoIds are provided without a GitHub token", () => {
+    const {options, taskReporter} = example();
+    const optionsWithoutToken = {...options, githubToken: null};
+    expect.assertions(1);
+    return load(optionsWithoutToken, taskReporter).catch((e) =>
+      expect(e.message).toMatch("no GitHub token")
+    );
+  });
+
+  it("only loads GitHub if no Discourse server set", async () => {
+    const {options, taskReporter, sourcecredDirectory} = example();
+    const newProject = {...options.project, discourseServer: null};
+    const newOptions = {...options, project: newProject, discourseKey: null};
+    await load(newOptions, taskReporter);
+    expect(loadDiscourse).not.toHaveBeenCalled();
+    const projectDirectory = directoryForProjectId(
+      project.id,
+      sourcecredDirectory
+    );
+    const graphFile = path.join(projectDirectory, "graph.json");
+    const graphJSON = JSON.parse(await fs.readFile(graphFile));
+    const expectedJSON = githubGraph().toJSON();
+    expect(graphJSON).toEqual(expectedJSON);
+  });
+
+  it("only loads Discourse if no GitHub repoIds set ", async () => {
+    const {options, taskReporter, sourcecredDirectory} = example();
+    const newProject = {...options.project, repoIds: []};
+    const newOptions = {...options, project: newProject, githubToken: null};
+    await load(newOptions, taskReporter);
+    expect(loadGraph).not.toHaveBeenCalled();
+    const projectDirectory = directoryForProjectId(
+      project.id,
+      sourcecredDirectory
+    );
+    const graphFile = path.join(projectDirectory, "graph.json");
+    const graphJSON = JSON.parse(await fs.readFile(graphFile));
+    const expectedJSON = discourseGraph().toJSON();
+    expect(graphJSON).toEqual(expectedJSON);
+  });
 });

src/cli/load.js

@@ -110,6 +110,7 @@ const loadCommand: Command = async (args, std) => {
     params,
     sourcecredDirectory: Common.sourcecredDirectory(),
     githubToken,
+    discourseKey: null,
   }));
   // Deliberately load in serial because GitHub requests that their API not
   // be called concurrently

src/cli/load.test.js

@@ -7,6 +7,7 @@ import {LoggingTaskReporter} from "../util/taskReporter";
 import {NodeAddress} from "../core/graph";
 import {run} from "./testUtil";
 import loadCommand, {help} from "./load";
+import type {LoadOptions} from "../api/load";
 import {defaultWeights, toJSON as weightsToJSON} from "../analysis/weights";
 import * as Common from "./common";
 
@@ -67,11 +68,16 @@ describe("cli/load", () => {
 
     it("calls load with a single repo", async () => {
       const invocation = run(loadCommand, ["foo/bar"]);
-      const expectedOptions = {
-        project: {id: "foo/bar", repoIds: [makeRepoId("foo", "bar")]},
+      const expectedOptions: LoadOptions = {
+        project: {
+          id: "foo/bar",
+          repoIds: [makeRepoId("foo", "bar")],
+          discourseServer: null,
+        },
         params: {alpha: 0.05, intervalDecay: 0.5, weights: defaultWeights()},
         sourcecredDirectory: Common.sourcecredDirectory(),
         githubToken: fakeGithubToken,
+        discourseKey: null,
       };
       expect(await invocation).toEqual({
         exitCode: 0,
@@ -86,11 +92,16 @@ describe("cli/load", () => {
 
     it("calls load with multiple repos", async () => {
       const invocation = run(loadCommand, ["foo/bar", "zoink/zod"]);
-      const expectedOptions = (projectId: string) => ({
-        project: {id: projectId, repoIds: [stringToRepoId(projectId)]},
+      const expectedOptions: (string) => LoadOptions = (projectId: string) => ({
+        project: {
+          id: projectId,
+          repoIds: [stringToRepoId(projectId)],
+          discourseServer: null,
+        },
         params: {alpha: 0.05, intervalDecay: 0.5, weights: defaultWeights()},
         sourcecredDirectory: Common.sourcecredDirectory(),
         githubToken: fakeGithubToken,
+        discourseKey: null,
       });
       expect(await invocation).toEqual({
         exitCode: 0,
@@ -118,11 +129,16 @@ describe("cli/load", () => {
         "--weights",
         weightsFile,
       ]);
-      const expectedOptions = {
-        project: {id: "foo/bar", repoIds: [makeRepoId("foo", "bar")]},
+      const expectedOptions: LoadOptions = {
+        project: {
+          id: "foo/bar",
+          repoIds: [makeRepoId("foo", "bar")],
+          discourseServer: null,
+        },
         params: {alpha: 0.05, intervalDecay: 0.5, weights},
         sourcecredDirectory: Common.sourcecredDirectory(),
         githubToken: fakeGithubToken,
+        discourseKey: null,
       };
       expect(await invocation).toEqual({
         exitCode: 0,

src/core/project.js

@@ -12,8 +12,9 @@ export type ProjectId = string;
  * Right now it has an `id` (which should be unique across a user's projects)
  * and an array of GitHub RepoIds.
  *
- * In the future, we will add support for more plugins (and remove the
- * hardcoded GitHub support).
+ * In the future, instead of hardcoding support for plugins like GitHub and Discourse,
+ * we will have a generic system for storing plugin-specific config, keyed by plugin
+ * identifier.
  *
  * We may add more fields (e.g. a description) to this object in the futre.
  *
@@ -24,6 +25,10 @@ export type ProjectId = string;
 export type Project = {|
   +id: ProjectId,
   +repoIds: $ReadOnlyArray<RepoId>,
+  +discourseServer: {|
+    +serverUrl: string,
+    +apiUsername: string,
+  |} | null,
 |};
 
 const COMPAT_INFO = {type: "sourcecred/project", version: "0.1.0"};

src/core/project.test.js

@@ -17,10 +17,12 @@ describe("core/project", () => {
   const p1: Project = deepFreeze({
     id: "foo/bar",
     repoIds: [foobar],
+    discourseServer: null,
   });
   const p2: Project = deepFreeze({
     id: "@foo",
     repoIds: [foobar, foozod],
+    discourseServer: {serverUrl: "https://example.com", apiUsername: "credbot"},
   });
   describe("to/fro JSON", () => {
     it("round trip is identity", () => {

src/core/project_io.test.js

@@ -22,10 +22,12 @@ describe("core/project_io", () => {
   const p1: Project = deepFreeze({
     id: "foo/bar",
     repoIds: [foobar],
+    discourseServer: null,
   });
   const p2: Project = deepFreeze({
     id: "@foo",
     repoIds: [foobar, foozod],
+    discourseServer: {serverUrl: "https://example.com", apiUsername: "credbot"},
   });
 
   it("setupProjectDirectory results in a loadable project", async () => {

src/plugins/github/specToProject.js

@@ -36,12 +36,17 @@ export async function specToProject(
     const project: Project = {
       id: spec,
       repoIds: [stringToRepoId(spec)],
+      discourseServer: null,
     };
     return project;
   } else if (spec.match(ownerSpecMatcher)) {
     const owner = spec.slice(1);
     const org = await fetchGithubOrg(owner, token);
-    const project: Project = {id: spec, repoIds: org.repos};
+    const project: Project = {
+      id: spec,
+      repoIds: org.repos,
+      discourseServer: null,
+    };
     return project;
   }
   throw new Error(`invalid spec: ${spec}`);

src/plugins/github/specToProject.test.js

@@ -2,6 +2,7 @@
 
 import {specToProject} from "./specToProject";
 import {stringToRepoId} from "../../core/repoId";
+import {type Project} from "../../core/project";
 jest.mock("./fetchGithubOrg", () => ({fetchGithubOrg: jest.fn()}));
 type JestMockFn = $Call<typeof jest.fn>;
 const fetchGithubOrg: JestMockFn = (require("./fetchGithubOrg")
@@ -13,9 +14,10 @@ describe("plugins/github/specToProject", () => {
   });
   it("works for a repoId", async () => {
     const spec = "foo/bar";
-    const expected = {
+    const expected: Project = {
       id: spec,
       repoIds: [stringToRepoId(spec)],
+      discourseServer: null,
     };
     const actual = await specToProject(spec, "FAKE_TOKEN");
     expect(expected).toEqual(actual);
@@ -29,7 +31,7 @@ describe("plugins/github/specToProject", () => {
     fetchGithubOrg.mockResolvedValueOnce(fakeOrg);
     const actual = await specToProject(spec, token);
     expect(fetchGithubOrg).toHaveBeenCalledWith(fakeOrg.name, token);
-    const expected = {id: spec, repoIds: repos};
+    const expected: Project = {id: spec, repoIds: repos, discourseServer: null};
     expect(actual).toEqual(expected);
   });
   describe("fails for malformed spec strings", () => {