refactor: consolidate duplicated methods into BaseGitService (#17)

This commit is contained in:
ClaudiaFang 2026-04-26 10:46:44 +08:00 committed by GitHub
parent 655cd69252
commit 591dd6e82a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 381 additions and 100 deletions

40
package-lock.json generated
View file

@ -1,13 +1,13 @@
{
"name": "git-file-push",
"version": "1.0.0",
"name": "git-file-sync",
"version": "1.1.0",
"lockfileVersion": 3,
"requires": true,
"packages": {
"": {
"name": "git-file-push",
"version": "1.0.0",
"license": "0-BSD",
"name": "git-file-sync",
"version": "1.1.0",
"license": "MIT",
"dependencies": {
"ignore": "^7.0.5",
"obsidian": "latest"
@ -1556,9 +1556,6 @@
"arm64"
],
"dev": true,
"libc": [
"glibc"
],
"license": "MIT",
"optional": true,
"os": [
@ -1576,9 +1573,6 @@
"arm64"
],
"dev": true,
"libc": [
"musl"
],
"license": "MIT",
"optional": true,
"os": [
@ -1596,9 +1590,6 @@
"ppc64"
],
"dev": true,
"libc": [
"glibc"
],
"license": "MIT",
"optional": true,
"os": [
@ -1616,9 +1607,6 @@
"s390x"
],
"dev": true,
"libc": [
"glibc"
],
"license": "MIT",
"optional": true,
"os": [
@ -1636,9 +1624,6 @@
"x64"
],
"dev": true,
"libc": [
"glibc"
],
"license": "MIT",
"optional": true,
"os": [
@ -1656,9 +1641,6 @@
"x64"
],
"dev": true,
"libc": [
"musl"
],
"license": "MIT",
"optional": true,
"os": [
@ -7320,9 +7302,6 @@
"arm64"
],
"dev": true,
"libc": [
"glibc"
],
"license": "MPL-2.0",
"optional": true,
"os": [
@ -7344,9 +7323,6 @@
"arm64"
],
"dev": true,
"libc": [
"musl"
],
"license": "MPL-2.0",
"optional": true,
"os": [
@ -7368,9 +7344,6 @@
"x64"
],
"dev": true,
"libc": [
"glibc"
],
"license": "MPL-2.0",
"optional": true,
"os": [
@ -7392,9 +7365,6 @@
"x64"
],
"dev": true,
"libc": [
"musl"
],
"license": "MPL-2.0",
"optional": true,
"os": [

View file

@ -88,10 +88,43 @@ export abstract class BaseGitService {
return cleanRoot + cleanPath;
}
protected encodeContent(content: string): string {
const bytes = new TextEncoder().encode(content);
let binary = '';
for (let i = 0; i < bytes.byteLength; i++) {
const byte = bytes[i];
if (byte !== undefined) {
binary += String.fromCodePoint(byte);
}
}
return btoa(binary);
}
protected decodeContent(base64: string): string {
const binary = atob(base64.replace(/\s/g, ''));
const bytes = new Uint8Array(binary.length);
for (let i = 0; i < binary.length; i++) {
const cp = binary.codePointAt(i);
bytes[i] = cp !== undefined ? cp : 0;
}
return new TextDecoder().decode(bytes);
}
protected handleFileNotFound(e: unknown): GitFile {
if (e instanceof Error && e.message.includes('404')) {
return { content: '', sha: '' };
}
throw e;
}
async getRepoGitignores(branch: string): Promise<string[]> {
const allFiles = await this.listFiles(branch);
return allFiles.filter(p => p.endsWith('.gitignore'));
}
abstract getFile(path: string, branch: string): Promise<GitFile>;
abstract pushFile(path: string, content: string, branch: string, message: string, sha?: string): Promise<string>;
abstract listFiles(branch: string): Promise<string[]>;
abstract deleteFile(path: string, branch: string, message: string): Promise<void>;
abstract testConnection(): Promise<boolean>;
abstract getRepoGitignores(branch: string): Promise<string[]>;
}

View file

@ -32,10 +32,7 @@ export class GitHubService extends BaseGitService implements GitServiceInterface
sha: data.sha
};
} catch (e) {
if (e instanceof Error && e.message.includes('404')) {
return { content: '', sha: '' };
}
throw e;
return this.handleFileNotFound(e);
}
}
@ -86,30 +83,4 @@ export class GitHubService extends BaseGitService implements GitServiceInterface
}
}
async getRepoGitignores(branch: string): Promise<string[]> {
const allFiles = await this.listFiles(branch);
return allFiles.filter(p => p.endsWith('.gitignore'));
}
private encodeContent(content: string): string {
const bytes = new TextEncoder().encode(content);
let binary = '';
for (let i = 0; i < bytes.byteLength; i++) {
const byte = bytes[i];
if (byte !== undefined) {
binary += String.fromCodePoint(byte);
}
}
return btoa(binary);
}
private decodeContent(base64: string): string {
const binary = atob(base64.replace(/\s/g, ''));
const bytes = new Uint8Array(binary.length);
for (let i = 0; i < binary.length; i++) {
const cp = binary.codePointAt(i);
bytes[i] = cp !== undefined ? cp : 0;
}
return new TextDecoder().decode(bytes);
}
}

