mirror of
https://github.com/firstsun-dev/git-files-sync.git
synced 2026-07-22 17:20:30 +00:00
perf(delete): batch-commit remote-only file deletion
Follow-up to the push-all batching work: batch-deleting remote-only files via the sync status panel's checkbox multi-select was still N separate commits (SyncStatusView.performRemoteDeletion looped gitService.deleteFile once per file). - Add optional GitServiceInterface.deleteBatch, mirroring pushBatch. GitHub/Gitea implement it via the git blob->tree->commit->ref Data API, reusing resolveGitHubStyleBaseTree/resolveBaseTree and commitGitHubStyleTree (widened its tree-item sha type to string | null -- a null sha removes that path from the resulting tree, GitHub's way of expressing a delete at the tree level). GitLab implements it via the same Commits API endpoint pushBatch already uses, with action: 'delete' entries. - SyncStatusView.performRemoteDeletion now calls deleteBatch once per chunk (MAX_BATCH_PUSH_SIZE, reused from the push work) when the provider supports it; a failed chunk marks every path in it as failed rather than dropping results silently. The original per-file loop is preserved verbatim as performRemoteDeletionSequential, the fallback for providers without deleteBatch. Local deletion is unaffected -- it's a local vault operation, not a git commit. Evidence: npx eslint . -> 0 errors; npm run build -> clean; npx vitest run -> 339/339 passed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
c28e0ec09a
commit
d8e3663b8f
12 changed files with 288 additions and 17 deletions
|
|
@ -89,6 +89,14 @@
|
||||||
"dependencies": [],
|
"dependencies": [],
|
||||||
"status": "done",
|
"status": "done",
|
||||||
"evidence": "npx eslint . -> 0 errors; npm run build -> clean; npx vitest run -> 330/330 passed; consolidated onto PR #51"
|
"evidence": "npx eslint . -> 0 errors; npm run build -> clean; npx vitest run -> 330/330 passed; consolidated onto PR #51"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"id": "feat-012",
|
||||||
|
"name": "perf(delete): batch-commit remote-only file deletion",
|
||||||
|
"description": "Follow-up to feat-011: batch-deleting remote-only files via the sync status panel's checkbox multi-select was still N separate commits (SyncStatusView.performRemoteDeletion looped gitService.deleteFile per file). Added optional GitServiceInterface.deleteBatch, mirroring pushBatch: GitHub/Gitea via the git blob->tree->commit->ref Data API (a tree entry with sha:null removes that path); GitLab via the same Commits API actions array used for pushBatch, with action:'delete'. SyncStatusView.performRemoteDeletion now calls deleteBatch once (chunked at MAX_BATCH_PUSH_SIZE) when the provider supports it, falling back to the original sequential per-file performRemoteDeletionSequential otherwise",
|
||||||
|
"dependencies": ["feat-011"],
|
||||||
|
"status": "done",
|
||||||
|
"evidence": "npx eslint . -> 0 errors; npm run build -> clean; npx vitest run -> 339/339 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."
|
"_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."
|
||||||
|
|
|
||||||
18
progress.md
18
progress.md
|
|
@ -12,9 +12,9 @@ Completed work is archived in [archive/](./archive/), one file per calendar mont
|
||||||
|
|
||||||
## Current State
|
## Current State
|
||||||
|
|
||||||
**Last Updated:** 2026-07-14 10:10
|
**Last Updated:** 2026-07-14 10:30
|
||||||
**Session ID:** current
|
**Session ID:** current
|
||||||
**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.
|
**Active Feature:** feat-012 (perf: batch-commit remote-only file deletion) — done, lint/build/test all green. feat-011 (push-all batching) also done this session. Previous active item (issue #78 delete-remote-only-file fix) still awaiting user re-test after rebuild.
|
||||||
|
|
||||||
## Status
|
## Status
|
||||||
|
|
||||||
|
|
@ -44,6 +44,16 @@ Completed work is archived in [archive/](./archive/), one file per calendar mont
|
||||||
- Pull path (`pullAllFiles`) intentionally untouched — out of scope, still needs full remote content regardless of sha comparison.
|
- 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`.
|
- Not yet committed as of this note — pending commit onto `claude/fix-directory-symlink-pull-260713`.
|
||||||
|
|
||||||
|
- [x] feat-012: perf(delete): batch-commit remote-only file deletion. User asked whether batch-deleting remote-only files (checkbox multi-select in the sync status panel) was also one commit; it wasn't (`performRemoteDeletion` looped `gitService.deleteFile` per file). Applied the same batching pattern as feat-011:
|
||||||
|
1. New optional `GitServiceInterface.deleteBatch?(paths, branch, commitMessage)`, mirroring `pushBatch?`. No result payload (deletes don't produce a new sha to report back).
|
||||||
|
2. `GitHubService`/`GiteaService.deleteBatch` reuse `resolveGitHubStyleBaseTree`/`resolveBaseTree` + `commitGitHubStyleTree` (widened `commitGitHubStyleTree`'s tree-item `sha` type to `string | null` — a `null` sha removes that path from the resulting tree, which is how GitHub's Git Data API expresses a delete at the tree level).
|
||||||
|
3. `GitLabService.deleteBatch` reuses the same Commits API endpoint as `pushBatch`, with `action: 'delete'` entries (no `content`/`encoding`).
|
||||||
|
4. `src/ui/SyncStatusView.ts`'s `performRemoteDeletion` now calls `deleteBatch` once per chunk (`MAX_BATCH_PUSH_SIZE`, reused from feat-011) when the provider supports it, updating progress per-file during a fast local pass before the grouped network call — same UX pattern as push. The original per-file loop was extracted verbatim into `performRemoteDeletionSequential`, used as the fallback when a provider has no `deleteBatch` (e.g. future Bitbucket). A failed chunk marks every path in it as failed (kept in `fileStatuses`/`selectedFiles`, not silently dropped); earlier successful chunks stay deleted.
|
||||||
|
- Local deletion (`performLocalDeletion`) untouched — pure local vault operation, nothing to batch.
|
||||||
|
- Added `deleteBatch` tests to `github-service.test.ts`/`gitea-service.test.ts`/`gitlab-service.test.ts` (happy path, empty-array short-circuit) and new `SyncStatusView.test.ts` cases (grouped call, whole-chunk-failure, fallback when `deleteBatch` is absent). Existing vaultFolder-prefix-stripping and real-error-message tests still pass unchanged against the fallback path.
|
||||||
|
- Evidence: `npx eslint .` → 0 errors; `npm run build` → clean; `npx vitest run` → 339/339 passed.
|
||||||
|
- Not yet committed as of this note — pending commit onto `claude/fix-directory-symlink-pull-260713`.
|
||||||
|
|
||||||
### What's In Progress
|
### What's In Progress
|
||||||
|
|
||||||
- Nothing else actively in progress.
|
- Nothing else actively in progress.
|
||||||
|
|
@ -51,8 +61,8 @@ Completed work is archived in [archive/](./archive/), one file per calendar mont
|
||||||
### What's Next
|
### 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 (issue #78).
|
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.
|
2. Manually verify feat-011/feat-012 in Obsidian if possible: push-all and batch-delete on a mixed vault should each produce exactly one new commit.
|
||||||
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.
|
3. Issue #37 (Bitbucket provider support, feat-010) — large, was deferred until #38 (i18n) landed. Now unblocked. Its `GitServiceInterface` implementation should simply omit `pushBatch`/`deleteBatch` and use the existing per-file fallbacks.
|
||||||
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).
|
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.
|
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.
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -299,7 +299,10 @@ export abstract class BaseGitService {
|
||||||
branch: string,
|
branch: string,
|
||||||
baseTreeSha: string,
|
baseTreeSha: string,
|
||||||
latestCommitSha: string,
|
latestCommitSha: string,
|
||||||
treeItems: Array<{ path: string; mode: string; type: 'blob'; sha: string }>,
|
// A null sha removes that path from the resulting tree — how a batch
|
||||||
|
// delete is expressed at the tree level (mode/type are still required
|
||||||
|
// fields on the entry but are otherwise irrelevant for a deletion).
|
||||||
|
treeItems: Array<{ path: string; mode: string; type: 'blob'; sha: string | null }>,
|
||||||
message: string
|
message: string
|
||||||
): Promise<string> {
|
): Promise<string> {
|
||||||
const treeResp = await this.safeRequest(`${base}/git/trees`, 'POST', { base_tree: baseTreeSha, tree: treeItems });
|
const treeResp = await this.safeRequest(`${base}/git/trees`, 'POST', { base_tree: baseTreeSha, tree: treeItems });
|
||||||
|
|
|
||||||
|
|
@ -66,6 +66,14 @@ export interface GitServiceInterface {
|
||||||
*/
|
*/
|
||||||
pushBatch?(items: BatchPushItem[], branch: string, commitMessage: string): Promise<BatchPushResult[]>;
|
pushBatch?(items: BatchPushItem[], branch: string, commitMessage: string): Promise<BatchPushResult[]>;
|
||||||
deleteFile(path: string, branch: string, commitMessage: string): Promise<void>;
|
deleteFile(path: string, branch: string, commitMessage: string): Promise<void>;
|
||||||
|
/**
|
||||||
|
* Delete many files in a single commit. Optional: only providers with a way
|
||||||
|
* to write multiple changes atomically implement it; callers must fall back
|
||||||
|
* to sequential deleteFile calls when it's absent (mirrors pushBatch?). Must
|
||||||
|
* be atomic: on failure it throws rather than partially deleting, so the
|
||||||
|
* caller can mark every path in the attempted batch as failed.
|
||||||
|
*/
|
||||||
|
deleteBatch?(paths: string[], branch: string, commitMessage: string): Promise<void>;
|
||||||
getRepoGitignores(branch: string): Promise<string[]>;
|
getRepoGitignores(branch: string): Promise<string[]>;
|
||||||
/**
|
/**
|
||||||
* Fetches a blob's content directly by its git SHA (from a GitTreeEntry),
|
* Fetches a blob's content directly by its git SHA (from a GitTreeEntry),
|
||||||
|
|
|
||||||
|
|
@ -153,6 +153,21 @@ export class GiteaService extends BaseGitService implements GitServiceInterface
|
||||||
await this.safeRequest(url, 'DELETE', body);
|
await this.safeRequest(url, 'DELETE', body);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async deleteBatch(paths: string[], branch: string, message: string): Promise<void> {
|
||||||
|
if (paths.length === 0) return;
|
||||||
|
const base = this.getGitDataApiBase();
|
||||||
|
const { latestCommitSha, baseTreeSha } = await this.resolveBaseTree(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);
|
||||||
|
}
|
||||||
|
|
||||||
async testConnection(branch: string): Promise<ConnectionTestResult> {
|
async testConnection(branch: string): Promise<ConnectionTestResult> {
|
||||||
try {
|
try {
|
||||||
const url = `${this.baseUrl}/api/v1/repos/${this.owner}/${this.repo}`;
|
const url = `${this.baseUrl}/api/v1/repos/${this.owner}/${this.repo}`;
|
||||||
|
|
|
||||||
|
|
@ -159,6 +159,21 @@ export class GitHubService extends BaseGitService implements GitServiceInterface
|
||||||
await this.safeRequest(url, 'DELETE', body);
|
await this.safeRequest(url, 'DELETE', body);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
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 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);
|
||||||
|
}
|
||||||
|
|
||||||
async testConnection(branch: string): Promise<ConnectionTestResult> {
|
async testConnection(branch: string): Promise<ConnectionTestResult> {
|
||||||
try {
|
try {
|
||||||
const url = `https://api.github.com/repos/${this.owner}/${this.repo}`;
|
const url = `https://api.github.com/repos/${this.owner}/${this.repo}`;
|
||||||
|
|
|
||||||
|
|
@ -137,6 +137,16 @@ export class GitLabService extends BaseGitService implements GitServiceInterface
|
||||||
await this.safeRequest(url, 'DELETE', body);
|
await this.safeRequest(url, 'DELETE', body);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async deleteBatch(paths: string[], branch: string, message: string): Promise<void> {
|
||||||
|
if (paths.length === 0) return;
|
||||||
|
const encodedProjectId = encodeURIComponent(this.projectId);
|
||||||
|
const url = `${this.baseUrl}/api/v4/projects/${encodedProjectId}/repository/commits`;
|
||||||
|
|
||||||
|
const actions = paths.map(path => ({ action: 'delete', file_path: this.getFullPath(path) }));
|
||||||
|
|
||||||
|
await this.safeRequest(url, 'POST', { branch, commit_message: message, actions });
|
||||||
|
}
|
||||||
|
|
||||||
async testConnection(branch: string): Promise<ConnectionTestResult> {
|
async testConnection(branch: string): Promise<ConnectionTestResult> {
|
||||||
const encodedProjectId = encodeURIComponent(this.projectId);
|
const encodedProjectId = encodeURIComponent(this.projectId);
|
||||||
try {
|
try {
|
||||||
|
|
|
||||||
|
|
@ -11,6 +11,7 @@ import { isBinaryPath, contentsEqual } from '../utils/path';
|
||||||
import { readLocalSymlinkTarget } from '../utils/symlink';
|
import { readLocalSymlinkTarget } from '../utils/symlink';
|
||||||
import { gitBlobSha } from '../utils/git-blob-sha';
|
import { gitBlobSha } from '../utils/git-blob-sha';
|
||||||
import { type GitTreeEntry } from '../services/git-service-interface';
|
import { type GitTreeEntry } from '../services/git-service-interface';
|
||||||
|
import { MAX_BATCH_PUSH_SIZE } from '../services/git-service-base';
|
||||||
import { t, type TranslationKey } from '../i18n';
|
import { t, type TranslationKey } from '../i18n';
|
||||||
|
|
||||||
export const SYNC_STATUS_VIEW_TYPE = 'sync-status-view';
|
export const SYNC_STATUS_VIEW_TYPE = 'sync-status-view';
|
||||||
|
|
@ -45,6 +46,10 @@ export class SyncStatusView extends ItemView {
|
||||||
private renderView(): void {
|
private renderView(): void {
|
||||||
const container = this.containerEl.children[1] as HTMLElement;
|
const container = this.containerEl.children[1] as HTMLElement;
|
||||||
if (!container) return;
|
if (!container) return;
|
||||||
|
|
||||||
|
const prevListEl = container.querySelector<HTMLElement>('.ssv-list');
|
||||||
|
const scrollTop = prevListEl?.scrollTop ?? 0;
|
||||||
|
|
||||||
container.empty();
|
container.empty();
|
||||||
|
|
||||||
this.renderInfoStrip(container);
|
this.renderInfoStrip(container);
|
||||||
|
|
@ -61,6 +66,8 @@ export class SyncStatusView extends ItemView {
|
||||||
} else {
|
} else {
|
||||||
this.renderFileList(listEl);
|
this.renderFileList(listEl);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
listEl.scrollTop = scrollTop;
|
||||||
}
|
}
|
||||||
|
|
||||||
private renderProgressBar(container: HTMLElement): void {
|
private renderProgressBar(container: HTMLElement): void {
|
||||||
|
|
@ -775,20 +782,62 @@ export class SyncStatusView extends ItemView {
|
||||||
}
|
}
|
||||||
|
|
||||||
private async performRemoteDeletion(remote: FileStatus[], total: number, localCount: number, prog: Notice, errors: { path: string, message: string }[]): Promise<void> {
|
private async performRemoteDeletion(remote: FileStatus[], total: number, localCount: number, prog: Notice, errors: { path: string, message: string }[]): Promise<void> {
|
||||||
|
if (remote.length === 0) return;
|
||||||
|
|
||||||
|
// s.path is a vault-relative path (may carry the vaultFolder prefix); the
|
||||||
|
// git service expects a path relative to rootPath only, so strip
|
||||||
|
// vaultFolder first, same as every other gitService call site.
|
||||||
|
const entries = remote.map(s => ({ status: s, repoPath: this.plugin.getNormalizedPath(s.path) }));
|
||||||
|
|
||||||
|
if (!this.plugin.gitService.deleteBatch) {
|
||||||
|
await this.performRemoteDeletionSequential(entries, total, localCount, prog, errors);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
let cur = localCount;
|
let cur = localCount;
|
||||||
for (const s of remote) {
|
for (const e of entries) {
|
||||||
cur++;
|
cur++;
|
||||||
prog.setMessage(t('syncStatus.progress.deletingRemote', { current: cur, total, path: s.path }));
|
prog.setMessage(t('syncStatus.progress.deletingRemote', { current: cur, total, path: e.status.path }));
|
||||||
|
}
|
||||||
|
|
||||||
|
const branch = this.plugin.settings.branch;
|
||||||
|
for (let i = 0; i < entries.length; i += MAX_BATCH_PUSH_SIZE) {
|
||||||
|
const chunk = entries.slice(i, i + MAX_BATCH_PUSH_SIZE);
|
||||||
try {
|
try {
|
||||||
// s.path is a vault-relative path (may carry the vaultFolder prefix);
|
const message = `Delete ${chunk.length} file(s) from Obsidian`;
|
||||||
// the git service expects a path relative to rootPath only, so strip
|
await this.plugin.gitService.deleteBatch(chunk.map(e => e.repoPath), branch, message);
|
||||||
// vaultFolder first, same as every other gitService call site.
|
for (const e of chunk) {
|
||||||
const repoPath = this.plugin.getNormalizedPath(s.path);
|
this.fileStatuses.delete(e.status.path);
|
||||||
await this.plugin.gitService.deleteFile(repoPath, this.plugin.settings.branch, `Delete ${repoPath}`);
|
this.selectedFiles.delete(e.status.path);
|
||||||
this.fileStatuses.delete(s.path);
|
}
|
||||||
this.selectedFiles.delete(s.path);
|
} catch (err) {
|
||||||
} catch (e) {
|
// Atomic per-provider failure: none of this chunk's files were
|
||||||
errors.push({ path: s.path, message: e instanceof Error ? e.message : String(e) });
|
// actually deleted, so every path in it is failed, not dropped.
|
||||||
|
const message = err instanceof Error ? err.message : String(err);
|
||||||
|
for (const e of chunk) errors.push({ path: e.status.path, message });
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Provider doesn't support a batch/atomic multi-file delete commit —
|
||||||
|
* fall back to the original sequential per-file delete. */
|
||||||
|
private async performRemoteDeletionSequential(
|
||||||
|
entries: Array<{ status: FileStatus; repoPath: string }>,
|
||||||
|
total: number,
|
||||||
|
localCount: number,
|
||||||
|
prog: Notice,
|
||||||
|
errors: { path: string, message: string }[]
|
||||||
|
): Promise<void> {
|
||||||
|
let cur = localCount;
|
||||||
|
for (const e of entries) {
|
||||||
|
cur++;
|
||||||
|
prog.setMessage(t('syncStatus.progress.deletingRemote', { current: cur, total, path: e.status.path }));
|
||||||
|
try {
|
||||||
|
await this.plugin.gitService.deleteFile(e.repoPath, this.plugin.settings.branch, `Delete ${e.repoPath}`);
|
||||||
|
this.fileStatuses.delete(e.status.path);
|
||||||
|
this.selectedFiles.delete(e.status.path);
|
||||||
|
} catch (err) {
|
||||||
|
errors.push({ path: e.status.path, message: err instanceof Error ? err.message : String(err) });
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -300,6 +300,35 @@ describe('GiteaService', () => {
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('deleteBatch', () => {
|
||||||
|
it('returns and makes no requests for an empty path list', async () => {
|
||||||
|
await service.deleteBatch([], 'main', 'delete nothing');
|
||||||
|
expect(requestUrl).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('resolves branch via /branches/{branch}, then deletes 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: '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
|
||||||
|
|
||||||
|
await service.deleteBatch(['a.md'], 'main', 'Delete 1 file(s) from Obsidian');
|
||||||
|
|
||||||
|
const calls = vi.mocked(requestUrl).mock.calls.map(c => c[0] as RequestUrlParam);
|
||||||
|
expect(calls).toHaveLength(5);
|
||||||
|
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 treeBody = JSON.parse(calls[2]?.body as string) as { tree: Array<{ path: string; sha: string | null }> };
|
||||||
|
expect(treeBody.tree).toEqual([{ path: 'a.md', mode: '100644', type: 'blob', sha: null }]);
|
||||||
|
|
||||||
|
expect(calls[4]?.method).toBe('PATCH');
|
||||||
|
expect(calls[4]?.url).toBe(`${baseUrl}/api/v1/repos/${owner}/${repo}/git/refs/heads/main`);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe('testConnection', () => {
|
describe('testConnection', () => {
|
||||||
sharedTestConnection(() => service);
|
sharedTestConnection(() => service);
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -312,6 +312,40 @@ describe('GitHubService', () => {
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('deleteBatch', () => {
|
||||||
|
it('returns and makes no requests for an empty path list', async () => {
|
||||||
|
await service.deleteBatch([], 'main', 'delete nothing');
|
||||||
|
expect(requestUrl).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('deletes N files in one commit via ref -> commit -> tree(sha:null) -> 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: '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
|
||||||
|
|
||||||
|
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);
|
||||||
|
|
||||||
|
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 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'] });
|
||||||
|
|
||||||
|
expect(calls[4]?.method).toBe('PATCH');
|
||||||
|
expect(JSON.parse(calls[4]?.body as string)).toEqual({ sha: 'commit2' });
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe('testConnection', () => {
|
describe('testConnection', () => {
|
||||||
sharedTestConnection(() => service);
|
sharedTestConnection(() => service);
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -242,6 +242,30 @@ describe('GitLabService', () => {
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('deleteBatch', () => {
|
||||||
|
it('returns and makes no requests for an empty path list', async () => {
|
||||||
|
await service.deleteBatch([], 'main', 'delete nothing');
|
||||||
|
expect(requestUrl).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('posts a Commits API actions array with action: delete, no content/encoding', async () => {
|
||||||
|
mockRequest({ status: 201, json: { id: 'commit-sha' } });
|
||||||
|
|
||||||
|
await service.deleteBatch(['a.md', 'b.md'], 'main', 'Delete 2 file(s) from Obsidian');
|
||||||
|
|
||||||
|
const call = getLastRequestCall();
|
||||||
|
expect(call.url).toBe(`${baseUrl}/api/v4/projects/${projectId}/repository/commits`);
|
||||||
|
expect(call.method).toBe('POST');
|
||||||
|
const body = JSON.parse(call.body as string) 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('Delete 2 file(s) from Obsidian');
|
||||||
|
expect(body.actions).toEqual([
|
||||||
|
{ action: 'delete', file_path: 'a.md' },
|
||||||
|
{ action: 'delete', file_path: 'b.md' },
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe('testConnection', () => {
|
describe('testConnection', () => {
|
||||||
sharedTestConnection(() => service);
|
sharedTestConnection(() => service);
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -10,6 +10,7 @@ import type { GitTreeEntry } from '../../src/services/git-service-interface';
|
||||||
function makePlugin(overrides: {
|
function makePlugin(overrides: {
|
||||||
vaultFolder?: string;
|
vaultFolder?: string;
|
||||||
deleteFile?: ReturnType<typeof vi.fn>;
|
deleteFile?: ReturnType<typeof vi.fn>;
|
||||||
|
deleteBatch?: ReturnType<typeof vi.fn>;
|
||||||
adapterExists?: ReturnType<typeof vi.fn>;
|
adapterExists?: ReturnType<typeof vi.fn>;
|
||||||
adapterStat?: ReturnType<typeof vi.fn>;
|
adapterStat?: ReturnType<typeof vi.fn>;
|
||||||
getAbstractFileByPath?: ReturnType<typeof vi.fn>;
|
getAbstractFileByPath?: ReturnType<typeof vi.fn>;
|
||||||
|
|
@ -29,7 +30,7 @@ function makePlugin(overrides: {
|
||||||
|
|
||||||
const plugin = {
|
const plugin = {
|
||||||
settings: { branch: 'main', vaultFolder },
|
settings: { branch: 'main', vaultFolder },
|
||||||
gitService: { deleteFile },
|
gitService: { deleteFile, deleteBatch: overrides.deleteBatch },
|
||||||
getNormalizedPath(path: string): string {
|
getNormalizedPath(path: string): string {
|
||||||
if (!vaultFolder) return path;
|
if (!vaultFolder) return path;
|
||||||
const prefix = `${vaultFolder}/`;
|
const prefix = `${vaultFolder}/`;
|
||||||
|
|
@ -98,6 +99,71 @@ describe('SyncStatusView remote deletion', () => {
|
||||||
|
|
||||||
expect(errors).toEqual([{ path: 'notes/todo.md', message: 'Cannot delete "notes/todo.md": file was not found on branch "main".' }]);
|
expect(errors).toEqual([{ path: 'notes/todo.md', message: 'Cannot delete "notes/todo.md": file was not found on branch "main".' }]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('groups all remote-only deletes into one gitService.deleteBatch call when the provider supports it', async () => {
|
||||||
|
const deleteBatch = vi.fn().mockResolvedValue(undefined);
|
||||||
|
const { plugin, leaf, deleteFile } = makePlugin({ deleteBatch });
|
||||||
|
const view = new SyncStatusView(leaf, plugin);
|
||||||
|
|
||||||
|
const targets: FileStatus[] = [
|
||||||
|
{ path: 'a.md', status: 'remote-only' },
|
||||||
|
{ path: 'b.md', status: 'remote-only' },
|
||||||
|
];
|
||||||
|
const errors: { path: string, message: string }[] = [];
|
||||||
|
const prog = new Notice('', 0);
|
||||||
|
|
||||||
|
await (view as unknown as {
|
||||||
|
performRemoteDeletion(remote: FileStatus[], total: number, localCount: number, prog: Notice, errors: { path: string, message: string }[]): Promise<void>
|
||||||
|
}).performRemoteDeletion(targets, 2, 0, prog, errors);
|
||||||
|
|
||||||
|
expect(deleteBatch).toHaveBeenCalledTimes(1);
|
||||||
|
expect(deleteBatch).toHaveBeenCalledWith(['a.md', 'b.md'], 'main', expect.any(String));
|
||||||
|
expect(deleteFile).not.toHaveBeenCalled();
|
||||||
|
expect(errors).toHaveLength(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('marks every path in a failed deleteBatch chunk as failed, not dropped', async () => {
|
||||||
|
const deleteBatch = vi.fn().mockRejectedValue(new Error('commit failed'));
|
||||||
|
const { plugin, leaf } = makePlugin({ deleteBatch });
|
||||||
|
const view = new SyncStatusView(leaf, plugin);
|
||||||
|
|
||||||
|
const targets: FileStatus[] = [
|
||||||
|
{ path: 'a.md', status: 'remote-only' },
|
||||||
|
{ path: 'b.md', status: 'remote-only' },
|
||||||
|
];
|
||||||
|
const errors: { path: string, message: string }[] = [];
|
||||||
|
const prog = new Notice('', 0);
|
||||||
|
|
||||||
|
await (view as unknown as {
|
||||||
|
performRemoteDeletion(remote: FileStatus[], total: number, localCount: number, prog: Notice, errors: { path: string, message: string }[]): Promise<void>
|
||||||
|
}).performRemoteDeletion(targets, 2, 0, prog, errors);
|
||||||
|
|
||||||
|
expect(errors).toEqual([
|
||||||
|
{ path: 'a.md', message: 'commit failed' },
|
||||||
|
{ path: 'b.md', message: 'commit failed' },
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('falls back to the sequential deleteFile loop when the provider has no deleteBatch', async () => {
|
||||||
|
const { plugin, leaf, deleteFile } = makePlugin();
|
||||||
|
const view = new SyncStatusView(leaf, plugin);
|
||||||
|
|
||||||
|
const targets: FileStatus[] = [
|
||||||
|
{ path: 'a.md', status: 'remote-only' },
|
||||||
|
{ path: 'b.md', status: 'remote-only' },
|
||||||
|
];
|
||||||
|
const errors: { path: string, message: string }[] = [];
|
||||||
|
const prog = new Notice('', 0);
|
||||||
|
|
||||||
|
await (view as unknown as {
|
||||||
|
performRemoteDeletion(remote: FileStatus[], total: number, localCount: number, prog: Notice, errors: { path: string, message: string }[]): Promise<void>
|
||||||
|
}).performRemoteDeletion(targets, 2, 0, prog, errors);
|
||||||
|
|
||||||
|
expect(deleteFile).toHaveBeenCalledTimes(2);
|
||||||
|
expect(deleteFile).toHaveBeenCalledWith('a.md', 'main', expect.any(String));
|
||||||
|
expect(deleteFile).toHaveBeenCalledWith('b.md', 'main', expect.any(String));
|
||||||
|
expect(errors).toHaveLength(0);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('SyncStatusView.identifyExtraFiles folder/remote-record collisions', () => {
|
describe('SyncStatusView.identifyExtraFiles folder/remote-record collisions', () => {
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue