timeline cred config is stored in JSON (#1356)

This modifies the TimelineCred serialization so that it includes the
CredConfig in the JSON. This means that it's easier to coordinate which
plugins and types are in scope, as the data itself can contain that
information.

Rather than define a new hand-rolled serializer, I just passed the
config directly through for stringification. Unit tests verify that this
still works (round-trip serialization is tested). As an added sanity
check, I generated a new small `cred.json`, and inspected the file via
`cat` to ensure that it's still legible text, and isn't interpreted as a
binary file due to the `NUL` bytes in node addresses.

Every client that previously depended on the `DEFAULT_CRED_CONFIG` now
properly gets its cred configuration from the JSON.

Test plan: Unit tests for serialization already exist. Generated a fresh
`cred.json` file and tested the frontend with it. Also,
`yarn test --full` passes.
This commit is contained in:
Dandelion Mané 2019-09-08 00:04:01 +02:00 committed by GitHub
parent ab0628a7ce
commit 5996dd710a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 53 additions and 41 deletions

View File

@ -179,20 +179,18 @@ export class TimelineCred {
graphJSON: this._graph.toJSON(),
credJSON: filteredTimelineCredToJSON(this._cred),
paramsJSON: paramsToJSON(this._params),
configJSON: this._config,
};
return toCompat(COMPAT_INFO, rawJSON);
}
static fromJSON(
j: TimelineCredJSON,
config: TimelineCredConfig
): TimelineCred {
static fromJSON(j: TimelineCredJSON): TimelineCred {
const json = fromCompat(COMPAT_INFO, j);
const {graphJSON, credJSON, paramsJSON} = json;
const {graphJSON, credJSON, paramsJSON, configJSON} = json;
const graph = Graph.fromJSON(graphJSON);
const cred = filteredTimelineCredFromJSON(credJSON);
const params = paramsFromJSON(paramsJSON);
return new TimelineCred(graph, cred, params, config);
return new TimelineCred(graph, cred, params, configJSON);
}
static async compute(
@ -231,11 +229,12 @@ async function _computeTimelineCred(
return filtered;
}
const COMPAT_INFO = {type: "sourcecred/timelineCred", version: "0.1.0"};
const COMPAT_INFO = {type: "sourcecred/timelineCred", version: "0.2.0"};
export opaque type TimelineCredJSON = Compatible<{|
+graphJSON: GraphJSON,
+paramsJSON: ParamsJSON,
+configJSON: TimelineCredConfig,
+credJSON: FilteredTimelineCredJSON,
|}>;

View File

@ -56,7 +56,7 @@ describe("src/analysis/timeline/timelineCred", () => {
it("JSON serialization works", () => {
const tc = exampleTimelineCred();
const json = exampleTimelineCred().toJSON();
const tc_ = TimelineCred.fromJSON(json, credConfig());
const tc_ = TimelineCred.fromJSON(json);
expect(tc.graph()).toEqual(tc_.graph());
expect(tc.params()).toEqual(tc_.params());
expect(tc.config()).toEqual(tc_.config());

View File

@ -8,11 +8,10 @@ import {Graph} from "../core/graph";
import {loadGraph} from "../plugins/github/loadGraph";
import {
type TimelineCredParameters,
type TimelineCredConfig,
TimelineCred,
} from "../analysis/timeline/timelineCred";
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";
@ -21,6 +20,7 @@ import * as NullUtil from "../util/null";
export type LoadOptions = {|
+project: Project,
+params: TimelineCredParameters,
+config: TimelineCredConfig,
+sourcecredDirectory: string,
+githubToken: string | null,
+discourseKey: string | null,
@ -44,7 +44,7 @@ export async function load(
options: LoadOptions,
taskReporter: TaskReporter
): Promise<void> {
const {project, params, sourcecredDirectory, githubToken} = options;
const {project, params, config, sourcecredDirectory, githubToken} = options;
const loadTask = `load-${options.project.id}`;
taskReporter.start(loadTask);
const cacheDirectory = path.join(sourcecredDirectory, "cache");
@ -101,7 +101,7 @@ export async function load(
await fs.writeFile(graphFile, JSON.stringify(graph.toJSON()));
taskReporter.start("compute-cred");
const cred = await TimelineCred.compute(graph, params, DEFAULT_CRED_CONFIG);
const cred = await TimelineCred.compute(graph, params, config);
const credJSON = cred.toJSON();
const credFile = path.join(projectDirectory, "cred.json");
await fs.writeFile(credFile, JSON.stringify(credJSON));

View File

@ -19,7 +19,6 @@ 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";
type JestMockFn = $Call<typeof jest.fn>;
jest.mock("../plugins/github/loadGraph", () => ({
@ -72,6 +71,11 @@ describe("api/load", () => {
weights.nodeManualWeights.set(NodeAddress.empty, 33);
// Deep freeze will freeze the weights, too
const params = deepFreeze({alpha: 0.05, intervalDecay: 0.5, weights});
const config = deepFreeze({
scoreNodePrefix: NodeAddress.empty,
filterNodePrefixes: [],
types: {nodeTypes: [], edgeTypes: []},
});
const example = () => {
const sourcecredDirectory = tmp.dirSync().name;
const taskReporter = new TestTaskReporter();
@ -79,6 +83,7 @@ describe("api/load", () => {
sourcecredDirectory,
githubToken,
params,
config,
project,
discourseKey,
};
@ -141,7 +146,7 @@ describe("api/load", () => {
expect(timelineCredCompute).toHaveBeenCalledWith(
expect.anything(),
params,
DEFAULT_CRED_CONFIG
config
);
expect(timelineCredCompute.mock.calls[0][0].equals(combinedGraph())).toBe(
true

View File

@ -0,0 +1,14 @@
// @flow
import deepFreeze from "deep-freeze";
import * as Github from "../plugins/github/declaration";
import type {TimelineCredConfig} from "../analysis/timeline/timelineCred";
export const DEFAULT_CRED_CONFIG: TimelineCredConfig = deepFreeze({
scoreNodePrefix: Github.userNodeType.prefix,
filterNodePrefixes: [Github.userNodeType.prefix, Github.repoNodeType.prefix],
types: {
nodeTypes: Github.declaration.nodeTypes.slice(),
edgeTypes: Github.declaration.edgeTypes.slice(),
},
});

View File

@ -9,6 +9,7 @@ import {defaultWeights, fromJSON as weightsFromJSON} from "../analysis/weights";
import {load} from "../api/load";
import {specToProject} from "../plugins/github/specToProject";
import fs from "fs-extra";
import {DEFAULT_CRED_CONFIG} from "./defaultCredConfig";
function usage(print: (string) => void): void {
print(
@ -105,9 +106,11 @@ const loadCommand: Command = async (args, std) => {
projectSpecs.map((s) => specToProject(s, githubToken))
);
const params = {alpha: 0.05, intervalDecay: 0.5, weights};
const config = DEFAULT_CRED_CONFIG;
const optionses = projects.map((project) => ({
project,
params,
config,
sourcecredDirectory: Common.sourcecredDirectory(),
githubToken,
discourseKey: Common.discourseKey(),

View File

@ -10,6 +10,7 @@ import loadCommand, {help} from "./load";
import type {LoadOptions} from "../api/load";
import {defaultWeights, toJSON as weightsToJSON} from "../analysis/weights";
import * as Common from "./common";
import {DEFAULT_CRED_CONFIG} from "./defaultCredConfig";
import {makeRepoId, stringToRepoId} from "../core/repoId";
@ -76,6 +77,7 @@ describe("cli/load", () => {
repoIds: [makeRepoId("foo", "bar")],
discourseServer: null,
},
config: DEFAULT_CRED_CONFIG,
params: {alpha: 0.05, intervalDecay: 0.5, weights: defaultWeights()},
sourcecredDirectory: Common.sourcecredDirectory(),
githubToken: fakeGithubToken,
@ -101,6 +103,7 @@ describe("cli/load", () => {
discourseServer: null,
},
params: {alpha: 0.05, intervalDecay: 0.5, weights: defaultWeights()},
config: DEFAULT_CRED_CONFIG,
sourcecredDirectory: Common.sourcecredDirectory(),
githubToken: fakeGithubToken,
discourseKey: fakeDiscourseKey,
@ -138,6 +141,7 @@ describe("cli/load", () => {
discourseServer: null,
},
params: {alpha: 0.05, intervalDecay: 0.5, weights},
config: DEFAULT_CRED_CONFIG,
sourcecredDirectory: Common.sourcecredDirectory(),
githubToken: fakeGithubToken,
discourseKey: fakeDiscourseKey,

View File

@ -13,7 +13,6 @@ import {
type Interval,
type CredNode,
} from "../analysis/timeline/timelineCred";
import {DEFAULT_CRED_CONFIG} from "../plugins/defaultCredConfig";
import {userNodeType} from "../plugins/github/declaration";
import * as GN from "../plugins/github/nodes";
import {directoryForProjectId} from "../core/project_io";
@ -100,7 +99,7 @@ export const scores: Command = async (args, std) => {
const credBlob = await fs.readFile(credFile);
const credJSON = JSON.parse(credBlob.toString());
const timelineCred = TimelineCred.fromJSON(credJSON, DEFAULT_CRED_CONFIG);
const timelineCred = TimelineCred.fromJSON(credJSON);
const userOutput: NodeOutput[] = timelineCred
.credSortedNodes(userNodeType.prefix)
.map((n: CredNode) => {

View File

@ -9,7 +9,6 @@ import {
userNodeType,
repoNodeType,
} from "../plugins/github/declaration";
import {DEFAULT_CRED_CONFIG} from "../plugins/defaultCredConfig";
import {encodeProjectId, type ProjectId} from "../core/project";
export type Props = {|
@ -96,7 +95,7 @@ export async function defaultLoader(
if (!response.ok) {
return Promise.reject(response);
}
return TimelineCred.fromJSON(await response.json(), DEFAULT_CRED_CONFIG);
return TimelineCred.fromJSON(await response.json());
}
try {

View File

@ -5,10 +5,13 @@ import {timeWeek} from "d3-time";
import type {Assets} from "../webutil/assets";
import {TimelineCredView} from "./TimelineCredView";
import {Graph, NodeAddress} from "../core/graph";
import {type Interval, TimelineCred} from "../analysis/timeline/timelineCred";
import {
type Interval,
TimelineCred,
type TimelineCredConfig,
} from "../analysis/timeline/timelineCred";
import {type FilteredTimelineCred} from "../analysis/timeline/filterTimelineCred";
import {defaultWeights} from "../analysis/weights";
import {DEFAULT_CRED_CONFIG} from "../plugins/defaultCredConfig";
export default class TimelineCredViewInspectiontest extends React.Component<{|
+assets: Assets,
@ -53,12 +56,12 @@ export default class TimelineCredViewInspectiontest extends React.Component<{|
addressToCred,
};
const params = {alpha: 0.05, intervalDecay: 0.5, weights: defaultWeights()};
return new TimelineCred(
graph,
filteredTimelineCred,
params,
DEFAULT_CRED_CONFIG
);
const config: TimelineCredConfig = {
scoreNodePrefix: NodeAddress.empty,
filterNodePrefixes: [NodeAddress.empty],
types: {nodeTypes: [], edgeTypes: []},
};
return new TimelineCred(graph, filteredTimelineCred, params, config);
}
render() {

View File

@ -1,14 +0,0 @@
// @flow
import deepFreeze from "deep-freeze";
import {userNodeType, repoNodeType, declaration} from "./github/declaration";
import type {TimelineCredConfig} from "../analysis/timeline/timelineCred";
export const DEFAULT_CRED_CONFIG: TimelineCredConfig = deepFreeze({
scoreNodePrefix: userNodeType.prefix,
filterNodePrefixes: [userNodeType.prefix, repoNodeType.prefix],
types: {
nodeTypes: declaration.nodeTypes.slice(),
edgeTypes: declaration.edgeTypes.slice(),
},
});