Skip to content

Commit

Permalink
fix(uploader): properly propagate upload cancelled status
Browse files Browse the repository at this point in the history
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
  • Loading branch information
susnux committed Feb 25, 2025
1 parent 88ccd90 commit 21b6756
Show file tree
Hide file tree
Showing 5 changed files with 236 additions and 15 deletions.
191 changes: 191 additions & 0 deletions cypress/components/UploadPicker/cancel.cy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,191 @@
/*!
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
import { File, Folder, Permission } from '@nextcloud/files'
import { generateRemoteUrl } from '@nextcloud/router'
import { UploadPicker, UploadStatus, getUploader } from '../../../lib/index.ts'

let state: string | undefined

before(() => {
cy.window().then((win) => {
state = win.document.body.innerHTML
})
})

/**
* Reset the inner body of the document to remove any previous state of the uploader.
*/
function resetDocument(): void {
if (state) {
cy.window().then((win) => {
win.document.body.innerHTML = state!
})
}
}

describe('UploadPicker: progress handling', () => {
let dirContent: File[] = []

afterEach(() => {
resetDocument()
})

beforeEach(() => {
dirContent = []

// Make sure we reset the destination
// so other tests do not interfere
const propsData = {
content: () => dirContent,
destination: new Folder({
id: 56,
owner: 'user',
source: generateRemoteUrl('dav/files/user'),
permissions: Permission.ALL,
root: '/files/user',
}),
}

// Start paused
getUploader(false, true)
.pause()

// Mount picker
cy.mount(UploadPicker, {
propsData,
}).as('uploadPicker')

// Check and init aliases
cy.get('[data-cy-upload-picker] [data-cy-upload-picker-input]').as('input').should('exist')
cy.get('[data-cy-upload-picker] [data-cy-upload-picker-progress]').as('progress').should('exist')
cy.get('[data-cy-upload-picker] [data-cy-upload-picker-progress-label]').as('progressLabel').should('exist')
})

it('cancels single file upload', () => {
const notify = cy.spy()
getUploader()
.addNotifier(notify)

cy.get('@input').attachFile({
// file of 5 MiB
fileContent: new Blob([new ArrayBuffer(5 * 1024 * 1024)]),
fileName: 'file.txt',
mimeType: 'text/plain',
encoding: 'utf8',
lastModified: new Date().getTime(),
})

cy.intercept('PUT', '/remote.php/dav/files/user/file.txt', { statusCode: 201, delay: 2000 })

// See there is no progress yet
cy.get('@progress')
.should('be.visible')
.should('have.value', 0)
cy.get('@progressLabel')
.should('contain.text', 'paused')
// start the uploader
.then(() => getUploader().start())

cy.get('@progress', { timeout: 2000 })
.should((el) => expect(el.val()).to.be.greaterThan(10))
.and((el) => expect(el.val()).to.be.lessThan(95))

// Now cancel the upload
cy.get('[data-cy-upload-picker-cancel]')
.should('be.visible')
.click()
cy.get('@progress')
.should('not.be.visible')
.then(() => {
// eslint-disable-next-line no-unused-expressions
expect(notify).to.be.calledOnce
expect(notify.getCall(0).args[0].status).to.eq(UploadStatus.CANCELLED)
})
})

it('cancels single chunked file upload', () => {
const notify = cy.spy()
getUploader()
.addNotifier(notify)

cy.get('@input').attachFile({
fileContent: new Blob([new ArrayBuffer(15 * 1024 * 1024)]),
fileName: 'file.txt',
mimeType: 'text/plain',
encoding: 'utf8',
lastModified: new Date().getTime(),
})

cy.intercept('MKCOL', '/remote.php/dav/uploads/user/*', { statusCode: 201 })
cy.intercept('DELETE', '/remote.php/dav/uploads/user/*', { statusCode: 201 })
cy.intercept('PUT', '/remote.php/dav/uploads/user/web-file-upload-*/*', { statusCode: 201, delay: 2000 })

// See there is no progress yet
cy.get('@progress')
.should('be.visible')
.should('have.value', 0)
cy.get('@progressLabel')
.should('contain.text', 'paused')
// start the uploader
.then(() => getUploader().start())

cy.get('@progress', { timeout: 2000 })
.should((el) => expect(el.val()).to.be.greaterThan(10))
.and((el) => expect(el.val()).to.be.lessThan(95))

// Now cancel the upload
cy.get('[data-cy-upload-picker-cancel]')
.should('be.visible')
.click()
cy.get('@progress')
.should('not.be.visible')
.then(() => {
// eslint-disable-next-line no-unused-expressions
expect(notify).to.be.calledTwice
expect(notify.getCall(0).args[0].status).to.eq(UploadStatus.CANCELLED)
expect(notify.getCall(1).args[0].status).to.eq(UploadStatus.CANCELLED)
})
})

it('cancels single file conflict', () => {
dirContent.push(new File({ owner: 'test', source: 'http://example.com/remote.php/dav/files/test/file.txt', mime: 'text/plain' }))
const notify = cy.spy()
getUploader()
.addNotifier(notify)

cy.get('@input').attachFile({
// file of 5 MiB
fileContent: new Blob([new ArrayBuffer(5 * 1024 * 1024)]),
fileName: 'file.txt',
mimeType: 'text/plain',
encoding: 'utf8',
lastModified: new Date().getTime(),
})

cy.intercept('PUT', '/remote.php/dav/files/user/file.txt', { statusCode: 409, delay: 2000 })

// See there is no progress yet
cy.get('@progress')
.should('be.visible')
.should('have.value', 0)
// start the uploader
.then(() => getUploader().start())

cy.get('[role="dialog"]')
.should('be.visible')
.find('[data-cy-conflict-picker-cancel]')
.click()

cy.contains('.toast-warning', 'Upload has been cancelled').should('be.visible')

cy.get('@progress')
.should('not.be.visible')
.then(() => {
// eslint-disable-next-line no-unused-expressions
expect(notify).to.be.calledOnce
expect(notify.getCall(0).args[0].status).to.eq(UploadStatus.CANCELLED)
})
})
})
6 changes: 6 additions & 0 deletions l10n/messages.pot
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@ msgstr ""
msgid "Existing version"
msgstr ""