View file

@ -34,10 +34,7 @@ export class GitLabService extends BaseGitService implements GitServiceInterface
sha: data.last_commit_id
};
} catch (e) {
if (e instanceof Error && e.message.includes('404')) {
return { content: '', sha: '' };
}
throw e;
return this.handleFileNotFound(e);
}
}
@ -90,30 +87,4 @@ export class GitLabService extends BaseGitService implements GitServiceInterface
}
}
async getRepoGitignores(branch: string): Promise<string[]> {
const allFiles = await this.listFiles(branch);
return allFiles.filter(p => p.endsWith('.gitignore'));
}
private encodeContent(content: string): string {
const bytes = new TextEncoder().encode(content);
let binary = '';
for (let i = 0; i < bytes.byteLength; i++) {
const byte = bytes[i];
if (byte !== undefined) {
binary += String.fromCodePoint(byte);
}
}
return btoa(binary);
}
private decodeContent(base64: string): string {
const binary = atob(base64.replace(/\s/g, ''));
const bytes = new Uint8Array(binary.length);
for (let i = 0; i < binary.length; i++) {
const cp = binary.codePointAt(i);
bytes[i] = cp !== undefined ? cp : 0;
}
return new TextDecoder().decode(bytes);
}
}

View file

@ -62,6 +62,34 @@ describe('GitignoreManager', () => {
expect(manager.isIgnored('secret.txt')).toBe(true);
});
it('should fall back to [".gitignore"] when getRepoGitignores throws', async () => {
vi.mocked(mockGitService.getRepoGitignores).mockRejectedValue(new Error('Network error'));
const adapter = mockApp.vault.adapter as Mocked<DataAdapter>;
vi.mocked(adapter.exists).mockResolvedValue(true);
vi.mocked(adapter.read).mockResolvedValue('node_modules/');
const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
await manager.loadGitignores();
expect(consoleSpy).toHaveBeenCalled();
expect(manager.isIgnored('node_modules/test.js')).toBe(true);
});
it('should fall back to remote when local .gitignore read throws', async () => {
vi.mocked(mockGitService.getRepoGitignores).mockResolvedValue(['.gitignore']);
const adapter = mockApp.vault.adapter as Mocked<DataAdapter>;
vi.mocked(adapter.exists).mockResolvedValue(true);
vi.mocked(adapter.read).mockRejectedValue(new Error('Permission denied'));
vi.mocked(mockGitService.getFile).mockResolvedValue({ content: 'secret.txt', sha: 'sha' });
const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
await manager.loadGitignores();
expect(consoleSpy).toHaveBeenCalled();
expect(mockGitService.getFile).toHaveBeenCalledWith('/.gitignore', branch);
expect(manager.isIgnored('secret.txt')).toBe(true);
});
it('should handle subdirectory gitignores correctly', async () => {
// Repo structure:
// .gitignore

View file

@ -118,7 +118,7 @@ describe('SyncManager Batch Operations', () => {
it('should handle missing remote files during batch pull', async () => {
const files = ['exists.md', 'missing.md'];
vi.mocked(mockGitService.getFile)
.mockResolvedValueOnce({ content: 'content', sha: 'sha' })
.mockResolvedValueOnce({ content: '', sha: '' });
@ -130,4 +130,48 @@ describe('SyncManager Batch Operations', () => {
expect(results.errors[0]!.error).toContain('File not found in remote');
});
});
describe('onProgress callback', () => {
it('should call onProgress for each file processed', async () => {
const files = ['file1.md', 'file2.md', 'file3.md'];
const adapter = mockApp.vault.adapter as Mocked<DataAdapter>;
vi.mocked(adapter.exists).mockResolvedValue(true);
vi.mocked(adapter.read).mockResolvedValue('content');
vi.mocked(mockGitService.getFile).mockResolvedValue({ content: '', sha: 'sha' });
vi.mocked(mockGitService.pushFile).mockResolvedValue('path');
const onProgress = vi.fn();
await manager.pushAllFiles(files, onProgress);
expect(onProgress).toHaveBeenCalledTimes(3);
expect(onProgress).toHaveBeenCalledWith(1, 3, 'file1.md');
expect(onProgress).toHaveBeenCalledWith(2, 3, 'file2.md');
expect(onProgress).toHaveBeenCalledWith(3, 3, 'file3.md');
});
});
describe('batch push with rename detection', () => {
it('should detect and handle rename during batch push', async () => {
const oldPath = 'old.md';
const newPath = 'new.md';
const mockFile = Object.assign(new TFile(), { path: newPath, name: 'new.md' });
mockSettings.syncMetadata = {
[oldPath]: { lastSyncedSha: 'sha', lastSyncedAt: 0, lastKnownPath: oldPath }
};
vi.mocked(mockApp.vault.getFileByPath).mockImplementation(p => p === oldPath ? null : mockFile);
vi.mocked(mockApp.vault.read).mockResolvedValue('content');
vi.mocked(mockApp.vault.adapter.exists as ReturnType<typeof vi.fn>).mockResolvedValue(true);
vi.mocked(mockGitService.pushFile).mockResolvedValue(newPath);
vi.mocked(mockGitService.getFile).mockResolvedValue({ content: 'content', sha: 'new-sha' });
const results = await manager.pushAllFiles([mockFile]);
expect(results.success).toBe(1);
expect(mockGitService.pushFile).toHaveBeenCalledWith(
newPath, 'content', 'main', `Rename ${oldPath} to ${newPath}`, undefined
);
});
});
});

View file

@ -0,0 +1,68 @@
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { GitHubService } from '../../src/services/github-service';
import { requestUrl, RequestUrlResponse } from 'obsidian';
// Tests for BaseGitService protected methods exercised via GitHubService
describe('BaseGitService', () => {
let service: GitHubService;
beforeEach(() => {
vi.clearAllMocks();
service = new GitHubService();
service.updateConfig('token', 'owner', 'repo');
});
describe('getFullPath with rootPath ending in /', () => {
it('should not double-add slash when rootPath already ends with /', async () => {
service.updateConfig('token', 'owner', 'repo', 'vault/');
vi.mocked(requestUrl).mockResolvedValue({
status: 200,
json: { content: btoa('hello'), sha: 'abc' }
} as unknown as RequestUrlResponse);
await service.getFile('notes/test.md', 'main');
const calls = vi.mocked(requestUrl).mock.calls;
const call = calls[0]?.[0] as { url: string };
// rootPath 'vault/' + 'notes/test.md' should produce 'vault/notes/test.md'
expect(call.url).toContain('vault/notes/test.md');
expect(call.url).not.toContain('vault//notes/test.md');
});
});
describe('safeRequest with non-Error exception', () => {
it('should wrap non-Error throws in a new Error', async () => {
// Throw a plain string (not an Error instance) from requestUrl
vi.mocked(requestUrl).mockRejectedValue('plain string error');
await expect(service.getFile('test.md', 'main')).rejects.toThrow(
'Network error or unexpected failure: plain string error'
);
});
});
describe('encodeContent / decodeContent round-trip', () => {
it('should correctly encode and decode UTF-8 content', async () => {
const original = 'Hello, 世界! 🌍';
vi.mocked(requestUrl).mockResolvedValue({
status: 200,
json: {
content: { path: 'test.md' }
}
} as unknown as RequestUrlResponse);
// Push encodes content; we verify the encoded body decodes back to original
await service.pushFile('test.md', original, 'main', 'test');
const calls = vi.mocked(requestUrl).mock.calls;
const body = JSON.parse((calls[0]?.[0] as { body: string }).body) as { content: string };
// Decode what was sent and confirm round-trip
const decoded = atob(body.content.replace(/\s/g, ''));
const bytes = new Uint8Array(decoded.length);
for (let i = 0; i < decoded.length; i++) {
bytes[i] = decoded.codePointAt(i) ?? 0;
}
expect(new TextDecoder().decode(bytes)).toBe(original);
});
});
});

View file

@ -69,9 +69,9 @@ describe('GitHubService', () => {
});
it('should update existing file correctly (sha provided)', async () => {
vi.mocked(requestUrl).mockResolvedValue({
status: 200,
json: { content: { path: 'existing.md' } }
vi.mocked(requestUrl).mockResolvedValue({
status: 200,
json: { content: { path: 'existing.md' } }
} as unknown as RequestUrlResponse);
const result = await service.pushFile('existing.md', 'updated content', 'main', 'update', 'old-sha');
@ -85,4 +85,108 @@ describe('GitHubService', () => {
expect(lastCall.body).toContain('"sha":"old-sha"');
});
});
describe('listFiles', () => {
it('should list blob files from tree API', async () => {
vi.mocked(requestUrl).mockResolvedValue({
status: 200,
json: {
tree: [
{ path: 'file1.md', type: 'blob' },
{ path: 'dir/file2.md', type: 'blob' },
{ path: 'subdir', type: 'tree' },
]
}
} as unknown as RequestUrlResponse);
const result = await service.listFiles('main');
expect(result).toEqual(['file1.md', 'dir/file2.md']);
});
it('should filter by rootPath when set', async () => {
service.updateConfig(token, owner, repo, 'vault');
vi.mocked(requestUrl).mockResolvedValue({
status: 200,
json: {
tree: [
{ path: 'vault/file1.md', type: 'blob' },
{ path: 'other/file2.md', type: 'blob' },
]
}
} as unknown as RequestUrlResponse);
const result = await service.listFiles('main');
expect(result).toEqual(['vault/file1.md']);
});
});
describe('deleteFile', () => {
it('should delete file using its sha', async () => {
vi.mocked(requestUrl)
.mockResolvedValueOnce({
status: 200,
json: { content: btoa('content'), sha: 'file-sha' }
} as unknown as RequestUrlResponse)
.mockResolvedValueOnce({
status: 200,
json: {}
} as unknown as RequestUrlResponse);
await service.deleteFile('test.md', 'main', 'delete test.md');
const calls = vi.mocked(requestUrl).mock.calls;
expect(calls).toHaveLength(2);
const deleteCall = calls[1]?.[0] as RequestUrlParam;
expect(deleteCall.method).toBe('DELETE');
expect(deleteCall.body).toContain('"sha":"file-sha"');
});
});
describe('testConnection', () => {
it('should return true on successful connection', async () => {
vi.mocked(requestUrl).mockResolvedValue({ status: 200, json: {} } as unknown as RequestUrlResponse);
const result = await service.testConnection();
expect(result).toBe(true);
});
it('should return false on failed connection', async () => {
vi.mocked(requestUrl).mockResolvedValue({
status: 401,
json: { message: 'Unauthorized' },
text: 'Unauthorized'
} as unknown as RequestUrlResponse);
const result = await service.testConnection();
expect(result).toBe(false);
});
});
describe('getRepoGitignores', () => {
it('should return only .gitignore paths from file list', async () => {
vi.mocked(requestUrl).mockResolvedValue({
status: 200,
json: {
tree: [
{ path: '.gitignore', type: 'blob' },
{ path: 'src/main.ts', type: 'blob' },
{ path: 'sub/.gitignore', type: 'blob' },
]
}
} as unknown as RequestUrlResponse);
const result = await service.getRepoGitignores('main');
expect(result).toEqual(['.gitignore', 'sub/.gitignore']);
});
});
describe('getFile error handling', () => {
it('should rethrow non-404 errors', async () => {
vi.mocked(requestUrl).mockResolvedValue({
status: 500,
json: { message: 'Internal Server Error' },
text: 'Internal Server Error'
} as unknown as RequestUrlResponse);
await expect(service.getFile('test.md', 'main')).rejects.toThrow('500');
});
});
});

View file

@ -88,4 +88,96 @@ describe('GitLabService', () => {
expect(lastCall.body).toContain(btoa('updated content'));
});
});
describe('listFiles', () => {
it('should list blob files from tree API', async () => {
vi.mocked(requestUrl).mockResolvedValue({
status: 200,
json: [
{ path: 'file1.md', type: 'blob' },
{ path: 'dir/file2.md', type: 'blob' },
{ path: 'subdir', type: 'tree' },
]
} as unknown as RequestUrlResponse);
const result = await service.listFiles('main');
expect(result).toEqual(['file1.md', 'dir/file2.md']);
});
it('should filter by rootPath when set', async () => {
service.updateConfig(baseUrl, token, projectId, 'vault');
vi.mocked(requestUrl).mockResolvedValue({
status: 200,
json: [
{ path: 'vault/file1.md', type: 'blob' },
{ path: 'other/file2.md', type: 'blob' },
]
} as unknown as RequestUrlResponse);
const result = await service.listFiles('main');
expect(result).toEqual(['vault/file1.md']);
});
});
describe('deleteFile', () => {
it('should delete file with commit message', async () => {
vi.mocked(requestUrl).mockResolvedValue({
status: 200,
json: {}
} as unknown as RequestUrlResponse);
await service.deleteFile('test.md', 'main', 'delete test.md');
const calls = vi.mocked(requestUrl).mock.calls;
const deleteCall = calls[0]?.[0] as RequestUrlParam;
expect(deleteCall.method).toBe('DELETE');
expect(deleteCall.body).toContain('"commit_message":"delete test.md"');
});
});
describe('testConnection', () => {
it('should return true on successful connection', async () => {
vi.mocked(requestUrl).mockResolvedValue({ status: 200, json: {} } as unknown as RequestUrlResponse);
const result = await service.testConnection();
expect(result).toBe(true);
});
it('should return false on failed connection', async () => {
vi.mocked(requestUrl).mockResolvedValue({
status: 401,
json: { message: 'Unauthorized' },
text: 'Unauthorized'
} as unknown as RequestUrlResponse);
const result = await service.testConnection();
expect(result).toBe(false);
});
});
describe('getRepoGitignores', () => {
it('should return only .gitignore paths from file list', async () => {
vi.mocked(requestUrl).mockResolvedValue({
status: 200,
json: [
{ path: '.gitignore', type: 'blob' },
{ path: 'src/main.ts', type: 'blob' },
{ path: 'sub/.gitignore', type: 'blob' },
]
} as unknown as RequestUrlResponse);
const result = await service.getRepoGitignores('main');
expect(result).toEqual(['.gitignore', 'sub/.gitignore']);
});
});
describe('getFile error handling', () => {
it('should rethrow non-404 errors', async () => {
vi.mocked(requestUrl).mockResolvedValue({
status: 500,
json: { message: 'Internal Server Error' },
text: 'Internal Server Error'
} as unknown as RequestUrlResponse);
await expect(service.getFile('test.md', 'main')).rejects.toThrow('500');
});
});
});