From 4978831bdd21a972abdde42a67c9c5ca7a621fdf Mon Sep 17 00:00:00 2001 From: Crash-- Date: Mon, 6 Apr 2026 13:33:24 +0700 Subject: [PATCH 1/6] test: add failing tests for file.name identity collision in upload queue The reducer matches queue items by file.name, so two files with the same name (e.g. from different sub-folders) collide: an action targeting one updates both. These 3 tests demonstrate the bug. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/modules/upload/index.spec.js | 74 ++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/src/modules/upload/index.spec.js b/src/modules/upload/index.spec.js index a6348fb0f5..f3401acb94 100644 --- a/src/modules/upload/index.spec.js +++ b/src/modules/upload/index.spec.js @@ -503,6 +503,80 @@ describe('queue reducer', () => { expect(result).toEqual(expected) }) + it('should only update the targeted item when files share the same name', () => { + const stateWithDuplicateNames = [ + { + status: 'pending', + file: { name: 'photo.jpg' }, + progress: null + }, + { + status: 'pending', + file: { name: 'photo.jpg' }, + progress: null + } + ] + const action = { + type: 'UPLOAD_FILE', + file: { name: 'photo.jpg' } + } + const result = queue(stateWithDuplicateNames, action) + + // Bug: both items get updated because the reducer matches on file.name + // Only the first should transition to 'loading' + expect(result[0].status).toBe('loading') + expect(result[1].status).toBe('pending') + }) + + it('should correctly track success for files with duplicate names', () => { + const stateWithDuplicateNames = [ + { + status: 'loading', + file: { name: 'photo.jpg' }, + progress: null + }, + { + status: 'pending', + file: { name: 'photo.jpg' }, + progress: null + } + ] + const action = { + type: 'RECEIVE_UPLOAD_SUCCESS', + file: { name: 'photo.jpg' } + } + const result = queue(stateWithDuplicateNames, action) + + // Bug: both items get updated because the reducer matches on file.name + expect(result[0].status).toBe('created') + expect(result[1].status).toBe('pending') + }) + + it('should correctly track errors for files with duplicate names', () => { + const stateWithDuplicateNames = [ + { + status: 'loading', + file: { name: 'photo.jpg' }, + progress: null + }, + { + status: 'loading', + file: { name: 'photo.jpg' }, + progress: null + } + ] + const action = { + type: 'RECEIVE_UPLOAD_ERROR', + file: { name: 'photo.jpg' }, + status: 'failed' + } + const result = queue(stateWithDuplicateNames, action) + + // Bug: both items get updated because the reducer matches on file.name + expect(result[0].status).toBe('failed') + expect(result[1].status).toBe('loading') + }) + describe('progress action', () => { const file = { name: 'doc1.odt' From 9192bba55d05fc7b67c52c0521123290d1b98a3b Mon Sep 17 00:00:00 2001 From: Crash-- Date: Mon, 6 Apr 2026 13:34:38 +0700 Subject: [PATCH 2/6] fix: use fileId instead of file.name for upload queue identity The reducer matched queue items by file.name, causing two files with the same name (e.g. from different sub-folders) to collide. Switching to a dedicated fileId field (defaults to file.name for backward compat) fixes this and prepares for folder upload flattening. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/modules/upload/index.js | 24 ++++++++------ src/modules/upload/index.spec.js | 57 ++++++++++++++++++++++++++------ 2 files changed, 61 insertions(+), 20 deletions(-) diff --git a/src/modules/upload/index.js b/src/modules/upload/index.js index 966f97dd6b..e390eb7a73 100644 --- a/src/modules/upload/index.js +++ b/src/modules/upload/index.js @@ -55,6 +55,7 @@ const CONFLICT_ERROR = 409 const itemInitialState = item => ({ ...item, + fileId: item.fileId ?? item.file.name, status: PENDING, progress: null }) @@ -157,7 +158,7 @@ export const queue = (state = [], action) => { case RECEIVE_UPLOAD_ERROR: case UPLOAD_PROGRESS: return state.map(i => - i.file.name !== action.file.name ? i : item(i, action) + i.fileId !== action.fileId ? i : item(i, action) ) default: return state @@ -168,8 +169,9 @@ export default combineReducers({ queue }) -export const uploadProgress = (file, event, date) => ({ +export const uploadProgress = (fileId, file, event, date) => ({ type: UPLOAD_PROGRESS, + fileId, file, loaded: event.loaded, total: event.total, @@ -202,12 +204,12 @@ export const processNextFile = return dispatch(onQueueEmpty(queueCompletedCallback)) } - const { file, entry, isDirectory } = item + const { file, fileId, entry, isDirectory } = item const encryptionKey = flag('drive.enable-encryption') ? await getEncryptionKeyFromDirId(client, dirID) : null try { - dispatch({ type: UPLOAD_FILE, file }) + dispatch({ type: UPLOAD_FILE, fileId, file }) if (entry && isDirectory) { const newDir = await uploadDirectory( client, @@ -220,11 +222,11 @@ export const processNextFile = driveId ) safeCallback(newDir) - dispatch({ type: RECEIVE_UPLOAD_SUCCESS, file, uploadedItem: newDir }) + dispatch({ type: RECEIVE_UPLOAD_SUCCESS, fileId, file, uploadedItem: newDir }) } else { const withProgress = { onUploadProgress: event => { - dispatch(uploadProgress(file, event)) + dispatch(uploadProgress(fileId, file, event)) } } @@ -242,6 +244,7 @@ export const processNextFile = safeCallback(uploadedFile) dispatch({ type: RECEIVE_UPLOAD_SUCCESS, + fileId, file, uploadedItem: uploadedFile }) @@ -260,7 +263,7 @@ export const processNextFile = path, { onUploadProgress: event => { - dispatch(uploadProgress(file, event)) + dispatch(uploadProgress(fileId, file, event)) } }, driveId @@ -268,6 +271,7 @@ export const processNextFile = safeCallback(uploadedFile) dispatch({ type: RECEIVE_UPLOAD_SUCCESS, + fileId, file, isUpdate: true, uploadedItem: uploadedFile @@ -301,7 +305,7 @@ export const processNextFile = } // Dispatch an action to handle the upload error with the determined status - dispatch({ type: RECEIVE_UPLOAD_ERROR, file, status }) + dispatch({ type: RECEIVE_UPLOAD_ERROR, fileId, file, status }) } } dispatch( @@ -478,8 +482,8 @@ export const overwriteFile = async ( return resp.data } -export const removeFileToUploadQueue = file => async dispatch => { - dispatch({ type: RECEIVE_UPLOAD_SUCCESS, file, isUpdate: true }) +export const removeFileToUploadQueue = (file, fileId) => async dispatch => { + dispatch({ type: RECEIVE_UPLOAD_SUCCESS, fileId: fileId ?? file.name, file, isUpdate: true }) } export const addToUploadQueue = diff --git a/src/modules/upload/index.spec.js b/src/modules/upload/index.spec.js index f3401acb94..7f0520be8d 100644 --- a/src/modules/upload/index.spec.js +++ b/src/modules/upload/index.spec.js @@ -84,6 +84,7 @@ describe('processNextFile function', () => { upload: { queue: [ { + fileId: 'my-doc.odt', status: 'pending', file, entry: '', @@ -107,6 +108,7 @@ describe('processNextFile function', () => { await asyncProcess(dispatchSpy, getState) expect(dispatchSpy).toHaveBeenCalledWith({ type: 'UPLOAD_FILE', + fileId: 'my-doc.odt', file }) expect(createFileSpy).toHaveBeenCalledWith(file, { @@ -120,6 +122,7 @@ describe('processNextFile function', () => { upload: { queue: [ { + fileId: 'my-doc.odt', status: 'pending', file, entry: '', @@ -155,6 +158,7 @@ describe('processNextFile function', () => { expect(dispatchSpy).toHaveBeenNthCalledWith(1, { type: 'UPLOAD_FILE', + fileId: 'my-doc.odt', file }) expect(createFileSpy).toHaveBeenCalledWith(file, { @@ -174,6 +178,7 @@ describe('processNextFile function', () => { expect(dispatchSpy).toHaveBeenNthCalledWith(2, { type: 'RECEIVE_UPLOAD_SUCCESS', + fileId: 'my-doc.odt', file, isUpdate: true, uploadedItem: file @@ -186,6 +191,7 @@ describe('processNextFile function', () => { upload: { queue: [ { + fileId: 'my-doc.odt', status: 'pending', file, entry: '', @@ -221,6 +227,7 @@ describe('processNextFile function', () => { expect(fileUploadedCallbackSpy).not.toHaveBeenCalled() expect(dispatchSpy).toHaveBeenNthCalledWith(2, { + fileId: 'my-doc.odt', file, status: 'quota', type: 'RECEIVE_UPLOAD_ERROR' @@ -233,6 +240,7 @@ describe('processNextFile function', () => { upload: { queue: [ { + fileId: 'my-doc.odt', status: 'pending', file, entry: '', @@ -260,6 +268,7 @@ describe('processNextFile function', () => { expect(fileUploadedCallbackSpy).not.toHaveBeenCalled() expect(dispatchSpy).toHaveBeenNthCalledWith(2, { + fileId: 'my-doc.odt', file, status: 'quota', type: 'RECEIVE_UPLOAD_ERROR' @@ -328,6 +337,7 @@ describe('selectors', () => { describe('queue reducer', () => { const state = [ { + fileId: 'doc1.odt', status: 'pending', file: { name: 'doc1.odt' @@ -335,6 +345,7 @@ describe('queue reducer', () => { progress: null }, { + fileId: 'doc2.odt', status: 'pending', file: { name: 'doc2.odt' @@ -342,6 +353,7 @@ describe('queue reducer', () => { progress: null }, { + fileId: 'doc3.odt', status: 'pending', file: { name: 'doc3.odt' @@ -366,12 +378,14 @@ describe('queue reducer', () => { it('should handle UPLOAD_FILE action type', () => { const action = { type: 'UPLOAD_FILE', + fileId: 'doc1.odt', file: { name: 'doc1.odt' } } const expected = [ { + fileId: 'doc1.odt', status: 'loading', file: { name: 'doc1.odt' @@ -379,6 +393,7 @@ describe('queue reducer', () => { progress: null }, { + fileId: 'doc2.odt', status: 'pending', file: { name: 'doc2.odt' @@ -386,6 +401,7 @@ describe('queue reducer', () => { progress: null }, { + fileId: 'doc3.odt', status: 'pending', file: { name: 'doc3.odt' @@ -400,6 +416,7 @@ describe('queue reducer', () => { it('should handle RECEIVE_UPLOAD_SUCCESS action type', () => { const action = { type: 'RECEIVE_UPLOAD_SUCCESS', + fileId: 'doc3.odt', file: { name: 'doc3.odt' }, @@ -407,6 +424,7 @@ describe('queue reducer', () => { } const expected = [ { + fileId: 'doc1.odt', status: 'pending', file: { name: 'doc1.odt' @@ -414,6 +432,7 @@ describe('queue reducer', () => { progress: null }, { + fileId: 'doc2.odt', status: 'pending', file: { name: 'doc2.odt' @@ -421,6 +440,7 @@ describe('queue reducer', () => { progress: null }, { + fileId: 'doc3.odt', status: 'created', file: { name: 'doc3.odt' @@ -435,6 +455,7 @@ describe('queue reducer', () => { it('should handle RECEIVE_UPLOAD_SUCCESS action type (update)', () => { const action = { type: 'RECEIVE_UPLOAD_SUCCESS', + fileId: 'doc3.odt', file: { name: 'doc3.odt' }, @@ -442,6 +463,7 @@ describe('queue reducer', () => { } const expected = [ { + fileId: 'doc1.odt', status: 'pending', file: { name: 'doc1.odt' @@ -449,6 +471,7 @@ describe('queue reducer', () => { progress: null }, { + fileId: 'doc2.odt', status: 'pending', file: { name: 'doc2.odt' @@ -456,6 +479,7 @@ describe('queue reducer', () => { progress: null }, { + fileId: 'doc3.odt', status: 'updated', file: { name: 'doc3.odt' @@ -470,6 +494,7 @@ describe('queue reducer', () => { it('should handle RECEIVE_UPLOAD_ERROR action type', () => { const action = { type: 'RECEIVE_UPLOAD_ERROR', + fileId: 'doc2.odt', file: { name: 'doc2.odt' }, @@ -478,6 +503,7 @@ describe('queue reducer', () => { } const expected = [ { + fileId: 'doc1.odt', status: 'pending', file: { name: 'doc1.odt' @@ -485,6 +511,7 @@ describe('queue reducer', () => { progress: null }, { + fileId: 'doc2.odt', status: 'conflict', file: { name: 'doc2.odt' @@ -492,6 +519,7 @@ describe('queue reducer', () => { progress: null }, { + fileId: 'doc3.odt', status: 'pending', file: { name: 'doc3.odt' @@ -506,11 +534,13 @@ describe('queue reducer', () => { it('should only update the targeted item when files share the same name', () => { const stateWithDuplicateNames = [ { + fileId: 'summer/photo.jpg', status: 'pending', file: { name: 'photo.jpg' }, progress: null }, { + fileId: 'winter/photo.jpg', status: 'pending', file: { name: 'photo.jpg' }, progress: null @@ -518,12 +548,11 @@ describe('queue reducer', () => { ] const action = { type: 'UPLOAD_FILE', + fileId: 'summer/photo.jpg', file: { name: 'photo.jpg' } } const result = queue(stateWithDuplicateNames, action) - // Bug: both items get updated because the reducer matches on file.name - // Only the first should transition to 'loading' expect(result[0].status).toBe('loading') expect(result[1].status).toBe('pending') }) @@ -531,11 +560,13 @@ describe('queue reducer', () => { it('should correctly track success for files with duplicate names', () => { const stateWithDuplicateNames = [ { + fileId: 'summer/photo.jpg', status: 'loading', file: { name: 'photo.jpg' }, progress: null }, { + fileId: 'winter/photo.jpg', status: 'pending', file: { name: 'photo.jpg' }, progress: null @@ -543,11 +574,11 @@ describe('queue reducer', () => { ] const action = { type: 'RECEIVE_UPLOAD_SUCCESS', + fileId: 'summer/photo.jpg', file: { name: 'photo.jpg' } } const result = queue(stateWithDuplicateNames, action) - // Bug: both items get updated because the reducer matches on file.name expect(result[0].status).toBe('created') expect(result[1].status).toBe('pending') }) @@ -555,11 +586,13 @@ describe('queue reducer', () => { it('should correctly track errors for files with duplicate names', () => { const stateWithDuplicateNames = [ { + fileId: 'summer/photo.jpg', status: 'loading', file: { name: 'photo.jpg' }, progress: null }, { + fileId: 'winter/photo.jpg', status: 'loading', file: { name: 'photo.jpg' }, progress: null @@ -567,12 +600,12 @@ describe('queue reducer', () => { ] const action = { type: 'RECEIVE_UPLOAD_ERROR', + fileId: 'summer/photo.jpg', file: { name: 'photo.jpg' }, status: 'failed' } const result = queue(stateWithDuplicateNames, action) - // Bug: both items get updated because the reducer matches on file.name expect(result[0].status).toBe('failed') expect(result[1].status).toBe('loading') }) @@ -581,6 +614,7 @@ describe('queue reducer', () => { const file = { name: 'doc1.odt' } + const fileId = 'doc1.odt' const date1 = 1000 const date2 = 2000 @@ -589,6 +623,7 @@ describe('queue reducer', () => { const expected = [ { + fileId: 'doc1.odt', status: 'pending', file: { name: 'doc1.odt' @@ -602,6 +637,7 @@ describe('queue reducer', () => { } }, { + fileId: 'doc2.odt', status: 'pending', file: { name: 'doc2.odt' @@ -609,6 +645,7 @@ describe('queue reducer', () => { progress: null }, { + fileId: 'doc3.odt', status: 'pending', file: { name: 'doc3.odt' @@ -618,15 +655,15 @@ describe('queue reducer', () => { ] it('should handle UPLOAD_PROGRESS', () => { - const action = uploadProgress(file, event1, date1) + const action = uploadProgress(fileId, file, event1, date1) const result = queue(state, action) expect(result).toEqual(expected) }) it('should compute speed and remaining time', () => { - const result = queue(state, uploadProgress(file, event1, date1)) + const result = queue(state, uploadProgress(fileId, file, event1, date1)) expect(result[0].progress.remainingTime).toBe(null) - const result2 = queue(result, uploadProgress(file, event2, date2)) + const result2 = queue(result, uploadProgress(fileId, file, event2, date2)) expect(result2[0].progress).toEqual({ lastUpdated: expect.any(Number), loaded: 200, @@ -637,9 +674,9 @@ describe('queue reducer', () => { }) it('should handle upload error', () => { - const result = queue(state, uploadProgress(file, event1, date1)) - const result2 = queue(result, uploadProgress(file, event2, date2)) - const result3 = queue(result2, { type: 'RECEIVE_UPLOAD_ERROR', file }) + const result = queue(state, uploadProgress(fileId, file, event1, date1)) + const result2 = queue(result, uploadProgress(fileId, file, event2, date2)) + const result3 = queue(result2, { type: 'RECEIVE_UPLOAD_ERROR', fileId, file }) expect(result3[0].progress).toEqual(null) }) }) From 6dba0338c43d0784a7ac57e5d63d432c46cbbdf2 Mon Sep 17 00:00:00 2001 From: Crash-- Date: Mon, 6 Apr 2026 16:29:45 +0700 Subject: [PATCH 3/6] style: fix Prettier formatting and null-safe fileId derivation - Fix Prettier formatting violations (line wrapping) - Use optional chaining in itemInitialState to handle directory entries where file is null: item.file?.name ?? item.entry?.name Co-Authored-By: Claude Opus 4.6 (1M context) --- src/modules/upload/index.js | 20 ++++++++++++++------ src/modules/upload/index.spec.js | 6 +++++- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/modules/upload/index.js b/src/modules/upload/index.js index e390eb7a73..9cbb17f9de 100644 --- a/src/modules/upload/index.js +++ b/src/modules/upload/index.js @@ -55,7 +55,7 @@ const CONFLICT_ERROR = 409 const itemInitialState = item => ({ ...item, - fileId: item.fileId ?? item.file.name, + fileId: item.fileId ?? item.file?.name ?? item.entry?.name, status: PENDING, progress: null }) @@ -157,9 +157,7 @@ export const queue = (state = [], action) => { case RECEIVE_UPLOAD_SUCCESS: case RECEIVE_UPLOAD_ERROR: case UPLOAD_PROGRESS: - return state.map(i => - i.fileId !== action.fileId ? i : item(i, action) - ) + return state.map(i => (i.fileId !== action.fileId ? i : item(i, action))) default: return state } @@ -222,7 +220,12 @@ export const processNextFile = driveId ) safeCallback(newDir) - dispatch({ type: RECEIVE_UPLOAD_SUCCESS, fileId, file, uploadedItem: newDir }) + dispatch({ + type: RECEIVE_UPLOAD_SUCCESS, + fileId, + file, + uploadedItem: newDir + }) } else { const withProgress = { onUploadProgress: event => { @@ -483,7 +486,12 @@ export const overwriteFile = async ( } export const removeFileToUploadQueue = (file, fileId) => async dispatch => { - dispatch({ type: RECEIVE_UPLOAD_SUCCESS, fileId: fileId ?? file.name, file, isUpdate: true }) + dispatch({ + type: RECEIVE_UPLOAD_SUCCESS, + fileId: fileId ?? file.name, + file, + isUpdate: true + }) } export const addToUploadQueue = diff --git a/src/modules/upload/index.spec.js b/src/modules/upload/index.spec.js index 7f0520be8d..a813232668 100644 --- a/src/modules/upload/index.spec.js +++ b/src/modules/upload/index.spec.js @@ -676,7 +676,11 @@ describe('queue reducer', () => { it('should handle upload error', () => { const result = queue(state, uploadProgress(fileId, file, event1, date1)) const result2 = queue(result, uploadProgress(fileId, file, event2, date2)) - const result3 = queue(result2, { type: 'RECEIVE_UPLOAD_ERROR', fileId, file }) + const result3 = queue(result2, { + type: 'RECEIVE_UPLOAD_ERROR', + fileId, + file + }) expect(result3[0].progress).toEqual(null) }) }) From d6a166fbc228a26a2cb7d0f90e21d817c015df15 Mon Sep 17 00:00:00 2001 From: Crash-- Date: Mon, 6 Apr 2026 16:40:33 +0700 Subject: [PATCH 4/6] fix: reducer fallback on file.name for Flagship compatibility The Flagship upload path (UploadUtils.ts) dispatches RECEIVE_UPLOAD_SUCCESS and RECEIVE_UPLOAD_ERROR without fileId. The reducer now falls back to matching on file.name when fileId is absent, ensuring these dispatches still update the correct queue item. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/modules/upload/index.js | 6 ++++-- src/modules/upload/index.spec.js | 17 +++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/modules/upload/index.js b/src/modules/upload/index.js index 9cbb17f9de..5c7254c612 100644 --- a/src/modules/upload/index.js +++ b/src/modules/upload/index.js @@ -156,8 +156,10 @@ export const queue = (state = [], action) => { case UPLOAD_FILE: case RECEIVE_UPLOAD_SUCCESS: case RECEIVE_UPLOAD_ERROR: - case UPLOAD_PROGRESS: - return state.map(i => (i.fileId !== action.fileId ? i : item(i, action))) + case UPLOAD_PROGRESS: { + const matchId = action.fileId ?? action.file?.name + return state.map(i => (i.fileId !== matchId ? i : item(i, action))) + } default: return state } diff --git a/src/modules/upload/index.spec.js b/src/modules/upload/index.spec.js index a813232668..dbdb625824 100644 --- a/src/modules/upload/index.spec.js +++ b/src/modules/upload/index.spec.js @@ -610,6 +610,23 @@ describe('queue reducer', () => { expect(result[1].status).toBe('loading') }) + it('should fall back to file.name when action has no fileId (Flagship compat)', () => { + const stateWithItems = [ + { + fileId: 'my-doc.odt', + status: 'loading', + file: { name: 'my-doc.odt' }, + progress: null + } + ] + const action = { + type: 'RECEIVE_UPLOAD_SUCCESS', + file: { name: 'my-doc.odt' } + } + const result = queue(stateWithItems, action) + expect(result[0].status).toBe('created') + }) + describe('progress action', () => { const file = { name: 'doc1.odt' From fa56779a6dd847190906b8700dc347e95c39b785 Mon Sep 17 00:00:00 2001 From: Crash-- Date: Tue, 7 Apr 2026 10:28:42 +0700 Subject: [PATCH 5/6] refactor: extract helpers from processNextFile to reduce complexity Extract getUploadErrorStatus() and handleConflictOverwrite() to lower cyclomatic complexity flagged by CodeScene. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/modules/upload/index.js | 246 +++++++++++++++++++++--------------- 1 file changed, 142 insertions(+), 104 deletions(-) diff --git a/src/modules/upload/index.js b/src/modules/upload/index.js index 5c7254c612..cab96174a6 100644 --- a/src/modules/upload/index.js +++ b/src/modules/upload/index.js @@ -178,6 +178,123 @@ export const uploadProgress = (fileId, file, event, date) => ({ date: date || Date.now() }) +const getUploadErrorStatus = error => { + const statusError = { + 409: CONFLICT, + 413: QUOTA + } + + if (error.message?.includes(ERR_MAX_FILE_SIZE)) { + return ERR_MAX_FILE_SIZE + } else if (error.status in statusError) { + return statusError[error.status] + } else if (/Failed to fetch$/.exec(error.toString())) { + return NETWORK + } + return FAILED +} + +const handleConflictOverwrite = async ( + client, + file, + fileId, + dirID, + driveId, + dispatch +) => { + const path = driveId + ? await getFullpath(client, dirID, file.name, driveId) + : await CozyFile.getFullpath(dirID, file.name) + + const uploadedFile = await overwriteFile( + client, + file, + path, + { + onUploadProgress: event => { + dispatch(uploadProgress(fileId, file, event)) + } + }, + driveId + ) + dispatch({ + type: RECEIVE_UPLOAD_SUCCESS, + fileId, + file, + isUpdate: true, + uploadedItem: uploadedFile + }) + return uploadedFile +} + +const performUpload = async ( + client, + item, + dirID, + { vaultClient, encryptionKey }, + driveId, + dispatch +) => { + const { file, fileId, entry, isDirectory } = item + dispatch({ type: UPLOAD_FILE, fileId, file }) + + if (entry && isDirectory) { + return uploadDirectory( + client, + entry, + dirID, + { vaultClient, encryptionKey }, + driveId + ) + } + + return uploadFile( + client, + file, + dirID, + { + vaultClient, + encryptionKey, + onUploadProgress: event => { + dispatch(uploadProgress(fileId, file, event)) + } + }, + driveId + ) +} + +const handleUploadError = async ( + uploadError, + { client, file, fileId, dirID, driveId, dispatch, safeCallback } +) => { + let error = uploadError + if (uploadError.status === CONFLICT_ERROR) { + try { + const uploaded = await handleConflictOverwrite( + client, + file, + fileId, + dirID, + driveId, + dispatch + ) + safeCallback(uploaded) + return + } catch (updateError) { + error = updateError + } + } + logger.error( + `Upload module catches an error when executing processNextFile(): ${error}` + ) + dispatch({ + type: RECEIVE_UPLOAD_ERROR, + fileId, + file, + status: getUploadErrorStatus(error) + }) +} + export const processNextFile = ( fileUploadedCallback, @@ -193,7 +310,6 @@ export const processNextFile = typeof fileUploadedCallback === 'function' ? fileUploadedCallback : () => {} - let error if (!client) { throw new Error( 'Upload module needs a cozy-client instance to work. This instance should be made available by using the extraArgument function of redux-thunk' @@ -204,114 +320,36 @@ export const processNextFile = return dispatch(onQueueEmpty(queueCompletedCallback)) } - const { file, fileId, entry, isDirectory } = item + const { file, fileId } = item const encryptionKey = flag('drive.enable-encryption') ? await getEncryptionKeyFromDirId(client, dirID) : null try { - dispatch({ type: UPLOAD_FILE, fileId, file }) - if (entry && isDirectory) { - const newDir = await uploadDirectory( - client, - entry, - dirID, - { - vaultClient, - encryptionKey - }, - driveId - ) - safeCallback(newDir) - dispatch({ - type: RECEIVE_UPLOAD_SUCCESS, - fileId, - file, - uploadedItem: newDir - }) - } else { - const withProgress = { - onUploadProgress: event => { - dispatch(uploadProgress(fileId, file, event)) - } - } - - const uploadedFile = await uploadFile( - client, - file, - dirID, - { - vaultClient, - encryptionKey, - ...withProgress - }, - driveId - ) - safeCallback(uploadedFile) - dispatch({ - type: RECEIVE_UPLOAD_SUCCESS, - fileId, - file, - uploadedItem: uploadedFile - }) - } + const uploaded = await performUpload( + client, + item, + dirID, + { vaultClient, encryptionKey }, + driveId, + dispatch + ) + safeCallback(uploaded) + dispatch({ + type: RECEIVE_UPLOAD_SUCCESS, + fileId, + file, + uploadedItem: uploaded + }) } catch (uploadError) { - error = uploadError - if (uploadError.status === CONFLICT_ERROR) { - try { - const path = driveId - ? await getFullpath(client, dirID, file.name, driveId) - : await CozyFile.getFullpath(dirID, file.name) - - const uploadedFile = await overwriteFile( - client, - file, - path, - { - onUploadProgress: event => { - dispatch(uploadProgress(fileId, file, event)) - } - }, - driveId - ) - safeCallback(uploadedFile) - dispatch({ - type: RECEIVE_UPLOAD_SUCCESS, - fileId, - file, - isUpdate: true, - uploadedItem: uploadedFile - }) - error = null - } catch (updateError) { - error = updateError - } - } - if (error) { - logger.error( - `Upload module catches an error when executing processNextFile(): ${error}` - ) - - // Define mapping for specific status codes to our constants - const statusError = { - 409: CONFLICT, - 413: QUOTA - } - - // Determine the status based on the error details - let status - if (error.message?.includes(ERR_MAX_FILE_SIZE)) { - status = ERR_MAX_FILE_SIZE // File size exceeded maximum size allowed by the server - } else if (error.status in statusError) { - status = statusError[error.status] - } else if (/Failed to fetch$/.exec(error.toString())) { - status = NETWORK - } else { - status = FAILED - } - - // Dispatch an action to handle the upload error with the determined status - dispatch({ type: RECEIVE_UPLOAD_ERROR, fileId, file, status }) - } + await handleUploadError(uploadError, { + client, + file, + fileId, + dirID, + driveId, + dispatch, + safeCallback + }) } dispatch( processNextFile( From 133e44ba2b48210b1ee9cdc9347858cd266110ea Mon Sep 17 00:00:00 2001 From: Crash-- Date: Tue, 7 Apr 2026 10:55:59 +0700 Subject: [PATCH 6/6] fix: populate fileId at enqueue paths to prevent basename collisions extractFilesEntries now sets fileId from entry.fullPath, and generateForQueue uses file.filePath for Flagship uploads. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/modules/upload/index.js | 1 + src/modules/upload/index.spec.js | 6 ++++-- src/modules/views/Upload/UploadTypes.ts | 1 + src/modules/views/Upload/UploadUtils.ts | 6 +++++- 4 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/modules/upload/index.js b/src/modules/upload/index.js index cab96174a6..4b92a507ae 100644 --- a/src/modules/upload/index.js +++ b/src/modules/upload/index.js @@ -635,6 +635,7 @@ export const extractFilesEntries = items => { const entry = item.webkitGetAsEntry() results.push({ file: item.getAsFile(), + fileId: entry.fullPath, isDirectory: entry.isDirectory === true, entry }) diff --git a/src/modules/upload/index.spec.js b/src/modules/upload/index.spec.js index dbdb625824..b4046536c9 100644 --- a/src/modules/upload/index.spec.js +++ b/src/modules/upload/index.spec.js @@ -743,7 +743,7 @@ describe('extractFilesEntries', () => { it('should extract DataTransferItem with file entry', () => { const file = new File(['a'], 'a.txt') - const fileEntry = { isFile: true, isDirectory: false } + const fileEntry = { isFile: true, isDirectory: false, fullPath: '/a.txt' } const items = [ { webkitGetAsEntry: () => fileEntry, @@ -754,13 +754,14 @@ describe('extractFilesEntries', () => { expect(result).toHaveLength(1) expect(result[0]).toEqual({ file, + fileId: '/a.txt', isDirectory: false, entry: fileEntry }) }) it('should extract DataTransferItem with directory entry', () => { - const dirEntry = { isFile: false, isDirectory: true } + const dirEntry = { isFile: false, isDirectory: true, fullPath: '/photos' } const items = [ { webkitGetAsEntry: () => dirEntry, @@ -771,6 +772,7 @@ describe('extractFilesEntries', () => { expect(result).toHaveLength(1) expect(result[0]).toEqual({ file: null, + fileId: '/photos', isDirectory: true, entry: dirEntry }) diff --git a/src/modules/views/Upload/UploadTypes.ts b/src/modules/views/Upload/UploadTypes.ts index fa256325bd..e9dc632223 100644 --- a/src/modules/views/Upload/UploadTypes.ts +++ b/src/modules/views/Upload/UploadTypes.ts @@ -1,6 +1,7 @@ export interface FileForQueue { name: string file?: { name: string } + fileId?: string isDirectory?: false } diff --git a/src/modules/views/Upload/UploadUtils.ts b/src/modules/views/Upload/UploadUtils.ts index a59e997779..cf5f891a81 100644 --- a/src/modules/views/Upload/UploadUtils.ts +++ b/src/modules/views/Upload/UploadUtils.ts @@ -11,7 +11,11 @@ export const generateForQueue = ( files: FileFromNative['file'][] ): FileForQueue[] => { // @ts-expect-error fix file types mismatch - return files.map(file => ({ file: file, isDirectory: false })) + return files.map(file => ({ + file: file, + fileId: String(file.filePath || file.name), + isDirectory: false + })) } export const onFileUploaded = (