From 4cc20e3a3cb3da409aeaab9b94533657f2c943bc Mon Sep 17 00:00:00 2001 From: Vladimir Urushev Date: Sun, 21 Jun 2026 21:26:53 +0200 Subject: [PATCH] fix: skip re-uploading unchanged attachments to avoid Confluence rollback error --- src/confluence/client.ts | 43 -------------------- src/sync.ts | 26 ++++++------- test/sync.test.ts | 84 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 57 deletions(-) diff --git a/src/confluence/client.ts b/src/confluence/client.ts index 6af1d87..e1c67cb 100644 --- a/src/confluence/client.ts +++ b/src/confluence/client.ts @@ -150,49 +150,6 @@ export class ConfluenceClient { } return result } - - async updateAttachment( - pageId: string, - attachmentId: string, - filename: string, - data: Buffer, - contentType: string, - ): Promise { - const formData = new FormData() - const blob = new Blob([data], { type: contentType }) - formData.append('file', blob, filename) - - const url = `${this.baseUrl}/content/${pageId}/child/attachment/${attachmentId}/data` - const response = await fetch(url, { - method: 'POST', - headers: { - Authorization: this.authHeader, - 'X-Atlassian-Token': 'nocheck', - }, - body: formData, - }) - - if (!response.ok) { - let errorMessage: string - try { - const errorBody = (await response.json()) as { message?: string } - errorMessage = errorBody.message || response.statusText - } catch { - errorMessage = response.statusText - } - throw new ConfluenceApiError({ - statusCode: response.status, - message: errorMessage, - }) - } - - // API may return attachment directly or wrapped in results array - const result = (await response.json()) as Attachment | AttachmentSearchResult - if ('results' in result) { - return result.results[0] - } - return result - } } export class ConfluenceApiError extends Error { diff --git a/src/sync.ts b/src/sync.ts index 6554ef9..2217aa2 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -148,25 +148,23 @@ async function uploadAttachments( client: ConfluenceClient, config: Config, ): Promise { - // Get existing attachments to avoid duplicates const existing = await client.getAttachments(pageId) - const existingMap = new Map(existing.map((a) => [a.title, a])) + const existingNames = new Set(existing.map((a) => a.title)) for (const [filename, info] of attachments) { - const existingAttachment = existingMap.get(filename) - - if (existingAttachment) { - // Update existing attachment - await client.updateAttachment(pageId, existingAttachment.id, filename, info.data, info.contentType) - if (config.verbose) { - console.log(` Updated attachment: ${filename}`) - } - } else { - // Upload new attachment - await client.uploadAttachment(pageId, filename, info.data, info.contentType) + // Attachment names embed an md5 of their content, so a name match means the bytes are + // already identical. Re-uploading is a no-op that Confluence Cloud rejects with + // "UnexpectedRollbackException: marked as rollback-only" — skip instead of re-uploading. + if (existingNames.has(filename)) { if (config.verbose) { - console.log(` Uploaded attachment: ${filename}`) + console.log(` Skipped attachment (unchanged): ${filename}`) } + continue + } + + await client.uploadAttachment(pageId, filename, info.data, info.contentType) + if (config.verbose) { + console.log(` Uploaded attachment: ${filename}`) } } } diff --git a/test/sync.test.ts b/test/sync.test.ts index e8adcf1..0b641c8 100644 --- a/test/sync.test.ts +++ b/test/sync.test.ts @@ -180,6 +180,90 @@ describe('syncFiles', () => { }) }) + describe('attachment upload', () => { + // Writes a markdown file referencing a local image, returns the content-hashed + // attachment name Confluence would store it under (basename-.ext). + async function withLocalImage(): Promise<{ file: string; attachmentName: string; cleanup: () => void }> { + const fs = await import('node:fs') + const path = await import('node:path') + const os = await import('node:os') + const { createHash } = await import('node:crypto') + + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'markfluence-attach-')) + const data = Buffer.from('fake-png-bytes') + fs.writeFileSync(path.join(dir, 'pic.png'), data) + const file = path.join(dir, 'page.md') + fs.writeFileSync(file, '# Title\n\n![pic](./pic.png)\n') + + const hash = createHash('md5').update(data).digest('hex').slice(0, 12) + return { + file, + attachmentName: `pic-${hash}.png`, + cleanup: () => fs.rmSync(dir, { recursive: true, force: true }), + } + } + + const existingPage = { + id: '123', + title: 'Title', + version: { number: 1 }, + body: { storage: { value: '

old

' } }, + _links: { webui: '/pages/123' }, + } + const updatedPage = { id: '123', title: 'Title', version: { number: 2 }, _links: { webui: '/pages/123' } } + + it('skips re-uploading an attachment that already exists (idempotent re-sync)', async () => { + const { file, attachmentName, cleanup } = await withLocalImage() + try { + fetchMock + // getPageByTitle + .mockResolvedValueOnce({ ok: true, status: 200, json: () => Promise.resolve({ results: [existingPage] }) }) + // updatePage + .mockResolvedValueOnce({ ok: true, status: 200, json: () => Promise.resolve(updatedPage) }) + // getAttachments — the image is already there under the same content-hashed name + .mockResolvedValueOnce({ + ok: true, + status: 200, + json: () => Promise.resolve({ results: [{ id: 'att-1', title: attachmentName }] }), + }) + + const results = await syncFiles([file], mockConfig) + + // The file must still be reported as updated, not silently dropped on an attachment error. + expect(results).toHaveLength(1) + expect(results[0].action).toBe('updated') + // getPageByTitle + updatePage + getAttachments, and crucially NO re-upload POST. + expect(fetchMock).toHaveBeenCalledTimes(3) + expect(fetchMock.mock.calls.some((c) => c[1]?.method === 'POST')).toBe(false) + } finally { + cleanup() + } + }) + + it('uploads an attachment that is not yet present', async () => { + const { file, cleanup } = await withLocalImage() + try { + fetchMock + .mockResolvedValueOnce({ ok: true, status: 200, json: () => Promise.resolve({ results: [existingPage] }) }) + .mockResolvedValueOnce({ ok: true, status: 200, json: () => Promise.resolve(updatedPage) }) + // getAttachments — nothing there yet + .mockResolvedValueOnce({ ok: true, status: 200, json: () => Promise.resolve({ results: [] }) }) + // uploadAttachment + .mockResolvedValueOnce({ ok: true, status: 200, json: () => Promise.resolve({ id: 'att-2' }) }) + + const results = await syncFiles([file], mockConfig) + + expect(results).toHaveLength(1) + expect(results[0].action).toBe('updated') + expect(fetchMock).toHaveBeenCalledTimes(4) + const postCall = fetchMock.mock.calls.find((c) => c[1]?.method === 'POST') + expect(postCall?.[0]).toContain('/child/attachment') + } finally { + cleanup() + } + }) + }) + describe('per-file space override', () => { it('uses space from frontmatter when available', async () => { const fs = await import('node:fs')