From ca63ea00fb7850f24c2a755f0f9c27a757b1b4db Mon Sep 17 00:00:00 2001 From: Robin van Boven <497556+Beanow@users.noreply.github.com> Date: Mon, 3 Feb 2020 23:46:03 +0100 Subject: [PATCH] Github: switch to CacheProvider for fetchGithubRepo (#1614) Delegating to a CacheProvider instance, will limit the number of places where we need to handle filesystem details. It will also allow a mock, or in-memory cache to be provided. --- src/api/load.js | 5 ++++- src/api/load.test.js | 5 +++-- src/plugins/github/bin/fetchAndPrintGithubRepo.js | 6 +++--- src/plugins/github/fetchGithubRepo.js | 9 ++++----- src/plugins/github/loadGraph.js | 5 +++-- 5 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/api/load.js b/src/api/load.js index 78bb77c..7a66b8a 100644 --- a/src/api/load.js +++ b/src/api/load.js @@ -18,6 +18,8 @@ import * as Github from "../plugins/github/loadWeightedGraph"; import * as WeightedGraph from "../core/weightedGraph"; import {type Weights as WeightsT} from "../core/weights"; import {loadWeightedGraph} from "./loadWeightedGraph"; +import {DataDirectory} from "../backend/dataDirectory"; +import {type CacheProvider} from "../backend/cache"; export type LoadOptions = {| +project: Project, @@ -58,6 +60,7 @@ export async function load( const fullParams = params == null ? defaultParams() : partialParams(params); const loadTask = `load-${options.project.id}`; taskReporter.start(loadTask); + const dataDirectory = new DataDirectory(sourcecredDirectory); const cacheDirectory = path.join(sourcecredDirectory, "cache"); await fs.mkdirp(cacheDirectory); @@ -77,7 +80,7 @@ export async function load( githubOptions = { repoIds: project.repoIds, token: githubToken, - cacheDirectory, + cache: (dataDirectory: CacheProvider), }; } diff --git a/src/api/load.test.js b/src/api/load.test.js index fe18dbe..58848d3 100644 --- a/src/api/load.test.js +++ b/src/api/load.test.js @@ -26,6 +26,7 @@ import { partialParams, } from "../analysis/timeline/params"; import * as WeightedGraph from "../core/weightedGraph"; +import {DataDirectory} from "../backend/dataDirectory"; type JestMockFn = $Call; jest.mock("../plugins/github/loadWeightedGraph", () => ({ @@ -102,11 +103,11 @@ describe("api/load", () => { it("calls github githubWeightedGraph with the right options", async () => { const {options, taskReporter, sourcecredDirectory} = example(); await load(options, taskReporter); - const cacheDirectory = path.join(sourcecredDirectory, "cache"); + const cache = new DataDirectory(sourcecredDirectory); const expectedLoadGraphOptions: LoadGraphOptions = { repoIds: project.repoIds, token: exampleGithubToken, - cacheDirectory, + cache, }; expect(githubWeightedGraph).toHaveBeenCalledWith( expectedLoadGraphOptions, diff --git a/src/plugins/github/bin/fetchAndPrintGithubRepo.js b/src/plugins/github/bin/fetchAndPrintGithubRepo.js index 48c9745..6fe733f 100644 --- a/src/plugins/github/bin/fetchAndPrintGithubRepo.js +++ b/src/plugins/github/bin/fetchAndPrintGithubRepo.js @@ -13,10 +13,9 @@ */ import stringify from "json-stable-stringify"; -import tmp from "tmp"; - import fetchGithubRepo from "../fetchGithubRepo"; import {makeRepoId} from "../repoId"; +import {MemoryCacheProvider} from "../../../backend/memoryCacheProvider"; function parseArgs() { const argv = process.argv.slice(2); @@ -38,7 +37,8 @@ function parseArgs() { function main() { const args = parseArgs(); const repoId = makeRepoId(args.owner, args.name); - const options = {token: args.githubToken, cacheDirectory: tmp.dirSync().name}; + const cache = new MemoryCacheProvider(); + const options = {token: args.githubToken, cache}; fetchGithubRepo(repoId, options) .then((data) => { console.log(stringify(data, {space: 4})); diff --git a/src/plugins/github/fetchGithubRepo.js b/src/plugins/github/fetchGithubRepo.js index 6fed413..1f09f91 100644 --- a/src/plugins/github/fetchGithubRepo.js +++ b/src/plugins/github/fetchGithubRepo.js @@ -6,7 +6,6 @@ import Database from "better-sqlite3"; import fetch from "isomorphic-fetch"; -import path from "path"; import retry from "retry"; import {type RepoId, repoIdToString} from "./repoId"; @@ -19,6 +18,7 @@ import type {Repository} from "./graphqlTypes"; import schema from "./schema"; import {validateToken} from "./token"; import {cacheIdForRepoId} from "./cacheId"; +import {type CacheProvider} from "../../backend/cache"; /** * Scrape data from a GitHub repo using the GitHub API. @@ -35,9 +35,9 @@ import {cacheIdForRepoId} from "./cacheId"; */ export default async function fetchGithubRepo( repoId: RepoId, - options: {|+token: string, +cacheDirectory: string|} + options: {|+token: string, +cache: CacheProvider|} ): Promise { - const {token, cacheDirectory} = options; + const {token, cache} = options; // Right now, only warn on likely to be bad tokens (see #1461). // This lets us proceed to the GitHub API validating the token, @@ -59,8 +59,7 @@ export default async function fetchGithubRepo( // name is valid and uniquely identifying even on case-insensitive // filesystems (HFS, HFS+, APFS, NTFS) or filesystems preventing // equals signs in file names. - const dbFilename = `${cacheIdForRepoId(repoId)}.db`; - const db = new Database(path.join(cacheDirectory, dbFilename)); + const db: Database = await cache.database(cacheIdForRepoId(repoId)); const mirror = new Mirror(db, schema(), { blacklistedIds: BLACKLISTED_IDS, guessTypename: _guessTypename, diff --git a/src/plugins/github/loadGraph.js b/src/plugins/github/loadGraph.js index a3ec5d5..79d012f 100644 --- a/src/plugins/github/loadGraph.js +++ b/src/plugins/github/loadGraph.js @@ -15,11 +15,12 @@ import {RelationalView} from "./relationalView"; import {type RepoId, repoIdToString} from "./repoId"; import {Graph} from "../../core/graph"; import {type GithubToken} from "./token"; +import {type CacheProvider} from "../../backend/cache"; export type Options = {| +repoIds: $ReadOnlyArray, +token: GithubToken, - +cacheDirectory: string, + +cache: CacheProvider, |}; /** @@ -42,7 +43,7 @@ export async function loadGraph( repositories.push( await fetchGithubRepo(repoId, { token: options.token, - cacheDirectory: options.cacheDirectory, + cache: options.cache, }) ); taskReporter.finish(taskId);