From 1e8dd2c04ebe973400ee6f55f90702eba48d0f77 Mon Sep 17 00:00:00 2001 From: skjnldsv Date: Wed, 18 Dec 2024 19:11:31 +0100 Subject: [PATCH 1/2] feat: add assembling status to UploadPicker Signed-off-by: skjnldsv --- .../UploadPicker/UploadPicker.cy.ts | 6 +- cypress/components/UploadPicker/status.cy.ts | 143 ++++++++++++++++++ cypress/support/component.ts | 2 + l10n/messages.pot | 7 +- lib/components/UploadPicker.vue | 54 ++++--- lib/uploader.ts | 5 + 6 files changed, 194 insertions(+), 23 deletions(-) create mode 100644 cypress/components/UploadPicker/status.cy.ts diff --git a/cypress/components/UploadPicker/UploadPicker.cy.ts b/cypress/components/UploadPicker/UploadPicker.cy.ts index 7b290031..6368da0b 100644 --- a/cypress/components/UploadPicker/UploadPicker.cy.ts +++ b/cypress/components/UploadPicker/UploadPicker.cy.ts @@ -138,9 +138,6 @@ describe('UploadPicker valid uploads', () => { afterEach(() => resetDocument()) it('Uploads a file with chunking', () => { - // Init and reset chunk request spy - const chunksRequestsSpy = cy.spy() - // Intercept tmp upload chunks folder creation cy.intercept('MKCOL', '/remote.php/dav/uploads/*/web-file-upload*', { statusCode: 201, @@ -151,7 +148,6 @@ describe('UploadPicker valid uploads', () => { method: 'PUT', url: '/remote.php/dav/uploads/*/web-file-upload*/*', }, (req) => { - chunksRequestsSpy() req.reply({ statusCode: 201, }) @@ -193,7 +189,7 @@ describe('UploadPicker valid uploads', () => { cy.get('[data-cy-upload-picker] .upload-picker__progress') .as('progress') .should('not.be.visible') - expect(chunksRequestsSpy).to.have.always.been.callCount(26) + cy.get('@chunks.all').should('have.lengthOf', 26) }) }) diff --git a/cypress/components/UploadPicker/status.cy.ts b/cypress/components/UploadPicker/status.cy.ts new file mode 100644 index 00000000..a1ae39ba --- /dev/null +++ b/cypress/components/UploadPicker/status.cy.ts @@ -0,0 +1,143 @@ +/* eslint-disable no-unused-expressions */ +/** + * SPDX-FileCopyrightText: 2022 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +// dist file might not be built when running eslint only +// eslint-disable-next-line import/no-unresolved,n/no-missing-import +import { Folder, Permission } from '@nextcloud/files' +import { generateRemoteUrl } from '@nextcloud/router' +import { getUploader, UploadPicker } from '../../../lib/index.ts' + +let state: string | undefined +before(() => { + cy.window().then((win) => { + state = win.document.body.innerHTML + }) +}) + +const resetDocument = () => { + if (state) { + cy.window().then((win) => { + win.document.body.innerHTML = state! + }) + } +} + +describe('UploadPicker: status testing', () => { + beforeEach(() => { + // Make sure we reset the destination + // so other tests do not interfere + const propsData = { + destination: new Folder({ + id: 56, + owner: 'user', + source: generateRemoteUrl('dav/files/user'), + permissions: Permission.ALL, + root: '/files/user', + }), + } + + // Mount picker + const onPause = cy.spy().as('pausedListener') + const onResume = cy.spy().as('resumedListener') + cy.mount(UploadPicker, { + propsData, + listeners: { + paused: onPause, + resumed: onResume, + }, + }).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] .upload-picker__progress').as('progress').should('exist') + }) + + afterEach(() => resetDocument()) + + it('shows paused status on pause', () => { + // Intercept tmp upload chunks folder creation + cy.intercept('MKCOL', '/remote.php/dav/uploads/*/web-file-upload*', { + statusCode: 201, + }).as('init') + + // Intercept chunks upload + cy.intercept({ + method: 'PUT', + url: '/remote.php/dav/uploads/*/web-file-upload*/*', + }, (req) => { + req.reply({ + statusCode: 201, + }) + }).as('chunks') + + // Intercept final assembly request + const assemblyStartStub = cy.stub().as('assemblyStart') + cy.intercept('MOVE', '/remote.php/dav/uploads/*/web-file-upload*/.file', (req) => { + assemblyStartStub() + req.reply({ + statusCode: 204, + // Fake assembling chunks + delay: 5000, + }) + }).as('assemblyEnd') + + // Start upload + cy.get('@input').attachFile({ + // Fake file of 256MB + fileContent: new Blob([new ArrayBuffer(256 * 1024 * 1024)]), + fileName: 'photos.zip', + mimeType: 'application/zip', + encoding: 'utf8', + lastModified: new Date().getTime(), + }) + + cy.wait('@init').then(() => { + cy.get('[data-cy-upload-picker] .upload-picker__progress') + .as('progress') + .should('be.visible') + }) + + cy.wait('@chunks').then(() => { + cy.get('[data-cy-upload-picker] .upload-picker__progress') + .as('progress') + .should('be.visible') + cy.get('@progress') + .children('progress') + .should('not.have.value', '0') + cy.get('[data-cy-upload-picker-progress-label]').should('not.contain', 'estimating time left') + cy.get('[data-cy-upload-picker-progress-label]').should('not.contain', 'paused') + + cy.wait(1000).then(() => { + getUploader().pause() + }) + + cy.get('[data-cy-upload-picker-progress-label]').should('contain', 'paused') + cy.get('@pausedListener').should('have.been.calledOnce') + + cy.wait(1000).then(() => { + getUploader().start() + }) + + cy.get('[data-cy-upload-picker-progress-label]').should('not.contain', 'paused') + cy.get('@resumedListener').should('have.been.calledOnce') + }) + + // Should will retry until success or timeout + cy.get('@assemblyStart', { timeout: 30000 }).should('have.been.calledOnce').then(() => { + cy.get('[data-cy-upload-picker] .upload-picker__progress') + .as('progress') + .should('be.visible') + + cy.get('[data-cy-upload-picker-progress-label]').should('not.contain', 'paused') + cy.get('[data-cy-upload-picker-progress-label]').should('contain', 'assembling') + }) + + cy.wait('@assemblyEnd', { timeout: 60000 }).then(() => { + cy.get('[data-cy-upload-picker] .upload-picker__progress') + .as('progress') + .should('not.be.visible') + }) + }) +}) diff --git a/cypress/support/component.ts b/cypress/support/component.ts index 6d04272b..66061a24 100644 --- a/cypress/support/component.ts +++ b/cypress/support/component.ts @@ -29,6 +29,8 @@ import { mount } from '@cypress/vue2' // @ts-expect-error Mock window so this is an internal property window._oc_capabilities = { files: {} } +// @ts-expect-error Mock window so this is an internal property +window._oc_debug = true // Example use: // cy.mount(MyComponent) diff --git a/l10n/messages.pot b/l10n/messages.pot index 152a0f10..2d7ed389 100644 --- a/l10n/messages.pot +++ b/l10n/messages.pot @@ -22,7 +22,9 @@ msgstr[0] "" msgstr[1] "" msgid "{seconds} seconds left" -msgstr "" +msgid_plural "{seconds} seconds left" +msgstr[0] "" +msgstr[1] "" #. TRANSLATORS time has the format 00:00:00 msgid "{time} left" @@ -31,6 +33,9 @@ msgstr "" msgid "a few seconds left" msgstr "" +msgid "assembling" +msgstr "" + msgid "Cancel" msgstr "" diff --git a/lib/components/UploadPicker.vue b/lib/components/UploadPicker.vue index 3210cd15..fde44638 100644 --- a/lib/components/UploadPicker.vue +++ b/lib/components/UploadPicker.vue @@ -113,7 +113,7 @@ :value="progress" size="medium" />

- {{ timeLeft }} + {{ status }}

@@ -167,7 +167,7 @@ import IconUpload from 'vue-material-design-icons/Upload.vue' import { getUploader } from '../index.ts' import { Status } from '../uploader.ts' import { Status as UploadStatus } from '../upload.ts' -import { t } from '../utils/l10n.ts' +import { n, t } from '../utils/l10n.ts' import { uploadConflictHandler } from '../utils/conflicts.ts' import logger from '../utils/logger.ts' @@ -259,7 +259,7 @@ export default defineComponent({ return { eta: null as null|ReturnType, openedMenu: false, - timeLeft: '', + status: '', newFileMenuEntries: [] as Entry[], uploadManager: getUploader(), @@ -303,16 +303,26 @@ export default defineComponent({ }, hasFailure(): boolean { - return this.queue?.filter((upload: Upload) => upload.status === UploadStatus.FAILED).length !== 0 + return this.queue.some((upload: Upload) => upload.status === UploadStatus.FAILED) }, isUploading(): boolean { - return this.queue?.filter((upload: Upload) => upload.status !== UploadStatus.CANCELLED).length > 0 + return this.queue.some((upload: Upload) => upload.status !== UploadStatus.CANCELLED) }, - isAssembling(): boolean { - return this.queue?.filter((upload: Upload) => upload.status === UploadStatus.ASSEMBLING).length !== 0 + isOnlyAssembling(): boolean { + return this.queue.every((upload: Upload) => ( + // ignore empty uploads or meta uploads + upload.size === 0 + // all the uploads are assembling or finished + || upload.status === UploadStatus.ASSEMBLING + || upload.status === UploadStatus.FINISHED + )) }, isPaused(): boolean { - return this.uploadManager.info?.status === Status.PAUSED + return this.uploaderStatus === Status.PAUSED + }, + + uploaderStatus(): Status { + return this.uploadManager.info.status }, buttonLabel(): string { @@ -356,12 +366,17 @@ export default defineComponent({ this.updateStatus() }, - isPaused(isPaused) { - if (isPaused) { + uploaderStatus(status, oldStatus) { + if (status === Status.PAUSED) { this.$emit('paused', this.queue) - } else { + } else if (oldStatus === Status.PAUSED) { this.$emit('resumed', this.queue) } + this.updateStatus() + }, + + isOnlyAssembling() { + this.updateStatus() }, }, @@ -453,28 +468,33 @@ export default defineComponent({ updateStatus() { if (this.isPaused) { - this.timeLeft = t('paused') + this.status = t('paused') + return + } + + if (this.isOnlyAssembling) { + this.status = t('assembling') return } - const estimate = Math.round(this.eta!.estimate()) + const estimate = Math.round(this.eta?.estimate?.() || 0) if (estimate === Infinity) { - this.timeLeft = t('estimating time left') + this.status = t('estimating time left') return } if (estimate < 10) { - this.timeLeft = t('a few seconds left') + this.status = t('a few seconds left') return } if (estimate > 60) { const date = new Date(0) date.setSeconds(estimate) const time = date.toISOString().slice(11, 11 + 8) - this.timeLeft = t('{time} left', { time }) // TRANSLATORS time has the format 00:00:00 + this.status = t('{time} left', { time }) // TRANSLATORS time has the format 00:00:00 return } - this.timeLeft = t('{seconds} seconds left', { seconds: estimate }) + this.status = n('{seconds} seconds left', '{seconds} seconds left', estimate, { seconds: estimate }) }, setDestination(destination: Folder) { diff --git a/lib/uploader.ts b/lib/uploader.ts index fc4cf2a0..176e09f8 100644 --- a/lib/uploader.ts +++ b/lib/uploader.ts @@ -175,6 +175,8 @@ export class Uploader { public pause() { this._jobQueue.pause() this._queueStatus = Status.PAUSED + this.updateStats() + logger.debug('Upload paused') } /** @@ -184,6 +186,7 @@ export class Uploader { this._jobQueue.start() this._queueStatus = Status.UPLOADING this.updateStats() + logger.debug('Upload resumed') } /** @@ -547,6 +550,8 @@ export class Uploader { await Promise.all(chunksQueue) this.updateStats() + // Assemble the chunks + upload.status = UploadStatus.ASSEMBLING upload.response = await axios.request({ method: 'MOVE', url: `${tempUrl}/.file`, From 5d015761b7353cf4ef51998c8b657a794cccac3a Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Tue, 25 Feb 2025 15:28:30 +0100 Subject: [PATCH 2/2] fix(uploader): Ensure jobQueue is not reset (empty) during chunk assembling Signed-off-by: Ferdinand Thiessen --- .../components/UploadPicker/progress.cy.ts | 74 +++++++++++++++++- lib/uploader.ts | 75 ++++++++++--------- 2 files changed, 112 insertions(+), 37 deletions(-) diff --git a/cypress/components/UploadPicker/progress.cy.ts b/cypress/components/UploadPicker/progress.cy.ts index 1d246112..4ca16c2b 100644 --- a/cypress/components/UploadPicker/progress.cy.ts +++ b/cypress/components/UploadPicker/progress.cy.ts @@ -60,6 +60,9 @@ describe('UploadPicker: progress handling', () => { }), } + // Start paused + getUploader(false, true).pause() + // Mount picker cy.mount(UploadPicker, { propsData, @@ -74,7 +77,6 @@ describe('UploadPicker: progress handling', () => { it('has increasing progress bar during non-chunked upload', () => { // Start in paused mode const uploader = getUploader() - uploader.pause() cy.get('@input').attachFile({ // file of 5 MiB @@ -150,7 +152,6 @@ describe('UploadPicker: progress handling', () => { // Start in paused mode const uploader = getUploader() - uploader.pause() // 3 MiB/s meaning upload will take 5 seconds throttleUpload(3 * 1024 * 1024) @@ -205,6 +206,74 @@ describe('UploadPicker: progress handling', () => { cy.get('@progress') .should('not.be.visible') }) + + it('shows the progress bar while assembling', () => { + // Maximum the responses can take + Cypress.config({ defaultCommandTimeout: 7000 }) + + const { promise, resolve } = Promise.withResolvers() + + cy.intercept('PUT', '/remote.php/dav/files/user/file.txt', { statusCode: 201 }).as('upload') + cy.intercept('MKCOL', '/remote.php/dav/uploads/user/*', { statusCode: 201 }).as('mkdir') + cy.intercept('PUT', '/remote.php/dav/uploads/user/*/*', (rq) => { + rq.reply({ statusCode: 201 }) + if (rq.url.endsWith('/2')) { + rq.on('response', async () => await promise) + } + }).as('uploadBig') + cy.intercept('MOVE', '/remote.php/dav/uploads/user/*/.file', { statusCode: 201, delay: 1000 }).as('move') + + // Start in paused mode + const uploader = getUploader() + + cy.get('@input').attachFile([ + { + // file of 5 MiB so it is not chunked + fileContent: new Blob([new ArrayBuffer(5 * 1024 * 1024)]), + fileName: 'file.txt', + mimeType: 'text/plain', + encoding: 'utf8', + lastModified: new Date().getTime(), + }, + { + // file of 15 MiB so it is chunked in 10MiB and 5 MiB + fileContent: new Blob([new ArrayBuffer(15 * 1024 * 1024)]), + fileName: 'big-file.txt', + mimeType: 'text/plain', + encoding: 'utf8', + lastModified: new Date().getTime(), + }, + ]) + + // 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(() => uploader.start()) + + // MKCOL was successfully so the upload can begin + cy.wait('@mkdir') + + cy.get('@progress', { timeout: 2000 }) + .should((el) => expect(el.val()).to.be.greaterThan(10)) + .and((el) => expect(el.val()).to.be.lessThan(95)) + + cy.wait('@upload') + cy.wait('@uploadBig') + .then(() => resolve()) + + cy.get('@progressLabel') + .should('be.visible') + .and('contain.text', 'assembling') + + cy.wait('@move') + + cy.get('@progress') + .should('not.be.visible') + }) }) describe('UploadPicker: reset progress on retry', () => { @@ -217,6 +286,7 @@ describe('UploadPicker: reset progress on retry', () => { cy.window() .then((win) => { // Internal global variable + // eslint-disable-next-line @typescript-eslint/no-explicit-any (win as any)._oc_capabilities = { files: { chunked_upload: { max_parallel_count: 1 } } } }) diff --git a/lib/uploader.ts b/lib/uploader.ts index 176e09f8..07b784a0 100644 --- a/lib/uploader.ts +++ b/lib/uploader.ts @@ -545,46 +545,51 @@ export class Uploader { chunksQueue.push(this._jobQueue.add(request)) } - try { - // Once all chunks are sent, assemble the final file - await Promise.all(chunksQueue) - this.updateStats() + const request = async () => { + try { + // Once all chunks are sent, assemble the final file + await Promise.all(chunksQueue) - // Assemble the chunks - upload.status = UploadStatus.ASSEMBLING - upload.response = await axios.request({ - method: 'MOVE', - url: `${tempUrl}/.file`, - headers: { - ...this._customHeaders, - 'X-OC-Mtime': Math.floor(file.lastModified / 1000), - 'OC-Total-Length': file.size, - Destination: encodedDestinationFile, - }, - }) + // Assemble the chunks + upload.status = UploadStatus.ASSEMBLING + this.updateStats() - this.updateStats() - upload.status = UploadStatus.FINISHED - logger.debug(`Successfully uploaded ${file.name}`, { file, upload }) - resolve(upload) - } catch (error) { - if (isCancel(error) || error instanceof UploadCancelledError) { - upload.status = UploadStatus.CANCELLED - reject(new UploadCancelledError(error)) - } else { - upload.status = UploadStatus.FAILED - reject(t('Failed assembling the chunks together')) - } + // Send the assemble request + upload.response = await axios.request({ + method: 'MOVE', + url: `${tempUrl}/.file`, + headers: { + ...this._customHeaders, + 'X-OC-Mtime': Math.floor(file.lastModified / 1000), + 'OC-Total-Length': file.size, + Destination: encodedDestinationFile, + }, + }) + upload.status = UploadStatus.FINISHED + this.updateStats() - // Cleaning up temp directory - axios.request({ - method: 'DELETE', - url: `${tempUrl}`, - }) + logger.debug(`Successfully uploaded ${file.name}`, { file, upload }) + resolve(upload) + } catch (error) { + if (isCancel(error) || error instanceof UploadCancelledError) { + upload.status = UploadStatus.CANCELLED + reject(new UploadCancelledError(error)) + } else { + upload.status = UploadStatus.FAILED + reject(t('Failed assembling the chunks together')) + } + // Cleaning up temp directory + axios.request({ + method: 'DELETE', + url: `${tempUrl}`, + }) + } finally { + // Notify listeners of the upload completion + this._notifyAll(upload) + } } - // Notify listeners of the upload completion - this._notifyAll(upload) + this._jobQueue.add(request) } else { logger.debug('Initializing regular upload', { file, upload })