msgid "Failed assembling the chunks together"
msgstr ""

msgid "Failed uploading the file"
msgstr ""

msgid "Filenames must not end with \"{segment}\"."
msgstr ""

Expand Down
2 changes: 1 addition & 1 deletion lib/components/UploadPicker.vue
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ export default defineComponent({
return this.queue?.filter((upload: Upload) => upload.status === UploadStatus.FAILED).length !== 0
},
isUploading(): boolean {
return this.queue?.length > 0
return this.queue?.filter((upload: Upload) => upload.status !== UploadStatus.CANCELLED).length > 0
},
isAssembling(): boolean {
return this.queue?.filter((upload: Upload) => upload.status === UploadStatus.ASSEMBLING).length !== 0
Expand Down
13 changes: 13 additions & 0 deletions lib/errors/UploadCancelledError.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*!
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
import { t } from '../utils/l10n.ts'

export class UploadCancelledError extends Error {

public constructor(cause?: unknown) {
super(t('Upload has been cancelled'), { cause })
}

}
39 changes: 25 additions & 14 deletions lib/uploader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import axios, { isCancel } from '@nextcloud/axios'
import PCancelable from 'p-cancelable'
import PQueue from 'p-queue'

import { UploadCancelledError } from './errors/UploadCancelledError.ts'
import { getChunk, initChunkWorkspace, uploadData } from './utils/upload.js'
import { getMaxChunksSize } from './utils/config.js'
import { Status as UploadStatus, Upload } from './upload.js'
Expand Down Expand Up @@ -294,9 +295,15 @@ export class Uploader {
upload.status = UploadStatus.FINISHED
resolve(uploads)
} catch (error) {
logger.error('Error in batch upload', { error })
upload.status = UploadStatus.FAILED
reject(t('Upload has been cancelled'))
if (isCancel(error) || error instanceof UploadCancelledError) {
logger.info('Upload cancelled by user', { error })
upload.status = UploadStatus.CANCELLED
reject(new UploadCancelledError(error))
} else {
logger.error('Error in batch upload', { error })
upload.status = UploadStatus.FAILED
reject(error)
}
} finally {
this._notifyAll(upload)
this.updateStats()
Expand Down Expand Up @@ -335,10 +342,14 @@ export class Uploader {
await client.createDirectory(folderPath, { signal: abort.signal })
resolve(currentUpload)
} catch (error) {
if (error && typeof error === 'object' && 'status' in error && error.status === 405) {
if (isCancel(error) || error instanceof UploadCancelledError) {
currentUpload.status = UploadStatus.CANCELLED
reject(new UploadCancelledError(error))
} else if (error && typeof error === 'object' && 'status' in error && error.status === 405) {
// Directory already exists, so just write into it and ignore the error
currentUpload.status = UploadStatus.FINISHED
logger.debug('Directory already exists, writing into it', { directory: directory.name })
currentUpload.status = UploadStatus.FINISHED
resolve(currentUpload)
} else {
// Another error happened, so abort uploading the directory
currentUpload.status = UploadStatus.FAILED
Expand Down Expand Up @@ -371,7 +382,7 @@ export class Uploader {
const selectedForUpload = await callback(directory.children, folderPath)
if (selectedForUpload === false) {
logger.debug('Upload canceled by user', { directory })
reject(t('Upload has been cancelled'))
reject(new UploadCancelledError('Conflict resolution cancelled by user'))
return
} else if (selectedForUpload.length === 0 && directory.children.length > 0) {
logger.debug('Skipping directory, as all files were skipped by user', { directory })
Expand Down Expand Up @@ -552,12 +563,12 @@ export class Uploader {
logger.debug(`Successfully uploaded ${file.name}`, { file, upload })
resolve(upload)
} catch (error) {
if (!isCancel(error)) {
upload.status = UploadStatus.FAILED
reject('Failed assembling the chunks together')
if (isCancel(error) || error instanceof UploadCancelledError) {
upload.status = UploadStatus.CANCELLED
reject(new UploadCancelledError(error))
} else {
upload.status = UploadStatus.FAILED
reject(t('Upload has been cancelled'))
reject(t('Failed assembling the chunks together'))
}

// Cleaning up temp directory
Expand Down Expand Up @@ -607,9 +618,9 @@ export class Uploader {
logger.debug(`Successfully uploaded ${file.name}`, { file, upload })
resolve(upload)
} catch (error) {
if (isCancel(error)) {
upload.status = UploadStatus.FAILED
reject(t('Upload has been cancelled'))
if (isCancel(error) || error instanceof UploadCancelledError) {
upload.status = UploadStatus.CANCELLED
reject(new UploadCancelledError(error))
return
}

Expand All @@ -620,7 +631,7 @@ export class Uploader {

upload.status = UploadStatus.FAILED
logger.error(`Failed uploading ${file.name}`, { error, file, upload })
reject('Failed uploading the file')
reject(t('Failed uploading the file'))
}

// Notify listeners of the upload completion
Expand Down

0 comments on commit 21b6756

Please sign in to comment.