mirror of
https://github.com/firstsun-dev/git-files-sync.git
synced 2026-07-22 06:54:27 +00:00
perf(push): GitHub batch push/delete via GraphQL createCommitOnBranch
GitHubService.pushBatch/deleteBatch previously created each file's blob with a REST POST .../git/blobs call before the tree/commit/ref sequence, so an N-file batch still cost N/8 rounds of latency-bound round trips even with concurrency. Switched to GitHub's createCommitOnBranch GraphQL mutation, which carries file content directly in the request body, cutting an N-file batch to 2-3 HTTP calls total. - Extracted BaseGitService.getLatestCommitSha from resolveGitHubStyleBaseTree, which is still used by pushSymlink (GraphQL's FileAddition has no file-mode field) and Gitea's batch methods (no GraphQL API). - Added explicit handling for GraphQL's 200-status-with-errors-array failure mode, which REST's status-code check can't catch. - Verified against a live GitHub repo: pushBatch/deleteBatch each produced one commit with correct content and blob shas.
This commit is contained in:
ClaudiaFang
parent
12cce6497e
commit
114a5759a7
3 changed files with 126 additions and 116 deletions
|
|
@ -281,16 +281,25 @@ export abstract class BaseGitService {
|
|||
}
|
||||
|
||||
/**
|
||||
* Resolves a branch to its latest commit sha and that commit's base tree
|
||||
* sha, via GitHub's `git/ref/heads/{branch}` endpoint. Gitea's older
|
||||
* versions require a different branch-resolution endpoint, so it provides
|
||||
* its own override rather than using this helper.
|
||||
* Resolves a branch to its latest commit sha via GitHub's
|
||||
* `git/ref/heads/{branch}` endpoint. Gitea's older versions require a
|
||||
* different branch-resolution endpoint, so it provides its own override
|
||||
* rather than using this helper.
|
||||
*/
|
||||
protected async resolveGitHubStyleBaseTree(branch: string): Promise<{ latestCommitSha: string; baseTreeSha: string }> {
|
||||
protected async getLatestCommitSha(branch: string): Promise<string> {
|
||||
const base = this.getGitDataApiBase();
|
||||
const refResp = await this.safeRequest(`${base}/git/ref/heads/${branch}`, 'GET');
|
||||
const latestCommitSha = this.parseJson<{ object: { sha: string } }>(refResp).object.sha;
|
||||
return this.parseJson<{ object: { sha: string } }>(refResp).object.sha;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves a branch to its latest commit sha and that commit's base tree
|
||||
* sha. Only needed by the REST Git Data API flow (pushSymlink, and
|
||||
* Gitea's pushBatch/deleteBatch which lack a GraphQL alternative).
|
||||
*/
|
||||
protected async resolveGitHubStyleBaseTree(branch: string): Promise<{ latestCommitSha: string; baseTreeSha: string }> {
|
||||
const latestCommitSha = await this.getLatestCommitSha(branch);
|
||||
const base = this.getGitDataApiBase();
|
||||
const commitResp = await this.safeRequest(`${base}/git/commits/${latestCommitSha}`, 'GET');
|
||||
const baseTreeSha = this.parseJson<{ tree: { sha: string } }>(commitResp).tree.sha;
|
||||
|
||||
|
|
|
|||
|
|
@ -1,7 +1,22 @@
|
|||
import { GitServiceInterface, GitTreeEntry, BatchPushItem, BatchPushResult } from './git-service-interface';
|
||||
import { BaseGitService, ConnectionTestResult, GitFile, GitHubContentResponse, GitHubTreeResponse, GIT_SYMLINK_MODE, BLOB_CREATE_CONCURRENCY } from './git-service-base';
|
||||
import { BaseGitService, ConnectionTestResult, GitFile, GitHubContentResponse, GitHubTreeResponse, GIT_SYMLINK_MODE } from './git-service-base';
|
||||
import { logger } from '../utils/logger';
|
||||
|
||||
/**
|
||||
* Commits any mix of file additions/deletions in one request. Used instead of
|
||||
* the REST Git Data API's blob -> tree -> commit -> ref sequence for
|
||||
* pushBatch/deleteBatch: those need one HTTP round trip per blob to upload
|
||||
* content, while this mutation carries file content directly in the request
|
||||
* body, so an N-file batch is one call instead of N+~5.
|
||||
*/
|
||||
const CREATE_COMMIT_MUTATION = `
|
||||
mutation ($input: CreateCommitOnBranchInput!) {
|
||||
createCommitOnBranch(input: $input) {
|
||||
commit { oid }
|
||||
}
|
||||
}
|
||||
`;
|
||||
|
||||
export class GitHubService extends BaseGitService implements GitServiceInterface {
|
||||
private owner: string = '';
|
||||
private repo: string = '';
|
||||
|
|
@ -87,29 +102,48 @@ export class GitHubService extends BaseGitService implements GitServiceInterface
|
|||
return { path: fullPath, sha: blobSha };
|
||||
}
|
||||
|
||||
/**
|
||||
* Calls a GitHub GraphQL mutation. GraphQL reports mutation-level failures
|
||||
* (e.g. a stale expectedHeadOid) as a 200 response with an `errors` array
|
||||
* rather than an HTTP error status, so this checks for that on top of
|
||||
* safeRequest's status-code check.
|
||||
*/
|
||||
private async githubGraphQL<T>(query: string, variables: Record<string, unknown>): Promise<T> {
|
||||
const response = await this.safeRequest('https://api.github.com/graphql', 'POST', { query, variables });
|
||||
const body = this.parseJson<{ data?: T; errors?: Array<{ message: string }> }>(response);
|
||||
if (body.errors && body.errors.length > 0) {
|
||||
throw new Error(`GitHub GraphQL error: ${body.errors.map(e => e.message).join('; ')}`);
|
||||
}
|
||||
if (!body.data) {
|
||||
throw new Error('GitHub GraphQL response returned no data');
|
||||
}
|
||||
return body.data;
|
||||
}
|
||||
|
||||
async pushBatch(items: BatchPushItem[], branch: string, message: string): Promise<BatchPushResult[]> {
|
||||
if (items.length === 0) return [];
|
||||
const base = this.getGitDataApiBase();
|
||||
const { latestCommitSha, baseTreeSha } = await this.resolveGitHubStyleBaseTree(branch);
|
||||
const expectedHeadOid = await this.getLatestCommitSha(branch);
|
||||
|
||||
const blobShas = await this.mapWithConcurrency(items, BLOB_CREATE_CONCURRENCY, async item => {
|
||||
const blobResp = await this.safeRequest(`${base}/git/blobs`, 'POST', {
|
||||
content: this.encodeContent(item.content),
|
||||
encoding: 'base64',
|
||||
});
|
||||
return this.parseJson<{ sha: string }>(blobResp).sha;
|
||||
await this.githubGraphQL(CREATE_COMMIT_MUTATION, {
|
||||
input: {
|
||||
branch: { repositoryNameWithOwner: `${this.owner}/${this.repo}`, branchName: branch },
|
||||
message: { headline: message },
|
||||
expectedHeadOid,
|
||||
fileChanges: {
|
||||
additions: items.map(item => ({
|
||||
path: this.getFullPath(item.path),
|
||||
contents: this.encodeContent(item.content),
|
||||
})),
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const treeItems = items.map((item, i) => ({
|
||||
path: this.getFullPath(item.path),
|
||||
mode: '100644',
|
||||
type: 'blob' as const,
|
||||
sha: blobShas[i] as string,
|
||||
}));
|
||||
|
||||
await this.commitGitHubStyleTree(base, branch, baseTreeSha, latestCommitSha, treeItems, message);
|
||||
|
||||
return items.map((item, i) => ({ path: item.path, sha: blobShas[i] }));
|
||||
// createCommitOnBranch only returns the new commit's oid, not each
|
||||
// file's blob sha, so read them back with one follow-up tree fetch
|
||||
// (mirrors GitLab's pushBatch, which has the same limitation).
|
||||
const freshTree = await this.listFilesDetailed(branch, false);
|
||||
const shaByPath = new Map(freshTree.map(e => [e.path, e.sha]));
|
||||
return items.map(item => ({ path: item.path, sha: shaByPath.get(this.getFullPath(item.path)) }));
|
||||
}
|
||||
|
||||
async listFilesDetailed(branch: string, useFilter = true): Promise<GitTreeEntry[]> {
|
||||
|
|
@ -160,17 +194,18 @@ export class GitHubService extends BaseGitService implements GitServiceInterface
|
|||
|
||||
async deleteBatch(paths: string[], branch: string, message: string): Promise<void> {
|
||||
if (paths.length === 0) return;
|
||||
const base = this.getGitDataApiBase();
|
||||
const { latestCommitSha, baseTreeSha } = await this.resolveGitHubStyleBaseTree(branch);
|
||||
const expectedHeadOid = await this.getLatestCommitSha(branch);
|
||||
|
||||
const treeItems = paths.map(path => ({
|
||||
path: this.getFullPath(path),
|
||||
mode: '100644',
|
||||
type: 'blob' as const,
|
||||
sha: null,
|
||||
}));
|
||||
|
||||
await this.commitGitHubStyleTree(base, branch, baseTreeSha, latestCommitSha, treeItems, message);
|
||||
await this.githubGraphQL(CREATE_COMMIT_MUTATION, {
|
||||
input: {
|
||||
branch: { repositoryNameWithOwner: `${this.owner}/${this.repo}`, branchName: branch },
|
||||
message: { headline: message },
|
||||
expectedHeadOid,
|
||||
fileChanges: {
|
||||
deletions: paths.map(path => ({ path: this.getFullPath(path) })),
|
||||
},
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
async testConnection(branch: string): Promise<ConnectionTestResult> {
|
||||
|
|
|
|||
|
|
@ -105,15 +105,11 @@ describe('GitHubService', () => {
|
|||
expect(requestUrl).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('commits N files in one commit via ref -> commit -> blobs -> tree -> commit -> ref', async () => {
|
||||
it('commits N files in one GraphQL mutation via ref -> createCommitOnBranch -> tree', async () => {
|
||||
vi.mocked(requestUrl)
|
||||
.mockResolvedValueOnce({ status: 200, json: { object: { sha: 'commit1' } } } as unknown as RequestUrlResponse) // get ref
|
||||
.mockResolvedValueOnce({ status: 200, json: { tree: { sha: 'tree1' } } } as unknown as RequestUrlResponse) // get commit
|
||||
.mockResolvedValueOnce({ status: 201, json: { sha: 'blob-a' } } as unknown as RequestUrlResponse) // blob a
|
||||
.mockResolvedValueOnce({ status: 201, json: { sha: 'blob-b' } } as unknown as RequestUrlResponse) // blob b
|
||||
.mockResolvedValueOnce({ status: 201, json: { sha: 'tree2' } } as unknown as RequestUrlResponse) // create tree
|
||||
.mockResolvedValueOnce({ status: 201, json: { sha: 'commit2' } } as unknown as RequestUrlResponse) // create commit
|
||||
.mockResolvedValueOnce({ status: 200, json: {} } as unknown as RequestUrlResponse); // update ref
|
||||
.mockResolvedValueOnce({ status: 200, json: { data: { createCommitOnBranch: { commit: { oid: 'commit2' } } } } } as unknown as RequestUrlResponse) // mutation
|
||||
.mockResolvedValueOnce({ status: 200, json: { tree: [{ path: 'a.md', type: 'blob', sha: 'blob-a' }, { path: 'b.md', type: 'blob', sha: 'blob-b' }], truncated: false } } as unknown as RequestUrlResponse); // fresh tree
|
||||
|
||||
const result = await service.pushBatch(
|
||||
[{ path: 'a.md', content: 'hello' }, { path: 'b.md', content: 'world' }],
|
||||
|
|
@ -124,69 +120,37 @@ describe('GitHubService', () => {
|
|||
expect(result).toEqual([{ path: 'a.md', sha: 'blob-a' }, { path: 'b.md', sha: 'blob-b' }]);
|
||||
|
||||
const calls = vi.mocked(requestUrl).mock.calls.map(c => c[0] as RequestUrlParam);
|
||||
expect(calls).toHaveLength(7);
|
||||
expect(calls).toHaveLength(3);
|
||||
expect(calls[1]?.url).toBe('https://api.github.com/graphql');
|
||||
expect(calls[1]?.method).toBe('POST');
|
||||
|
||||
// blobs are base64-encoded (not pushSymlink's raw utf-8 path), so binary content works too.
|
||||
const blobABody = JSON.parse(calls[2]?.body as string) as { content: string; encoding: string };
|
||||
expect(blobABody.encoding).toBe('base64');
|
||||
expect(atob(blobABody.content)).toBe('hello');
|
||||
|
||||
const treeBody = JSON.parse(calls[4]?.body as string) as { base_tree: string; tree: Array<{ path: string; mode: string; type: string; sha: string }> };
|
||||
expect(treeBody.base_tree).toBe('tree1');
|
||||
expect(treeBody.tree).toEqual([
|
||||
{ path: 'a.md', mode: '100644', type: 'blob', sha: 'blob-a' },
|
||||
{ path: 'b.md', mode: '100644', type: 'blob', sha: 'blob-b' },
|
||||
const mutationBody = JSON.parse(calls[1]?.body as string) as {
|
||||
variables: { input: { branch: { repositoryNameWithOwner: string; branchName: string }; message: { headline: string }; expectedHeadOid: string; fileChanges: { additions: Array<{ path: string; contents: string }> } } };
|
||||
};
|
||||
const input = mutationBody.variables.input;
|
||||
expect(input.branch).toEqual({ repositoryNameWithOwner: `${owner}/${repo}`, branchName: 'main' });
|
||||
expect(input.message).toEqual({ headline: 'Push 2 file(s) from Obsidian' });
|
||||
expect(input.expectedHeadOid).toBe('commit1');
|
||||
// contents are base64-encoded (not pushSymlink's raw utf-8 path), so binary content works too.
|
||||
expect(atob(input.fileChanges.additions[0]!.contents)).toBe('hello');
|
||||
expect(input.fileChanges.additions).toEqual([
|
||||
{ path: 'a.md', contents: btoa('hello') },
|
||||
{ path: 'b.md', contents: btoa('world') },
|
||||
]);
|
||||
|
||||
const commitBody = JSON.parse(calls[5]?.body as string) as { message: string; tree: string; parents: string[] };
|
||||
expect(commitBody).toEqual({ message: 'Push 2 file(s) from Obsidian', tree: 'tree2', parents: ['commit1'] });
|
||||
|
||||
expect(calls[6]?.method).toBe('PATCH');
|
||||
expect(JSON.parse(calls[6]?.body as string)).toEqual({ sha: 'commit2' });
|
||||
});
|
||||
|
||||
it('creates blobs concurrently, not one at a time (regression guard against a sequential loop)', async () => {
|
||||
// Deferred blob responses: none of them resolve until every blob
|
||||
// POST has already been dispatched. A sequential `for` loop would
|
||||
// deadlock here since call N+1 never fires until call N resolves.
|
||||
const blobDeferreds = Array.from({ length: 4 }, () => {
|
||||
let resolve!: (v: RequestUrlResponse) => void;
|
||||
const promise = new Promise<RequestUrlResponse>(r => { resolve = r; });
|
||||
return { promise, resolve };
|
||||
});
|
||||
let blobCallCount = 0;
|
||||
it('throws when the GraphQL response reports errors on an HTTP 200', async () => {
|
||||
// GraphQL reports mutation failures (e.g. a stale expectedHeadOid) as a
|
||||
// 200 response with an `errors` array, not an HTTP error status.
|
||||
vi.mocked(requestUrl)
|
||||
.mockResolvedValueOnce({ status: 200, json: { object: { sha: 'commit1' } } } as unknown as RequestUrlResponse) // get ref
|
||||
.mockResolvedValueOnce({ status: 200, json: { errors: [{ message: 'Head sha was modified' }] } } as unknown as RequestUrlResponse); // mutation failure
|
||||
|
||||
vi.mocked(requestUrl).mockImplementation(((param: string | RequestUrlParam) => {
|
||||
const url = (param as RequestUrlParam).url;
|
||||
if (url.endsWith('/git/ref/heads/main')) {
|
||||
return Promise.resolve({ status: 200, json: { object: { sha: 'commit1' } } } as unknown as RequestUrlResponse);
|
||||
}
|
||||
if (url.includes('/git/commits/commit1')) {
|
||||
return Promise.resolve({ status: 200, json: { tree: { sha: 'tree1' } } } as unknown as RequestUrlResponse);
|
||||
}
|
||||
if (url.endsWith('/git/blobs')) {
|
||||
return blobDeferreds[blobCallCount++]!.promise;
|
||||
}
|
||||
if (url.endsWith('/git/trees')) {
|
||||
return Promise.resolve({ status: 201, json: { sha: 'tree2' } } as unknown as RequestUrlResponse);
|
||||
}
|
||||
if (url.endsWith('/git/commits')) {
|
||||
return Promise.resolve({ status: 201, json: { sha: 'commit2' } } as unknown as RequestUrlResponse);
|
||||
}
|
||||
return Promise.resolve({ status: 200, json: {} } as unknown as RequestUrlResponse);
|
||||
}) as unknown as typeof requestUrl);
|
||||
|
||||
const items = Array.from({ length: 4 }, (_, i) => ({ path: `f${i}.md`, content: `content${i}` }));
|
||||
const resultPromise = service.pushBatch(items, 'main', 'push');
|
||||
|
||||
// Wait for every blob POST to have been dispatched — a sequential
|
||||
// loop would never get past the first one, since its promise never resolves.
|
||||
await vi.waitFor(() => expect(blobCallCount).toBe(4));
|
||||
|
||||
blobDeferreds.forEach((d, i) => d.resolve({ status: 201, json: { sha: `blob-${i}` } } as unknown as RequestUrlResponse));
|
||||
const result = await resultPromise;
|
||||
|
||||
expect(result).toEqual(items.map((item, i) => ({ path: item.path, sha: `blob-${i}` })));
|
||||
await expect(service.pushBatch(
|
||||
[{ path: 'a.md', content: 'hello' }],
|
||||
'main',
|
||||
'Push 1 file(s) from Obsidian'
|
||||
)).rejects.toThrow('Head sha was modified');
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -362,31 +326,33 @@ describe('GitHubService', () => {
|
|||
expect(requestUrl).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('deletes N files in one commit via ref -> commit -> tree(sha:null) -> commit -> ref', async () => {
|
||||
it('deletes N files in one GraphQL mutation via ref -> createCommitOnBranch', async () => {
|
||||
vi.mocked(requestUrl)
|
||||
.mockResolvedValueOnce({ status: 200, json: { object: { sha: 'commit1' } } } as unknown as RequestUrlResponse) // get ref
|
||||
.mockResolvedValueOnce({ status: 200, json: { tree: { sha: 'tree1' } } } as unknown as RequestUrlResponse) // get commit
|
||||
.mockResolvedValueOnce({ status: 201, json: { sha: 'tree2' } } as unknown as RequestUrlResponse) // create tree
|
||||
.mockResolvedValueOnce({ status: 201, json: { sha: 'commit2' } } as unknown as RequestUrlResponse) // create commit
|
||||
.mockResolvedValueOnce({ status: 200, json: {} } as unknown as RequestUrlResponse); // update ref
|
||||
.mockResolvedValueOnce({ status: 200, json: { data: { createCommitOnBranch: { commit: { oid: 'commit2' } } } } } as unknown as RequestUrlResponse); // mutation
|
||||
|
||||
await service.deleteBatch(['a.md', 'b.md'], 'main', 'Delete 2 file(s) from Obsidian');
|
||||
|
||||
const calls = vi.mocked(requestUrl).mock.calls.map(c => c[0] as RequestUrlParam);
|
||||
expect(calls).toHaveLength(5);
|
||||
expect(calls).toHaveLength(2);
|
||||
expect(calls[1]?.url).toBe('https://api.github.com/graphql');
|
||||
|
||||
const treeBody = JSON.parse(calls[2]?.body as string) as { base_tree: string; tree: Array<{ path: string; mode: string; type: string; sha: string | null }> };
|
||||
expect(treeBody.base_tree).toBe('tree1');
|
||||
expect(treeBody.tree).toEqual([
|
||||
{ path: 'a.md', mode: '100644', type: 'blob', sha: null },
|
||||
{ path: 'b.md', mode: '100644', type: 'blob', sha: null },
|
||||
]);
|
||||
const mutationBody = JSON.parse(calls[1]?.body as string) as {
|
||||
variables: { input: { expectedHeadOid: string; message: { headline: string }; fileChanges: { deletions: Array<{ path: string }> } } };
|
||||
};
|
||||
const input = mutationBody.variables.input;
|
||||
expect(input.expectedHeadOid).toBe('commit1');
|
||||
expect(input.message).toEqual({ headline: 'Delete 2 file(s) from Obsidian' });
|
||||
expect(input.fileChanges.deletions).toEqual([{ path: 'a.md' }, { path: 'b.md' }]);
|
||||
});
|
||||
|
||||
const commitBody = JSON.parse(calls[3]?.body as string) as { message: string; tree: string; parents: string[] };
|
||||
expect(commitBody).toEqual({ message: 'Delete 2 file(s) from Obsidian', tree: 'tree2', parents: ['commit1'] });
|
||||
it('throws when the GraphQL response reports errors on an HTTP 200', async () => {
|
||||
vi.mocked(requestUrl)
|
||||
.mockResolvedValueOnce({ status: 200, json: { object: { sha: 'commit1' } } } as unknown as RequestUrlResponse) // get ref
|
||||
.mockResolvedValueOnce({ status: 200, json: { errors: [{ message: 'Head sha was modified' }] } } as unknown as RequestUrlResponse); // mutation failure
|
||||
|
||||
expect(calls[4]?.method).toBe('PATCH');
|
||||
expect(JSON.parse(calls[4]?.body as string)).toEqual({ sha: 'commit2' });
|
||||
await expect(service.deleteBatch(['a.md'], 'main', 'Delete 1 file(s) from Obsidian'))
|
||||
.rejects.toThrow('Head sha was modified');
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue