From e79cca6c6c5e0df08357ebf154f4b8712dd19727 Mon Sep 17 00:00:00 2001 From: Robin van Boven <497556+Beanow@users.noreply.github.com> Date: Sat, 16 Nov 2019 13:46:09 +0100 Subject: [PATCH] Discourse: add mirror options to 0.4.0 projects (#1451) N.b. this is an alternative to #1433, removing multi-server support for discourse. --- .../project.json | 2 +- src/api/load.js | 3 +- src/api/load.test.js | 8 ++--- src/core/project.js | 21 ++++++++---- src/core/project.test.js | 2 +- src/plugins/discourse/fetch.js | 1 + src/plugins/discourse/loadDiscourse.js | 26 +++++++-------- src/plugins/discourse/mirror.js | 32 +++++++++++++++++-- src/plugins/discourse/mirror.test.js | 14 ++++++-- 9 files changed, 74 insertions(+), 35 deletions(-) diff --git a/sharness/__snapshots__/example-github-load/projects/c291cmNlY3JlZC10ZXN0L2V4YW1wbGUtZ2l0aHVi/project.json b/sharness/__snapshots__/example-github-load/projects/c291cmNlY3JlZC10ZXN0L2V4YW1wbGUtZ2l0aHVi/project.json index 5f00c54..8301ab5 100644 --- a/sharness/__snapshots__/example-github-load/projects/c291cmNlY3JlZC10ZXN0L2V4YW1wbGUtZ2l0aHVi/project.json +++ b/sharness/__snapshots__/example-github-load/projects/c291cmNlY3JlZC10ZXN0L2V4YW1wbGUtZ2l0aHVi/project.json @@ -1 +1 @@ -[{"type":"sourcecred/project","version":"0.3.1"},{"discourseServer":null,"id":"sourcecred-test/example-github","identities":[],"repoIds":[{"name":"example-github","owner":"sourcecred-test"}]}] \ No newline at end of file +[{"type":"sourcecred/project","version":"0.4.0"},{"discourseServer":null,"id":"sourcecred-test/example-github","identities":[],"repoIds":[{"name":"example-github","owner":"sourcecred-test"}]}] \ No newline at end of file diff --git a/src/api/load.js b/src/api/load.js index 6b57b45..ebcdbb3 100644 --- a/src/api/load.js +++ b/src/api/load.js @@ -53,9 +53,8 @@ export async function load( function discourseGraph(): ?Promise { const discourseServer = project.discourseServer; if (discourseServer != null) { - const {serverUrl} = discourseServer; const discourseOptions = { - fetchOptions: {serverUrl}, + discourseServer, cacheDirectory, }; return loadDiscourse(discourseOptions, taskReporter); diff --git a/src/api/load.test.js b/src/api/load.test.js index f83ba23..f813abc 100644 --- a/src/api/load.test.js +++ b/src/api/load.test.js @@ -62,9 +62,7 @@ describe("api/load", () => { const project: Project = { id: "foo", repoIds: [makeRepoId("foo", "bar")], - discourseServer: { - serverUrl: discourseServerUrl, - }, + discourseServer: {serverUrl: discourseServerUrl}, identities: [], }; deepFreeze(project); @@ -115,9 +113,7 @@ describe("api/load", () => { await load(options, taskReporter); const cacheDirectory = path.join(sourcecredDirectory, "cache"); const expectedOptions: LoadDiscourseOptions = { - fetchOptions: { - serverUrl: discourseServerUrl, - }, + discourseServer: {serverUrl: discourseServerUrl}, cacheDirectory, }; expect(loadDiscourse).toHaveBeenCalledWith(expectedOptions, taskReporter); diff --git a/src/core/project.js b/src/core/project.js index 5ea44d4..476a771 100644 --- a/src/core/project.js +++ b/src/core/project.js @@ -4,6 +4,7 @@ import base64url from "base64url"; import {type RepoId} from "../core/repoId"; import {toCompat, fromCompat, type Compatible} from "../util/compat"; import {type Identity} from "../plugins/identity/identity"; +import {type DiscourseServer} from "../plugins/discourse/loadDiscourse"; export type ProjectId = string; @@ -17,7 +18,7 @@ export type ProjectId = string; * 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. + * We may add more fields (e.g. a description) to this object in the future. * * We may create a complimentary object with load/cache info for the project in * the future (e.g. showing the last update time for each of the project's data @@ -26,16 +27,22 @@ export type ProjectId = string; export type Project = {| +id: ProjectId, +repoIds: $ReadOnlyArray, - +discourseServer: {| - +serverUrl: string, - +apiUsername?: string, - |} | null, + +discourseServer: DiscourseServer | null, +identities: $ReadOnlyArray, |}; -const COMPAT_INFO = {type: "sourcecred/project", version: "0.3.1"}; +const COMPAT_INFO = {type: "sourcecred/project", version: "0.4.0"}; -const upgrades = {"0.3.0": (p) => p}; +const upgradeFrom030 = (p) => ({ + ...p, + discourseServer: + p.discourseServer != null ? {serverUrl: p.discourseServer.serverUrl} : null, +}); + +const upgrades = { + "0.3.0": upgradeFrom030, + "0.3.1": upgradeFrom030, +}; export type ProjectJSON = Compatible; diff --git a/src/core/project.test.js b/src/core/project.test.js index 5db12dc..06da27b 100644 --- a/src/core/project.test.js +++ b/src/core/project.test.js @@ -31,7 +31,7 @@ describe("core/project", () => { }, ], }); - describe("to/fro JSON", () => { + describe("to/from JSON", () => { it("round trip is identity", () => { function check(p: Project) { const json = projectToJSON(p); diff --git a/src/plugins/discourse/fetch.js b/src/plugins/discourse/fetch.js index 8048404..6ac6969 100644 --- a/src/plugins/discourse/fetch.js +++ b/src/plugins/discourse/fetch.js @@ -18,6 +18,7 @@ import * as NullUtil from "../../util/null"; export type UserId = number; export type PostId = number; export type TopicId = number; +export type CategoryId = number; export type Topic = {| +id: TopicId, diff --git a/src/plugins/discourse/loadDiscourse.js b/src/plugins/discourse/loadDiscourse.js index 74f91e0..5114769 100644 --- a/src/plugins/discourse/loadDiscourse.js +++ b/src/plugins/discourse/loadDiscourse.js @@ -2,18 +2,21 @@ import Database from "better-sqlite3"; import base64url from "base64url"; -import {Fetcher, type DiscourseFetchOptions} from "./fetch"; +import {Fetcher} from "./fetch"; import {SqliteMirrorRepository, type ReadRepository} from "./mirrorRepository"; -import {Mirror} from "./mirror"; +import {Mirror, type MirrorOptions} from "./mirror"; import {createGraph} from "./createGraph"; import {TaskReporter} from "../../util/taskReporter"; import {Graph} from "../../core/graph"; import path from "path"; -export type {DiscourseFetchOptions} from "./fetch"; +export type DiscourseServer = {| + +serverUrl: string, + +mirrorOptions?: $Shape, +|}; export type Options = {| - +fetchOptions: DiscourseFetchOptions, + +discourseServer: DiscourseServer, +cacheDirectory: string, |}; @@ -21,15 +24,12 @@ export async function loadDiscourse( options: Options, reporter: TaskReporter ): Promise { - const filename = base64url.encode(options.fetchOptions.serverUrl) + ".db"; + const {serverUrl, mirrorOptions} = options.discourseServer; + const filename = base64url.encode(serverUrl) + ".db"; const db = new Database(path.join(options.cacheDirectory, filename)); - const repo = new SqliteMirrorRepository(db, options.fetchOptions.serverUrl); - const fetcher = new Fetcher(options.fetchOptions); - const mirror = new Mirror(repo, fetcher, options.fetchOptions.serverUrl); + const repo = new SqliteMirrorRepository(db, serverUrl); + const fetcher = new Fetcher({serverUrl}); + const mirror = new Mirror(repo, fetcher, serverUrl, mirrorOptions); await mirror.update(reporter); - const graph = createGraph( - options.fetchOptions.serverUrl, - (repo: ReadRepository) - ); - return graph; + return createGraph(serverUrl, (repo: ReadRepository)); } diff --git a/src/plugins/discourse/mirror.js b/src/plugins/discourse/mirror.js index 4395185..663e4c8 100644 --- a/src/plugins/discourse/mirror.js +++ b/src/plugins/discourse/mirror.js @@ -1,9 +1,27 @@ // @flow import type {TaskReporter} from "../../util/taskReporter"; -import {type Discourse} from "./fetch"; +import type {Discourse, CategoryId} from "./fetch"; import {MirrorRepository} from "./mirrorRepository"; +export type MirrorOptions = {| + // Category definition topics don't show up in the list of bumped topics. + // We need to proactively check them. This sets the interval at which we + // should check. + +recheckCategoryDefinitionsAfterMs: number, + + // When you're concerned about potentially missed edits, + // this option lets you recheck all existing topics in a + // given set of category IDs (where 1 is uncategorized). + // It does not propagate into subcategories. + +recheckTopicsInCategories: $ReadOnlyArray, +|}; + +const defaultOptions: MirrorOptions = { + recheckCategoryDefinitionsAfterMs: 24 * 3600 * 1000, // 24h + recheckTopicsInCategories: [], +}; + /** * Mirrors data from the Discourse API into a local sqlite db. * @@ -22,6 +40,7 @@ import {MirrorRepository} from "./mirrorRepository"; * for multiple Discourse servers is not permitted; use separate Mirrors. */ export class Mirror { + +_options: MirrorOptions; +_repo: MirrorRepository; +_fetcher: Discourse; +_serverUrl: string; @@ -35,10 +54,19 @@ export class Mirror { * A serverUrl is required so that we can ensure that this Mirror is only storing * data from a particular Discourse server. */ - constructor(repo: MirrorRepository, fetcher: Discourse, serverUrl: string) { + constructor( + repo: MirrorRepository, + fetcher: Discourse, + serverUrl: string, + options?: $Shape + ) { this._repo = repo; this._fetcher = fetcher; this._serverUrl = serverUrl; + this._options = { + ...defaultOptions, + ...(options || {}), + }; } async update(reporter: TaskReporter) { diff --git a/src/plugins/discourse/mirror.test.js b/src/plugins/discourse/mirror.test.js index afe2600..64c2d0d 100644 --- a/src/plugins/discourse/mirror.test.js +++ b/src/plugins/discourse/mirror.test.js @@ -2,7 +2,7 @@ import sortBy from "lodash.sortby"; import Database from "better-sqlite3"; -import {Mirror} from "./mirror"; +import {Mirror, type MirrorOptions} from "./mirror"; import {SqliteMirrorRepository} from "./mirrorRepository"; import { type Discourse, @@ -165,12 +165,20 @@ describe("plugins/discourse/mirror", () => { spyWarn().mockRestore(); } }); - const example = () => { + const example = (optionOverrides?: $Shape) => { + // Explicitly set all options, so we know what to expect in tests. + const options: MirrorOptions = { + recheckCategoryDefinitionsAfterMs: 3600000, // 1h + recheckTopicsInCategories: [], + }; const fetcher = new MockFetcher(); const db = new Database(":memory:"); const url = "http://example.com"; const repo = new SqliteMirrorRepository(db, url); - const mirror = new Mirror(repo, fetcher, url); + const mirror = new Mirror(repo, fetcher, url, { + ...options, + ...(optionOverrides || {}), + }); const reporter = new TestTaskReporter(); return {fetcher, mirror, reporter, url, repo}; };