Skip to content

Commit

Permalink
Cache prs and check runs for fast load
Browse files Browse the repository at this point in the history
- adds a LocalCache helper for lscache
  • Loading branch information
mtsgrd committed Nov 20, 2024
1 parent 2e263ab commit 4d6b82f
Show file tree
Hide file tree
Showing 7 changed files with 158 additions and 62 deletions.
84 changes: 30 additions & 54 deletions apps/desktop/src/lib/forge/github/githubChecksMonitor.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { scurveBackoff } from '$lib/backoff/scurve';
import { DEFAULT_HEADERS } from '$lib/forge/github/headers';
import { parseGitHubCheckSuites } from '$lib/forge/github/types';
import { parseGitHubCheckRuns, parseGitHubCheckSuites } from '$lib/forge/github/types';
import { sleep } from '$lib/utils/sleep';
import { Octokit, type RestEndpointMethodTypes } from '@octokit/rest';
import { LocalCache } from '@gitbutler/shared/localCache';
import { Octokit } from '@octokit/rest';
import { writable } from 'svelte/store';
import type { CheckSuites, ChecksStatus } from '$lib/forge/interface/types';
import type { RepoInfo } from '$lib/url/gitUrl';
Expand All @@ -27,13 +28,17 @@ export class GitHubChecksMonitor implements ForgeChecksMonitor {
private timeout: any;
private hasCheckSuites: boolean | undefined;

// Stores previously fetched checks results by pr number.
private cache = new LocalCache({ keyPrefix: 'pr-checks', expiry: 1440 });

constructor(
private octokit: Octokit,
private repo: RepoInfo,
private sourceBranch: string
) {}

async start() {
this.loadFromCache();
this.update();
}

Expand All @@ -42,15 +47,25 @@ export class GitHubChecksMonitor implements ForgeChecksMonitor {
delete this.timeout;
}

loadFromCache() {
const cachedValue = this.cache.get(this.sourceBranch);
try {
const status = parseGitHubCheckRuns(cachedValue);
this._status = status;
this.status.set(status);
} catch {
this.cache.remove(this.sourceBranch);
}
}

async update() {
this.error.set(undefined);
this.loading.set(true);

try {
const checks = await this.fetchChecksWithRetries(this.sourceBranch, 5, 2000);
const status = parseChecks(checks);
this.status.set(status);
this._status = status;
this.status.set(checks);
this._status = checks;
} catch (e: any) {
console.error(e);
this.error.set(e.message);
Expand Down Expand Up @@ -84,9 +99,13 @@ export class GitHubChecksMonitor implements ForgeChecksMonitor {
return scurveBackoff(ageMs, 10000, 600000);
}

private async fetchChecksWithRetries(ref: string, retries: number, delayMs: number) {
private async fetchChecksWithRetries(
ref: string,
retries: number,
delayMs: number
): Promise<ChecksStatus | null> {
let checks = await this.fetchChecks(ref);
if (checks.total_count > 0) {
if (checks && checks?.totalCount > 0) {
this.hasCheckSuites = true;
return checks;
}
Expand All @@ -99,7 +118,7 @@ export class GitHubChecksMonitor implements ForgeChecksMonitor {
if (!this.hasCheckSuites) return checks;

let attempts = 0;
while (checks.total_count === 0 && attempts < retries) {
while (checks && checks.totalCount === 0 && attempts < retries) {
attempts++;
await sleep(delayMs);
checks = await this.fetchChecks(ref);
Expand All @@ -117,57 +136,14 @@ export class GitHubChecksMonitor implements ForgeChecksMonitor {
return { count: resp.data.total_count, items: parseGitHubCheckSuites(resp.data) };
}

private async fetchChecks(ref: string) {
private async fetchChecks(ref: string): Promise<ChecksStatus | null> {
const resp = await this.octokit.checks.listForRef({
headers: DEFAULT_HEADERS,
owner: this.repo.owner,
repo: this.repo.name,
ref: ref
});
return resp.data;
this.cache.set(ref, resp.data);
return parseGitHubCheckRuns(resp.data);
}
}

function parseChecks(
data: RestEndpointMethodTypes['checks']['listForRef']['response']['data']
): ChecksStatus | null {
// Fetch with retries since checks might not be available _right_ after
// the pull request has been created.

// If there are no checks then there is no status to report
const checkRuns = data.check_runs;
if (checkRuns.length === 0) return null;

// Establish when the first check started running, useful for showing
// how long something has been running.
const starts = checkRuns
.map((run) => run.started_at)
.filter((startedAt) => startedAt !== null) as string[];
const startTimes = starts.map((startedAt) => new Date(startedAt));

const queued = checkRuns.filter((c) => c.status === 'queued').length;
const failed = checkRuns.filter((c) => c.conclusion === 'failure').length;
const skipped = checkRuns.filter((c) => c.conclusion === 'skipped').length;
const succeeded = checkRuns.filter((c) => c.conclusion === 'success').length;

const firstStart = new Date(Math.min(...startTimes.map((date) => date.getTime())));
const completed = checkRuns.every((check) => !!check.completed_at);
const totalCount = data.total_count;

const success = queued === 0 && failed === 0 && skipped + succeeded === totalCount;
const finished = checkRuns.filter(
(c) => c.conclusion && ['failure', 'success'].includes(c.conclusion)
).length;

return {
startedAt: firstStart,
hasChecks: !!totalCount,
success,
failed,
completed,
queued,
totalCount,
skipped,
finished
};
}
8 changes: 8 additions & 0 deletions apps/desktop/src/lib/forge/github/githubPrMonitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,15 @@ export class GitHubPrMonitor implements ForgePrMonitor {
private prNumber: number
) {}

private loadFromCache() {
const cachedValue = this.prService.getCached(this.prNumber);
if (cachedValue) {
this.pr.set(cachedValue);
}
}

private start() {
this.loadFromCache();
this.fetch();
this.intervalId = setInterval(() => {
this.fetch();
Expand Down
30 changes: 29 additions & 1 deletion apps/desktop/src/lib/forge/github/githubPrService.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { GitHubPrMonitor } from './githubPrMonitor';
import { DEFAULT_HEADERS } from './headers';
import { ghResponseToInstance, parseGitHubDetailedPullRequest } from './types';
import { ghResponseToInstance, parseGitHubDetailedPullRequest, parseGitHubReview } from './types';
import { sleep } from '$lib/utils/sleep';
import { LocalCache } from '@gitbutler/shared/localCache';
import posthog from 'posthog-js';
import { writable } from 'svelte/store';
import type { ForgePrService } from '$lib/forge/interface/forgePrService';
Expand All @@ -17,6 +18,9 @@ import type { Octokit } from '@octokit/rest';
export class GitHubPrService implements ForgePrService {
loading = writable(false);

// Stores previously fetched pull requests by their number.
private cache = new LocalCache({ keyPrefix: 'pr', expiry: 1440 });

constructor(
private octokit: Octokit,
private repo: RepoInfo
Expand Down Expand Up @@ -72,9 +76,33 @@ export class GitHubPrService implements ForgePrService {
repo: this.repo.name,
pull_number: prNumber
});
this.cache.set(prNumber, resp.data);
return parseGitHubDetailedPullRequest(resp.data);
}

getCached(prNumber: number): DetailedPullRequest | undefined {
const cachedValue = this.cache.get(prNumber);
try {
return cachedValue ? parseGitHubDetailedPullRequest(cachedValue) : undefined;
} catch {
this.cache.remove(prNumber);
}
}

private cacheKey(prNumber: number) {
return 'pr-cache-' + prNumber;
}

async getReviewStatus(prNumber: number): Promise<ReviewStatus> {
const resp = await this.octokit.pulls.listReviews({
headers: DEFAULT_HEADERS,
owner: this.repo.owner,
repo: this.repo.name,
pull_number: prNumber
});
return parseGitHubReview(resp.data);
}

async merge(method: MergeMethod, prNumber: number) {
await this.octokit.pulls.merge({
owner: this.repo.owner,
Expand Down
53 changes: 52 additions & 1 deletion apps/desktop/src/lib/forge/github/types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import { parseRemoteUrl } from '$lib/url/gitUrl';
import type { CheckSuite, DetailedPullRequest, Label, PullRequest } from '../interface/types';
import type {
ChecksStatus,
CheckSuite,
DetailedPullRequest,
Label,
PullRequest
} from '../interface/types';
import type { RestEndpointMethodTypes } from '@octokit/rest';

export type DetailedGitHubPullRequest = RestEndpointMethodTypes['pulls']['get']['response']['data'];
Expand Down Expand Up @@ -82,3 +88,48 @@ export function parseGitHubCheckSuites(data: GitHubListCheckSuitesResp): CheckSu
}));
return result;
}

export type GitHubListChecksResp =
RestEndpointMethodTypes['checks']['listForRef']['response']['data'];

export function parseGitHubCheckRuns(data: GitHubListChecksResp): ChecksStatus | null {
// Fetch with retries since checks might not be available _right_ after
// the pull request has been created.

// If there are no checks then there is no status to report
const checkRuns = data.check_runs;
if (checkRuns.length === 0) return null;

// Establish when the first check started running, useful for showing
// how long something has been running.
const starts = checkRuns
.map((run) => run.started_at)
.filter((startedAt) => startedAt !== null) as string[];
const startTimes = starts.map((startedAt) => new Date(startedAt));

const queued = checkRuns.filter((c) => c.status === 'queued').length;
const failed = checkRuns.filter((c) => c.conclusion === 'failure').length;
const skipped = checkRuns.filter((c) => c.conclusion === 'skipped').length;
const succeeded = checkRuns.filter((c) => c.conclusion === 'success').length;

const firstStart = new Date(Math.min(...startTimes.map((date) => date.getTime())));
const completed = checkRuns.every((check) => !!check.completed_at);
const totalCount = data.total_count;

const success = queued === 0 && failed === 0 && skipped + succeeded === totalCount;
const finished = checkRuns.filter(
(c) => c.conclusion && ['failure', 'success'].includes(c.conclusion)
).length;

return {
startedAt: firstStart,
hasChecks: !!totalCount,
success,
failed,
completed,
queued,
totalCount,
skipped,
finished
};
}
2 changes: 1 addition & 1 deletion apps/desktop/src/lib/forge/interface/forgePrService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export const [getForgePrService, createForgePrServiceStore] = buildContextStore<

export interface ForgePrService {
loading: Writable<boolean>;
get(prNumber: number): Promise<DetailedPullRequest>;
get(prNumber: number): Promise<DetailedPullRequest | void>;
createPr({
title,
body,
Expand Down
8 changes: 3 additions & 5 deletions packages/shared/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,12 @@
"@sveltejs/vite-plugin-svelte": "catalog:svelte",
"@terrazzo/cli": "^0.0.11",
"@terrazzo/plugin-css": "^0.0.9",
"@types/lscache": "^1.3.4",
"@types/lscache": "^1.3.2",
"@types/postcss-pxtorem": "^6.0.3",
"autoprefixer": "^10.4.19",
"cpy-cli": "^5.0.0",
"dayjs": "^1.11.13",
"lscache": "^1.3.2",
"postcss": "^8.4.38",
"postcss-cli": "^11.0.0",
"postcss-minify": "^1.1.0",
Expand All @@ -63,8 +64,5 @@
"vite": "catalog:",
"vitest": "^2.0.5"
},
"type": "module",
"dependencies": {
"lscache": "^1.3.2"
}
"type": "module"
}
35 changes: 35 additions & 0 deletions packages/shared/src/lib/localCache.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import lscache from 'lscache';

const DEFAULT_PREFIX = 'cache';

export interface CacheOptions {
expiry: number;
keyPrefix: string;
}
/**
* Helper for getting/setting values with lscache.
*/
export class LocalCache {
expiry: number;
keyPrefix: string | undefined;
constructor(options: CacheOptions) {
this.expiry = options.expiry;
this.keyPrefix = options.keyPrefix;
}

get(key: string | number) {
return lscache.get(this.transformKey(key));
}

set(key: string | number, value: string | number | object) {
lscache.set(this.transformKey(key), value, this.expiry);
}

remove(key: string | number) {
lscache.remove(this.transformKey(key));
}

private transformKey(key: string | number) {
return DEFAULT_PREFIX + ':' + this.keyPrefix + ':' + key;
}
}

0 comments on commit 4d6b82f

Please sign in to comment.