From 1d3cbb86504de3b5b58ffaae0a037f64d6f95e8f Mon Sep 17 00:00:00 2001 From: sam Date: Tue, 22 Jul 2025 12:21:28 +0300 Subject: [PATCH 01/11] feat: add individual attachment upload for forms with large attachments --- src/lib/insert-or-replace.js | 201 +++++++++++++++++++++-- test/fn/upload-large-attachments.spec.js | 151 +++++++++++++++++ 2 files changed, 340 insertions(+), 12 deletions(-) create mode 100644 test/fn/upload-large-attachments.spec.js diff --git a/src/lib/insert-or-replace.js b/src/lib/insert-or-replace.js index ac7834ac..dc917225 100644 --- a/src/lib/insert-or-replace.js +++ b/src/lib/insert-or-replace.js @@ -1,12 +1,189 @@ -module.exports = (db, doc) => - db.get(doc._id) - .then(existingDoc => doc._rev = existingDoc._rev) - .catch(e => { - if(e.status === 404) { - return; - } - else { - throw e; - } - }) - .then(() => db.put(doc)); +const mime = require('mime-types'); + +const upsertDoc = async (db, doc, maxRetries = 3, retryDelay = 100) => { + let attempts = 0; + + while (attempts < maxRetries) { + try { + // Fetch the latest _rev on each attempt + try { + const existingDoc = await db.get(doc._id); + // Creating a copy to avoid mutating the original doc + const docToSave = { ...doc, _rev: existingDoc._rev }; + const result = await db.put(docToSave); + return result; + } catch (e) { + if (e.status === 404) { + // Document doesn't exist, try to create it + const docToSave = { ...doc }; + delete docToSave._rev; // Remove _rev for new documents + const result = await db.put(docToSave); + return result; + } else { + throw e; + } + } + + } catch (err) { + if (err.status === 409) { + // Conflict - document was updated by another process + attempts++; + if (attempts >= maxRetries) { + throw new Error(`Document update failed after ${maxRetries} attempts due to conflicts. + Last error: ${err.message}`); + } + + console.log(`Conflict detected, retrying attempt ${attempts}/${maxRetries}`); + // Wait before retrying (exponential backoff) + await new Promise(resolve => setTimeout(resolve, retryDelay * Math.pow(2, attempts - 1))); + continue; + + } else if (err.status === 413) { + // Document too large - handle attachments separately + return handleLargeDocument(db, doc, maxRetries, retryDelay); + + } else { + throw err; + } + } + } +}; + +const handleLargeDocument = async (db, doc, maxRetries = 5, retryDelay = 200) => { + let attempts = 0; + + while (attempts < maxRetries) { + try { + // Backup attachments + const attachments = doc._attachments; + const docWithoutAttachments = { ...doc }; + delete docWithoutAttachments._attachments; + + // Get the very latest document state + let currentRev = null; + try { + const existingDoc = await db.get(doc._id, { latest: true }); + currentRev = existingDoc._rev; + docWithoutAttachments._rev = currentRev; + console.log(`Got latest rev for ${doc._id}: ${currentRev}`); + } catch (e) { + if (e.status !== 404) { + throw e; + } + delete docWithoutAttachments._rev; + console.log(`Document ${doc._id} does not exist, creating new`); + } + + // Try to save the document without attachments + console.log(`Attempting to save ${doc._id} (attempt ${attempts + 1}/${maxRetries})`); + const res = await db.put(docWithoutAttachments); + console.log(`Successfully saved ${doc._id}, new rev: ${res.rev}`); + + // Handle attachments one by one with individual error handling + if (attachments && Object.keys(attachments).length > 0) { + console.log(`Processing ${Object.keys(attachments).length} attachments for ${doc._id}`); + + let currentDocRev = res.rev; + for (const attachmentName of Object.keys(attachments)) { + try { + const att = attachments[attachmentName]; + const contentType = att.content_type || mime.lookup(attachmentName) || 'application/octet-stream'; + + const attachResult = await db.putAttachment(doc._id, attachmentName, currentDocRev, att.data, contentType); + currentDocRev = attachResult.rev; // Update rev for next attachment + console.log(`Added attachment ${attachmentName} to ${doc._id}`); + } catch (attachErr) { + console.error(`Failed to add attachment ${attachmentName}:`, attachErr.message); + if (attachErr.status === 409) { + // Get the latest rev and retry this attachment + const latestDoc = await db.get(doc._id); + currentDocRev = latestDoc._rev; + const att = attachments[attachmentName]; + const contentType = att.content_type || mime.lookup(attachmentName) || 'application/octet-stream'; + const attachResult = await db.putAttachment(doc._id, attachmentName, currentDocRev, + att.data, contentType); + currentDocRev = attachResult.rev; + console.log(`Retried and added attachment ${attachmentName} to ${doc._id}`); + } else { + throw attachErr; + } + } + } + } + + return res; + + } catch (err) { + if (err.status === 409) { + attempts++; + if (attempts >= maxRetries) { + console.error(`All retry attempts failed for ${doc._id}. Trying force save strategy...`); + try { + return await forceSaveDocument(db, doc); + } catch (forceErr) { + throw new Error(`Large document update failed after ${maxRetries} attempts and + force save failed. Document ID: ${doc._id}. Last error: ${forceErr.message}`); + } + } + + console.log(`Large document conflict detected for ${doc._id}, retrying attempt ${attempts}/${maxRetries}`); + // Increase delay progressively + const delay = retryDelay * Math.pow(2, attempts - 1) + Math.random() * 100; // Add jitter + await new Promise(resolve => setTimeout(resolve, delay)); + continue; + } else { + console.error(`Non-conflict error for ${doc._id}:`, err.message); + throw err; + } + } + } +}; + +const forceSaveDocument = async (db, doc) => { + console.log(`Attempting force save for ${doc._id}`); + + // Remove attachments temporarily and create a clean document + const attachments = doc._attachments; + const cleanDoc = { ...doc }; + delete cleanDoc._attachments; + delete cleanDoc._rev; // Remove rev entirely to force creation of new document + + try { + // Try to delete the existing problematic document + try { + const existingDoc = await db.get(doc._id); + await db.remove(existingDoc._id, existingDoc._rev); + console.log(`Removed existing conflicted document ${doc._id}`); + } catch (deleteErr) { + if (deleteErr.status !== 404) { + console.log(`Could not delete existing document: ${deleteErr.message}`); + } + } + + // Try to PUT the clean document + const result = await db.put(cleanDoc); + console.log(`Force saved ${doc._id} with rev: ${result.rev}`); + + // Add attachments back if they exist + if (attachments && Object.keys(attachments).length > 0) { + console.log(`Adding ${Object.keys(attachments).length} attachments to force-saved document`); + let currentRev = result.rev; + + for (const attachmentName of Object.keys(attachments)) { + const att = attachments[attachmentName]; + const contentType = att.content_type || mime.lookup(attachmentName) || 'application/octet-stream'; + + const attachResult = await db.putAttachment(result.id, attachmentName, currentRev, att.data, contentType); + currentRev = attachResult.rev; + console.log(`Added attachment ${attachmentName} to force-saved document`); + } + } + + return result; + } catch (err) { + console.error(`Force save failed for ${doc._id}:`, err.message); + throw err; + } +}; + +module.exports = upsertDoc; diff --git a/test/fn/upload-large-attachments.spec.js b/test/fn/upload-large-attachments.spec.js new file mode 100644 index 00000000..f8aa3abe --- /dev/null +++ b/test/fn/upload-large-attachments.spec.js @@ -0,0 +1,151 @@ +const { expect } = require('chai'); +const sinon = require('sinon'); + +describe('forms with large attachments', () => { + + it('falls back to uploading attachments separately when doc is too large', async () => { + const largeAttachment = { + data: Buffer.alloc(340 * 1024 * 1024), // 340MB + content_type: 'image/png' + }; + + const doc = { + _id: 'form:large', + someField: 'test data', + _attachments: { + 'large.png': largeAttachment + } + }; + + const fakeDb = { + // First get() call in upsertDoc - document exists + get: sinon.stub() + .onFirstCall().resolves({ _id: 'form:large', _rev: '1-abc' }) + // Second get() call in handleLargeDocument with {latest: true} + .onSecondCall().resolves({ _id: 'form:large', _rev: '1-abc' }), + + // First put() call fails with 413, second put() (without attachments) succeeds + put: sinon.stub() + .onFirstCall().rejects({ status: 413, message: 'Document too large' }) + .onSecondCall().resolves({ id: 'form:large', rev: '2-def', ok: true }), + + // putAttachment succeeds + putAttachment: sinon.stub().resolves({ id: 'form:large', rev: '3-ghi', ok: true }) + }; + + const uploadFn = require('../../src/lib/insert-or-replace'); + const result = await uploadFn(fakeDb, doc); + + // Verify the flow + expect(fakeDb.get.calledTwice).to.be.true; + expect(fakeDb.put.calledTwice).to.be.true; + expect(fakeDb.putAttachment.calledOnce).to.be.true; + + // Verify first put() call included attachments and proper _rev + const firstPutCall = fakeDb.put.firstCall.args[0]; + expect(firstPutCall._id).to.equal('form:large'); + expect(firstPutCall._rev).to.equal('1-abc'); + expect(firstPutCall._attachments).to.exist; + expect(firstPutCall._attachments['large.png']).to.equal(largeAttachment); + + // Verify second put() call (fallback) excluded attachments but kept other fields + const secondPutCall = fakeDb.put.secondCall.args[0]; + expect(secondPutCall._id).to.equal('form:large'); + expect(secondPutCall._rev).to.equal('1-abc'); + expect(secondPutCall.someField).to.equal('test data'); + expect(secondPutCall._attachments).to.be.undefined; + + // Verify putAttachment was called with correct parameters + const [id, name, rev, data, contentType] = fakeDb.putAttachment.firstCall.args; + expect(id).to.equal('form:large'); + expect(name).to.equal('large.png'); + expect(rev).to.equal('2-def'); // Should use the rev from the second put() result + expect(data).to.equal(largeAttachment.data); + expect(contentType).to.equal('image/png'); + + // Verify the final result + expect(result).to.deep.equal({ id: 'form:large', rev: '2-def', ok: true }); + }); + + // Additional test for multiple attachments + it('handles multiple attachments when document is too large', async () => { + const attachment1 = { data: Buffer.alloc(1024), content_type: 'image/png' }; + const attachment2 = { data: Buffer.alloc(2048), content_type: 'image/jpeg' }; + + const doc = { + _id: 'form:multi-attach', + _attachments: { + 'image1.png': attachment1, + 'image2.jpg': attachment2 + } + }; + + const fakeDb = { + get: sinon.stub() + .onFirstCall().resolves({ _id: 'form:multi-attach', _rev: '1-abc' }) + .onSecondCall().resolves({ _id: 'form:multi-attach', _rev: '1-abc' }), + + put: sinon.stub() + .onFirstCall().rejects({ status: 413 }) + .onSecondCall().resolves({ id: 'form:multi-attach', rev: '2-def', ok: true }), + + putAttachment: sinon.stub() + .onFirstCall().resolves({ id: 'form:multi-attach', rev: '3-ghi', ok: true }) + .onSecondCall().resolves({ id: 'form:multi-attach', rev: '4-jkl', ok: true }) + }; + + const uploadFn = require('../../src/lib/insert-or-replace'); + await uploadFn(fakeDb, doc); + + expect(fakeDb.putAttachment.calledTwice).to.be.true; + + // Verify first attachment + const [id1, name1, rev1, data1, contentType1] = fakeDb.putAttachment.firstCall.args; + expect(id1).to.equal('form:multi-attach'); + expect(name1).to.equal('image1.png'); + expect(rev1).to.equal('2-def'); + expect(contentType1).to.equal('image/png'); + + // Verify second attachment uses updated rev + const [id2, name2, rev2, data2, contentType2] = fakeDb.putAttachment.secondCall.args; + expect(id2).to.equal('form:multi-attach'); + expect(name2).to.equal('image2.jpg'); + expect(rev2).to.equal('3-ghi'); // Updated rev from first attachment + expect(contentType2).to.equal('image/jpeg'); + }); + + // Test for attachment conflict handling + it('handles conflicts when adding attachments separately', async () => { + const attachment = { data: Buffer.alloc(1024), content_type: 'image/png' }; + + const doc = { + _id: 'form:conflict', + _attachments: { 'test.png': attachment } + }; + + const fakeDb = { + get: sinon.stub() + .onCall(0).resolves({ _id: 'form:conflict', _rev: '1-abc' }) + .onCall(1).resolves({ _id: 'form:conflict', _rev: '1-abc' }) + .onCall(2).resolves({ _id: 'form:conflict', _rev: '2-updated' }), // For retry + + put: sinon.stub() + .onFirstCall().rejects({ status: 413 }) + .onSecondCall().resolves({ id: 'form:conflict', rev: '2-def', ok: true }), + + putAttachment: sinon.stub() + .onFirstCall().rejects({ status: 409, message: 'Document update conflict' }) + .onSecondCall().resolves({ id: 'form:conflict', rev: '3-final', ok: true }) + }; + + const uploadFn = require('../../src/lib/insert-or-replace'); + await uploadFn(fakeDb, doc); + + expect(fakeDb.get.calledThrice).to.be.true; + expect(fakeDb.putAttachment.calledTwice).to.be.true; + + // Verify retry used updated rev + const [, , retryRev] = fakeDb.putAttachment.secondCall.args; + expect(retryRev).to.equal('2-updated'); + }); +}); From 9c7638ee0c5c7eaa4f9da480b462f76f6ab1323f Mon Sep 17 00:00:00 2001 From: sam Date: Tue, 12 Aug 2025 06:37:07 +0300 Subject: [PATCH 02/11] refactor: implement recommendations and add unit test for creating brand new documents without _rev --- src/lib/insert-or-replace.js | 254 ++++++++--------------- test/fn/upload-large-attachments.spec.js | 33 +++ 2 files changed, 122 insertions(+), 165 deletions(-) diff --git a/src/lib/insert-or-replace.js b/src/lib/insert-or-replace.js index dc917225..a0d657b7 100644 --- a/src/lib/insert-or-replace.js +++ b/src/lib/insert-or-replace.js @@ -1,187 +1,111 @@ const mime = require('mime-types'); +const log = require('./log'); -const upsertDoc = async (db, doc, maxRetries = 3, retryDelay = 100) => { - let attempts = 0; - - while (attempts < maxRetries) { - try { - // Fetch the latest _rev on each attempt - try { - const existingDoc = await db.get(doc._id); - // Creating a copy to avoid mutating the original doc - const docToSave = { ...doc, _rev: existingDoc._rev }; - const result = await db.put(docToSave); - return result; - } catch (e) { - if (e.status === 404) { - // Document doesn't exist, try to create it - const docToSave = { ...doc }; - delete docToSave._rev; // Remove _rev for new documents - const result = await db.put(docToSave); - return result; - } else { - throw e; - } - } +const MAX_RETRY = 3; - } catch (err) { - if (err.status === 409) { - // Conflict - document was updated by another process - attempts++; - if (attempts >= maxRetries) { - throw new Error(`Document update failed after ${maxRetries} attempts due to conflicts. - Last error: ${err.message}`); - } - - console.log(`Conflict detected, retrying attempt ${attempts}/${maxRetries}`); - // Wait before retrying (exponential backoff) - await new Promise(resolve => setTimeout(resolve, retryDelay * Math.pow(2, attempts - 1))); - continue; - - } else if (err.status === 413) { - // Document too large - handle attachments separately - return handleLargeDocument(db, doc, maxRetries, retryDelay); - - } else { - throw err; - } +const getDoc = async (db, docId) => { + try { + return await db.get(docId); + } catch (e) { + if (e.status === 404) { + return null; // Document doesn't exist } + throw e; } }; -const handleLargeDocument = async (db, doc, maxRetries = 5, retryDelay = 200) => { - let attempts = 0; - - while (attempts < maxRetries) { - try { - // Backup attachments - const attachments = doc._attachments; - const docWithoutAttachments = { ...doc }; - delete docWithoutAttachments._attachments; +const putDoc = async (db, doc, existingRev = null) => { + const docToSave = { ...doc }; + if (existingRev) { + docToSave._rev = existingRev; + } else { + delete docToSave._rev; // Ensure no _rev for new documents + } + return await db.put(docToSave); +}; - // Get the very latest document state - let currentRev = null; - try { - const existingDoc = await db.get(doc._id, { latest: true }); - currentRev = existingDoc._rev; - docWithoutAttachments._rev = currentRev; - console.log(`Got latest rev for ${doc._id}: ${currentRev}`); - } catch (e) { - if (e.status !== 404) { - throw e; - } - delete docWithoutAttachments._rev; - console.log(`Document ${doc._id} does not exist, creating new`); - } +const addDocAttachment = async (db, docId, attachmentName, attachment, currentRev, retries = MAX_RETRY) => { + if (retries < 0) { + throw new Error(`Failed to add attachment ${attachmentName} to ${docId} after retries`); + } - // Try to save the document without attachments - console.log(`Attempting to save ${doc._id} (attempt ${attempts + 1}/${maxRetries})`); - const res = await db.put(docWithoutAttachments); - console.log(`Successfully saved ${doc._id}, new rev: ${res.rev}`); - - // Handle attachments one by one with individual error handling - if (attachments && Object.keys(attachments).length > 0) { - console.log(`Processing ${Object.keys(attachments).length} attachments for ${doc._id}`); - - let currentDocRev = res.rev; - for (const attachmentName of Object.keys(attachments)) { - try { - const att = attachments[attachmentName]; - const contentType = att.content_type || mime.lookup(attachmentName) || 'application/octet-stream'; - - const attachResult = await db.putAttachment(doc._id, attachmentName, currentDocRev, att.data, contentType); - currentDocRev = attachResult.rev; // Update rev for next attachment - console.log(`Added attachment ${attachmentName} to ${doc._id}`); - } catch (attachErr) { - console.error(`Failed to add attachment ${attachmentName}:`, attachErr.message); - if (attachErr.status === 409) { - // Get the latest rev and retry this attachment - const latestDoc = await db.get(doc._id); - currentDocRev = latestDoc._rev; - const att = attachments[attachmentName]; - const contentType = att.content_type || mime.lookup(attachmentName) || 'application/octet-stream'; - const attachResult = await db.putAttachment(doc._id, attachmentName, currentDocRev, - att.data, contentType); - currentDocRev = attachResult.rev; - console.log(`Retried and added attachment ${attachmentName} to ${doc._id}`); - } else { - throw attachErr; - } - } - } - } - - return res; - - } catch (err) { - if (err.status === 409) { - attempts++; - if (attempts >= maxRetries) { - console.error(`All retry attempts failed for ${doc._id}. Trying force save strategy...`); - try { - return await forceSaveDocument(db, doc); - } catch (forceErr) { - throw new Error(`Large document update failed after ${maxRetries} attempts and - force save failed. Document ID: ${doc._id}. Last error: ${forceErr.message}`); - } - } - - console.log(`Large document conflict detected for ${doc._id}, retrying attempt ${attempts}/${maxRetries}`); - // Increase delay progressively - const delay = retryDelay * Math.pow(2, attempts - 1) + Math.random() * 100; // Add jitter - await new Promise(resolve => setTimeout(resolve, delay)); - continue; - } else { - console.error(`Non-conflict error for ${doc._id}:`, err.message); - throw err; + try { + const contentType = attachment.content_type || mime.lookup(attachmentName) || 'application/octet-stream'; + const result = await db.putAttachment(docId, attachmentName, currentRev, attachment.data, contentType); + log.info(`Added attachment ${attachmentName} to ${docId}`); + return result; + } catch (err) { + if (err.status === 409) { + log.info(`Attachment conflict for ${attachmentName} in ${docId}, retrying`); + const latestDoc = await getDoc(db, docId); + if (!latestDoc) { + throw new Error(`Document ${docId} not found during attachment retry`); } + return addDocAttachment(db, docId, attachmentName, attachment, latestDoc._rev, retries - 1); } + throw err; + } +}; + +const handleAttachments = async (db, docId, attachments, initialRev) => { + let currentRev = initialRev; + for (const attachmentName of Object.keys(attachments)) { + const result = await addDocAttachment(db, docId, attachmentName, attachments[attachmentName], currentRev); + currentRev = result.rev; // Update rev for next attachment } + return currentRev; }; -const forceSaveDocument = async (db, doc) => { - console.log(`Attempting force save for ${doc._id}`); - - // Remove attachments temporarily and create a clean document - const attachments = doc._attachments; - const cleanDoc = { ...doc }; - delete cleanDoc._attachments; - delete cleanDoc._rev; // Remove rev entirely to force creation of new document - +const handleLargeDocument = async (db, doc, retries = MAX_RETRY) => { + if (retries < 0) { + throw new Error(`Large document update failed for ${doc._id} after retries`); + } + try { - // Try to delete the existing problematic document - try { - const existingDoc = await db.get(doc._id); - await db.remove(existingDoc._id, existingDoc._rev); - console.log(`Removed existing conflicted document ${doc._id}`); - } catch (deleteErr) { - if (deleteErr.status !== 404) { - console.log(`Could not delete existing document: ${deleteErr.message}`); - } - } - - // Try to PUT the clean document - const result = await db.put(cleanDoc); - console.log(`Force saved ${doc._id} with rev: ${result.rev}`); - - // Add attachments back if they exist + const attachments = doc._attachments; + const docWithoutAttachments = { ...doc }; + delete docWithoutAttachments._attachments; + + const existingDoc = await getDoc(db, doc._id); + log.info(existingDoc ? `Got latest rev for ${doc._id}: ${existingDoc._rev}` : + `Document ${doc._id} does not exist, creating new`); + + log.info(`Attempting to save ${doc._id}`); + const res = await putDoc(db, docWithoutAttachments, existingDoc ? existingDoc._rev : null); + log.info(`Successfully saved ${doc._id}, new rev: ${res.rev}`); + if (attachments && Object.keys(attachments).length > 0) { - console.log(`Adding ${Object.keys(attachments).length} attachments to force-saved document`); - let currentRev = result.rev; - - for (const attachmentName of Object.keys(attachments)) { - const att = attachments[attachmentName]; - const contentType = att.content_type || mime.lookup(attachmentName) || 'application/octet-stream'; - - const attachResult = await db.putAttachment(result.id, attachmentName, currentRev, att.data, contentType); - currentRev = attachResult.rev; - console.log(`Added attachment ${attachmentName} to force-saved document`); - } + log.info(`Processing ${Object.keys(attachments).length} attachments for ${doc._id}`); + await handleAttachments(db, doc._id, attachments, res.rev); + } + + return res; + } catch (err) { + if (err.status === 409) { + log.info(`Large document conflict detected for ${doc._id}, retrying`); + return handleLargeDocument(db, doc, retries - 1); } - + log.error(`Non-conflict error for ${doc._id}: ${err.message}`); + throw err; + } +}; + +const upsertDoc = async (db, doc, retries = MAX_RETRY) => { + if (retries < 0) { + throw new Error(`Document update failed for ${doc._id} after retries due to conflicts`); + } + + try { + const existingDoc = await getDoc(db, doc._id); + const result = await putDoc(db, doc, existingDoc ? existingDoc._rev : null); return result; } catch (err) { - console.error(`Force save failed for ${doc._id}:`, err.message); + if (err.status === 409) { + log.info(`Conflict detected for ${doc._id}, retrying`); + return upsertDoc(db, doc, retries - 1); + } else if (err.status === 413) { + return handleLargeDocument(db, doc); + } throw err; } }; diff --git a/test/fn/upload-large-attachments.spec.js b/test/fn/upload-large-attachments.spec.js index f8aa3abe..4c527ad3 100644 --- a/test/fn/upload-large-attachments.spec.js +++ b/test/fn/upload-large-attachments.spec.js @@ -148,4 +148,37 @@ describe('forms with large attachments', () => { const [, , retryRev] = fakeDb.putAttachment.secondCall.args; expect(retryRev).to.equal('2-updated'); }); + + it('creates a new document when it does not exist', async () => { + const doc = { + _id: 'new-doc', + someField: 'test data', + // no _rev + }; + + const fakeDb = { + get: sinon.stub() + .rejects({ status: 404 }), // Document does not exist + put: sinon.stub() + .resolves({ id: 'new-doc', rev: '1-newrev', ok: true }), + }; + + const uploadFn = require('../../src/lib/insert-or-replace'); + const result = await uploadFn(fakeDb, doc); + + // get() called once to check existence + expect(fakeDb.get.calledOnce).to.be.true; + expect(fakeDb.get.firstCall.args[0]).to.equal('new-doc'); + + // put() called once to create new doc + expect(fakeDb.put.calledOnce).to.be.true; + const putArg = fakeDb.put.firstCall.args[0]; + + expect(putArg._id).to.equal('new-doc'); + expect(putArg._rev).to.be.undefined; // _rev should be removed for new docs + expect(putArg.someField).to.equal('test data'); + + // Result matches put() return value + expect(result).to.deep.equal({ id: 'new-doc', rev: '1-newrev', ok: true }); + }); }); From af912c55cec9221e19b2b96c0b1e9da88a6b3064 Mon Sep 17 00:00:00 2001 From: sam Date: Wed, 20 Aug 2025 15:25:05 +0300 Subject: [PATCH 03/11] refactor: preserve non-media attachments, optimize error handling and update tests for uploading large attachments --- src/lib/insert-or-replace.js | 59 ++++-- test/fn/upload-large-attachments.spec.js | 221 +++++++++++++++++------ 2 files changed, 212 insertions(+), 68 deletions(-) diff --git a/src/lib/insert-or-replace.js b/src/lib/insert-or-replace.js index a0d657b7..e774612b 100644 --- a/src/lib/insert-or-replace.js +++ b/src/lib/insert-or-replace.js @@ -29,19 +29,22 @@ const addDocAttachment = async (db, docId, attachmentName, attachment, currentRe throw new Error(`Failed to add attachment ${attachmentName} to ${docId} after retries`); } + // Check for document existence before attempting attachment + const latestDoc = await getDoc(db, docId); + if (!latestDoc) { + throw new Error(`Document ${docId} not found before adding attachment ${attachmentName}`); + } + const revToUse = latestDoc._rev || currentRev; + try { const contentType = attachment.content_type || mime.lookup(attachmentName) || 'application/octet-stream'; - const result = await db.putAttachment(docId, attachmentName, currentRev, attachment.data, contentType); + const result = await db.putAttachment(docId, attachmentName, revToUse, attachment.data, contentType); log.info(`Added attachment ${attachmentName} to ${docId}`); return result; } catch (err) { if (err.status === 409) { log.info(`Attachment conflict for ${attachmentName} in ${docId}, retrying`); - const latestDoc = await getDoc(db, docId); - if (!latestDoc) { - throw new Error(`Document ${docId} not found during attachment retry`); - } - return addDocAttachment(db, docId, attachmentName, attachment, latestDoc._rev, retries - 1); + return addDocAttachment(db, docId, attachmentName, attachment, revToUse, retries - 1); } throw err; } @@ -62,21 +65,46 @@ const handleLargeDocument = async (db, doc, retries = MAX_RETRY) => { } try { - const attachments = doc._attachments; - const docWithoutAttachments = { ...doc }; - delete docWithoutAttachments._attachments; + const attachments = doc._attachments || {}; + // Regex for specific functional files: form.xml, model.xml, form.html + const functionalRegex = /^(form|model)\.xml$|^form\.html$/i; + + // Separate functional vs media attachments + const functionalAttachments = {}; + const mediaAttachments = {}; + for (const [name, value] of Object.entries(attachments)) { + if (functionalRegex.test(name)) { + functionalAttachments[name] = value; + } else { + if (value.data) { // Validate attachment has data + mediaAttachments[name] = value; + } else { + log.warn(`Skipping invalid attachment ${name} for ${doc._id}: missing data`); + } + } + } + + // Prepare doc with only functional attachments + const docToSave = { ...doc, _attachments: functionalAttachments }; + if (Object.keys(docToSave._attachments).length === 0) { + delete docToSave._attachments; + } const existingDoc = await getDoc(db, doc._id); - log.info(existingDoc ? `Got latest rev for ${doc._id}: ${existingDoc._rev}` : - `Document ${doc._id} does not exist, creating new`); + log.info( + existingDoc + ? `Got latest rev for ${doc._id}: ${existingDoc._rev}` + : `Document ${doc._id} does not exist, creating new` + ); log.info(`Attempting to save ${doc._id}`); - const res = await putDoc(db, docWithoutAttachments, existingDoc ? existingDoc._rev : null); + const res = await putDoc(db, docToSave, existingDoc ? existingDoc._rev : null); log.info(`Successfully saved ${doc._id}, new rev: ${res.rev}`); - if (attachments && Object.keys(attachments).length > 0) { - log.info(`Processing ${Object.keys(attachments).length} attachments for ${doc._id}`); - await handleAttachments(db, doc._id, attachments, res.rev); + // Upload media attachments separately + if (Object.keys(mediaAttachments).length > 0) { + log.info(`Processing ${Object.keys(mediaAttachments).length} media attachments for ${doc._id}`); + await handleAttachments(db, doc._id, mediaAttachments, res.rev); } return res; @@ -90,6 +118,7 @@ const handleLargeDocument = async (db, doc, retries = MAX_RETRY) => { } }; + const upsertDoc = async (db, doc, retries = MAX_RETRY) => { if (retries < 0) { throw new Error(`Document update failed for ${doc._id} after retries due to conflicts`); diff --git a/test/fn/upload-large-attachments.spec.js b/test/fn/upload-large-attachments.spec.js index 4c527ad3..9002ff91 100644 --- a/test/fn/upload-large-attachments.spec.js +++ b/test/fn/upload-large-attachments.spec.js @@ -1,35 +1,37 @@ const { expect } = require('chai'); const sinon = require('sinon'); +const log = require('../../src/lib/log'); // Import the log module directly describe('forms with large attachments', () => { + afterEach(() => { + sinon.restore(); + }); it('falls back to uploading attachments separately when doc is too large', async () => { const largeAttachment = { - data: Buffer.alloc(340 * 1024 * 1024), // 340MB + data: Buffer.alloc(340 * 1024 * 1024), content_type: 'image/png' }; - + const doc = { _id: 'form:large', someField: 'test data', _attachments: { - 'large.png': largeAttachment + 'large.png': largeAttachment, + 'form.xml': { data: Buffer.from('xml content'), content_type: 'application/xml' } } }; const fakeDb = { - // First get() call in upsertDoc - document exists get: sinon.stub() - .onFirstCall().resolves({ _id: 'form:large', _rev: '1-abc' }) - // Second get() call in handleLargeDocument with {latest: true} - .onSecondCall().resolves({ _id: 'form:large', _rev: '1-abc' }), - - // First put() call fails with 413, second put() (without attachments) succeeds + .onCall(0).resolves({ _id: 'form:large', _rev: '1-abc' }) // upsertDoc + .onCall(1).resolves({ _id: 'form:large', _rev: '1-abc' }) // handleLargeDocument + .onCall(2).resolves({ _id: 'form:large', _rev: '2-def' }), // addDocAttachment for large.png + put: sinon.stub() .onFirstCall().rejects({ status: 413, message: 'Document too large' }) .onSecondCall().resolves({ id: 'form:large', rev: '2-def', ok: true }), - - // putAttachment succeeds + putAttachment: sinon.stub().resolves({ id: 'form:large', rev: '3-ghi', ok: true }) }; @@ -37,58 +39,64 @@ describe('forms with large attachments', () => { const result = await uploadFn(fakeDb, doc); // Verify the flow - expect(fakeDb.get.calledTwice).to.be.true; + expect(fakeDb.get.calledThrice).to.be.true; expect(fakeDb.put.calledTwice).to.be.true; expect(fakeDb.putAttachment.calledOnce).to.be.true; - // Verify first put() call included attachments and proper _rev + // Verify first put() call included all attachments and proper _rev const firstPutCall = fakeDb.put.firstCall.args[0]; expect(firstPutCall._id).to.equal('form:large'); expect(firstPutCall._rev).to.equal('1-abc'); expect(firstPutCall._attachments).to.exist; expect(firstPutCall._attachments['large.png']).to.equal(largeAttachment); + expect(firstPutCall._attachments['form.xml']).to.exist; - // Verify second put() call (fallback) excluded attachments but kept other fields + // Verify second put() call (fallback) included only functional attachments const secondPutCall = fakeDb.put.secondCall.args[0]; expect(secondPutCall._id).to.equal('form:large'); expect(secondPutCall._rev).to.equal('1-abc'); expect(secondPutCall.someField).to.equal('test data'); - expect(secondPutCall._attachments).to.be.undefined; + expect(secondPutCall._attachments).to.deep.equal({ + 'form.xml': { data: Buffer.from('xml content'), content_type: 'application/xml' } + }); - // Verify putAttachment was called with correct parameters - const [id, name, rev, data, contentType] = fakeDb.putAttachment.firstCall.args; - expect(id).to.equal('form:large'); - expect(name).to.equal('large.png'); - expect(rev).to.equal('2-def'); // Should use the rev from the second put() result - expect(data).to.equal(largeAttachment.data); - expect(contentType).to.equal('image/png'); + // Verify putAttachment was called with correct parameters for media attachment + const putAttachmentArgs = fakeDb.putAttachment.firstCall.args; + expect(putAttachmentArgs[0]).to.equal('form:large'); + expect(putAttachmentArgs[1]).to.equal('large.png'); + expect(putAttachmentArgs[2]).to.equal('2-def'); + expect(putAttachmentArgs[3]).to.equal(largeAttachment.data); + expect(putAttachmentArgs[4]).to.equal('image/png'); // Verify the final result expect(result).to.deep.equal({ id: 'form:large', rev: '2-def', ok: true }); }); - // Additional test for multiple attachments it('handles multiple attachments when document is too large', async () => { const attachment1 = { data: Buffer.alloc(1024), content_type: 'image/png' }; const attachment2 = { data: Buffer.alloc(2048), content_type: 'image/jpeg' }; - + const functionalAttachment = { data: Buffer.from('html content'), content_type: 'text/html' }; + const doc = { _id: 'form:multi-attach', _attachments: { 'image1.png': attachment1, - 'image2.jpg': attachment2 + 'image2.jpg': attachment2, + 'form.html': functionalAttachment } }; const fakeDb = { get: sinon.stub() - .onFirstCall().resolves({ _id: 'form:multi-attach', _rev: '1-abc' }) - .onSecondCall().resolves({ _id: 'form:multi-attach', _rev: '1-abc' }), - + .onCall(0).resolves({ _id: 'form:multi-attach', _rev: '1-abc' }) // upsertDoc + .onCall(1).resolves({ _id: 'form:multi-attach', _rev: '1-abc' }) // handleLargeDocument + .onCall(2).resolves({ _id: 'form:multi-attach', _rev: '2-def' }) // addDocAttachment for image1.png + .onCall(3).resolves({ _id: 'form:multi-attach', _rev: '3-ghi' }), // addDocAttachment for image2.jpg + put: sinon.stub() .onFirstCall().rejects({ status: 413 }) .onSecondCall().resolves({ id: 'form:multi-attach', rev: '2-def', ok: true }), - + putAttachment: sinon.stub() .onFirstCall().resolves({ id: 'form:multi-attach', rev: '3-ghi', ok: true }) .onSecondCall().resolves({ id: 'form:multi-attach', rev: '4-jkl', ok: true }) @@ -97,27 +105,36 @@ describe('forms with large attachments', () => { const uploadFn = require('../../src/lib/insert-or-replace'); await uploadFn(fakeDb, doc); + expect(fakeDb.get.callCount).to.equal(4); + expect(fakeDb.put.calledTwice).to.be.true; expect(fakeDb.putAttachment.calledTwice).to.be.true; - + + // Verify second put() call included only functional attachments + const secondPutCall = fakeDb.put.secondCall.args[0]; + expect(secondPutCall._attachments).to.deep.equal({ + 'form.html': functionalAttachment + }); + // Verify first attachment - const [id1, name1, rev1, data1, contentType1] = fakeDb.putAttachment.firstCall.args; - expect(id1).to.equal('form:multi-attach'); - expect(name1).to.equal('image1.png'); - expect(rev1).to.equal('2-def'); - expect(contentType1).to.equal('image/png'); - + const firstAttachmentArgs = fakeDb.putAttachment.firstCall.args; + expect(firstAttachmentArgs[0]).to.equal('form:multi-attach'); + expect(firstAttachmentArgs[1]).to.equal('image1.png'); + expect(firstAttachmentArgs[2]).to.equal('2-def'); + expect(firstAttachmentArgs[3]).to.equal(attachment1.data); + expect(firstAttachmentArgs[4]).to.equal('image/png'); + // Verify second attachment uses updated rev - const [id2, name2, rev2, data2, contentType2] = fakeDb.putAttachment.secondCall.args; - expect(id2).to.equal('form:multi-attach'); - expect(name2).to.equal('image2.jpg'); - expect(rev2).to.equal('3-ghi'); // Updated rev from first attachment - expect(contentType2).to.equal('image/jpeg'); + const secondAttachmentArgs = fakeDb.putAttachment.secondCall.args; + expect(secondAttachmentArgs[0]).to.equal('form:multi-attach'); + expect(secondAttachmentArgs[1]).to.equal('image2.jpg'); + expect(secondAttachmentArgs[2]).to.equal('3-ghi'); + expect(secondAttachmentArgs[3]).to.equal(attachment2.data); + expect(secondAttachmentArgs[4]).to.equal('image/jpeg'); }); - // Test for attachment conflict handling it('handles conflicts when adding attachments separately', async () => { const attachment = { data: Buffer.alloc(1024), content_type: 'image/png' }; - + const doc = { _id: 'form:conflict', _attachments: { 'test.png': attachment } @@ -125,14 +142,15 @@ describe('forms with large attachments', () => { const fakeDb = { get: sinon.stub() - .onCall(0).resolves({ _id: 'form:conflict', _rev: '1-abc' }) - .onCall(1).resolves({ _id: 'form:conflict', _rev: '1-abc' }) - .onCall(2).resolves({ _id: 'form:conflict', _rev: '2-updated' }), // For retry - + .onCall(0).resolves({ _id: 'form:conflict', _rev: '1-abc' }) // upsertDoc + .onCall(1).resolves({ _id: 'form:conflict', _rev: '1-abc' }) // handleLargeDocument + .onCall(2).resolves({ _id: 'form:conflict', _rev: '2-def' }) // First addDocAttachment + .onCall(3).resolves({ _id: 'form:conflict', _rev: '2-updated' }), // Retry addDocAttachment + put: sinon.stub() .onFirstCall().rejects({ status: 413 }) .onSecondCall().resolves({ id: 'form:conflict', rev: '2-def', ok: true }), - + putAttachment: sinon.stub() .onFirstCall().rejects({ status: 409, message: 'Document update conflict' }) .onSecondCall().resolves({ id: 'form:conflict', rev: '3-final', ok: true }) @@ -141,12 +159,13 @@ describe('forms with large attachments', () => { const uploadFn = require('../../src/lib/insert-or-replace'); await uploadFn(fakeDb, doc); - expect(fakeDb.get.calledThrice).to.be.true; + expect(fakeDb.get.callCount).to.equal(4); + expect(fakeDb.put.calledTwice).to.be.true; expect(fakeDb.putAttachment.calledTwice).to.be.true; - + // Verify retry used updated rev - const [, , retryRev] = fakeDb.putAttachment.secondCall.args; - expect(retryRev).to.equal('2-updated'); + const retryAttachmentArgs = fakeDb.putAttachment.secondCall.args; + expect(retryAttachmentArgs[2]).to.equal('2-updated'); }); it('creates a new document when it does not exist', async () => { @@ -175,10 +194,106 @@ describe('forms with large attachments', () => { const putArg = fakeDb.put.firstCall.args[0]; expect(putArg._id).to.equal('new-doc'); - expect(putArg._rev).to.be.undefined; // _rev should be removed for new docs + expect(putArg._rev).to.be.undefined; expect(putArg.someField).to.equal('test data'); // Result matches put() return value expect(result).to.deep.equal({ id: 'new-doc', rev: '1-newrev', ok: true }); }); + + it('skips invalid media attachments and logs warning', async () => { + const validAttachment = { data: Buffer.alloc(1024), content_type: 'image/png' }; + const invalidAttachment = { content_type: 'image/jpeg' }; // Missing data + + const doc = { + _id: 'form:invalid-attach', + _attachments: { + 'valid.png': validAttachment, + 'invalid.jpg': invalidAttachment, + 'form.xml': { data: Buffer.from('xml content'), content_type: 'application/xml' } + } + }; + + const fakeDb = { + get: sinon.stub() + .onCall(0).resolves({ _id: 'form:invalid-attach', _rev: '1-abc' }) // upsertDoc + .onCall(1).resolves({ _id: 'form:invalid-attach', _rev: '1-abc' }) // handleLargeDocument + .onCall(2).resolves({ _id: 'form:invalid-attach', _rev: '2-def' }), // addDocAttachment for valid.png + + put: sinon.stub() + .onFirstCall().rejects({ status: 413 }) + .onSecondCall().resolves({ id: 'form:invalid-attach', rev: '2-def', ok: true }), + + putAttachment: sinon.stub() + .resolves({ id: 'form:invalid-attach', rev: '3-ghi', ok: true }) + }; + + const logWarnSpy = sinon.spy(log, 'warn'); + const uploadFn = require('../../src/lib/insert-or-replace'); + await uploadFn(fakeDb, doc); + + expect(fakeDb.get.callCount).to.equal(3); + expect(fakeDb.put.calledTwice).to.be.true; + expect(fakeDb.putAttachment.calledOnce).to.be.true; + + // Verify second put() call included only functional attachments + const secondPutCall = fakeDb.put.secondCall.args[0]; + expect(secondPutCall._attachments).to.deep.equal({ + 'form.xml': { data: Buffer.from('xml content'), content_type: 'application/xml' } + }); + + // Verify only valid attachment was processed + const putAttachmentArgs = fakeDb.putAttachment.firstCall.args; + expect(putAttachmentArgs[1]).to.equal('valid.png'); + expect(putAttachmentArgs[2]).to.equal('2-def'); + expect(putAttachmentArgs[3]).to.equal(validAttachment.data); + expect(putAttachmentArgs[4]).to.equal('image/png'); + + // Verify warning was logged for invalid attachment + expect(logWarnSpy.calledOnce).to.be.true; + expect(logWarnSpy.calledWith( + 'Skipping invalid attachment invalid.jpg for form:invalid-attach: missing data' + )).to.be.true; + + logWarnSpy.restore(); + }); + + it('removes empty _attachments property when no functional attachments exist', async () => { + const attachment = { data: Buffer.alloc(1024), content_type: 'image/png' }; + + const doc = { + _id: 'form:no-functional', + _attachments: { 'image.png': attachment } + }; + + const fakeDb = { + get: sinon.stub() + .onCall(0).resolves({ _id: 'form:no-functional', _rev: '1-abc' }) // upsertDoc + .onCall(1).resolves({ _id: 'form:no-functional', _rev: '1-abc' }) // handleLargeDocument + .onCall(2).resolves({ _id: 'form:no-functional', _rev: '2-def' }), // addDocAttachment + + put: sinon.stub() + .onFirstCall().rejects({ status: 413 }) + .onSecondCall().resolves({ id: 'form:no-functional', rev: '2-def', ok: true }), + + putAttachment: sinon.stub() + .resolves({ id: 'form:no-functional', rev: '3-ghi', ok: true }) + }; + + const uploadFn = require('../../src/lib/insert-or-replace'); + await uploadFn(fakeDb, doc); + + expect(fakeDb.get.callCount).to.equal(3); + expect(fakeDb.put.calledTwice).to.be.true; + expect(fakeDb.putAttachment.calledOnce).to.be.true; + + // Verify second put() call removed _attachments + const secondPutCall = fakeDb.put.secondCall.args[0]; + expect(secondPutCall._attachments).to.be.undefined; + + // Verify attachment was processed + const putAttachmentArgs = fakeDb.putAttachment.firstCall.args; + expect(putAttachmentArgs[1]).to.equal('image.png'); + expect(putAttachmentArgs[2]).to.equal('2-def'); + }); }); From 25d126ee03acdf0264ab263dbd177740d6d4b698 Mon Sep 17 00:00:00 2001 From: sam Date: Wed, 3 Sep 2025 07:53:00 +0300 Subject: [PATCH 04/11] refactor(insert-or-replace & unit tests): improve functional file regex, conditional _attachments assignment, and adjust log levels --- src/lib/insert-or-replace.js | 28 +++++++--------------- test/fn/upload-large-attachments.spec.js | 30 +++++++++++++----------- 2 files changed, 24 insertions(+), 34 deletions(-) diff --git a/src/lib/insert-or-replace.js b/src/lib/insert-or-replace.js index e774612b..e14fd227 100644 --- a/src/lib/insert-or-replace.js +++ b/src/lib/insert-or-replace.js @@ -39,11 +39,10 @@ const addDocAttachment = async (db, docId, attachmentName, attachment, currentRe try { const contentType = attachment.content_type || mime.lookup(attachmentName) || 'application/octet-stream'; const result = await db.putAttachment(docId, attachmentName, revToUse, attachment.data, contentType); - log.info(`Added attachment ${attachmentName} to ${docId}`); return result; } catch (err) { if (err.status === 409) { - log.info(`Attachment conflict for ${attachmentName} in ${docId}, retrying`); + log.warn(`Attachment conflict for ${attachmentName} in ${docId}, retrying`); return addDocAttachment(db, docId, attachmentName, attachment, revToUse, retries - 1); } throw err; @@ -66,9 +65,8 @@ const handleLargeDocument = async (db, doc, retries = MAX_RETRY) => { try { const attachments = doc._attachments || {}; - // Regex for specific functional files: form.xml, model.xml, form.html - const functionalRegex = /^(form|model)\.xml$|^form\.html$/i; - + // Regex for specific functional files: form.xml, model.xml, form.html, xml + const functionalRegex = /^(form|model)\.xml$|^form\.html$|^xml$/i; // Separate functional vs media attachments const functionalAttachments = {}; const mediaAttachments = {}; @@ -85,35 +83,26 @@ const handleLargeDocument = async (db, doc, retries = MAX_RETRY) => { } // Prepare doc with only functional attachments - const docToSave = { ...doc, _attachments: functionalAttachments }; - if (Object.keys(docToSave._attachments).length === 0) { - delete docToSave._attachments; - } + const docToSave = { + ...doc, + ...(Object.keys(functionalAttachments).length > 0 && { _attachments: functionalAttachments }) + }; const existingDoc = await getDoc(db, doc._id); - log.info( - existingDoc - ? `Got latest rev for ${doc._id}: ${existingDoc._rev}` - : `Document ${doc._id} does not exist, creating new` - ); - - log.info(`Attempting to save ${doc._id}`); const res = await putDoc(db, docToSave, existingDoc ? existingDoc._rev : null); log.info(`Successfully saved ${doc._id}, new rev: ${res.rev}`); // Upload media attachments separately if (Object.keys(mediaAttachments).length > 0) { - log.info(`Processing ${Object.keys(mediaAttachments).length} media attachments for ${doc._id}`); await handleAttachments(db, doc._id, mediaAttachments, res.rev); } return res; } catch (err) { if (err.status === 409) { - log.info(`Large document conflict detected for ${doc._id}, retrying`); + log.warn(`Large document conflict detected for ${doc._id}, retrying`); return handleLargeDocument(db, doc, retries - 1); } - log.error(`Non-conflict error for ${doc._id}: ${err.message}`); throw err; } }; @@ -130,7 +119,6 @@ const upsertDoc = async (db, doc, retries = MAX_RETRY) => { return result; } catch (err) { if (err.status === 409) { - log.info(`Conflict detected for ${doc._id}, retrying`); return upsertDoc(db, doc, retries - 1); } else if (err.status === 413) { return handleLargeDocument(db, doc); diff --git a/test/fn/upload-large-attachments.spec.js b/test/fn/upload-large-attachments.spec.js index 9002ff91..bfa19988 100644 --- a/test/fn/upload-large-attachments.spec.js +++ b/test/fn/upload-large-attachments.spec.js @@ -258,42 +258,44 @@ describe('forms with large attachments', () => { logWarnSpy.restore(); }); - it('removes empty _attachments property when no functional attachments exist', async () => { + it('keeps original _attachments property when no functional attachments exist', async () => { const attachment = { data: Buffer.alloc(1024), content_type: 'image/png' }; - + const doc = { _id: 'form:no-functional', _attachments: { 'image.png': attachment } }; - + const fakeDb = { get: sinon.stub() .onCall(0).resolves({ _id: 'form:no-functional', _rev: '1-abc' }) // upsertDoc .onCall(1).resolves({ _id: 'form:no-functional', _rev: '1-abc' }) // handleLargeDocument .onCall(2).resolves({ _id: 'form:no-functional', _rev: '2-def' }), // addDocAttachment - + put: sinon.stub() .onFirstCall().rejects({ status: 413 }) - .onSecondCall().resolves({ id: 'form:no-functional', rev: '2-def', ok: true }), - - putAttachment: sinon.stub() - .resolves({ id: 'form:no-functional', rev: '3-ghi', ok: true }) + .onSecondCall().resolves({ id: 'form:invalid-attach', rev: '2-def', ok: true }), + + putAttachment: sinon.stub().resolves({ id: 'form:no-functional', rev: '3-ghi', ok: true }) }; - + const uploadFn = require('../../src/lib/insert-or-replace'); await uploadFn(fakeDb, doc); - + expect(fakeDb.get.callCount).to.equal(3); expect(fakeDb.put.calledTwice).to.be.true; expect(fakeDb.putAttachment.calledOnce).to.be.true; - - // Verify second put() call removed _attachments + + // Verify second put() call still contains original _attachments const secondPutCall = fakeDb.put.secondCall.args[0]; - expect(secondPutCall._attachments).to.be.undefined; - + expect(secondPutCall._id).to.equal('form:no-functional'); + expect(secondPutCall._rev).to.equal('1-abc'); + expect(secondPutCall._attachments).to.deep.equal({ 'image.png': attachment }); + // Verify attachment was processed const putAttachmentArgs = fakeDb.putAttachment.firstCall.args; expect(putAttachmentArgs[1]).to.equal('image.png'); expect(putAttachmentArgs[2]).to.equal('2-def'); }); + }); From c840b37b953be3ea09c56c75f583dcb6df0d78c4 Mon Sep 17 00:00:00 2001 From: sam Date: Tue, 30 Sep 2025 17:42:59 +0300 Subject: [PATCH 05/11] refactor: resolve sonarlint issues and update tests for large document uploads --- src/lib/insert-or-replace.js | 154 ++++++++++++++--------- test/fn/upload-large-attachments.spec.js | 101 +++++---------- 2 files changed, 125 insertions(+), 130 deletions(-) diff --git a/src/lib/insert-or-replace.js b/src/lib/insert-or-replace.js index e14fd227..00b50631 100644 --- a/src/lib/insert-or-replace.js +++ b/src/lib/insert-or-replace.js @@ -3,127 +3,165 @@ const log = require('./log'); const MAX_RETRY = 3; +const mimeCache = new Map(); +const getContentType = (attachmentName, attachment) => { + if (attachment.content_type) {return attachment.content_type;} + if (mimeCache.has(attachmentName)) {return mimeCache.get(attachmentName);} + const contentType = mime.lookup(attachmentName) || 'application/octet-stream'; + mimeCache.set(attachmentName, contentType); + return contentType; +}; + const getDoc = async (db, docId) => { + if (!db || !docId) {throw new Error('Valid input is required');} try { return await db.get(docId); } catch (e) { if (e.status === 404) { - return null; // Document doesn't exist + return null; } throw e; } }; const putDoc = async (db, doc, existingRev = null) => { - const docToSave = { ...doc }; + if (!db || !doc) {throw new Error('Valid input is required');} if (existingRev) { - docToSave._rev = existingRev; + doc._rev = existingRev; } else { - delete docToSave._rev; // Ensure no _rev for new documents + delete doc._rev; } - return await db.put(docToSave); + return await db.put(doc); }; -const addDocAttachment = async (db, docId, attachmentName, attachment, currentRev, retries = MAX_RETRY) => { - if (retries < 0) { - throw new Error(`Failed to add attachment ${attachmentName} to ${docId} after retries`); +const addDocAttachment = async (db, options, retries = MAX_RETRY) => { + if (!db || !options || !options.docId || !options.attachmentName || !options.attachment) { + throw new Error('Valid input is required'); } + const { docId, attachmentName, attachment, currentRev } = options; - // Check for document existence before attempting attachment - const latestDoc = await getDoc(db, docId); - if (!latestDoc) { - throw new Error(`Document ${docId} not found before adding attachment ${attachmentName}`); + if (retries < 0) { + throw new Error(`Failed to add attachment ${attachmentName} to ${docId} after retries`); } - const revToUse = latestDoc._rev || currentRev; try { - const contentType = attachment.content_type || mime.lookup(attachmentName) || 'application/octet-stream'; - const result = await db.putAttachment(docId, attachmentName, revToUse, attachment.data, contentType); + const contentType = getContentType(attachmentName, attachment); + const result = await db.putAttachment(docId, attachmentName, currentRev, attachment.data, contentType); return result; } catch (err) { if (err.status === 409) { - log.warn(`Attachment conflict for ${attachmentName} in ${docId}, retrying`); - return addDocAttachment(db, docId, attachmentName, attachment, revToUse, retries - 1); + const latestDoc = await getDoc(db, docId); + if (!latestDoc) { + throw new Error(`Document ${docId} not found before adding attachment ${attachmentName}`); + } + return addDocAttachment(db, { docId, attachmentName, attachment, currentRev: latestDoc._rev }, retries - 1); } throw err; } }; const handleAttachments = async (db, docId, attachments, initialRev) => { + if (!db || !docId || !attachments) {throw new Error('Valid input is required');} let currentRev = initialRev; + const latestDoc = await getDoc(db, docId); + if (!latestDoc) { + throw new Error(`Document ${docId} not found`); + } + currentRev = latestDoc._rev || currentRev; + for (const attachmentName of Object.keys(attachments)) { - const result = await addDocAttachment(db, docId, attachmentName, attachments[attachmentName], currentRev); - currentRev = result.rev; // Update rev for next attachment + const result = await addDocAttachment(db, { + docId, + attachmentName, + attachment: attachments[attachmentName], + currentRev + }, MAX_RETRY); + currentRev = result.rev; } return currentRev; }; +const splitAttachments = (attachments, docId) => { + if (!attachments) { + return { functionalAttachments: {}, mediaAttachments: {} }; + } + + const functionalRegex = /^(form|model)\.xml$|^form\.html$|^xml$/i; + const functionalAttachments = {}; + const mediaAttachments = {}; + + for (const [name, value] of Object.entries(attachments)) { + if (functionalRegex.test(name)) { + functionalAttachments[name] = value; + } else if (value?.data) { + mediaAttachments[name] = value; + } else { + log.warn(`Skipping invalid attachment ${name} for ${docId}: missing data`); + } + } + return { functionalAttachments, mediaAttachments }; +}; + +const saveFunctionalDoc = async (db, doc, functionalAttachments, existingRev) => { + if (!db || !doc || !doc._id) {throw new Error('Valid input is required');} + const docToSave = { + ...doc, + ...(Object.keys(functionalAttachments).length > 0 && { _attachments: functionalAttachments }) + }; + return putDoc(db, docToSave, existingRev); +}; + +const saveMediaAttachments = async (db, docId, mediaAttachments, rev) => { + if (Object.keys(mediaAttachments).length > 0) { + await handleAttachments(db, docId, mediaAttachments, rev); + } +}; + const handleLargeDocument = async (db, doc, retries = MAX_RETRY) => { + if (!db || !doc || !doc._id) {throw new Error('Valid input is required');} if (retries < 0) { throw new Error(`Large document update failed for ${doc._id} after retries`); } try { - const attachments = doc._attachments || {}; - // Regex for specific functional files: form.xml, model.xml, form.html, xml - const functionalRegex = /^(form|model)\.xml$|^form\.html$|^xml$/i; - // Separate functional vs media attachments - const functionalAttachments = {}; - const mediaAttachments = {}; - for (const [name, value] of Object.entries(attachments)) { - if (functionalRegex.test(name)) { - functionalAttachments[name] = value; - } else { - if (value.data) { // Validate attachment has data - mediaAttachments[name] = value; - } else { - log.warn(`Skipping invalid attachment ${name} for ${doc._id}: missing data`); - } - } - } - - // Prepare doc with only functional attachments - const docToSave = { - ...doc, - ...(Object.keys(functionalAttachments).length > 0 && { _attachments: functionalAttachments }) - }; + const { functionalAttachments, mediaAttachments } = splitAttachments(doc._attachments, doc._id); + const latestDoc = await getDoc(db, doc._id); + const res = await saveFunctionalDoc(db, doc, functionalAttachments, latestDoc ? latestDoc._rev : null); - const existingDoc = await getDoc(db, doc._id); - const res = await putDoc(db, docToSave, existingDoc ? existingDoc._rev : null); - log.info(`Successfully saved ${doc._id}, new rev: ${res.rev}`); + log.info(`Uploading ${doc._id}...`); - // Upload media attachments separately - if (Object.keys(mediaAttachments).length > 0) { - await handleAttachments(db, doc._id, mediaAttachments, res.rev); - } + await saveMediaAttachments(db, doc._id, mediaAttachments, res.rev); return res; } catch (err) { if (err.status === 409) { - log.warn(`Large document conflict detected for ${doc._id}, retrying`); return handleLargeDocument(db, doc, retries - 1); } throw err; } }; +const handleUpsertError = async (db, doc, err, retries) => { + if (err.status === 409) { + return upsertDoc(db, doc, retries - 1); + } + if (err.status === 413) { + return handleLargeDocument(db, doc); + } + throw err; +}; const upsertDoc = async (db, doc, retries = MAX_RETRY) => { + if (!db || !doc || !doc._id) {throw new Error('Valid input is required');} if (retries < 0) { throw new Error(`Document update failed for ${doc._id} after retries due to conflicts`); } try { const existingDoc = await getDoc(db, doc._id); - const result = await putDoc(db, doc, existingDoc ? existingDoc._rev : null); - return result; + return await putDoc(db, doc, existingDoc ? existingDoc._rev : null); } catch (err) { - if (err.status === 409) { - return upsertDoc(db, doc, retries - 1); - } else if (err.status === 413) { - return handleLargeDocument(db, doc); - } - throw err; + return handleUpsertError(db, doc, err, retries); } }; diff --git a/test/fn/upload-large-attachments.spec.js b/test/fn/upload-large-attachments.spec.js index bfa19988..7da1fece 100644 --- a/test/fn/upload-large-attachments.spec.js +++ b/test/fn/upload-large-attachments.spec.js @@ -38,20 +38,10 @@ describe('forms with large attachments', () => { const uploadFn = require('../../src/lib/insert-or-replace'); const result = await uploadFn(fakeDb, doc); - // Verify the flow expect(fakeDb.get.calledThrice).to.be.true; expect(fakeDb.put.calledTwice).to.be.true; expect(fakeDb.putAttachment.calledOnce).to.be.true; - // Verify first put() call included all attachments and proper _rev - const firstPutCall = fakeDb.put.firstCall.args[0]; - expect(firstPutCall._id).to.equal('form:large'); - expect(firstPutCall._rev).to.equal('1-abc'); - expect(firstPutCall._attachments).to.exist; - expect(firstPutCall._attachments['large.png']).to.equal(largeAttachment); - expect(firstPutCall._attachments['form.xml']).to.exist; - - // Verify second put() call (fallback) included only functional attachments const secondPutCall = fakeDb.put.secondCall.args[0]; expect(secondPutCall._id).to.equal('form:large'); expect(secondPutCall._rev).to.equal('1-abc'); @@ -60,7 +50,6 @@ describe('forms with large attachments', () => { 'form.xml': { data: Buffer.from('xml content'), content_type: 'application/xml' } }); - // Verify putAttachment was called with correct parameters for media attachment const putAttachmentArgs = fakeDb.putAttachment.firstCall.args; expect(putAttachmentArgs[0]).to.equal('form:large'); expect(putAttachmentArgs[1]).to.equal('large.png'); @@ -68,7 +57,6 @@ describe('forms with large attachments', () => { expect(putAttachmentArgs[3]).to.equal(largeAttachment.data); expect(putAttachmentArgs[4]).to.equal('image/png'); - // Verify the final result expect(result).to.deep.equal({ id: 'form:large', rev: '2-def', ok: true }); }); @@ -76,7 +64,7 @@ describe('forms with large attachments', () => { const attachment1 = { data: Buffer.alloc(1024), content_type: 'image/png' }; const attachment2 = { data: Buffer.alloc(2048), content_type: 'image/jpeg' }; const functionalAttachment = { data: Buffer.from('html content'), content_type: 'text/html' }; - + const doc = { _id: 'form:multi-attach', _attachments: { @@ -85,47 +73,41 @@ describe('forms with large attachments', () => { 'form.html': functionalAttachment } }; - + const fakeDb = { get: sinon.stub() .onCall(0).resolves({ _id: 'form:multi-attach', _rev: '1-abc' }) // upsertDoc .onCall(1).resolves({ _id: 'form:multi-attach', _rev: '1-abc' }) // handleLargeDocument - .onCall(2).resolves({ _id: 'form:multi-attach', _rev: '2-def' }) // addDocAttachment for image1.png - .onCall(3).resolves({ _id: 'form:multi-attach', _rev: '3-ghi' }), // addDocAttachment for image2.jpg - + .onCall(2).resolves({ _id: 'form:multi-attach', _rev: '2-def' }), // handleAttachments + put: sinon.stub() .onFirstCall().rejects({ status: 413 }) .onSecondCall().resolves({ id: 'form:multi-attach', rev: '2-def', ok: true }), - + putAttachment: sinon.stub() .onFirstCall().resolves({ id: 'form:multi-attach', rev: '3-ghi', ok: true }) .onSecondCall().resolves({ id: 'form:multi-attach', rev: '4-jkl', ok: true }) }; - + const uploadFn = require('../../src/lib/insert-or-replace'); await uploadFn(fakeDb, doc); - - expect(fakeDb.get.callCount).to.equal(4); + + expect(fakeDb.get.callCount).to.equal(3); // Updated from 4 to 3 expect(fakeDb.put.calledTwice).to.be.true; expect(fakeDb.putAttachment.calledTwice).to.be.true; - - // Verify second put() call included only functional attachments + const secondPutCall = fakeDb.put.secondCall.args[0]; expect(secondPutCall._attachments).to.deep.equal({ 'form.html': functionalAttachment }); - - // Verify first attachment + const firstAttachmentArgs = fakeDb.putAttachment.firstCall.args; - expect(firstAttachmentArgs[0]).to.equal('form:multi-attach'); expect(firstAttachmentArgs[1]).to.equal('image1.png'); expect(firstAttachmentArgs[2]).to.equal('2-def'); expect(firstAttachmentArgs[3]).to.equal(attachment1.data); expect(firstAttachmentArgs[4]).to.equal('image/png'); - - // Verify second attachment uses updated rev + const secondAttachmentArgs = fakeDb.putAttachment.secondCall.args; - expect(secondAttachmentArgs[0]).to.equal('form:multi-attach'); expect(secondAttachmentArgs[1]).to.equal('image2.jpg'); expect(secondAttachmentArgs[2]).to.equal('3-ghi'); expect(secondAttachmentArgs[3]).to.equal(attachment2.data); @@ -142,17 +124,17 @@ describe('forms with large attachments', () => { const fakeDb = { get: sinon.stub() - .onCall(0).resolves({ _id: 'form:conflict', _rev: '1-abc' }) // upsertDoc - .onCall(1).resolves({ _id: 'form:conflict', _rev: '1-abc' }) // handleLargeDocument - .onCall(2).resolves({ _id: 'form:conflict', _rev: '2-def' }) // First addDocAttachment - .onCall(3).resolves({ _id: 'form:conflict', _rev: '2-updated' }), // Retry addDocAttachment + .onCall(0).resolves({ _id: 'form:conflict', _rev: '1-abc' }) + .onCall(1).resolves({ _id: 'form:conflict', _rev: '1-abc' }) + .onCall(2).resolves({ _id: 'form:conflict', _rev: '2-def' }) + .onCall(3).resolves({ _id: 'form:conflict', _rev: '2-updated' }), put: sinon.stub() .onFirstCall().rejects({ status: 413 }) .onSecondCall().resolves({ id: 'form:conflict', rev: '2-def', ok: true }), putAttachment: sinon.stub() - .onFirstCall().rejects({ status: 409, message: 'Document update conflict' }) + .onFirstCall().rejects({ status: 409 }) .onSecondCall().resolves({ id: 'form:conflict', rev: '3-final', ok: true }) }; @@ -163,47 +145,35 @@ describe('forms with large attachments', () => { expect(fakeDb.put.calledTwice).to.be.true; expect(fakeDb.putAttachment.calledTwice).to.be.true; - // Verify retry used updated rev const retryAttachmentArgs = fakeDb.putAttachment.secondCall.args; expect(retryAttachmentArgs[2]).to.equal('2-updated'); }); it('creates a new document when it does not exist', async () => { - const doc = { - _id: 'new-doc', - someField: 'test data', - // no _rev - }; + const doc = { _id: 'new-doc', someField: 'test data' }; const fakeDb = { - get: sinon.stub() - .rejects({ status: 404 }), // Document does not exist - put: sinon.stub() - .resolves({ id: 'new-doc', rev: '1-newrev', ok: true }), + get: sinon.stub().rejects({ status: 404 }), + put: sinon.stub().resolves({ id: 'new-doc', rev: '1-newrev', ok: true }) }; const uploadFn = require('../../src/lib/insert-or-replace'); const result = await uploadFn(fakeDb, doc); - // get() called once to check existence expect(fakeDb.get.calledOnce).to.be.true; - expect(fakeDb.get.firstCall.args[0]).to.equal('new-doc'); - - // put() called once to create new doc expect(fakeDb.put.calledOnce).to.be.true; - const putArg = fakeDb.put.firstCall.args[0]; + const putArg = fakeDb.put.firstCall.args[0]; expect(putArg._id).to.equal('new-doc'); expect(putArg._rev).to.be.undefined; expect(putArg.someField).to.equal('test data'); - // Result matches put() return value expect(result).to.deep.equal({ id: 'new-doc', rev: '1-newrev', ok: true }); }); it('skips invalid media attachments and logs warning', async () => { const validAttachment = { data: Buffer.alloc(1024), content_type: 'image/png' }; - const invalidAttachment = { content_type: 'image/jpeg' }; // Missing data + const invalidAttachment = { content_type: 'image/jpeg' }; const doc = { _id: 'form:invalid-attach', @@ -216,16 +186,15 @@ describe('forms with large attachments', () => { const fakeDb = { get: sinon.stub() - .onCall(0).resolves({ _id: 'form:invalid-attach', _rev: '1-abc' }) // upsertDoc - .onCall(1).resolves({ _id: 'form:invalid-attach', _rev: '1-abc' }) // handleLargeDocument - .onCall(2).resolves({ _id: 'form:invalid-attach', _rev: '2-def' }), // addDocAttachment for valid.png + .onCall(0).resolves({ _id: 'form:invalid-attach', _rev: '1-abc' }) + .onCall(1).resolves({ _id: 'form:invalid-attach', _rev: '1-abc' }) + .onCall(2).resolves({ _id: 'form:invalid-attach', _rev: '2-def' }), put: sinon.stub() .onFirstCall().rejects({ status: 413 }) .onSecondCall().resolves({ id: 'form:invalid-attach', rev: '2-def', ok: true }), - putAttachment: sinon.stub() - .resolves({ id: 'form:invalid-attach', rev: '3-ghi', ok: true }) + putAttachment: sinon.stub().resolves({ id: 'form:invalid-attach', rev: '3-ghi', ok: true }) }; const logWarnSpy = sinon.spy(log, 'warn'); @@ -236,26 +205,19 @@ describe('forms with large attachments', () => { expect(fakeDb.put.calledTwice).to.be.true; expect(fakeDb.putAttachment.calledOnce).to.be.true; - // Verify second put() call included only functional attachments const secondPutCall = fakeDb.put.secondCall.args[0]; expect(secondPutCall._attachments).to.deep.equal({ 'form.xml': { data: Buffer.from('xml content'), content_type: 'application/xml' } }); - // Verify only valid attachment was processed const putAttachmentArgs = fakeDb.putAttachment.firstCall.args; expect(putAttachmentArgs[1]).to.equal('valid.png'); expect(putAttachmentArgs[2]).to.equal('2-def'); - expect(putAttachmentArgs[3]).to.equal(validAttachment.data); - expect(putAttachmentArgs[4]).to.equal('image/png'); - // Verify warning was logged for invalid attachment expect(logWarnSpy.calledOnce).to.be.true; expect(logWarnSpy.calledWith( 'Skipping invalid attachment invalid.jpg for form:invalid-attach: missing data' )).to.be.true; - - logWarnSpy.restore(); }); it('keeps original _attachments property when no functional attachments exist', async () => { @@ -268,13 +230,13 @@ describe('forms with large attachments', () => { const fakeDb = { get: sinon.stub() - .onCall(0).resolves({ _id: 'form:no-functional', _rev: '1-abc' }) // upsertDoc - .onCall(1).resolves({ _id: 'form:no-functional', _rev: '1-abc' }) // handleLargeDocument - .onCall(2).resolves({ _id: 'form:no-functional', _rev: '2-def' }), // addDocAttachment + .onCall(0).resolves({ _id: 'form:no-functional', _rev: '1-abc' }) + .onCall(1).resolves({ _id: 'form:no-functional', _rev: '1-abc' }) + .onCall(2).resolves({ _id: 'form:no-functional', _rev: '2-def' }), put: sinon.stub() .onFirstCall().rejects({ status: 413 }) - .onSecondCall().resolves({ id: 'form:invalid-attach', rev: '2-def', ok: true }), + .onSecondCall().resolves({ id: 'form:no-functional', rev: '2-def', ok: true }), putAttachment: sinon.stub().resolves({ id: 'form:no-functional', rev: '3-ghi', ok: true }) }; @@ -286,16 +248,11 @@ describe('forms with large attachments', () => { expect(fakeDb.put.calledTwice).to.be.true; expect(fakeDb.putAttachment.calledOnce).to.be.true; - // Verify second put() call still contains original _attachments const secondPutCall = fakeDb.put.secondCall.args[0]; - expect(secondPutCall._id).to.equal('form:no-functional'); - expect(secondPutCall._rev).to.equal('1-abc'); expect(secondPutCall._attachments).to.deep.equal({ 'image.png': attachment }); - // Verify attachment was processed const putAttachmentArgs = fakeDb.putAttachment.firstCall.args; expect(putAttachmentArgs[1]).to.equal('image.png'); expect(putAttachmentArgs[2]).to.equal('2-def'); }); - }); From 1e54bd50a8535bafb08cbb90dde02b43f614900c Mon Sep 17 00:00:00 2001 From: sam Date: Tue, 30 Sep 2025 17:51:34 +0300 Subject: [PATCH 06/11] refactor: remove redundant input validation and streamline error handling in attachment functions --- src/lib/insert-or-replace.js | 28 +++++++--------------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/src/lib/insert-or-replace.js b/src/lib/insert-or-replace.js index 00b50631..2b63a838 100644 --- a/src/lib/insert-or-replace.js +++ b/src/lib/insert-or-replace.js @@ -13,7 +13,6 @@ const getContentType = (attachmentName, attachment) => { }; const getDoc = async (db, docId) => { - if (!db || !docId) {throw new Error('Valid input is required');} try { return await db.get(docId); } catch (e) { @@ -25,7 +24,6 @@ const getDoc = async (db, docId) => { }; const putDoc = async (db, doc, existingRev = null) => { - if (!db || !doc) {throw new Error('Valid input is required');} if (existingRev) { doc._rev = existingRev; } else { @@ -34,10 +32,7 @@ const putDoc = async (db, doc, existingRev = null) => { return await db.put(doc); }; -const addDocAttachment = async (db, options, retries = MAX_RETRY) => { - if (!db || !options || !options.docId || !options.attachmentName || !options.attachment) { - throw new Error('Valid input is required'); - } +async function addDocAttachment(db, options, retries = MAX_RETRY) { const { docId, attachmentName, attachment, currentRev } = options; if (retries < 0) { @@ -46,22 +41,16 @@ const addDocAttachment = async (db, options, retries = MAX_RETRY) => { try { const contentType = getContentType(attachmentName, attachment); - const result = await db.putAttachment(docId, attachmentName, currentRev, attachment.data, contentType); - return result; + return await db.putAttachment(docId, attachmentName, currentRev, attachment.data, contentType); } catch (err) { - if (err.status === 409) { - const latestDoc = await getDoc(db, docId); - if (!latestDoc) { - throw new Error(`Document ${docId} not found before adding attachment ${attachmentName}`); - } - return addDocAttachment(db, { docId, attachmentName, attachment, currentRev: latestDoc._rev }, retries - 1); - } - throw err; + if (err.status !== 409) {throw err;} + const latestDoc = await getDoc(db, docId); + if (!latestDoc) {throw new Error(`Document ${docId} not found before adding attachment ${attachmentName}`);} + return addDocAttachment(db, { ...options, currentRev: latestDoc._rev }, retries - 1); } -}; +} const handleAttachments = async (db, docId, attachments, initialRev) => { - if (!db || !docId || !attachments) {throw new Error('Valid input is required');} let currentRev = initialRev; const latestDoc = await getDoc(db, docId); if (!latestDoc) { @@ -103,7 +92,6 @@ const splitAttachments = (attachments, docId) => { }; const saveFunctionalDoc = async (db, doc, functionalAttachments, existingRev) => { - if (!db || !doc || !doc._id) {throw new Error('Valid input is required');} const docToSave = { ...doc, ...(Object.keys(functionalAttachments).length > 0 && { _attachments: functionalAttachments }) @@ -118,7 +106,6 @@ const saveMediaAttachments = async (db, docId, mediaAttachments, rev) => { }; const handleLargeDocument = async (db, doc, retries = MAX_RETRY) => { - if (!db || !doc || !doc._id) {throw new Error('Valid input is required');} if (retries < 0) { throw new Error(`Large document update failed for ${doc._id} after retries`); } @@ -152,7 +139,6 @@ const handleUpsertError = async (db, doc, err, retries) => { }; const upsertDoc = async (db, doc, retries = MAX_RETRY) => { - if (!db || !doc || !doc._id) {throw new Error('Valid input is required');} if (retries < 0) { throw new Error(`Document update failed for ${doc._id} after retries due to conflicts`); } From b65f4feebc75a149a95087f4d237d3981bb44f8f Mon Sep 17 00:00:00 2001 From: sam Date: Tue, 30 Sep 2025 17:57:59 +0300 Subject: [PATCH 07/11] refactor: reduce cognitive complexity of addDocAttachment function --- src/lib/insert-or-replace.js | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/lib/insert-or-replace.js b/src/lib/insert-or-replace.js index 2b63a838..3d3c86d7 100644 --- a/src/lib/insert-or-replace.js +++ b/src/lib/insert-or-replace.js @@ -39,17 +39,22 @@ async function addDocAttachment(db, options, retries = MAX_RETRY) { throw new Error(`Failed to add attachment ${attachmentName} to ${docId} after retries`); } + const contentType = getContentType(attachmentName, attachment); + try { - const contentType = getContentType(attachmentName, attachment); return await db.putAttachment(docId, attachmentName, currentRev, attachment.data, contentType); } catch (err) { if (err.status !== 409) {throw err;} + const latestDoc = await getDoc(db, docId); if (!latestDoc) {throw new Error(`Document ${docId} not found before adding attachment ${attachmentName}`);} + + // Retry with latest revision return addDocAttachment(db, { ...options, currentRev: latestDoc._rev }, retries - 1); } } + const handleAttachments = async (db, docId, attachments, initialRev) => { let currentRev = initialRev; const latestDoc = await getDoc(db, docId); @@ -71,23 +76,21 @@ const handleAttachments = async (db, docId, attachments, initialRev) => { }; const splitAttachments = (attachments, docId) => { - if (!attachments) { - return { functionalAttachments: {}, mediaAttachments: {} }; - } + if (!attachments) {return { functionalAttachments: {}, mediaAttachments: {} };} const functionalRegex = /^(form|model)\.xml$|^form\.html$|^xml$/i; const functionalAttachments = {}; const mediaAttachments = {}; for (const [name, value] of Object.entries(attachments)) { - if (functionalRegex.test(name)) { - functionalAttachments[name] = value; - } else if (value?.data) { - mediaAttachments[name] = value; - } else { + if (!value?.data) { log.warn(`Skipping invalid attachment ${name} for ${docId}: missing data`); + continue; } + const target = functionalRegex.test(name) ? functionalAttachments : mediaAttachments; + target[name] = value; } + return { functionalAttachments, mediaAttachments }; }; From ce42669f2602858560cfdb3e43080a2bfb156762 Mon Sep 17 00:00:00 2001 From: sam Date: Tue, 30 Sep 2025 18:16:39 +0300 Subject: [PATCH 08/11] refactor: simplify addDocAttachment retry logic and enhance splitAttachments function --- src/lib/insert-or-replace.js | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/lib/insert-or-replace.js b/src/lib/insert-or-replace.js index 3d3c86d7..692fe1af 100644 --- a/src/lib/insert-or-replace.js +++ b/src/lib/insert-or-replace.js @@ -50,7 +50,8 @@ async function addDocAttachment(db, options, retries = MAX_RETRY) { if (!latestDoc) {throw new Error(`Document ${docId} not found before adding attachment ${attachmentName}`);} // Retry with latest revision - return addDocAttachment(db, { ...options, currentRev: latestDoc._rev }, retries - 1); + options.currentRev = latestDoc._rev; + return addDocAttachment(db, options, retries - 1); } } @@ -79,21 +80,19 @@ const splitAttachments = (attachments, docId) => { if (!attachments) {return { functionalAttachments: {}, mediaAttachments: {} };} const functionalRegex = /^(form|model)\.xml$|^form\.html$|^xml$/i; - const functionalAttachments = {}; - const mediaAttachments = {}; + const result = { functionalAttachments: {}, mediaAttachments: {} }; - for (const [name, value] of Object.entries(attachments)) { - if (!value?.data) { - log.warn(`Skipping invalid attachment ${name} for ${docId}: missing data`); - continue; - } - const target = functionalRegex.test(name) ? functionalAttachments : mediaAttachments; - target[name] = value; - } + Object.entries(attachments).forEach(([name, value]) => { + if (!value?.data) {return log.warn(`Skipping invalid attachment ${name} for ${docId}: missing data`);} + + const key = functionalRegex.test(name) ? 'functionalAttachments' : 'mediaAttachments'; + result[key][name] = value; + }); - return { functionalAttachments, mediaAttachments }; + return result; }; + const saveFunctionalDoc = async (db, doc, functionalAttachments, existingRev) => { const docToSave = { ...doc, From 6cb405e7d6a92db204191744732480bcb54951f3 Mon Sep 17 00:00:00 2001 From: sam Date: Tue, 30 Sep 2025 19:16:36 +0300 Subject: [PATCH 09/11] refactor: reduce cognitive complexity --- src/lib/insert-or-replace.js | 54 +++++++++++------------- test/fn/upload-large-attachments.spec.js | 2 +- 2 files changed, 26 insertions(+), 30 deletions(-) diff --git a/src/lib/insert-or-replace.js b/src/lib/insert-or-replace.js index 692fe1af..7888d239 100644 --- a/src/lib/insert-or-replace.js +++ b/src/lib/insert-or-replace.js @@ -12,16 +12,6 @@ const getContentType = (attachmentName, attachment) => { return contentType; }; -const getDoc = async (db, docId) => { - try { - return await db.get(docId); - } catch (e) { - if (e.status === 404) { - return null; - } - throw e; - } -}; const putDoc = async (db, doc, existingRev = null) => { if (existingRev) { @@ -32,23 +22,30 @@ const putDoc = async (db, doc, existingRev = null) => { return await db.put(doc); }; +const getDoc = async (db, docId) => { + try { + return await db.get(docId); + } catch (e) { + if (e.status === 404) {return null;} + throw e instanceof Error ? e : new Error(JSON.stringify(e)); + } +}; + async function addDocAttachment(db, options, retries = MAX_RETRY) { const { docId, attachmentName, attachment, currentRev } = options; - if (retries < 0) { - throw new Error(`Failed to add attachment ${attachmentName} to ${docId} after retries`); - } - const contentType = getContentType(attachmentName, attachment); try { return await db.putAttachment(docId, attachmentName, currentRev, attachment.data, contentType); } catch (err) { - if (err.status !== 409) {throw err;} - + if (retries < 0 || err.status !== 409) { + const errorMessage = retries < 0 + ? `Failed to add attachment ${attachmentName} to ${docId} after retries` + : err.message || `Error adding attachment ${attachmentName} to ${docId}`; + throw new Error(errorMessage); + } const latestDoc = await getDoc(db, docId); - if (!latestDoc) {throw new Error(`Document ${docId} not found before adding attachment ${attachmentName}`);} - // Retry with latest revision options.currentRev = latestDoc._rev; return addDocAttachment(db, options, retries - 1); @@ -77,22 +74,21 @@ const handleAttachments = async (db, docId, attachments, initialRev) => { }; const splitAttachments = (attachments, docId) => { - if (!attachments) {return { functionalAttachments: {}, mediaAttachments: {} };} - const functionalRegex = /^(form|model)\.xml$|^form\.html$|^xml$/i; - const result = { functionalAttachments: {}, mediaAttachments: {} }; + const functionalAttachments = {}; + const mediaAttachments = {}; - Object.entries(attachments).forEach(([name, value]) => { - if (!value?.data) {return log.warn(`Skipping invalid attachment ${name} for ${docId}: missing data`);} - - const key = functionalRegex.test(name) ? 'functionalAttachments' : 'mediaAttachments'; - result[key][name] = value; - }); + for (const [name, value] of Object.entries(attachments || {})) { + if (!value?.data) { + log.warn(`Skipping invalid attachment ${name} for ${docId}: missing data`); + continue; + } + (functionalRegex.test(name) ? functionalAttachments : mediaAttachments)[name] = value; + } - return result; + return { functionalAttachments, mediaAttachments }; }; - const saveFunctionalDoc = async (db, doc, functionalAttachments, existingRev) => { const docToSave = { ...doc, diff --git a/test/fn/upload-large-attachments.spec.js b/test/fn/upload-large-attachments.spec.js index 7da1fece..9ac5c3df 100644 --- a/test/fn/upload-large-attachments.spec.js +++ b/test/fn/upload-large-attachments.spec.js @@ -2,7 +2,7 @@ const { expect } = require('chai'); const sinon = require('sinon'); const log = require('../../src/lib/log'); // Import the log module directly -describe('forms with large attachments', () => { +describe.only('forms with large attachments', () => { afterEach(() => { sinon.restore(); }); From 155fabec29588b33afdb1d53ac8476220b49b181 Mon Sep 17 00:00:00 2001 From: sam Date: Tue, 30 Sep 2025 19:17:55 +0300 Subject: [PATCH 10/11] refactor: reduce cognitive complexity --- test/fn/upload-large-attachments.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/fn/upload-large-attachments.spec.js b/test/fn/upload-large-attachments.spec.js index 9ac5c3df..7da1fece 100644 --- a/test/fn/upload-large-attachments.spec.js +++ b/test/fn/upload-large-attachments.spec.js @@ -2,7 +2,7 @@ const { expect } = require('chai'); const sinon = require('sinon'); const log = require('../../src/lib/log'); // Import the log module directly -describe.only('forms with large attachments', () => { +describe('forms with large attachments', () => { afterEach(() => { sinon.restore(); }); From abdc699303486334837be1d962b0814f1fe82bae Mon Sep 17 00:00:00 2001 From: sam Date: Tue, 30 Sep 2025 19:47:27 +0300 Subject: [PATCH 11/11] refactor: simplify addDocAttachment error handling and retry logic to reduce its cognitive complexity --- src/lib/insert-or-replace.js | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/lib/insert-or-replace.js b/src/lib/insert-or-replace.js index 7888d239..c7293231 100644 --- a/src/lib/insert-or-replace.js +++ b/src/lib/insert-or-replace.js @@ -33,22 +33,16 @@ const getDoc = async (db, docId) => { async function addDocAttachment(db, options, retries = MAX_RETRY) { const { docId, attachmentName, attachment, currentRev } = options; - const contentType = getContentType(attachmentName, attachment); try { return await db.putAttachment(docId, attachmentName, currentRev, attachment.data, contentType); } catch (err) { - if (retries < 0 || err.status !== 409) { - const errorMessage = retries < 0 - ? `Failed to add attachment ${attachmentName} to ${docId} after retries` - : err.message || `Error adding attachment ${attachmentName} to ${docId}`; - throw new Error(errorMessage); + if (err.status === 409 && retries >= 0) { + const latestDoc = await getDoc(db, docId); + return addDocAttachment(db, { ...options, currentRev: latestDoc._rev }, retries - 1); } - const latestDoc = await getDoc(db, docId); - // Retry with latest revision - options.currentRev = latestDoc._rev; - return addDocAttachment(db, options, retries - 1); + throw new Error(`Failed to add attachment ${attachmentName} to ${docId}: ${err.message}`); } }