refactor: address SonarCloud issues and reduce code duplication (#20)

* ci: use shared obsidian plugin workflow and update obsidian version

* Add quality gate status badge to README

Added a quality gate status badge to the README.

* refactor: address SonarCloud issues and reduce code duplication

---------

Co-authored-by: ClaudiaFang <tianyao.firstsun@gmail.coim>
This commit is contained in:
ClaudiaFang 2026-04-27 01:20:40 +08:00 committed by GitHub
parent 67bf95ae89
commit 4574abc294
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 108 additions and 138 deletions

View file

@ -16,7 +16,7 @@ jobs:
uses: firstsun-dev/general-workflows/.github/workflows/obsidian-plugin-ci.yml@main
with:
plugin-id: "git-file-sync"
skip-sonar: false
skip-sonar: ${{ secrets.SONAR_TOKEN == '' || vars.ENABLE_CI_SONAR != 'true' }}
secrets:
RELEASE_TOKEN: ${{ secrets.RELEASE_TOKEN }}
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}

View file

@ -14,6 +14,19 @@ export class SyncManager {
this.settings = settings;
}
private get serviceName(): string {
return this.settings.serviceType === 'gitlab' ? 'GitLab' : 'GitHub';
}
public async updateMetadata(path: string, sha: string): Promise<void> {
this.settings.syncMetadata[path] = {
lastSyncedSha: sha,
lastSyncedAt: Date.now(),
lastKnownPath: path
};
await this.saveSettings();
}
updateGitService(gitService: GitServiceInterface): void {
this.gitService = gitService;
}
@ -27,7 +40,6 @@ export class SyncManager {
}
const content = await this.getFileContent(fileOrPath);
const serviceName = this.settings.serviceType === 'gitlab' ? 'GitLab' : 'GitHub';
try {
// Check if this is a renamed file
let renamedFrom = null;
@ -65,7 +77,7 @@ export class SyncManager {
await this.performPush({ path, name }, content, remote.sha);
} catch (e) {
console.error(e);
new Notice(`Failed to push ${name} to ${serviceName}: ${e instanceof Error ? e.message : String(e)}`);
new Notice(`Failed to push ${name} to ${this.serviceName}: ${e instanceof Error ? e.message : String(e)}`);
}
}
@ -88,8 +100,6 @@ export class SyncManager {
}
private async handleRename(file: TFile, oldPath: string, content: string): Promise<void> {
const serviceName = this.settings.serviceType === 'gitlab' ? 'GitLab' : 'GitHub';
try {
// Push the file to the new location
await this.gitService.pushFile(
@ -106,25 +116,20 @@ export class SyncManager {
// Update metadata
const newRemote = await this.gitService.getFile(file.path, this.settings.branch);
this.settings.syncMetadata[file.path] = {
lastSyncedSha: newRemote.sha,
lastSyncedAt: Date.now(),
lastKnownPath: file.path
};
await this.updateMetadata(file.path, newRemote.sha);
// Remove old metadata
delete this.settings.syncMetadata[oldPath];
await this.saveSettings();
new Notice(`Renamed and pushed ${file.name} to ${serviceName}\nNote: Old file at ${oldPath} may need manual deletion from remote`);
new Notice(`Renamed and pushed ${file.name} to ${this.serviceName}\nNote: Old file at ${oldPath} may need manual deletion from remote`);
} catch (e) {
console.error(e);
new Notice(`Failed to handle rename: ${e instanceof Error ? e.message : String(e)}`);
}
}
private async performPush(file: {path: string, name: string}, content: string, existingSha?: string) {
const serviceName = this.settings.serviceType === 'gitlab' ? 'GitLab' : 'GitHub';
private async performPush(file: {path: string, name: string}, content: string, existingSha?: string, silent = false) {
await this.gitService.pushFile(
file.path,
content,
@ -135,53 +140,40 @@ export class SyncManager {
// Update metadata
const newRemote = await this.gitService.getFile(file.path, this.settings.branch);
this.settings.syncMetadata[file.path] = {
lastSyncedSha: newRemote.sha,
lastSyncedAt: Date.now(),
lastKnownPath: file.path
};
await this.saveSettings();
new Notice(`Pushed ${file.name} to ${serviceName}`);
await this.updateMetadata(file.path, newRemote.sha);
if (!silent) new Notice(`Pushed ${file.name} to ${this.serviceName}`);
}
async pullFile(fileOrPath: TFile | string) {
const { path, name, isString } = this.getFileInfo(fileOrPath);
if (!await this.checkFileExists(path, isString)) {
new Notice(`File ${name} no longer exists in vault.`);
return;
}
const serviceName = this.settings.serviceType === 'gitlab' ? 'GitLab' : 'GitHub';
try {
const remote = await this.gitService.getFile(path, this.settings.branch);
if (!remote.sha) {
new Notice(`File ${name} not found on remote.`);
return;
}
const localContent = await this.getFileContent(fileOrPath);
const exists = await this.checkFileExists(path, isString);
const localContent = exists ? await this.getFileContent(fileOrPath) : null;
const lastSynced = this.settings.syncMetadata[path];
if (localContent === remote.content) {
if (exists && localContent === remote.content) {
// Still update metadata even if content matches
this.settings.syncMetadata[path] = {
lastSyncedSha: remote.sha,
lastSyncedAt: Date.now()
};
await this.saveSettings();
await this.updateMetadata(path, remote.sha);
new Notice(`${name} is already up to date.`);
return;
}
// Conflict detection for pull
if (remote.sha && lastSynced && remote.sha !== lastSynced.lastSyncedSha) {
new SyncConflictModal(this.app, name, localContent, remote.content, (choice) => {
// Conflict detection for pull (only if local exists)
if (exists && remote.sha && lastSynced && remote.sha !== lastSynced.lastSyncedSha) {
new SyncConflictModal(this.app, name, localContent || '', remote.content, (choice) => {
void (async () => {
try {
const fileRep = typeof fileOrPath === 'string' ? { path, name } : fileOrPath;
if (choice === 'local') {
await this.performPush({ path, name }, localContent, remote.sha);
await this.performPush({ path, name }, localContent || '', remote.sha);
} else {
await this.performPull(fileRep, remote.content, remote.sha);
}
@ -198,12 +190,12 @@ export class SyncManager {
await this.performPull(fileRep, remote.content, remote.sha);
} catch (e) {
console.error(e);
new Notice(`Failed to pull ${name} from ${serviceName}: ${e instanceof Error ? e.message : String(e)}`);
new Notice(`Failed to pull ${name} from ${this.serviceName}: ${e instanceof Error ? e.message : String(e)}`);
}
}
private async performPull(file: TFile | {path: string, name: string}, remoteContent: string, remoteSha: string) {
const serviceName = this.settings.serviceType === 'gitlab' ? 'GitLab' : 'GitHub';
private async performPull(file: TFile | {path: string, name: string}, remoteContent: string, remoteSha: string, silent = false) {
await this.ensureParentDirs(file.path);
if (file instanceof TFile) {
await this.app.vault.modify(file, remoteContent);
@ -212,15 +204,24 @@ export class SyncManager {
}
// Update metadata
this.settings.syncMetadata[file.path] = {
lastSyncedSha: remoteSha,
lastSyncedAt: Date.now(),
lastKnownPath: file.path
};
await this.updateMetadata(file.path, remoteSha);
await this.saveSettings();
const name = file.name;
new Notice(`Pulled ${name} from ${serviceName}`);
if (!silent) new Notice(`Pulled ${file.name} from ${this.serviceName}`);
}
private async ensureParentDirs(filePath: string): Promise<void> {
const parts = filePath.split('/');
let cur = '';
for (let i = 0; i < parts.length - 1; i++) {
cur += (i > 0 ? '/' : '') + parts[i];
if (!this.app.vault.getAbstractFileByPath(cur)) {
try {
await this.app.vault.createFolder(cur);
} catch {
// already exists or failed
}
}
}
}
private async saveSettings() {
@ -313,21 +314,14 @@ export class SyncManager {
}
const remote = await this.gitService.getFile(path, this.settings.branch);
await this.gitService.pushFile(path, content, this.settings.branch, `Update ${name} from Obsidian`, remote.sha || undefined);
const newRemote = await this.gitService.getFile(path, this.settings.branch);
this.settings.syncMetadata[path] = { lastSyncedSha: newRemote.sha, lastSyncedAt: Date.now(), lastKnownPath: path };
await this.performPush({ path, name }, content, remote.sha || undefined, true);
}
private async processSingleBatchPull(fileOrPath: TFile | string, path: string, name: string, isString: boolean) {
const remote = await this.gitService.getFile(path, this.settings.branch);
if (!remote.sha) throw new Error('File not found in remote');
if (typeof fileOrPath === 'string') {
await this.app.vault.adapter.write(fileOrPath, remote.content);
} else if (fileOrPath instanceof TFile) {
await this.app.vault.modify(fileOrPath, remote.content);
}
this.settings.syncMetadata[path] = { lastSyncedSha: remote.sha, lastSyncedAt: Date.now(), lastKnownPath: path };
const fileRep = typeof fileOrPath === 'string' ? { path, name } : fileOrPath;
await this.performPull(fileRep, remote.content, remote.sha, true);
}
}

View file

@ -39,9 +39,7 @@ export default class GitLabFilesPush extends Plugin {
this.gitignoreManager = new GitignoreManager(this.app, this.gitService, this.settings.branch, this.settings.rootPath);
this.sync = new SyncManager(this.app, this.gitService, this.settings);
const serviceName = this.settings.serviceType === 'gitlab' ? 'GitLab' : 'GitHub';
this.addRibbonIcon('upload-cloud', Platform.isMobile ? `Push` : `Push to ${serviceName}`, async () => {
this.addRibbonIcon('upload-cloud', Platform.isMobile ? `Push` : `Push to ${this.serviceName}`, async () => {
const activeView = this.app.workspace.getActiveViewOfType(MarkdownView);
if (activeView && activeView.file instanceof TFile) {
await this.sync.pushFile(activeView.file);
@ -52,7 +50,7 @@ export default class GitLabFilesPush extends Plugin {
this.addCommand({
id: 'push-current-file',
name: `Push current file to ${serviceName}`,
name: `Push current file to ${this.serviceName}`,
callback: async () => {
const activeView = this.app.workspace.getActiveViewOfType(MarkdownView);
if (activeView && activeView.file instanceof TFile) {
@ -63,7 +61,7 @@ export default class GitLabFilesPush extends Plugin {
this.addCommand({
id: 'pull-current-file',
name: `Pull current file from ${serviceName}`,
name: `Pull current file from ${this.serviceName}`,
callback: async () => {
const activeView = this.app.workspace.getActiveViewOfType(MarkdownView);
if (activeView && activeView.file instanceof TFile) {
@ -92,12 +90,12 @@ export default class GitLabFilesPush extends Plugin {
this.app.workspace.on('file-menu', (menu, file) => {
if (file instanceof TFile) {
menu.addItem((item) => {
item.setTitle(`Push to ${serviceName}`)
item.setTitle(`Push to ${this.serviceName}`)
.setIcon('upload-cloud')
.onClick(async () => { await this.sync.pushFile(file); });
});
menu.addItem((item) => {
item.setTitle(`Pull from ${serviceName}`)
item.setTitle(`Pull from ${this.serviceName}`)
.setIcon('download-cloud')
.onClick(async () => { await this.sync.pullFile(file); });
});
@ -115,6 +113,10 @@ export default class GitLabFilesPush extends Plugin {
);
}
private get serviceName(): string {
return this.settings.serviceType === 'gitlab' ? 'GitLab' : 'GitHub';
}
async activateSyncStatusView(): Promise<void> {
const { workspace } = this.app;
@ -137,74 +139,53 @@ export default class GitLabFilesPush extends Plugin {
}
async pushAllFiles(): Promise<void> {
const allFiles = this.app.vault.getFiles();
let files = this.filterFilesByVaultFolder(allFiles);
const serviceName = this.settings.serviceType === 'gitlab' ? 'GitLab' : 'GitHub';
await this.gitService.listFiles(this.settings.branch);
await this.gitignoreManager.loadGitignores();
files = files.filter(f => !this.gitignoreManager.isIgnored(f.path));
if (files.length === 0) {
new Notice('No files to push in the configured vault folder');
return;
}
const confirmed = await this.showConfirmDialog(`Push ${files.length} file(s) to ${serviceName}?`);
if (!confirmed) return;
const progressNotice = new Notice(`Pushing 0/${files.length} files...`, 0);
try {
const results = await this.sync.pushAllFiles(files, (current, total, fileName) => {
progressNotice.setMessage(`Pushing ${current}/${total}: ${fileName}`);
});
progressNotice.hide();
if (results.errors.length > 0) {
console.error('Push errors:', results.errors);
}
} catch (e) {
progressNotice.hide();
console.error(e);
new Notice(`Push failed: ${e instanceof Error ? e.message : String(e)}`);
}
await this.runAllFiles('push');
}
async pullAllFiles(): Promise<void> {
await this.runAllFiles('pull');
}
private async runAllFiles(op: 'push' | 'pull'): Promise<void> {
const allFiles = this.app.vault.getFiles();
let files = this.filterFilesByVaultFolder(allFiles);
const serviceName = this.settings.serviceType === 'gitlab' ? 'GitLab' : 'GitHub';
await this.gitService.listFiles(this.settings.branch);
await this.gitignoreManager.loadGitignores();
files = files.filter(f => !this.gitignoreManager.isIgnored(f.path));
if (files.length === 0) {
new Notice('No files to pull in the configured vault folder');
new Notice(`No files to ${op} in the configured vault folder`);
return;
}
const confirmed = await this.showConfirmDialog(`Pull ${files.length} file(s) from ${serviceName}? This will overwrite local changes.`);
const msg = op === 'push'
? `Push ${files.length} file(s) to ${this.serviceName}?`
: `Pull ${files.length} file(s) from ${this.serviceName}? This will overwrite local changes.`;
const confirmed = await this.showConfirmDialog(msg);
if (!confirmed) return;
const progressNotice = new Notice(`Pulling 0/${files.length} files...`, 0);
const progressNotice = new Notice(`${op === 'push' ? 'Pushing' : 'Pulling'} 0/${files.length} files...`, 0);
try {
const results = await this.sync.pullAllFiles(files, (current, total, fileName) => {
progressNotice.setMessage(`Pulling ${current}/${total}: ${fileName}`);
});
const results = op === 'push'
? await this.sync.pushAllFiles(files, (current, total, fileName) => {
progressNotice.setMessage(`Pushing ${current}/${total}: ${fileName}`);
})
: await this.sync.pullAllFiles(files, (current, total, fileName) => {
progressNotice.setMessage(`Pulling ${current}/${total}: ${fileName}`);
});
progressNotice.hide();
if (results.errors.length > 0) {
console.error('Pull errors:', results.errors);
console.error(`${op} errors:`, results.errors);
}
} catch (e) {
progressNotice.hide();
console.error(e);
new Notice(`Pull failed: ${e instanceof Error ? e.message : String(e)}`);
new Notice(`${op === 'push' ? 'Push' : 'Pull'} failed: ${e instanceof Error ? e.message : String(e)}`);
}
}

View file

@ -314,34 +314,16 @@ export class SyncStatusView extends ItemView {
private async runSingleFile(fileStatus: FileStatus, op: 'push' | 'pull'): Promise<void> {
try {
const originalStatus = fileStatus.status;
fileStatus.status = 'checking';
this.renderView();
if (op === 'push') {
await this.plugin.sync.pushFile(fileStatus.file || fileStatus.path);
await new Promise(r => setTimeout(r, 500));
} else if (originalStatus === 'remote-only' || !fileStatus.file) {
// pull remote-only
const remote = await this.plugin.gitService.getFile(fileStatus.path, this.plugin.settings.branch);
if (remote.content) {
await this.ensureParentDirs(fileStatus.path);
await this.app.vault.adapter.write(fileStatus.path, remote.content);
this.plugin.settings.syncMetadata[fileStatus.path] = {
lastSyncedSha: remote.sha,
lastSyncedAt: Date.now(),
lastKnownPath: fileStatus.path
};
await this.plugin.saveSettings();
}
await new Promise(r => setTimeout(r, 1000));
await this.refreshAllStatuses();
return;
} else {
await this.plugin.sync.pullFile(fileStatus.file || fileStatus.path);
await new Promise(r => setTimeout(r, 500));
}
await new Promise(r => setTimeout(r, 500));
await this.refreshFileStatus(fileStatus.file || fileStatus.path);
this.renderView();
} catch (e) {
@ -681,17 +663,6 @@ export class SyncStatusView extends ItemView {
return diff.join('\n');
}
private async ensureParentDirs(filePath: string): Promise<void> {
const parts = filePath.split('/');
let cur = '';
for (let i = 0; i < parts.length - 1; i++) {
cur += (i > 0 ? '/' : '') + parts[i];
if (!this.app.vault.getAbstractFileByPath(cur)) {
try { await this.app.vault.createFolder(cur); } catch { /* already exists */ }
}
}
}
async pushAllModified(): Promise<void> {
await this.runBatchOperation('modified', 'push');
}

View file

@ -28,6 +28,13 @@ const mockApp = {
read: vi.fn(),
modify: vi.fn(),
getFileByPath: vi.fn(),
getAbstractFileByPath: vi.fn(),
createFolder: vi.fn(),
adapter: {
exists: vi.fn(),
read: vi.fn(),
write: vi.fn(),
}
}
} as unknown as App;
@ -313,6 +320,23 @@ describe('SyncManager', () => {
expect(mockApp.vault.modify).not.toHaveBeenCalled();
});
it('should pull a new file that does not exist locally', async () => {
const path = 'new-remote-file.md';
vi.mocked(mockGitLab.getFile).mockResolvedValue({ content: 'remote content', sha: 'new-sha' });
vi.spyOn(mockApp.vault, 'getFileByPath').mockReturnValue(null);
const writeSpy = vi.spyOn(mockApp.vault.adapter, 'write').mockResolvedValue(undefined);
vi.spyOn(mockApp.vault.adapter, 'exists').mockResolvedValue(false);
// Mock ensureParentDirs by mocking getAbstractFileByPath to return folder for parent
vi.spyOn(mockApp.vault, 'getAbstractFileByPath').mockReturnValue(new TFile());
await manager.pullFile(path);
expect(writeSpy).toHaveBeenCalledWith(path, 'remote content');
expect(mockSettings.syncMetadata[path]?.lastSyncedSha).toBe('new-sha');
});
it('should handle pull errors gracefully', async () => {
const mockFile = Object.assign(new TFile(), { path: 'fail.md', name: 'fail.md' });
vi.mocked(mockGitLab.getFile).mockRejectedValue(new Error('Network error'));