Skip to content

Commit

Permalink
starts searching for uniquePath from known suffix
Browse files Browse the repository at this point in the history
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
  • Loading branch information
Antreesy authored and backportbot-nextcloud[bot] committed Nov 28, 2023
1 parent 1fd950e commit a9ea153
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 26 deletions.
12 changes: 11 additions & 1 deletion src/store/fileUploadStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
hasDuplicateUploadNames,
findUniquePath,
getFileExtension,
getFileNamePrompt,
separateDuplicateUploads,
} from '../utils/fileUpload.js'

Expand Down Expand Up @@ -309,15 +310,24 @@ const actions = {
EventBus.$emit('scroll-chat-to-bottom', { force: true })
}

// Store propfind attempts within one action to reduce amount of requests for duplicates
const knownPaths = {}

const performUpload = async ([index, uploadedFile]) => {
// currentFile to be uploaded
const currentFile = uploadedFile.file
// userRoot path
const fileName = (currentFile.newName || currentFile.name)
// Candidate rest of the path
const path = getters.getAttachmentFolder() + '/' + fileName

// Check if previous propfind attempt was stored
const promptPath = getFileNamePrompt(path)
const knownSuffix = knownPaths[promptPath]
// Get a unique relative path based on the previous path variable
const uniquePath = await findUniquePath(client, userRoot, path)
const { uniquePath, suffix } = await findUniquePath(client, userRoot, path, knownSuffix)
knownPaths[promptPath] = suffix

try {
// Upload the file
const currentFileBuffer = await new Blob([currentFile]).arrayBuffer()
Expand Down
22 changes: 8 additions & 14 deletions src/store/fileUploadStore.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,15 @@ import { showError } from '@nextcloud/dialogs'
import { getDavClient } from '../services/DavClient.js'
import { shareFile } from '../services/filesSharingServices.js'
import { setAttachmentFolder } from '../services/settingsService.js'
import { findUniquePath, getFileExtension } from '../utils/fileUpload.js'
import { findUniquePath } from '../utils/fileUpload.js'
import fileUploadStore from './fileUploadStore.js'

jest.mock('../services/DavClient', () => ({
getDavClient: jest.fn(),
}))
jest.mock('../utils/fileUpload', () => ({
...jest.requireActual('../utils/fileUpload'),
findUniquePath: jest.fn(),
getFileExtension: jest.fn(),
hasDuplicateUploadNames: jest.fn(),
separateDuplicateUploads: jest.fn(),
}))
jest.mock('../services/filesSharingServices', () => ({
shareFile: jest.fn(),
Expand Down Expand Up @@ -158,13 +156,13 @@ describe('fileUploadStore', () => {
expect(store.getters.currentUploadId).toBe('upload-id1')

const uniqueFileName = '/Talk/' + file.name + 'uniq'
findUniquePath.mockResolvedValueOnce(uniqueFileName)
findUniquePath.mockResolvedValueOnce({ uniquePath: uniqueFileName, suffix: 1 })
shareFile.mockResolvedValue()

await store.dispatch('uploadFiles', { uploadId: 'upload-id1', caption: 'text-caption' })

expect(findUniquePath).toHaveBeenCalledTimes(1)
expect(findUniquePath).toHaveBeenCalledWith(client, '/files/current-user', '/Talk/' + file.name)
expect(findUniquePath).toHaveBeenCalledWith(client, '/files/current-user', '/Talk/' + file.name, undefined)

expect(client.putFileContents).toHaveBeenCalledTimes(1)
expect(client.putFileContents).toHaveBeenCalledWith(`/files/current-user${uniqueFileName}`, fileBuffer, expect.anything())
Expand Down Expand Up @@ -204,8 +202,8 @@ describe('fileUploadStore', () => {
expect(store.getters.currentUploadId).toBe('upload-id1')

findUniquePath
.mockResolvedValueOnce('/Talk/' + files[0].name + 'uniq')
.mockResolvedValueOnce('/Talk/' + files[1].name + 'uniq')
.mockResolvedValueOnce({ uniquePath: '/Talk/' + files[0].name + 'uniq', suffix: 1 })
.mockResolvedValueOnce({ uniquePath: '/Talk/' + files[1].name + 'uniq', suffix: 1 })
shareFile
.mockResolvedValueOnce({ data: { ocs: { data: { id: '1' } } } })
.mockResolvedValueOnce({ data: { ocs: { data: { id: '2' } } } })
Expand All @@ -217,7 +215,7 @@ describe('fileUploadStore', () => {
expect(shareFile).toHaveBeenCalledTimes(2)

for (const index in files) {
expect(findUniquePath).toHaveBeenCalledWith(client, '/files/current-user', '/Talk/' + files[index].name)
expect(findUniquePath).toHaveBeenCalledWith(client, '/files/current-user', '/Talk/' + files[index].name, undefined)
expect(client.putFileContents).toHaveBeenCalledWith(`/files/current-user/Talk/${files[index].name}uniq`, fileBuffers[index], expect.anything())
}

Expand Down Expand Up @@ -246,7 +244,7 @@ describe('fileUploadStore', () => {
})

findUniquePath
.mockResolvedValueOnce('/Talk/' + files[0].name + 'uniq')
.mockResolvedValueOnce({ uniquePath: '/Talk/' + files[0].name + 'uniq', suffix: 1 })
client.putFileContents.mockRejectedValueOnce({
response: {
status: 403,
Expand Down Expand Up @@ -380,10 +378,6 @@ describe('fileUploadStore', () => {
},
]

getFileExtension
.mockReturnValueOnce('.png')
.mockReturnValueOnce('.txt')

await store.dispatch('initialiseUpload', {
uploadId: 'upload-id1',
token: 'XXTOKENXX',
Expand Down
6 changes: 3 additions & 3 deletions src/utils/__tests__/fileUpload.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ describe('fileUpload', () => {
const result = await findUniquePath(client, userRoot, path)

// Assert
expect(result).toBe(path)
expect(result).toStrictEqual({ uniquePath: path, suffix: 1 })
expect(client.exists).toHaveBeenCalledWith(userRoot + path)
})

Expand All @@ -108,7 +108,7 @@ describe('fileUpload', () => {
const result = await findUniquePath(client, userRoot, path)

// Assert
expect(result).toBe(uniquePath)
expect(result).toStrictEqual({ uniquePath, suffix: 3 })
expect(client.exists).toHaveBeenNthCalledWith(1, userRoot + path)
expect(client.exists).toHaveBeenNthCalledWith(2, userRoot + existingPath)
expect(client.exists).toHaveBeenNthCalledWith(3, userRoot + uniquePath)
Expand All @@ -128,7 +128,7 @@ describe('fileUpload', () => {
const result = await findUniquePath(client, userRoot, givenPath)

// Assert
expect(result).toBe(uniquePath)
expect(result).toStrictEqual({ uniquePath, suffix: 6 })
expect(client.exists).toHaveBeenNthCalledWith(1, userRoot + givenPath)
expect(client.exists).toHaveBeenNthCalledWith(2, userRoot + existingPath)
expect(client.exists).toHaveBeenNthCalledWith(3, userRoot + uniquePath)
Expand Down
29 changes: 21 additions & 8 deletions src/utils/fileUpload.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,18 @@ const suffixRegex = / \(\d+\)$/
* @return {string} file extension including the dot
*/
const getFileExtension = function(path) {
return path.match(extensionRegex) ? path.match(extensionRegex)[0] : ''
return path.match(extensionRegex)?.[0] ?? ''
}

/**
* Returns the file suffix for the given path
*
* @param {string} path path
* @return {number} file suffix excluding the parenthesis
*/
const getFileSuffix = function(path) {
return parseInt(path.replace(extensionRegex, '')
.match(suffixRegex)?.[0]?.match(/\d+/)?.[0] ?? 1)
}

/**
Expand Down Expand Up @@ -65,22 +76,23 @@ const getFileNamePrompt = function(path) {
* @param {string} userRoot user root path
* @param {string} path The path whose existence in the destination is to
* be checked
* @return {string} The unique path
* @param {number} knownSuffix The suffix to start looking from
* @return {object} The unique path and suffix
*/
const findUniquePath = async function(client, userRoot, path) {
const findUniquePath = async function(client, userRoot, path, knownSuffix) {
// Return the input path if it doesn't exist in the destination folder
if (await client.exists(userRoot + path) === false) {
return path
if (!knownSuffix && await client.exists(userRoot + path) === false) {
return { uniquePath: path, suffix: getFileSuffix(path) }
}

const fileExtension = getFileExtension(path)
const fileName = extractFileName(path)

// Loop until a unique path is found
for (let number = 2; true; number++) {
const uniquePath = fileName + ` (${number})` + fileExtension
for (let suffix = knownSuffix + 1 || getFileSuffix(path) + 1; true; suffix++) {
const uniquePath = fileName + ` (${suffix})` + fileExtension
if (await client.exists(userRoot + uniquePath) === false) {
return uniquePath
return { uniquePath, suffix }
}
}
}
Expand Down Expand Up @@ -130,6 +142,7 @@ export {
findUniquePath,
extractFileName,
getFileExtension,
getFileSuffix,
getFileNamePrompt,
hasDuplicateUploadNames,
separateDuplicateUploads,
Expand Down

0 comments on commit a9ea153

Please sign in to comment.