share uploads sequentially or in parallel depending on conditions

Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>

4 files changed, 205 insertions, 11 deletions

diff --git a/src/store/fileUploadStore.js b/src/store/fileUploadStore.js
index 449f14e73..dd110ee78 100644
--- a/src/store/fileUploadStore.js
+++ b/src/store/fileUploadStore.js
@@ -33,7 +33,12 @@ import {
 	shareFile,
 } from '../services/filesSharingServices.js'
 import { setAttachmentFolder } from '../services/settingsService.js'
-import { findUniquePath, getFileExtension } from '../utils/fileUpload.js'
+import {
+	hasDuplicateUploadNames,
+	findUniquePath,
+	getFileExtension,
+	separateDuplicateUploads,
+} from '../utils/fileUpload.js'
 
 const state = {
 	attachmentFolder: loadState('spreed', 'attachment_folder', ''),
@@ -304,16 +309,13 @@ const actions = {
 			EventBus.$emit('scroll-chat-to-bottom', { force: true })
 		}
 
-		// Iterate again and perform the uploads
-		await Promise.allSettled(getters.getUploadsArray(uploadId).map(async ([index, uploadedFile]) => {
+		const performUpload = async ([index, uploadedFile]) => {
 			// currentFile to be uploaded
 			const currentFile = uploadedFile.file
 			// userRoot path
-			const userRoot = '/files/' + getters.getUserId()
 			const fileName = (currentFile.newName || currentFile.name)
 			// Candidate rest of the path
 			const path = getters.getAttachmentFolder() + '/' + fileName
-			const client = getDavClient()
 			// Get a unique relative path based on the previous path variable
 			const uniquePath = await findUniquePath(client, userRoot, path)
 			try {
@@ -349,10 +351,9 @@ const actions = {
 				commit('markFileAsFailedUpload', { uploadId, index })
 				dispatch('markTemporaryMessageAsFailed', { message: uploadedFile.temporaryMessage, reason })
 			}
-		}))
+		}
 
-		// Share the files, that have successfully been uploaded from the store, to the conversation
-		await Promise.all(getters.getShareableFiles(uploadId).map(async ([index, shareableFile]) => {
+		const performShare = async ([index, shareableFile]) => {
 			const path = shareableFile.sharePath
 			const temporaryMessage = shareableFile.temporaryMessage
 			const metadata = (caption && index === lastIndex)
@@ -372,7 +373,41 @@ const actions = {
 				dispatch('markTemporaryMessageAsFailed', { message: temporaryMessage, reason: 'failed-share' })
 				console.error('An error happened when trying to share your file: ', error)
 			}
-		}))
+		}
+
+		const client = getDavClient()
+		const userRoot = '/files/' + getters.getUserId()
+
+		const uploads = getters.getUploadsArray(uploadId)
+		// Check for duplicate names in the uploads array
+		if (hasDuplicateUploadNames(uploads)) {
+			const { uniques, duplicates } = separateDuplicateUploads(uploads)
+			await Promise.all(uniques.map(upload => performUpload(upload)))
+			// Search for uniquePath and upload files one by one to avoid 423 (Locked)
+			for (const upload of duplicates) {
+				await performUpload(upload)
+			}
+		} else {
+			// All original names are unique, upload files in parallel
+			await Promise.all(uploads.map(upload => performUpload(upload)))
+		}
+
+		const shares = getters.getShareableFiles(uploadId)
+		// Check if caption message for share was provided
+		if (caption) {
+			const captionShareIndex = shares.findIndex(([index]) => index === lastIndex)
+
+			// Share all files in parallel, except for last one
+			const parallelShares = shares.slice(0, captionShareIndex).concat(shares.slice(captionShareIndex + 1))
+			await Promise.all(parallelShares.map(share => performShare(share)))
+
+			// Share a last file, where caption is attached
+			await performShare(shares.at(captionShareIndex))
+		} else {
+			// Share all files in parallel
+			await Promise.all(shares.map(share => performShare(share)))
+		}
+
 		EventBus.$emit('upload-finished')
 	},
 	/**
diff --git a/src/store/fileUploadStore.spec.js b/src/store/fileUploadStore.spec.js
index 8e6fbe3be..71e57c3aa 100644
--- a/src/store/fileUploadStore.spec.js
+++ b/src/store/fileUploadStore.spec.js
@@ -17,6 +17,8 @@ jest.mock('../services/DavClient', () => ({
 jest.mock('../utils/fileUpload', () => ({
 	findUniquePath: jest.fn(),
 	getFileExtension: jest.fn(),
+	hasDuplicateUploadNames: jest.fn(),
+	separateDuplicateUploads: jest.fn(),
 }))
 jest.mock('../services/filesSharingServices', () => ({
 	shareFile: jest.fn(),
diff --git a/src/utils/__tests__/fileUpload.spec.js b/src/utils/__tests__/fileUpload.spec.js
index fe8541e66..d47fe4472 100644
--- a/src/utils/__tests__/fileUpload.spec.js
+++ b/src/utils/__tests__/fileUpload.spec.js
@@ -1,7 +1,11 @@
 import {
 	getFileExtension,
+	getFileSuffix,
 	extractFileName,
+	getFileNamePrompt,
 	findUniquePath,
+	hasDuplicateUploadNames,
+	separateDuplicateUploads,
 } from '../fileUpload.js'
 
 const client = {
@@ -23,6 +27,20 @@ describe('fileUpload', () => {
 		})
 	})
 
+	describe('getFileSuffix', () => {
+		it('should return the file suffix when it exists in the path', () => {
+			const path = 'file (3).txt'
+			const suffix = getFileSuffix(path)
+			expect(suffix).toBe(3)
+		})
+
+		it('should return 1 when no file suffix exists in the path', () => {
+			const path = 'file.txt'
+			const suffix = getFileSuffix(path)
+			expect(suffix).toBe(1)
+		})
+	})
+
 	describe('extractFileName', () => {
 		it('should return the file name as-is when there is no extension or digit suffix', () => {
 			const path = 'file'
@@ -37,6 +55,26 @@ describe('fileUpload', () => {
 		})
 	})
 
+	describe('getFileNamePrompt', () => {
+		it('should return the file name prompt including extension', () => {
+			const path = 'file.mock.txt'
+			const fileNamePrompt = getFileNamePrompt(path)
+			expect(fileNamePrompt).toBe('file.mock.txt')
+		})
+
+		it('should return the file name prompt without extension when the extension is missing', () => {
+			const path = 'file'
+			const fileNamePrompt = getFileNamePrompt(path)
+			expect(fileNamePrompt).toBe('file')
+		})
+
+		it('should return the file name prompt with digit suffix when the suffix exists', () => {
+			const path = 'file (1).txt'
+			const fileNamePrompt = getFileNamePrompt(path)
+			expect(fileNamePrompt).toBe('file.txt')
+		})
+	})
+
 	describe('findUniquePath', () => {
 		const userRoot = '/files/userid/'
 		const path = 'file.txt'
@@ -75,5 +113,70 @@ describe('fileUpload', () => {
 			expect(client.exists).toHaveBeenNthCalledWith(2, userRoot + existingPath)
 			expect(client.exists).toHaveBeenNthCalledWith(3, userRoot + uniquePath)
 		})
+
+		it('should look for unique path starting from a known suffix when the input path already exists in the destination folder', async () => {
+			// Arrange
+			const givenPath = 'file (4).txt'
+			const existingPath = 'file (5).txt'
+			const uniquePath = 'file (6).txt'
+			client.exists
+				.mockResolvedValueOnce(true)
+				.mockResolvedValueOnce(true)
+				.mockResolvedValueOnce(false) // Simulate resolving unique path on 3rd attempt
+
+			// Act
+			const result = await findUniquePath(client, userRoot, givenPath)
+
+			// Assert
+			expect(result).toBe(uniquePath)
+			expect(client.exists).toHaveBeenNthCalledWith(1, userRoot + givenPath)
+			expect(client.exists).toHaveBeenNthCalledWith(2, userRoot + existingPath)
+			expect(client.exists).toHaveBeenNthCalledWith(3, userRoot + uniquePath)
+		})
+	})
+})
+
+describe('hasDuplicateUploadNames', () => {
+	it('should return true when array includes duplicate upload names', () => {
+		const uploads = [
+			[1, { file: { name: 'file.txt' } }],
+			[2, { file: { name: 'file (1).txt' } }],
+			[3, { file: { name: 'file (copy).txt', newName: 'file (2).txt' } }],
+		]
+
+		expect(hasDuplicateUploadNames(uploads)).toBe(true)
+	})
+
+	it('should return false when array does not include duplicate upload names', () => {
+		const uploads = [
+			[1, { file: { name: 'file.txt' } }],
+			[2, { file: { name: 'file 2.txt' } }],
+			[3, { file: { name: 'file.txt', newName: 'file (copy).txt' } }],
+		]
+
+		expect(hasDuplicateUploadNames(uploads)).toBe(false)
+	})
+
+	describe('separateDuplicateUploads', () => {
+		it('should separate unique and duplicate uploads based on file name prompt', () => {
+			const uploads = [
+				[1, { file: { name: 'file.txt' } }],
+				[2, { file: { name: 'file (1).jpeg' } }],
+				[3, { file: { name: 'file (copy).txt', newName: 'file (2).txt' } }],
+				[4, { file: { name: 'file (unique).txt' } }],
+			]
+
+			const { uniques, duplicates } = separateDuplicateUploads(uploads)
+
+			expect(uniques).toEqual([
+				[1, { file: { name: 'file.txt' } }],
+				[2, { file: { name: 'file (1).jpeg' } }],
+				[4, { file: { name: 'file (unique).txt' } }],
+			])
+
+			expect(duplicates).toEqual([
+				[3, { file: { name: 'file (copy).txt', newName: 'file (2).txt' } }],
+			])
+		})
 	})
 })
diff --git a/src/utils/fileUpload.js b/src/utils/fileUpload.js
index d374f2730..0bfe25e4f 100644
--- a/src/utils/fileUpload.js
+++ b/src/utils/fileUpload.js
@@ -41,13 +41,23 @@ const getFileExtension = function(path) {
  */
 const extractFileName = function(path) {
 	return path
-		// If there is a file extension, remove it from the path string
+	// If there is a file extension, remove it from the path string
 		.replace(extensionRegex, '')
-		// If a filename ends with suffix ` (n)`, remove it from the path string
+	// If a filename ends with suffix ` (n)`, remove it from the path string
 		.replace(suffixRegex, '')
 }
 
 /**
+ * Returns the file name prompt for the given path
+ *
+ * @param {string} path path
+ * @return {string} file name prompt
+ */
+const getFileNamePrompt = function(path) {
+	return extractFileName(path) + getFileExtension(path)
+}
+
+/**
  * Checks the existence of a path in a folder and if a match is found, returns
  * a unique path for that folder.
  *
@@ -75,8 +85,52 @@ const findUniquePath = async function(client, userRoot, path) {
 	}
 }
 
+/**
+ * Checks the existence of duplicated file names in provided array of uploads.
+ *
+ * @param {Array} uploads The array of uploads to share
+ * @return {boolean} Whether array includes duplicates or not
+ */
+const hasDuplicateUploadNames = function(uploads) {
+	const uploadNames = uploads.map(([_index, { file }]) => {
+		return getFileNamePrompt(file.newName || file.name)
+	})
+	const uploadNamesSet = new Set(uploadNames)
+
+	return uploadNames.length !== uploadNamesSet.size
+}
+
+/**
+ * Process array of upload and returns separated array with unique filenames and duplicates
+ *
+ * @param {Array} uploads The array of uploads to share
+ * @return {object} separated unique and duplicate uploads
+ */
+function separateDuplicateUploads(uploads) {
+	const nameCount = new Set()
+	const uniques = []
+	const duplicates = []
+
+	// Count the occurrences of each name
+	for (const upload of uploads) {
+		const name = getFileNamePrompt(upload.at(1).file.newName || upload.at(1).file.name)
+
+		if (nameCount.has(name)) {
+			duplicates.push(upload)
+		} else {
+			uniques.push(upload)
+			nameCount.add(name)
+		}
+	}
+
+	return { uniques, duplicates }
+}
+
 export {
 	findUniquePath,
 	extractFileName,
 	getFileExtension,
+	getFileNamePrompt,
+	hasDuplicateUploadNames,
+	separateDuplicateUploads,
 }