Remove `createdAt` from the `AnalysisAdapter`

As #1136 will be moving timestamps into the graph, we no longer need
`createdAt` method in the `AnalysisAdapter`. Actually, we no longer need
the adapter/loader distinction introduced in #1157. I haven't taken the
time to remove the `BackendAdapterLoader` concept because a) we may need
it later, and b) if we don't, I'll likely remove the `AnalysisAdapter`
concept entirely, in favor of having plugins directly save `graph.json`
files to a known location.

Test plan: `yarn test` passes.
This commit is contained in:
Dandelion Mané 2019-06-13 23:17:58 +03:00
parent 4b1763ebc6
commit e47a5bd84e
5 changed files with 8 additions and 99 deletions

View File

@ -12,16 +12,14 @@
* In general, every plugin will provide an AnalysisAdapter, and the analysis
* data pipeline will aggregate results across all plugins' adapters.
*
* TODO(@decentralion): As the AnalysisAdapter evolves, consider whether it
* would make sense to simply move the data the AnalysisAdapter provides
* directly into the core Graph. Note that doing so would require considerable
* changes to the Graph APIs, including having Node be a rich data type rather
* than just an address, and allowing edges to Nodes which do not exist in the
* graph. Due to the complexity, such a refactor should not be undertaken
* lightly.
* At the moment, the AnalysisAdapter only exists to provide the graph(), which
* is immediately consumed by `sourcecred load` and used to aggregate into one
* all-encompasing graph. Consider whether we should just have the plugins save
* their graph to a consistent location on load, thus removing the need for an
* AnalysisAdapter.
*/
import {Graph, type NodeAddressT} from "../core/graph";
import {Graph} from "../core/graph";
import type {RepoId} from "../core/repoId";
import type {PluginDeclaration} from "./pluginDeclaration";
@ -37,7 +35,6 @@ export interface IBackendAdapterLoader {
load(sourcecredDirectory: string, repoId: RepoId): Promise<IAnalysisAdapter>;
}
export type MsSinceEpoch = number;
/**
* Provides data needed for cred analysis for an individual plugin.
*
@ -58,5 +55,4 @@ export interface IAnalysisAdapter {
* always existing for purposes of cred analysis. (E.g. we may want to
* consider user identities timeless.)
*/
createdAt(n: NodeAddressT): MsSinceEpoch | null;
}

View File

@ -2,16 +2,14 @@
import fs from "fs-extra";
import path from "path";
import {Graph, type NodeAddressT} from "../../core/graph";
import {Graph} from "../../core/graph";
import type {
IAnalysisAdapter,
IBackendAdapterLoader,
MsSinceEpoch,
} from "../../analysis/analysisAdapter";
import {type RepoId, repoIdToString} from "../../core/repoId";
import {declaration} from "./declaration";
import {type Repository} from "./types";
import {type StructuredAddress, fromRaw} from "./nodes";
export class BackendAdapterLoader implements IBackendAdapterLoader {
declaration() {
@ -59,24 +57,4 @@ export class AnalysisAdapter implements IAnalysisAdapter {
// hotspot. If so, implement a do-not-modify flag and set it (for safety)
return this._graph.copy();
}
createdAt(n: NodeAddressT): MsSinceEpoch | null {
// Coerce the NodeAddressT into a Git plugin 'RawAddress'.
// If this coercion is false (i.e. the AnalysisAdapter was passed a non-Git NodeAddress)
// then this will throw a runtime error.
const addr: StructuredAddress = fromRaw((n: any));
switch (addr.type) {
case "COMMIT":
const hash = addr.hash;
const commit = this._repository.commits[hash];
if (commit == null) {
// Possibly this commit was merged to a non-master branch.
// It's a little hacky to return null. See #1163 for alternative
// solutions.
return null;
}
return commit.createdAt;
default:
throw new Error(`Unexpected type: ${(addr.type: empty)}`);
}
}
}

View File

@ -5,8 +5,7 @@ import path from "path";
import {BackendAdapterLoader} from "./analysisAdapter";
import {stringToRepoId} from "../../core/repoId";
import {declaration} from "./declaration";
import {Graph, NodeAddress} from "../../core/graph";
import {toRaw} from "./nodes";
import {Graph} from "../../core/graph";
describe("plugins/git/analysisAdapter", () => {
const sourcecredDirectory = path.join(
@ -44,27 +43,5 @@ describe("plugins/git/analysisAdapter", () => {
const aa = await loadAnalysisAdapter();
expect(aa.declaration()).toEqual(declaration);
});
describe("has a createdAt method which", () => {
it("provides createdAt times", async () => {
const aa = await loadAnalysisAdapter();
const hash = "0a223346b4e6dec0127b1e6aa892c4ee0424b66a";
const commitAddr = toRaw({type: "COMMIT", hash});
const actualCreatedAt = aa.createdAt(commitAddr);
expect(actualCreatedAt).toEqual(1519807427000);
});
it("returns null for an absent commit hash", async () => {
// This is a little hacky. See #1163 for discussion.
// https://github.com/sourcecred/sourcecred/issues/1163
const aa = await loadAnalysisAdapter();
const commitAddr = toRaw({type: "COMMIT", hash: "1234"});
expect(aa.createdAt(commitAddr)).toEqual(null);
});
it("throws an error for an invalid NodeAddress", async () => {
const aa = await loadAnalysisAdapter();
expect(() => aa.createdAt(NodeAddress.empty)).toThrowError(
"Bad address"
);
});
});
});
});

