diff --git a/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.test.ts b/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.test.ts index f5115c99af0d1..8ee4568495fd3 100644 --- a/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.test.ts +++ b/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.test.ts @@ -547,6 +547,48 @@ describe("GitCafePullRequestProvider", () => { }).pipe(Effect.provide(server.layer)); }); + it.effect("recounts summary lines only when the compared commits move", () => { + const landedBase = "fedcba9876543210fedcba9876543210fedcba98"; + let comparisonBaseOid = baseOid; + let pullReads = 0; + const server = fakeGitCafe({ + // A comment moves the version without moving the compared commits. + "GET /repos/owner/repo/pulls/7": () => ({ ...pull, version: 4 + pullReads++ }), + "GET /repos/owner/repo/pulls/7/changes": () => ({ + version: 4, + headOid, + comparisonBaseOid, + items: [{ path: "a.ts" }, { path: "b.ts" }], + next: null, + }), + "POST /repos/owner/repo/pulls/7/diff-files": (request: SentRequest) => ({ + version: 4, + headOid, + comparisonBaseOid, + items: (request.body as { paths: ReadonlyArray }).paths.map((path) => ({ + path, + additions: comparisonBaseOid === baseOid ? 3 : 1, + deletions: 1, + })), + }), + }); + const hunkReads = () => + server.sent.filter((request) => request.path.endsWith("/diff-files")).length; + return Effect.gen(function* () { + const provider = yield* GitCafePullRequestProvider.make; + const first = yield* provider.getChangeRequestSummary!(target); + const second = yield* provider.getChangeRequestSummary!(target); + assert.deepInclude(first, { additions: 6, deletions: 2, changedFiles: 2 }); + assert.deepInclude(second, { additions: 6, deletions: 2, changedFiles: 2 }); + assert.strictEqual(hunkReads(), 1); + // A landed parent layer moves the comparison base under the same head. + comparisonBaseOid = landedBase; + const third = yield* provider.getChangeRequestSummary!(target); + assert.deepInclude(third, { additions: 2, deletions: 2, changedFiles: 2 }); + assert.strictEqual(hunkReads(), 2); + }).pipe(Effect.provide(server.layer)); + }); + it.effect("fences a native merge on the target branch's current commit", () => { const branchOid = "fedcba9876543210fedcba9876543210fedcba98"; const server = fakeGitCafe({ diff --git a/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.ts b/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.ts index 41683de1767d7..4347a2acec433 100644 --- a/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.ts +++ b/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.ts @@ -53,6 +53,7 @@ const DIFF_FILES_BATCH = 64; */ const LINE_STATS_MAX_FILES = DIFF_FILES_BATCH * 4; const MAX_CHANGE_PAGES = 20; +const LINE_STATS_CACHE_SIZE = 256; const chunk = (items: ReadonlyArray, size: number): Array> => Array.from({ length: Math.ceil(items.length / size) }, (_, index) => @@ -354,6 +355,11 @@ export const make = Effect.gen(function* () { return yield* failure(operation, `GitCafe's stack operation ${outcome.state}.`); }); + // GitCafe's pulls carry no line totals, and linked pull requests are summarized every minute, + // so hunk totals are held per compared pair of commits. The file list that names the pair is + // cheap; a retarget or a landed parent layer moves the base without moving the head. + const lineTotals = new Map(); + const readLineStats = Effect.fn("GitCafePullRequestProvider.readLineStats")(function* ( input: PullRef, version: number, @@ -381,21 +387,33 @@ export const make = Effect.gen(function* () { after = batch.next; if (after === null || after === previous || batch.items.length === 0) break; } - const counted = - files.some((file) => file.additions !== undefined) || - files.length > LINE_STATS_MAX_FILES || - revision === undefined - ? files - : (yield* Effect.forEach( + if (revision === undefined) return { changedFiles: 0, additions: 0, deletions: 0 }; + const key = `${input.host} ${input.repository.toLowerCase()} ${input.number} ${revision.baseOid} ${revision.headOid}`; + const held = lineTotals.get(key); + if (held !== undefined) return { changedFiles: files.length, ...held }; + const details = files.some((file) => file.additions !== undefined) + ? Option.some(files) + : files.length > LINE_STATS_MAX_FILES + ? Option.none() + : yield* Effect.forEach( chunk(detailablePaths(files), DIFF_FILES_BATCH), (paths) => readDiffFiles(input, revision, paths), { concurrency: 4 }, - ).pipe(Effect.orElseSucceed(() => [[]]))).flat(); - return { - changedFiles: files.length, + ).pipe( + Effect.map((batches) => batches.flat()), + Effect.option, + ); + // Unread hunks leave the lines unknown rather than zero; the file count still stands. + if (Option.isNone(details)) return { changedFiles: files.length }; + const counted = details.value; + const totals = { additions: counted.reduce((total, file) => total + (file.additions ?? 0), 0), deletions: counted.reduce((total, file) => total + (file.deletions ?? 0), 0), }; + if (lineTotals.size >= LINE_STATS_CACHE_SIZE) + lineTotals.delete(lineTotals.keys().next().value!); + lineTotals.set(key, totals); + return { changedFiles: files.length, ...totals }; }); const merge = Effect.fn("GitCafePullRequestProvider.merge")(function* ( @@ -525,14 +543,27 @@ export const make = Effect.gen(function* () { }; }, ), - getChangeRequestSummary: (input) => - readPull("getChangeRequestSummary", input).pipe( - Effect.map((pull) => ({ - ...Json.toChangeRequest(pull, input), + getChangeRequestSummary: Effect.fn("GitCafePullRequestProvider.getChangeRequestSummary")( + function* (input) { + const pull = yield* readPull("getChangeRequestSummary", input); + const changes = + pull.headOid === null + ? null + : yield* readLineStats(input, pull.version).pipe(Effect.orElseSucceed(() => null)); + // A count that could not be read is absent rather than zero. + const { + additions: _additions, + deletions: _deletions, + ...summary + } = Json.toChangeRequest(pull, input); + return { + ...summary, + ...(changes === null ? {} : changes), closedAt: pull.closedAt, mergedAt: pull.mergedAt, - })), - ), + }; + }, + ), getChangeRequest: Effect.fn("GitCafePullRequestProvider.getChangeRequest")(function* (input) { const pull = yield* readPull("getChangeRequest", input); const path = yield* pullPath("getChangeRequest", input);