diff --git a/feature_list.json b/feature_list.json index 1de1749..c5c3aa3 100644 --- a/feature_list.json +++ b/feature_list.json @@ -81,6 +81,14 @@ "dependencies": ["feat-009"], "status": "not-started", "evidence": "" + }, + { + "id": "feat-011", + "name": "perf(push): batch-commit push-all + SHA-based diffing", + "description": "Push-all was up to 3N sequential HTTP calls and N separate commits for N files, plus two redundant full-tree fetches per run. Added optional GitServiceInterface.pushBatch (GitHub/Gitea via blob->tree->commit->ref, generalizing pushSymlink's pattern; GitLab via the native multi-action Commits API + a follow-up tree read for blob shas); sync-manager now classifies each file via a locally-computed git blob sha against one pre-fetched remote tree (no getFile per file) and commits all queued files in one grouped call (chunked at MAX_BATCH_PUSH_SIZE=200), falling back to the old per-file path when a provider has no pushBatch; main.ts/GitignoreManager now share one tree fetch instead of two", + "dependencies": [], + "status": "done", + "evidence": "npx eslint . -> 0 errors; npm run build -> clean; npx vitest run -> 330/330 passed; consolidated onto PR #51" } ], "_evidenceStyle": "Keep evidence to one line: commit hash + short pointer (e.g. 'Commit abc1234 - added X, tests pass'). Debugging narrative and design discussion belong in the commit message, not this file." diff --git a/progress.md b/progress.md index 1ea588c..25c3570 100644 --- a/progress.md +++ b/progress.md @@ -12,9 +12,9 @@ Completed work is archived in [archive/](./archive/), one file per calendar mont ## Current State -**Last Updated:** 2026-07-14 09:35 +**Last Updated:** 2026-07-14 10:10 **Session ID:** current -**Active Feature:** issue #78 (blog repo, misfiled — actually a git-files-sync bug: delete remote-only file fails) — three root causes found and fixed, awaiting user re-test after rebuild +**Active Feature:** feat-011 (perf: batch-commit push-all + SHA-based diffing) — done, lint/build/test all green. Previous active item (issue #78 delete-remote-only-file fix) still awaiting user re-test after rebuild. ## Status @@ -33,16 +33,28 @@ Completed work is archived in [archive/](./archive/), one file per calendar mont - All three root causes committed (`896d77b`, merge `563a28e`, `fa42fea`) and pushed to `claude/fix-directory-symlink-pull-260713`. Test coverage for #2/#3 added after that — pending commit as of this note. - Not yet done: comment on/reference issue #78 (in the `blog` repo) noting the fix landed in `git-files-sync` instead — flag to user before doing so since it crosses repos. +- [x] feat-011: perf(push): batch-commit push-all + SHA-based diffing. User asked "現在外掛的 push功能很慢,可能是什麼原因" then approved doing all three identified fixes (a/b/c) via a full plan-mode design pass. Implementation: + 1. **Batched commit instead of one-commit-per-file**: new optional `GitServiceInterface.pushBatch?(items, branch, message)`. `GitHubService`/`GiteaService` implement it via the git blob→tree→commit→ref Data API (generalizing `pushSymlink`'s existing pattern, factored into shared `resolveGitHubStyleBaseTree`/`commitGitHubStyleTree` helpers in `git-service-base.ts`); `GitLabService` implements it via the native multi-file Commits API (`POST .../repository/commits` with an `actions` array), with one follow-up `listFilesDetailed` call afterward to recover each file's new blob sha (that API doesn't return them). Symlinks stay on the existing per-file `pushSymlink` path, not folded into batches. + 2. **SHA-based diffing instead of per-file `getFile`**: `sync-manager.ts`'s push-all flow (`processPushBatch`/`classifyPushCandidate`/`classifyAgainstTreeEntry`) now compares a locally-computed git blob sha (`utils/git-blob-sha.ts`'s `gitBlobSha`, already used by feat-006's status refresh) against a pre-fetched remote tree's per-entry sha — no network round trip needed to detect unchanged/new/modified/conflicting files. Rename detection and symlink handling are untouched (still per-file, immediate). + 3. **Dedup the remote-tree fetch**: `main.ts:runAllFiles` now fetches the tree once (`listFilesDetailed(branch, false)`) and threads it into both `GitignoreManager.loadGitignores(tree)` (new optional param, replacing its own `getRepoGitignores` call when a tree is supplied) and `SyncManager.pushAllFiles(files, onProgress, tree)` — replacing the old discarded `listFiles()` call plus gitignore-manager's separate fetch with a single shared one. + - Batches are chunked at `MAX_BATCH_PUSH_SIZE = 200` files per commit call (`git-service-base.ts`); a failed chunk marks every file in that chunk as failed (not silently dropped), earlier successful chunks stay committed. + - Providers without `pushBatch` (future Bitbucket) fall back to the original sequential per-file push path, unchanged. + - Added `pushBatch` tests to `github-service.test.ts`/`gitea-service.test.ts`/`gitlab-service.test.ts` (happy path, empty-batch short-circuit, base64 encoding, GitLab's create-vs-update action + sha-recovery-miss case), a `MAX_BATCH_PUSH_SIZE` sanity test, new `sync-manager-batch.test.ts` cases (grouped pushBatch call, mixed binary+text batch, whole-chunk-failure, sha-match skips both `getFile` and `pushBatch`), and a `gitignore-manager.test.ts` case for the pre-fetched-tree path. Existing conflict/rename/symlink batch tests rewritten to mock `listFilesDetailed` instead of `getFile` for the equality/conflict check (rename detection's own `getFile` calls are untouched). + - Evidence: `npx eslint .` → 0 errors; `npm run build` → clean; `npx vitest run` → 330/330 passed (313 pre-existing + wording/mock updates + 17 new cases). + - Pull path (`pullAllFiles`) intentionally untouched — out of scope, still needs full remote content regardless of sha comparison. + - Not yet committed as of this note — pending commit onto `claude/fix-directory-symlink-pull-260713`. + ### What's In Progress - Nothing else actively in progress. ### What's Next -1. Get user confirmation that delete now works (or a new detailed error message) after they rebuild/reload with the latest push. -2. Issue #37 (Bitbucket provider support, feat-010) — large, was deferred until #38 (i18n) landed. Now unblocked. -3. Re-sync against `gh issue list --repo firstsun-dev/git-files-sync --state open`. Remaining genuinely-unstarted issues as of this session: #47 (regex ignore lists), #45 (SonarQube findings), #37 (Bitbucket), #28 (non-engineering: community visibility). -4. PR #51 is large (7+ issues' worth of changes now). If the user wants to review/merge it before more work piles on, flag this rather than continuing to add commits indefinitely. +1. Get user confirmation that delete now works (or a new detailed error message) after they rebuild/reload with the latest push (issue #78). +2. Manually verify feat-011 in Obsidian if possible: push-all on a mixed vault should produce one new commit containing only changed/new files. +3. Issue #37 (Bitbucket provider support, feat-010) — large, was deferred until #38 (i18n) landed. Now unblocked. Its `GitServiceInterface` implementation should simply omit `pushBatch` and use the existing per-file fallback. +4. Re-sync against `gh issue list --repo firstsun-dev/git-files-sync --state open`. Remaining genuinely-unstarted issues as of this session: #47 (regex ignore lists), #45 (SonarQube findings), #37 (Bitbucket), #28 (non-engineering: community visibility). +5. PR #51 is large (8+ issues' worth of changes now). If the user wants to review/merge it before more work piles on, flag this rather than continuing to add commits indefinitely. ## Blockers / Risks diff --git a/src/logic/gitignore-manager.ts b/src/logic/gitignore-manager.ts index 1688383..330914d 100644 --- a/src/logic/gitignore-manager.ts +++ b/src/logic/gitignore-manager.ts @@ -1,6 +1,6 @@ import ignore, { Ignore } from 'ignore'; import { App } from 'obsidian'; -import { GitServiceInterface } from '../services/git-service-interface'; +import { GitServiceInterface, GitTreeEntry } from '../services/git-service-interface'; import { logger } from '../utils/logger'; import { readLocalSymlinkTarget } from '../utils/symlink'; @@ -41,11 +41,34 @@ export class GitignoreManager { /** * Discovers and parses .gitignore files from the local filesystem and remote repository. * Local files take priority; remote supplements anything not found locally. + * + * @param remoteTree Optional pre-fetched, unfiltered remote tree (e.g. from + * `gitService.listFilesDetailed(branch, false)`). When supplied, it's scanned + * directly for `.gitignore` paths instead of making another remote fetch via + * `getRepoGitignores`. Falls back to that fetch when omitted, so this method + * still works standalone (e.g. in tests). */ - async loadGitignores(): Promise { + async loadGitignores(remoteTree?: GitTreeEntry[]): Promise { this.ignoreMap.clear(); - // 1. Collect all potential gitignore paths + const gitignorePaths = await this.collectGitignorePaths(remoteTree); + + // Load content and build ignore instances + for (const fullGitignorePath of gitignorePaths) { + const dirPath = fullGitignorePath === '.gitignore' + ? '' + : fullGitignorePath.slice(0, -(('.gitignore'.length) + 1)); + const content = await this.getGitignoreContent(fullGitignorePath); + if (content) { + this.ignoreMap.set(dirPath, ignore().add(content)); + } + } + } + + /** Collects every candidate .gitignore path: repo root, rootPath ancestors, + * local vault scan, and the remote listing (from a pre-fetched tree when + * supplied, else a dedicated remote fetch). */ + private async collectGitignorePaths(remoteTree?: GitTreeEntry[]): Promise> { const gitignorePaths = new Set(); // a. Repo root @@ -66,23 +89,26 @@ export class GitignoreManager { await this.scanLocalGitignores(gitignorePaths); // d. Supplement with remote repo's gitignore listing (filtered to rootPath) + await this.addRemoteGitignorePaths(gitignorePaths, remoteTree); + + return gitignorePaths; + } + + /** Adds remote .gitignore paths to `out`: scanned directly from a + * pre-fetched tree when supplied, else via a dedicated remote fetch. */ + private async addRemoteGitignorePaths(out: Set, remoteTree?: GitTreeEntry[]): Promise { + if (remoteTree) { + for (const entry of remoteTree) { + if (entry.path.endsWith('.gitignore')) out.add(entry.path); + } + return; + } try { const remotePaths = await this.gitService.getRepoGitignores(this.branch); - for (const p of remotePaths) gitignorePaths.add(p); + for (const p of remotePaths) out.add(p); } catch (e) { logger.warn('Failed to fetch repo gitignores', e); } - - // 2. Load content and build ignore instances - for (const fullGitignorePath of gitignorePaths) { - const dirPath = fullGitignorePath === '.gitignore' - ? '' - : fullGitignorePath.slice(0, -(('.gitignore'.length) + 1)); - const content = await this.getGitignoreContent(fullGitignorePath); - if (content) { - this.ignoreMap.set(dirPath, ignore().add(content)); - } - } } private async scanLocalGitignores(out: Set): Promise { diff --git a/src/logic/sync-manager.ts b/src/logic/sync-manager.ts index ab5cf27..79c16d6 100644 --- a/src/logic/sync-manager.ts +++ b/src/logic/sync-manager.ts @@ -1,14 +1,19 @@ import { TFile, App, Notice } from 'obsidian'; -import { GitServiceInterface } from '../services/git-service-interface'; +import { GitServiceInterface, GitTreeEntry } from '../services/git-service-interface'; +import { MAX_BATCH_PUSH_SIZE } from '../services/git-service-base'; import { GitLabFilesPushSettings, getServiceName, getEffectiveSymlinkHandling } from '../settings'; import { SyncConflictModal } from '../ui/SyncConflictModal'; import { logger } from '../utils/logger'; import { isBinaryPath, contentsEqual } from '../utils/path'; import { readLocalSymlinkTarget, createLocalSymlink } from '../utils/symlink'; +import { gitBlobSha } from '../utils/git-blob-sha'; /** Result of syncing one file within a batch push/pull. */ type BatchOutcome = 'done' | 'unchanged' | 'conflict'; +/** A file classified as needing a push, queued for the grouped batch-commit call. */ +type ToPushEntry = { path: string; name: string; repoPath: string; content: string | ArrayBuffer; existingSha?: string }; + export class SyncManager { private readonly app: App; private gitService: GitServiceInterface; @@ -351,17 +356,20 @@ export class SyncManager { new Notice(`${message}: ${detail}`); } - async pushAllFiles(files: (TFile | string)[], onProgress?: (current: number, total: number, fileName: string) => void): Promise<{ success: number; failed: number; conflicts: number; errors: Array<{ file: string; error: string }> }> { - return this.processBatch(files, 'push', onProgress); + async pushAllFiles( + files: (TFile | string)[], + onProgress?: (current: number, total: number, fileName: string) => void, + remoteTree?: GitTreeEntry[] + ): Promise<{ success: number; failed: number; conflicts: number; errors: Array<{ file: string; error: string }> }> { + return this.processPushBatch(files, onProgress, remoteTree); } async pullAllFiles(files: (TFile | string)[], onProgress?: (current: number, total: number, fileName: string) => void): Promise<{ success: number; failed: number; conflicts: number; errors: Array<{ file: string; error: string }> }> { - return this.processBatch(files, 'pull', onProgress); + return this.processPullBatch(files, onProgress); } - private async processBatch( + private async processPullBatch( files: (TFile | string)[], - op: 'push' | 'pull', onProgress?: (current: number, total: number, fileName: string) => void ): Promise<{ success: number; failed: number; conflicts: number; errors: Array<{ file: string; error: string }> }> { const results = { success: 0, failed: 0, conflicts: 0, errors: [] as Array<{ file: string; error: string }> }; @@ -374,25 +382,218 @@ export class SyncManager { onProgress?.(i + 1, files.length, name); try { - const outcome = op === 'push' - ? await this.processSingleBatchPush(fileOrPath, path, name, isString) - : await this.processSingleBatchPull(fileOrPath, path, name, isString); - + const outcome = await this.processSingleBatchPull(fileOrPath, path, name, isString); if (outcome === 'done') results.success++; else if (outcome === 'conflict') results.conflicts++; } catch (e) { - logger.error(`Failed to ${op} ${path}:`, e); + logger.error(`Failed to pull ${path}:`, e); results.failed++; results.errors.push({ file: path, error: e instanceof Error ? e.message : String(e) }); } } await this.saveSettings(); - this.notifyBatchResult(op, results.success, results.failed, results.conflicts); + this.notifyBatchResult('pull', results.success, results.failed, results.conflicts); return results; } + private async processPushBatch( + files: (TFile | string)[], + onProgress?: (current: number, total: number, fileName: string) => void, + remoteTree?: GitTreeEntry[] + ): Promise<{ success: number; failed: number; conflicts: number; errors: Array<{ file: string; error: string }> }> { + const results = { success: 0, failed: 0, conflicts: 0, errors: [] as Array<{ file: string; error: string }> }; + + const tree = remoteTree ?? await this.gitService.listFilesDetailed(this.settings.branch, false); + const treeByFullPath = new Map(tree.map(e => [e.path, e])); + const toPush: ToPushEntry[] = []; + + for (let i = 0; i < files.length; i++) { + const fileOrPath = files[i]; + if (!fileOrPath) continue; + + const { path, name, isString } = this.getFileInfo(fileOrPath); + onProgress?.(i + 1, files.length, name); + + try { + const outcome = await this.classifyPushCandidate(fileOrPath, path, name, isString, treeByFullPath, toPush); + if (outcome === 'done') results.success++; + else if (outcome === 'conflict') results.conflicts++; + // 'unchanged' and 'queued' don't move any of the counters directly: + // 'unchanged' never did, and 'queued' is resolved by commitPushBatch below. + } catch (e) { + logger.error(`Failed to push ${path}:`, e); + results.failed++; + results.errors.push({ file: path, error: e instanceof Error ? e.message : String(e) }); + } + } + + if (toPush.length > 0) { + await this.commitPushBatch(toPush, results); + } + + await this.saveSettings(); + this.notifyBatchResult('push', results.success, results.failed, results.conflicts); + + return results; + } + + /** + * Classifies one file for the batch-push flow using a purely local + * comparison (git blob sha vs. the pre-fetched remote tree's blob sha) — + * no getFile network call. Symlinks and confirmed renames are pushed + * immediately (as today) and never queued; everything else is either + * resolved immediately ('unchanged'/'conflict') or appended to `toPush` + * for the grouped commit. + */ + private async classifyPushCandidate( + fileOrPath: TFile | string, + path: string, + name: string, + isString: boolean, + treeByFullPath: Map, + toPush: ToPushEntry[] + ): Promise { + if (!await this.checkFileExists(path, isString)) throw new Error('File no longer exists'); + + // Symbolic link handling: real → push as a symlink (GitHub), skip → ignore. + const symlinkTarget = readLocalSymlinkTarget(this.app, path); + if (symlinkTarget !== null && await this.handleSymlinkPush({ path, name }, symlinkTarget, true)) { + return getEffectiveSymlinkHandling(this.settings) !== 'skip' ? 'done' : 'unchanged'; + } + + const content = await this.getFileContent(fileOrPath); + const repoPath = this.getNormalizedPath(path); + + // Rename detection + if (!isString && fileOrPath instanceof TFile) { + const renamedFrom = await this.detectRename(fileOrPath, content); + if (renamedFrom) { + await this.handleRename(fileOrPath, renamedFrom, content); + return 'done'; + } + } + + const treeEntry = treeByFullPath.get(this.getFullPathForTree(repoPath)); + const outcome = await this.classifyAgainstTreeEntry(path, content, treeEntry); + if (outcome !== 'queued') return outcome; + + toPush.push({ path, name, repoPath, content, existingSha: treeEntry?.sha }); + return 'queued'; + } + + /** + * Decides a non-symlink, non-renamed file's outcome purely from a + * pre-fetched tree entry and a locally-computed git blob sha — no network + * call. Split out of classifyPushCandidate to keep both under the + * cognitive-complexity limit. + */ + private async classifyAgainstTreeEntry( + path: string, + content: string | ArrayBuffer, + treeEntry: GitTreeEntry | undefined + ): Promise { + // Don't convert a remote symlink into a regular file. + if (treeEntry?.symlink) return 'unchanged'; + + // Skip if already in sync — compared locally, no network round trip. + if (treeEntry?.sha) { + const localSha = await gitBlobSha(content); + if (localSha === treeEntry.sha) { + await this.updateMetadata(path, treeEntry.sha); + return 'unchanged'; + } + } + + // Same conflict check as the single-file flow: if the remote has moved on + // from what we last synced, overwriting it here would silently discard + // whatever changed on the remote. Skip it instead of force-pushing so the + // batch action can't quietly clobber changes the way a single push would + // stop and ask about via SyncConflictModal. + const lastSynced = this.settings.syncMetadata[path]; + if (treeEntry?.sha && lastSynced && treeEntry.sha !== lastSynced.lastSyncedSha) { + return 'conflict'; + } + + return 'queued'; + } + + /** + * Path relative to rootPath, matching how each git service's getFullPath + * would resolve `repoPath` — mirrors that logic locally so pre-fetched tree + * entries (always full repo paths) can be looked up without depending on + * each service's protected getFullPath. + */ + private getFullPathForTree(repoPath: string): string { + if (repoPath.startsWith('/')) return repoPath.slice(1); + const rootPath = this.settings.rootPath; + if (!rootPath) return repoPath; + const cleanRoot = rootPath.endsWith('/') ? rootPath : `${rootPath}/`; + if (repoPath.startsWith(cleanRoot)) return repoPath; + return cleanRoot + repoPath; + } + + /** Commits every queued file in one or more grouped batch-commit calls. */ + private async commitPushBatch( + toPush: ToPushEntry[], + results: { success: number; failed: number; conflicts: number; errors: Array<{ file: string; error: string }> } + ): Promise { + if (!this.gitService.pushBatch) { + await this.pushSequentialFallback(toPush, results); + return; + } + + for (let i = 0; i < toPush.length; i += MAX_BATCH_PUSH_SIZE) { + await this.commitOneChunk(toPush.slice(i, i + MAX_BATCH_PUSH_SIZE), results); + } + } + + /** Provider doesn't support a batch/atomic multi-file commit — fall back to + * the same sequential per-file push used by the single-file flow. */ + private async pushSequentialFallback( + toPush: ToPushEntry[], + results: { success: number; failed: number; conflicts: number; errors: Array<{ file: string; error: string }> } + ): Promise { + for (const f of toPush) { + try { + await this.performPush({ path: f.path, name: f.name }, f.content, f.existingSha, true); + results.success++; + } catch (e) { + results.failed++; + results.errors.push({ file: f.path, error: e instanceof Error ? e.message : String(e) }); + } + } + } + + private async commitOneChunk( + chunk: ToPushEntry[], + results: { success: number; failed: number; conflicts: number; errors: Array<{ file: string; error: string }> } + ): Promise { + try { + const commitMessage = `Push ${chunk.length} file(s) from Obsidian`; + const batchResults = await this.gitService.pushBatch!( + chunk.map(f => ({ path: f.repoPath, content: f.content, existedRemotely: !!f.existingSha })), + this.settings.branch, + commitMessage + ); + const shaByPath = new Map(batchResults.map(r => [r.path, r.sha])); + for (const f of chunk) { + const sha = shaByPath.get(f.repoPath); + if (sha) await this.updateMetadata(f.path, sha); + results.success++; + } + } catch (e) { + // Atomic per-provider failure: none of this chunk's files were + // actually written, so every file in it is failed, not dropped. + const message = e instanceof Error ? e.message : String(e); + for (const f of chunk) { + results.failed++; + results.errors.push({ file: f.path, error: message }); + } + } + } + private notifyBatchResult(op: 'push' | 'pull', success: number, failed: number, conflicts: number): void { const opName = op === 'push' ? 'Pushed' : 'Pulled'; if (success > 0) { @@ -443,52 +644,6 @@ export class SyncManager { } } - private async processSingleBatchPush(fileOrPath: TFile | string, path: string, name: string, isString: boolean): Promise { - if (!await this.checkFileExists(path, isString)) throw new Error('File no longer exists'); - - // Symbolic link handling: real → push as a symlink (GitHub), skip → ignore. - const symlinkTarget = readLocalSymlinkTarget(this.app, path); - if (symlinkTarget !== null && await this.handleSymlinkPush({ path, name }, symlinkTarget, true)) { - return getEffectiveSymlinkHandling(this.settings) !== 'skip' ? 'done' : 'unchanged'; - } - - const content = await this.getFileContent(fileOrPath); - const repoPath = this.getNormalizedPath(path); - - // Rename detection - if (!isString && fileOrPath instanceof TFile) { - const renamedFrom = await this.detectRename(fileOrPath, content); - if (renamedFrom) { - await this.handleRename(fileOrPath, renamedFrom, content); - return 'done'; - } - } - - const remote = await this.gitService.getFile(repoPath, this.settings.branch); - - // Don't convert a remote symlink into a regular file. - if (remote.isSymlink) return 'unchanged'; - - // Skip if already in sync - if (remote.sha && this.contentsEqual(content, remote.content)) { - await this.updateMetadata(path, remote.sha); - return 'unchanged'; - } - - // Same conflict check as the single-file flow: if the remote has moved on - // from what we last synced, overwriting it here would silently discard - // whatever changed on the remote. Skip it instead of force-pushing so the - // batch action can't quietly clobber changes the way a single push would - // stop and ask about via SyncConflictModal. - const lastSynced = this.settings.syncMetadata[path]; - if (remote.sha && lastSynced && remote.sha !== lastSynced.lastSyncedSha) { - return 'conflict'; - } - - await this.performPush({ path, name }, content, remote.sha || undefined, true); - return 'done'; - } - private async processSingleBatchPull(fileOrPath: TFile | string, path: string, name: string, isString: boolean): Promise { const repoPath = this.getNormalizedPath(path); const remote = await this.gitService.getFile(repoPath, this.settings.branch); diff --git a/src/main.ts b/src/main.ts index 8e10948..d5ca022 100644 --- a/src/main.ts +++ b/src/main.ts @@ -3,7 +3,7 @@ import { DEFAULT_SETTINGS, GitLabFilesPushSettings, GitLabSyncSettingTab, getSer import { GitLabService } from './services/gitlab-service'; import { GitHubService } from './services/github-service'; import { GiteaService } from './services/gitea-service'; -import { GitServiceInterface } from './services/git-service-interface'; +import { GitServiceInterface, GitTreeEntry } from './services/git-service-interface'; import { SyncManager } from './logic/sync-manager'; import { SyncStatusView, SYNC_STATUS_VIEW_TYPE } from './ui/SyncStatusView'; import { GitignoreManager } from './logic/gitignore-manager'; @@ -201,8 +201,16 @@ export default class GitLabFilesPush extends Plugin { const startPath = this.settings.vaultFolder || ''; const allPaths = await this.listAllFilesFromAdapter(startPath); - await this.gitService.listFiles(this.settings.branch); - await this.gitignoreManager.loadGitignores(); + // Fetch the remote tree once and share it with both gitignore discovery + // and (for push) the SHA-based diff, instead of each fetching it separately. + let tree: GitTreeEntry[] | undefined; + try { + tree = await this.gitService.listFilesDetailed(this.settings.branch, false); + } catch (e) { + logger.warn('Failed to fetch remote tree; falling back to per-call fetches', e); + } + + await this.gitignoreManager.loadGitignores(tree); const files = allPaths.filter(p => !this.gitignoreManager.isIgnored(this.getNormalizedPath(p))); if (files.length === 0) { @@ -224,7 +232,7 @@ export default class GitLabFilesPush extends Plugin { const results = op === 'push' ? await this.sync.pushAllFiles(files, (current, total, fileName) => { progressNotice.setMessage(t('main.progress.step', { verb: t('main.verb.pushing'), current, total, fileName })); - }) + }, tree) : await this.sync.pullAllFiles(files, (current, total, fileName) => { progressNotice.setMessage(t('main.progress.step', { verb: t('main.verb.pulling'), current, total, fileName })); }); diff --git a/src/services/git-service-base.ts b/src/services/git-service-base.ts index 89a858c..125b971 100644 --- a/src/services/git-service-base.ts +++ b/src/services/git-service-base.ts @@ -55,6 +55,10 @@ export interface ConnectionTestResult { error?: string; } +/** Max files per single batch-commit call. Guards against oversized request + * bodies / provider payload limits when a vault has thousands of files. */ +export const MAX_BATCH_PUSH_SIZE = 200; + export abstract class BaseGitService { protected token: string = ''; protected rootPath: string = ''; @@ -258,6 +262,61 @@ export abstract class BaseGitService { return e instanceof Error ? e : new Error(String(e)); } + /** + * The base URL for a GitHub-shaped Git Data API (e.g. + * `https://api.github.com/repos/{owner}/{repo}` or + * `{baseUrl}/api/v1/repos/{owner}/{repo}` for Gitea). Only meaningful for + * providers that implement pushBatch/pushSymlink via this API shape. + */ + protected getGitDataApiBase(): string { + throw new Error('getGitDataApiBase is not implemented for this provider'); + } + + /** + * 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. + */ + protected async resolveGitHubStyleBaseTree(branch: string): Promise<{ latestCommitSha: string; baseTreeSha: 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; + + const commitResp = await this.safeRequest(`${base}/git/commits/${latestCommitSha}`, 'GET'); + const baseTreeSha = this.parseJson<{ tree: { sha: string } }>(commitResp).tree.sha; + + return { latestCommitSha, baseTreeSha }; + } + + /** + * Commits N tree items (already-created blobs) in one shot: builds a new + * tree on top of baseTreeSha, commits it, and moves the branch ref to point + * at the new commit. Shared by GitHub/Gitea's pushSymlink and pushBatch. + */ + protected async commitGitHubStyleTree( + base: string, + branch: string, + baseTreeSha: string, + latestCommitSha: string, + treeItems: Array<{ path: string; mode: string; type: 'blob'; sha: string }>, + message: string + ): Promise { + const treeResp = await this.safeRequest(`${base}/git/trees`, 'POST', { base_tree: baseTreeSha, tree: treeItems }); + const newTreeSha = this.parseJson<{ sha: string }>(treeResp).sha; + + const newCommitResp = await this.safeRequest(`${base}/git/commits`, 'POST', { + message, + tree: newTreeSha, + parents: [latestCommitSha], + }); + const newCommitSha = this.parseJson<{ sha: string }>(newCommitResp).sha; + + await this.safeRequest(`${base}/git/refs/heads/${branch}`, 'PATCH', { sha: newCommitSha }); + + return newCommitSha; + } + async getRepoGitignores(branch: string): Promise { try { const allFiles = await this.listFiles(branch, false); // Fetch ALL files to find gitignores diff --git a/src/services/git-service-interface.ts b/src/services/git-service-interface.ts index b1518ec..7944b96 100644 --- a/src/services/git-service-interface.ts +++ b/src/services/git-service-interface.ts @@ -22,6 +22,26 @@ export interface GitTreeEntry { sha?: string; } +/** One file's content to include in a batched multi-file commit. */ +export interface BatchPushItem { + /** Path relative to rootPath, same shape pushFile's `path` param takes. */ + path: string; + content: string | ArrayBuffer; + /** + * Whether this path already existed on the remote before this push, per the + * caller's pre-fetched tree. Only GitLab's Commits API needs this (to choose + * action 'create' vs 'update'); GitHub/Gitea's tree-based commit ignores it. + */ + existedRemotely?: boolean; +} + +/** Result for one file after a batch push completes. */ +export interface BatchPushResult { + path: string; + /** New blob sha, when the provider can report it directly. */ + sha?: string; +} + export interface GitServiceInterface { updateConfig(...args: unknown[]): void; getFile(path: string, branch: string): Promise; @@ -37,6 +57,14 @@ export interface GitServiceInterface { * implement it; callers must fall back to pushFile when it's absent. */ pushSymlink?(path: string, target: string, branch: string, commitMessage: string): Promise<{ path: string, sha?: string }>; + /** + * Push many files in a single commit. Optional: only providers with a way to + * write multiple files atomically implement it; callers must fall back to + * sequential pushFile calls when it's absent (mirrors pushSymlink?). Must be + * atomic: on failure it throws rather than returning partial results, so the + * caller can mark every item in the attempted batch as failed. + */ + pushBatch?(items: BatchPushItem[], branch: string, commitMessage: string): Promise; deleteFile(path: string, branch: string, commitMessage: string): Promise; getRepoGitignores(branch: string): Promise; /** diff --git a/src/services/gitea-service.ts b/src/services/gitea-service.ts index 2281197..e5c6650 100644 --- a/src/services/gitea-service.ts +++ b/src/services/gitea-service.ts @@ -1,4 +1,4 @@ -import { GitServiceInterface, GitTreeEntry } from './git-service-interface'; +import { GitServiceInterface, GitTreeEntry, BatchPushItem, BatchPushResult } from './git-service-interface'; import { BaseGitService, ConnectionTestResult, GitFile, GitHubContentResponse, GitHubTreeResponse, GIT_SYMLINK_MODE } from './git-service-base'; import { logger } from '../utils/logger'; @@ -25,6 +25,25 @@ export class GiteaService extends BaseGitService implements GitServiceInterface return `${this.baseUrl}/api/v1/repos/${this.owner}/${this.repo}/contents/${encodedPath}`; } + protected getGitDataApiBase(): string { + return `${this.baseUrl}/api/v1/repos/${this.owner}/${this.repo}`; + } + + // Gitea's git/commits/{sha} endpoint needs a resolved commit sha, not a + // branch ref name, and older Gitea versions don't expose GitHub's + // git/ref/heads/{branch} endpoint at all — resolve via /branches/{branch} + // instead, same as listFilesDetailed already does. + private async resolveBaseTree(branch: string): Promise<{ latestCommitSha: string; baseTreeSha: string }> { + const branchUrl = `${this.baseUrl}/api/v1/repos/${this.owner}/${this.repo}/branches/${branch}`; + const branchResp = await this.safeRequest(branchUrl, 'GET'); + const latestCommitSha = this.parseJson<{ commit: { id: string } }>(branchResp).commit.id; + + const commitResp = await this.safeRequest(`${this.getGitDataApiBase()}/git/commits/${latestCommitSha}`, 'GET'); + const baseTreeSha = this.parseJson<{ tree: { sha: string } }>(commitResp).tree.sha; + + return { latestCommitSha, baseTreeSha }; + } + async getFile(path: string, branch: string): Promise { try { const url = `${this.getApiUrl(path)}?ref=${branch}`; @@ -56,6 +75,32 @@ export class GiteaService extends BaseGitService implements GitServiceInterface return { path: data.content.path, sha: data.content.sha }; } + async pushBatch(items: BatchPushItem[], branch: string, message: string): Promise { + if (items.length === 0) return []; + const base = this.getGitDataApiBase(); + const { latestCommitSha, baseTreeSha } = await this.resolveBaseTree(branch); + + const blobShas: string[] = []; + for (const item of items) { + const blobResp = await this.safeRequest(`${base}/git/blobs`, 'POST', { + content: this.encodeContent(item.content), + encoding: 'base64', + }); + blobShas.push(this.parseJson<{ sha: string }>(blobResp).sha); + } + + 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] })); + } + async listFilesDetailed(branch: string, useFilter = true): Promise { // Resolve branch name to commit SHA first for compatibility with all Gitea versions, // since the git/trees endpoint requires a SHA (not a ref name) on older instances. diff --git a/src/services/github-service.ts b/src/services/github-service.ts index 0912de9..40f09da 100644 --- a/src/services/github-service.ts +++ b/src/services/github-service.ts @@ -1,4 +1,4 @@ -import { GitServiceInterface, GitTreeEntry } from './git-service-interface'; +import { GitServiceInterface, GitTreeEntry, BatchPushItem, BatchPushResult } from './git-service-interface'; import { BaseGitService, ConnectionTestResult, GitFile, GitHubContentResponse, GitHubTreeResponse, GIT_SYMLINK_MODE } from './git-service-base'; import { logger } from '../utils/logger'; @@ -23,6 +23,10 @@ export class GitHubService extends BaseGitService implements GitServiceInterface return `https://api.github.com/repos/${this.owner}/${this.repo}/contents/${encodedPath}`; } + protected getGitDataApiBase(): string { + return `https://api.github.com/repos/${this.owner}/${this.repo}`; + } + async getFile(path: string, branch: string): Promise { try { const url = `${this.getApiUrl(path)}?ref=${branch}`; @@ -67,35 +71,48 @@ export class GitHubService extends BaseGitService implements GitServiceInterface // (mode 120000) must be committed through the lower-level Git Data API: // blob -> tree (with the symlink mode) -> commit -> move the branch ref. const fullPath = this.getFullPath(path); - const base = `https://api.github.com/repos/${this.owner}/${this.repo}`; + 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; - - const commitResp = await this.safeRequest(`${base}/git/commits/${latestCommitSha}`, 'GET'); - const baseTreeSha = this.parseJson<{ tree: { sha: string } }>(commitResp).tree.sha; + const { latestCommitSha, baseTreeSha } = await this.resolveGitHubStyleBaseTree(branch); const blobResp = await this.safeRequest(`${base}/git/blobs`, 'POST', { content: target, encoding: 'utf-8' }); const blobSha = this.parseJson<{ sha: string }>(blobResp).sha; - const treeResp = await this.safeRequest(`${base}/git/trees`, 'POST', { - base_tree: baseTreeSha, - tree: [{ path: fullPath, mode: GIT_SYMLINK_MODE, type: 'blob', sha: blobSha }], - }); - const newTreeSha = this.parseJson<{ sha: string }>(treeResp).sha; - - const newCommitResp = await this.safeRequest(`${base}/git/commits`, 'POST', { - message, - tree: newTreeSha, - parents: [latestCommitSha], - }); - const newCommitSha = this.parseJson<{ sha: string }>(newCommitResp).sha; - - await this.safeRequest(`${base}/git/refs/heads/${branch}`, 'PATCH', { sha: newCommitSha }); + await this.commitGitHubStyleTree( + base, branch, baseTreeSha, latestCommitSha, + [{ path: fullPath, mode: GIT_SYMLINK_MODE, type: 'blob', sha: blobSha }], + message + ); return { path: fullPath, sha: blobSha }; } + async pushBatch(items: BatchPushItem[], branch: string, message: string): Promise { + if (items.length === 0) return []; + const base = this.getGitDataApiBase(); + const { latestCommitSha, baseTreeSha } = await this.resolveGitHubStyleBaseTree(branch); + + const blobShas: string[] = []; + for (const item of items) { + const blobResp = await this.safeRequest(`${base}/git/blobs`, 'POST', { + content: this.encodeContent(item.content), + encoding: 'base64', + }); + blobShas.push(this.parseJson<{ sha: string }>(blobResp).sha); + } + + 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] })); + } + async listFilesDetailed(branch: string, useFilter = true): Promise { const url = `https://api.github.com/repos/${this.owner}/${this.repo}/git/trees/${branch}?recursive=1`; let data: GitHubTreeResponse; diff --git a/src/services/gitlab-service.ts b/src/services/gitlab-service.ts index 8d0241d..a901572 100644 --- a/src/services/gitlab-service.ts +++ b/src/services/gitlab-service.ts @@ -1,4 +1,4 @@ -import { GitServiceInterface, GitTreeEntry } from './git-service-interface'; +import { GitServiceInterface, GitTreeEntry, BatchPushItem, BatchPushResult } from './git-service-interface'; import { BaseGitService, ConnectionTestResult, GitFile, GitLabFileResponse, GitLabTreeItem, GIT_SYMLINK_MODE } from './git-service-base'; export class GitLabService extends BaseGitService implements GitServiceInterface { @@ -55,6 +55,28 @@ export class GitLabService extends BaseGitService implements GitServiceInterface return { path: data.file_path }; } + async pushBatch(items: BatchPushItem[], branch: string, message: string): Promise { + if (items.length === 0) return []; + const encodedProjectId = encodeURIComponent(this.projectId); + const url = `${this.baseUrl}/api/v4/projects/${encodedProjectId}/repository/commits`; + + const actions = items.map(item => ({ + action: item.existedRemotely ? 'update' : 'create', + file_path: this.getFullPath(item.path), + content: this.encodeContent(item.content), + encoding: 'base64', + })); + + await this.safeRequest(url, 'POST', { branch, commit_message: message, actions }); + + // The Commits API response doesn't include each file's new blob sha, so + // read it back via a single follow-up tree fetch (one extra call for the + // whole batch, not per file) rather than per-file getFile calls. + 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 { const encodedProjectId = encodeURIComponent(this.projectId); let allEntries: GitTreeEntry[] = []; diff --git a/tests/logic/gitignore-manager.test.ts b/tests/logic/gitignore-manager.test.ts index 4992a41..1da0f53 100644 --- a/tests/logic/gitignore-manager.test.ts +++ b/tests/logic/gitignore-manager.test.ts @@ -120,6 +120,26 @@ describe('GitignoreManager', () => { expect(manager.isIgnored('sub/root-ignored.txt')).toBe(true); }); + it('scans a pre-fetched remote tree for .gitignore paths instead of calling getRepoGitignores', async () => { + const adapter = mockApp.vault.adapter as Mocked; + vi.mocked(adapter.exists).mockResolvedValue(false); + vi.mocked(mockGitService.getFile).mockImplementation((path) => { + if (path === '/.gitignore') return Promise.resolve({ content: 'node_modules/', sha: '1' }); + if (path === '/sub/.gitignore') return Promise.resolve({ content: '*.tmp', sha: '2' }); + return Promise.resolve({ content: '', sha: '' }); + }); + + await manager.loadGitignores([ + { path: '.gitignore', symlink: false, sha: 'a' }, + { path: 'sub/.gitignore', symlink: false, sha: 'b' }, + { path: 'src/main.ts', symlink: false, sha: 'c' }, + ]); + + expect(mockGitService.getRepoGitignores).not.toHaveBeenCalled(); + expect(manager.isIgnored('node_modules/test.js')).toBe(true); + expect(manager.isIgnored('sub/test.tmp')).toBe(true); + }); + it('should pick up local-only subdirectory .gitignore not yet on remote', async () => { // Remote only knows about root .gitignore; sub/.gitignore exists locally but not pushed yet vi.mocked(mockGitService.getRepoGitignores).mockResolvedValue(['.gitignore']); diff --git a/tests/logic/sync-manager-batch.test.ts b/tests/logic/sync-manager-batch.test.ts index 6c7faca..50d2a21 100644 --- a/tests/logic/sync-manager-batch.test.ts +++ b/tests/logic/sync-manager-batch.test.ts @@ -20,6 +20,8 @@ describe('SyncManager Batch Operations', () => { exists: vi.fn(), read: vi.fn(), write: vi.fn(), + readBinary: vi.fn(), + writeBinary: vi.fn(), } as unknown as Mocked; mockApp = { @@ -40,6 +42,7 @@ describe('SyncManager Batch Operations', () => { getFile: vi.fn(), testConnection: vi.fn(), listFiles: vi.fn(), + listFilesDetailed: vi.fn().mockResolvedValue([]), deleteFile: vi.fn(), getRepoGitignores: vi.fn(), updateConfig: vi.fn(), @@ -100,6 +103,95 @@ describe('SyncManager Batch Operations', () => { }); }); + describe('batch commit via gitService.pushBatch', () => { + it('groups all queued files into one pushBatch call when the provider supports it', async () => { + const files = ['a.md', 'b.md']; + const adapter = mockApp.vault.adapter as Mocked; + + vi.mocked(adapter.exists).mockResolvedValue(true); + vi.mocked(adapter.read).mockImplementation(async (p) => (p === 'a.md' ? 'content-a' : 'content-b')); + // No tree entries: both files are new, so both are queued. + vi.mocked(mockGitService.listFilesDetailed).mockResolvedValue([]); + mockGitService.pushBatch = vi.fn().mockResolvedValue([ + { path: 'a.md', sha: 'sha-a' }, + { path: 'b.md', sha: 'sha-b' }, + ]); + + const results = await manager.pushAllFiles(files); + + expect(results.success).toBe(2); + expect(results.failed).toBe(0); + expect(mockGitService.pushBatch).toHaveBeenCalledTimes(1); + expect(mockGitService.pushFile).not.toHaveBeenCalled(); + const [items] = vi.mocked(mockGitService.pushBatch).mock.calls[0]!; + expect(items).toEqual([ + { path: 'a.md', content: 'content-a', existedRemotely: false }, + { path: 'b.md', content: 'content-b', existedRemotely: false }, + ]); + }); + + it('handles a mixed binary + text batch, computing blob shas for both', async () => { + const files = ['note.md', 'image.png']; + const adapter = mockApp.vault.adapter as Mocked; + + vi.mocked(adapter.exists).mockResolvedValue(true); + vi.mocked(adapter.read).mockResolvedValue('text content'); + vi.mocked(adapter.readBinary).mockResolvedValue(new Uint8Array([1, 2, 3]).buffer); + vi.mocked(mockGitService.listFilesDetailed).mockResolvedValue([]); + mockGitService.pushBatch = vi.fn().mockResolvedValue([ + { path: 'note.md', sha: 'sha-note' }, + { path: 'image.png', sha: 'sha-image' }, + ]); + + const results = await manager.pushAllFiles(files); + + expect(results.success).toBe(2); + expect(mockGitService.pushBatch).toHaveBeenCalledTimes(1); + }); + + it('marks every file in a failed chunk as failed, not dropped', async () => { + const files = ['a.md', 'b.md']; + const adapter = mockApp.vault.adapter as Mocked; + + vi.mocked(adapter.exists).mockResolvedValue(true); + vi.mocked(adapter.read).mockResolvedValue('content'); + vi.mocked(mockGitService.listFilesDetailed).mockResolvedValue([]); + mockGitService.pushBatch = vi.fn().mockRejectedValue(new Error('commit failed')); + + const results = await manager.pushAllFiles(files); + + expect(results.success).toBe(0); + expect(results.failed).toBe(2); + expect(results.errors).toEqual([ + { file: 'a.md', error: 'commit failed' }, + { file: 'b.md', error: 'commit failed' }, + ]); + }); + + it('skips both getFile and pushBatch when the local blob sha already matches the tree', async () => { + const path = 'unchanged.md'; + const adapter = mockApp.vault.adapter as Mocked; + + vi.mocked(adapter.exists).mockResolvedValue(true); + vi.mocked(adapter.read).mockResolvedValue('same content'); + + // Compute the real git blob sha for the content so it matches the tree entry. + const { gitBlobSha } = await import('../../src/utils/git-blob-sha'); + const sha = await gitBlobSha('same content'); + vi.mocked(mockGitService.listFilesDetailed).mockResolvedValue([{ path, symlink: false, sha }]); + mockGitService.pushBatch = vi.fn(); + + const results = await manager.pushAllFiles([path]); + + expect(results.success).toBe(0); + expect(results.conflicts).toBe(0); + expect(results.failed).toBe(0); + expect(mockGitService.getFile).not.toHaveBeenCalled(); + expect(mockGitService.pushBatch).not.toHaveBeenCalled(); + expect(mockSettings.syncMetadata[path]?.lastSyncedSha).toBe(sha); + }); + }); + describe('pullAllAllFiles', () => { it('should pull multiple files correctly (strings and TFiles)', async () => { const mockFile = Object.assign(new TFile(), { path: 'file2.md', name: 'file2.md' }); @@ -161,8 +253,10 @@ describe('SyncManager Batch Operations', () => { vi.mocked(adapter.exists).mockResolvedValue(true); vi.mocked(adapter.read).mockResolvedValue('local edit'); - // Remote sha differs from what we last synced, and content differs too. - vi.mocked(mockGitService.getFile).mockResolvedValue({ content: 'remote edit', sha: 'sha-changed-on-remote' }); + // Remote tree entry's sha differs from what we last synced. + vi.mocked(mockGitService.listFilesDetailed).mockResolvedValue([ + { path, symlink: false, sha: 'sha-changed-on-remote' } + ]); const results = await manager.pushAllFiles([path]); @@ -170,6 +264,7 @@ describe('SyncManager Batch Operations', () => { expect(results.conflicts).toBe(1); expect(results.failed).toBe(0); expect(mockGitService.pushFile).not.toHaveBeenCalled(); + expect(mockGitService.getFile).not.toHaveBeenCalled(); }); it('should skip (not overwrite) a pull when the local file has diverged since last sync', async () => { @@ -197,13 +292,16 @@ describe('SyncManager Batch Operations', () => { vi.mocked(adapter.exists).mockResolvedValue(true); vi.mocked(adapter.read).mockResolvedValue('local content'); - vi.mocked(mockGitService.getFile).mockResolvedValue({ content: 'remote content', sha: 'some-sha' }); + vi.mocked(mockGitService.listFilesDetailed).mockResolvedValue([ + { path, symlink: false, sha: 'some-sha' } + ]); vi.mocked(mockGitService.pushFile).mockResolvedValue({ path, sha: 'new-sha' }); const results = await manager.pushAllFiles([path]); expect(results.success).toBe(1); expect(results.conflicts).toBe(0); + expect(mockGitService.pushFile).toHaveBeenCalledWith(path, 'local content', 'main', expect.any(String), 'some-sha'); }); }); @@ -274,10 +372,15 @@ describe('SyncManager Batch Operations', () => { vi.mocked(mockApp.vault.read).mockResolvedValue('unrelated content'); vi.mocked(mockApp.vault.adapter.exists as ReturnType).mockResolvedValue(true); vi.mocked(mockGitService.pushFile).mockResolvedValue({ path: pushedPath, sha: 'new-sha' }); + // detectRename still checks the orphaned metadata entry's remote content directly. vi.mocked(mockGitService.getFile).mockImplementation(async (path) => { if (path === orphanedPath) return { content: 'totally different content', sha: 'orphaned-sha' }; - return { content: 'old content', sha: 'remote-sha' }; + return { content: '', sha: '' }; }); + // The pushed path's own remote state comes from the pre-fetched tree, not getFile. + vi.mocked(mockGitService.listFilesDetailed).mockResolvedValue([ + { path: pushedPath, symlink: false, sha: 'remote-sha' } + ]); const results = await manager.pushAllFiles([mockFile]); diff --git a/tests/logic/sync-manager-test-helpers.ts b/tests/logic/sync-manager-test-helpers.ts index 259b44e..87f8314 100644 --- a/tests/logic/sync-manager-test-helpers.ts +++ b/tests/logic/sync-manager-test-helpers.ts @@ -38,6 +38,7 @@ export function createSyncManagerMocks(): SyncManagerMocks { getFile: vi.fn(), testConnection: vi.fn(), listFiles: vi.fn(), + listFilesDetailed: vi.fn().mockResolvedValue([]), deleteFile: vi.fn(), getRepoGitignores: vi.fn(), updateConfig: vi.fn(), diff --git a/tests/services/git-service-base.test.ts b/tests/services/git-service-base.test.ts index e0735af..f0593db 100644 --- a/tests/services/git-service-base.test.ts +++ b/tests/services/git-service-base.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { GitHubService } from '../../src/services/github-service'; import { requestUrl, RequestUrlResponse } from 'obsidian'; +import { MAX_BATCH_PUSH_SIZE } from '../../src/services/git-service-base'; // Tests for BaseGitService protected methods exercised via GitHubService describe('BaseGitService', () => { @@ -166,6 +167,12 @@ describe('BaseGitService', () => { }); }); + describe('MAX_BATCH_PUSH_SIZE', () => { + it('is a sane positive chunk size', () => { + expect(MAX_BATCH_PUSH_SIZE).toBeGreaterThan(0); + }); + }); + describe('encodeContent / decodeContent round-trip', () => { it('should correctly encode and decode UTF-8 content', async () => { const original = 'Hello, 世界! 🌍'; diff --git a/tests/services/gitea-service.test.ts b/tests/services/gitea-service.test.ts index 907fa89..1db183d 100644 --- a/tests/services/gitea-service.test.ts +++ b/tests/services/gitea-service.test.ts @@ -117,6 +117,40 @@ describe('GiteaService', () => { }); }); + describe('pushBatch', () => { + it('returns [] and makes no requests for an empty item list', async () => { + const result = await service.pushBatch([], 'main', 'push nothing'); + expect(result).toEqual([]); + expect(requestUrl).not.toHaveBeenCalled(); + }); + + it('resolves branch via /branches/{branch}, then commits N files in one commit', async () => { + vi.mocked(requestUrl) + .mockResolvedValueOnce({ status: 200, json: { commit: { id: 'commit1' } } } as unknown as RequestUrlResponse) // resolve branch + .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: '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 + + const result = await service.pushBatch([{ path: 'a.md', content: 'hello' }], 'main', 'Push 1 file(s) from Obsidian'); + + expect(result).toEqual([{ path: 'a.md', sha: 'blob-a' }]); + + const calls = vi.mocked(requestUrl).mock.calls.map(c => c[0] as RequestUrlParam); + expect(calls).toHaveLength(6); + expect(calls[0]?.url).toBe(`${baseUrl}/api/v1/repos/${owner}/${repo}/branches/main`); + expect(calls[1]?.url).toBe(`${baseUrl}/api/v1/repos/${owner}/${repo}/git/commits/commit1`); + + const blobBody = JSON.parse(calls[2]?.body as string) as { content: string; encoding: string }; + expect(blobBody.encoding).toBe('base64'); + expect(atob(blobBody.content)).toBe('hello'); + + expect(calls[5]?.method).toBe('PATCH'); + expect(calls[5]?.url).toBe(`${baseUrl}/api/v1/repos/${owner}/${repo}/git/refs/heads/main`); + }); + }); + describe('listFiles', () => { const commitSha = 'abc123commit'; diff --git a/tests/services/github-service.test.ts b/tests/services/github-service.test.ts index 2d5a184..3a9c747 100644 --- a/tests/services/github-service.test.ts +++ b/tests/services/github-service.test.ts @@ -98,6 +98,54 @@ describe('GitHubService', () => { }); }); + describe('pushBatch', () => { + it('returns [] and makes no requests for an empty item list', async () => { + const result = await service.pushBatch([], 'main', 'push nothing'); + expect(result).toEqual([]); + expect(requestUrl).not.toHaveBeenCalled(); + }); + + it('commits N files in one commit via ref -> commit -> blobs -> tree -> commit -> ref', 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 + + const result = await service.pushBatch( + [{ path: 'a.md', content: 'hello' }, { path: 'b.md', content: 'world' }], + 'main', + 'Push 2 file(s) from Obsidian' + ); + + 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); + + // 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 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' }); + }); + }); + describe('pushFile', () => { it('should push new file correctly (no sha provided)', async () => { vi.mocked(requestUrl).mockResolvedValueOnce({ diff --git a/tests/services/gitlab-service.test.ts b/tests/services/gitlab-service.test.ts index a1028da..19b0251 100644 --- a/tests/services/gitlab-service.test.ts +++ b/tests/services/gitlab-service.test.ts @@ -68,6 +68,56 @@ describe('GitLabService', () => { }); }); + describe('pushBatch', () => { + it('returns [] and makes no requests for an empty item list', async () => { + const result = await service.pushBatch([], 'main', 'push nothing'); + expect(result).toEqual([]); + expect(requestUrl).not.toHaveBeenCalled(); + }); + + it('commits via the Commits API actions array, then reads back shas from a follow-up tree fetch', async () => { + vi.mocked(requestUrl) + .mockResolvedValueOnce({ status: 201, json: { id: 'commit-sha' } } as unknown as RequestUrlResponse) // POST commits + .mockResolvedValueOnce({ status: 200, json: [ + { path: 'a.md', type: 'blob', id: 'new-sha-a' }, + { path: 'b.md', type: 'blob', id: 'new-sha-b' }, + ] } as unknown as RequestUrlResponse); // follow-up listFilesDetailed + + const result = await service.pushBatch( + [ + { path: 'a.md', content: 'hello', existedRemotely: true }, + { path: 'b.md', content: 'world', existedRemotely: false }, + ], + 'main', + 'Push 2 file(s) from Obsidian' + ); + + expect(result).toEqual([{ path: 'a.md', sha: 'new-sha-a' }, { path: 'b.md', sha: 'new-sha-b' }]); + + const calls = vi.mocked(requestUrl).mock.calls; + expect(calls).toHaveLength(2); + const commitCall = calls[0]?.[0] as { url: string; method: string; body: string }; + expect(commitCall.url).toBe(`${baseUrl}/api/v4/projects/${projectId}/repository/commits`); + expect(commitCall.method).toBe('POST'); + const body = JSON.parse(commitCall.body) as { branch: string; commit_message: string; actions: Array<{ action: string; file_path: string; content: string; encoding: string }> }; + expect(body.branch).toBe('main'); + expect(body.commit_message).toBe('Push 2 file(s) from Obsidian'); + expect(body.actions).toEqual([ + { action: 'update', file_path: 'a.md', content: btoa('hello'), encoding: 'base64' }, + { action: 'create', file_path: 'b.md', content: btoa('world'), encoding: 'base64' }, + ]); + }); + + it('returns sha: undefined for a pushed path missing from the follow-up tree', async () => { + vi.mocked(requestUrl) + .mockResolvedValueOnce({ status: 201, json: { id: 'commit-sha' } } as unknown as RequestUrlResponse) + .mockResolvedValueOnce({ status: 200, json: [] } as unknown as RequestUrlResponse); + + const result = await service.pushBatch([{ path: 'a.md', content: 'hello' }], 'main', 'push'); + expect(result).toEqual([{ path: 'a.md', sha: undefined }]); + }); + }); + describe('listFiles', () => { it('should list blob files from tree API', async () => { mockRequest({ status: 200, json: [