discourse: use parser combinators for config (#1824)

Summary:
This patch changes the Discourse plugin’s config parsing to use the
parser combinator approach, which simplifies it and removes the need for
all the tests.

Test Plan:
Prior to deleting the tests, they passed, except for the test that
mirror options could not be partially populated; the partial parsing now
works correctly.

wchargin-branch: discourse-config-combinator
This commit is contained in:
William Chargin 2020-05-31 21:52:05 -07:00 committed by GitHub
parent 930ac715fa
commit 64169da128
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 35 additions and 129 deletions

View File

@ -1,67 +1,33 @@
// @flow
import {type MirrorOptions} from "./mirror";
import * as Combo from "../../util/combo";
import {optionsShapeParser, type MirrorOptions} from "./mirror";
export type DiscourseConfig = {|
+serverUrl: string,
+mirrorOptions?: $Shape<MirrorOptions>,
|};
type JsonObject =
| string
| number
| boolean
| null
| JsonObject[]
| {[string]: JsonObject};
const parser: Combo.Parser<DiscourseConfig> = (() => {
const C = Combo;
return C.object(
{
serverUrl: C.fmap(C.string, (serverUrl) => {
const httpRE = new RegExp(/^https?:\/\//);
if (!httpRE.test(serverUrl)) {
throw new Error(
"expected server url to start with 'https://' or 'http://'"
);
}
return serverUrl;
}),
},
{
mirrorOptions: optionsShapeParser,
}
);
})();
export function parseConfig(raw: JsonObject): DiscourseConfig {
if (raw == null || typeof raw !== "object" || Array.isArray(raw)) {
throw new Error("bad config: " + JSON.stringify(raw));
}
let mirrorOptions = undefined;
const {serverUrl} = raw;
if (typeof serverUrl !== "string") {
throw new Error("serverUrl not string: " + JSON.stringify(serverUrl));
}
const httpRE = new RegExp(/^https?:\/\//);
if (!httpRE.test(serverUrl)) {
throw new Error(
"expected server url to start with 'https://' or 'http://'"
);
}
if (raw.mirrorOptions !== undefined) {
const {mirrorOptions: rawMO} = raw;
if (rawMO == null || typeof rawMO !== "object" || Array.isArray(rawMO)) {
throw new Error("bad config: " + JSON.stringify(rawMO));
}
const {recheckCategoryDefinitionsAfterMs} = rawMO;
let {recheckTopicsInCategories} = rawMO;
if (!Array.isArray(recheckTopicsInCategories)) {
throw new Error(
"mirrorOptions.recheckTopicsInCategories must be array, got " +
JSON.stringify(recheckTopicsInCategories)
);
}
if (!recheckTopicsInCategories.every((x) => typeof x === "number")) {
throw new Error(
"mirrorOptions.recheckTopicsInCategories must all be numbers, got " +
JSON.stringify(recheckTopicsInCategories)
);
}
recheckTopicsInCategories = recheckTopicsInCategories.map((x) => Number(x));
if (typeof recheckCategoryDefinitionsAfterMs !== "number") {
throw new Error(
"recheckCategoryDefinitionsAfterMs must be number, got " +
JSON.stringify(recheckCategoryDefinitionsAfterMs)
);
}
mirrorOptions = {
recheckCategoryDefinitionsAfterMs,
recheckTopicsInCategories,
};
}
return {serverUrl, mirrorOptions};
export function parseConfig(raw: Combo.JsonObject): DiscourseConfig {
return parser.parseOrThrow(raw);
}

View File

@ -1,72 +0,0 @@
// @flow
import {parseConfig} from "./config";
describe("plugins/discourse/config", () => {
describe("parseConfig", () => {
it("works on a config with just a serverUrl", () => {
const config = {serverUrl: "https://server.io"};
expect(parseConfig(config)).toEqual(config);
});
it("errors if the serverUrl is not a url", () => {
const config = {serverUrl: "1234"};
expect(() => parseConfig(config)).toThrowError();
});
it("errors if the serverUrl is not a string", () => {
const config = {serverUrl: 234};
expect(() => parseConfig(config)).toThrowError();
});
it("errors if the serverUrl is missing", () => {
const config = {};
expect(() => parseConfig(config)).toThrowError();
});
it("errors if the config is not an object", () => {
const config = [];
expect(() => parseConfig(config)).toThrowError();
});
it("works on a config with mirror options", () => {
const config = {
serverUrl: "https://server.io",
mirrorOptions: {
recheckCategoryDefinitionsAfterMs: 12,
recheckTopicsInCategories: [1, 2, 3, 4],
},
};
expect(parseConfig(config)).toEqual(config);
});
it("errors on a config with missing mirror options", () => {
const c1 = {
serverUrl: "https://server.io",
mirrorOptions: {
recheckTopicsInCategories: [1, 2, 3, 4],
},
};
expect(() => parseConfig(c1)).toThrowError();
const c2 = {
serverUrl: "https://server.io",
mirrorOptions: {
recheckCategoryDefinitionsAfterMs: 12,
},
};
expect(() => parseConfig(c2)).toThrowError();
});
it("errors with bad config options", () => {
const c1 = {
serverUrl: "https://server.io",
mirrorOptions: {
recheckTopicsInCategories: 12,
recheckCategoryDefinitionsAfterMs: 12,
},
};
expect(() => parseConfig(c1)).toThrowError();
const c2 = {
serverUrl: "https://server.io",
mirrorOptions: {
recheckTopicsInCategories: [1, 2, 3, 4],
recheckCategoryDefinitionsAfterMs: "foo",
},
};
expect(() => parseConfig(c2)).toThrowError();
});
});
});

View File

@ -1,5 +1,6 @@
// @flow
import * as Combo from "../../util/combo";
import type {TaskReporter} from "../../util/taskReporter";
import type {Discourse, CategoryId, Topic, TopicLatest} from "./fetch";
import {MirrorRepository} from "./mirrorRepository";
@ -17,6 +18,17 @@ export type MirrorOptions = {|
+recheckTopicsInCategories: $ReadOnlyArray<CategoryId>,
|};
const optionsParserFields = {
recheckCategoryDefinitionsAfterMs: Combo.number,
recheckTopicsInCategories: Combo.array<number>(Combo.number),
};
export const optionsParser: Combo.Parser<MirrorOptions> = Combo.object(
optionsParserFields
);
export const optionsShapeParser: Combo.Parser<
$Shape<MirrorOptions>
> = Combo.shape(optionsParserFields);
const defaultOptions: MirrorOptions = {
recheckCategoryDefinitionsAfterMs: 24 * 3600 * 1000, // 24h
recheckTopicsInCategories: [],