View File

@ -3,19 +3,14 @@
import fs from "fs-extra";
import path from "path";
import pako from "pako";
import stringify from "json-stable-stringify";
import {type RepoId, repoIdToString} from "../../core/repoId";
import type {
IAnalysisAdapter,
IBackendAdapterLoader,
MsSinceEpoch,
} from "../../analysis/analysisAdapter";
import {declaration} from "./declaration";
import {RelationalView} from "./relationalView";
import {createGraph} from "./createGraph";
import {createdAt} from "./createdAt";
import {fromRaw} from "./nodes";
import {type NodeAddressT} from "../../core/graph";
export class BackendAdapterLoader implements IBackendAdapterLoader {
declaration() {
@ -47,14 +42,6 @@ export class AnalysisAdapter implements IAnalysisAdapter {
declaration() {
return declaration;
}
createdAt(n: NodeAddressT): MsSinceEpoch | null {
const addr = fromRaw((n: any));
const entity = this._view.entity(addr);
if (entity == null) {
throw new Error(`No entity matching ${stringify(addr)}`);
}
return createdAt(entity);
}
graph() {
return createGraph(this._view);
}

View File

@ -8,8 +8,6 @@ import {stringToRepoId} from "../../core/repoId";
import {declaration} from "./declaration";
import {RelationalView} from "./relationalView";
import {createGraph} from "./createGraph";
import {NodeAddress} from "../../core/graph";
import {toRaw} from "./nodes";
describe("plugins/github/analysisAdapter", () => {
it("the loader provides the declaration", () => {
@ -52,32 +50,5 @@ describe("plugins/github/analysisAdapter", () => {
const aa = await loadAnalysisAdapter();
expect(aa.declaration()).toEqual(declaration);
});
describe("has a createdAt method which", () => {
it("provides createdAt times", async () => {
const aa = await loadAnalysisAdapter();
const addr = toRaw({
type: "ISSUE",
repo: {type: "REPO", owner: "sourcecred", name: "example-github"},
number: "1",
});
const actualCreatedAt = aa.createdAt(addr);
expect(actualCreatedAt).toMatchInlineSnapshot(`1519807088000`);
});
it("throws an error for an absent entity", async () => {
const aa = await loadAnalysisAdapter();
const addr = toRaw({
type: "ISSUE",
repo: {type: "REPO", owner: "sourcecred", name: "example-github"},
number: "1001",
});
expect(() => aa.createdAt(addr)).toThrowError("No entity matching");
});
it("throws an error for an invalid NodeAddress", async () => {
const aa = await loadAnalysisAdapter();
expect(() => aa.createdAt(NodeAddress.empty)).toThrowError(
"Bad address"
);
});
});
});
});