From 0cd7591dd525833111721fa236e0177d4f9efe71 Mon Sep 17 00:00:00 2001 From: Maksim Sukharev Date: Sat, 20 Jan 2024 01:22:38 +0100 Subject: [PATCH] fix(upload): use nextcloud/upload for file uploads, adjust tests Signed-off-by: Maksim Sukharev --- .../Message/MessagePart/FilePreview.spec.js | 23 +++++++--- .../Message/MessagePart/FilePreview.vue | 42 +++++++++++++++---- src/store/fileUploadStore.js | 28 +++---------- src/store/fileUploadStore.spec.js | 36 +++++++--------- src/test-setup.js | 4 ++ 5 files changed, 76 insertions(+), 57 deletions(-) diff --git a/src/components/MessagesList/MessagesGroup/Message/MessagePart/FilePreview.spec.js b/src/components/MessagesList/MessagesGroup/Message/MessagePart/FilePreview.spec.js index 683f46071fa6..5a5619b4dd7e 100644 --- a/src/components/MessagesList/MessagesGroup/Message/MessagePart/FilePreview.spec.js +++ b/src/components/MessagesList/MessagesGroup/Message/MessagePart/FilePreview.spec.js @@ -7,6 +7,7 @@ import PlayCircleOutline from 'vue-material-design-icons/PlayCircleOutline.vue' import { getCapabilities } from '@nextcloud/capabilities' import { imagePath, generateRemoteUrl } from '@nextcloud/router' +import { getUploader } from '@nextcloud/upload' import NcButton from '@nextcloud/vue/dist/Components/NcButton.js' @@ -151,22 +152,32 @@ describe('FilePreview.vue', () => { }) describe('uploading', () => { - let uploadProgressMock + const path = '/Talk/path-to-file.png' + let getUploadFileMock beforeEach(() => { - uploadProgressMock = jest.fn() - testStoreConfig.modules.fileUploadStore.getters.uploadProgress = () => uploadProgressMock + getUploadFileMock = jest.fn(() => ({ + sharePath: path, + status: 'uploading', + })) + testStoreConfig.modules.fileUploadStore.getters.getUploadFile = () => getUploadFileMock store = new Vuex.Store(testStoreConfig) }) test('renders progress bar while uploading', async () => { + getUploader.mockImplementation(() => ({ + queue: [{ + _source: path, + _uploaded: 85, + _size: 100, + }], + })) + propsData.id = 'temp-123' propsData.index = 'index-1' propsData.uploadId = 1000 propsData.localUrl = 'blob:XYZ' - uploadProgressMock.mockReturnValue(85) - const wrapper = shallowMount(FilePreview, { localVue, store, @@ -182,7 +193,7 @@ describe('FilePreview.vue', () => { expect(progressEl.exists()).toBe(true) expect(progressEl.props('value')).toBe(85) - expect(uploadProgressMock).toHaveBeenCalledWith(1000, 'index-1') + expect(getUploadFileMock).toHaveBeenCalledWith(1000, 'index-1') }) }) diff --git a/src/components/MessagesList/MessagesGroup/Message/MessagePart/FilePreview.vue b/src/components/MessagesList/MessagesGroup/Message/MessagePart/FilePreview.vue index 6a3b17128a4c..da00f55c7009 100644 --- a/src/components/MessagesList/MessagesGroup/Message/MessagePart/FilePreview.vue +++ b/src/components/MessagesList/MessagesGroup/Message/MessagePart/FilePreview.vue @@ -82,6 +82,7 @@ import PlayCircleOutline from 'vue-material-design-icons/PlayCircleOutline.vue' import { getCapabilities } from '@nextcloud/capabilities' import { encodePath } from '@nextcloud/paths' import { generateUrl, imagePath, generateRemoteUrl } from '@nextcloud/router' +import { getUploader } from '@nextcloud/upload' import NcButton from '@nextcloud/vue/dist/Components/NcButton.js' import NcProgressBar from '@nextcloud/vue/dist/Components/NcProgressBar.js' @@ -286,6 +287,8 @@ export default { return { isLoading: true, failed: false, + uploadManager: null, + uploadProgress: 0, } }, computed: { @@ -514,14 +517,12 @@ export default { return this.id.startsWith('temp') && this.index && this.uploadId }, - uploadProgress() { - if (this.isTemporaryUpload) { - if (this.$store.getters.uploadProgress(this.uploadId, this.index)) { - return this.$store.getters.uploadProgress(this.uploadId, this.index) - } - } - // likely never reached - return 0 + uploadFile() { + return this.$store.getters.getUploadFile(this.uploadId, this.index) + }, + + upload() { + return this.uploadManager?.queue.find(item => item._source.includes(this.uploadFile.sharePath)) }, hasTemporaryImageUrl() { @@ -537,6 +538,31 @@ export default { }, }, + watch: { + isTemporaryUpload: { + immediate: true, + handler(value) { + this.uploadManager = value ? getUploader() : null + } + }, + 'uploadFile.status'(value) { + if (['successUpload', 'sharing', 'shared'].includes(value)) { + this.uploadProgress = 100 + } + }, + upload: { + immediate: true, + deep: true, + handler(value) { + if (!value) { + return + } + const progress = value._uploaded / value._size * 100 + this.uploadProgress = Math.max(this.uploadProgress, progress) + } + } + }, + mounted() { const img = new Image() img.onerror = () => { diff --git a/src/store/fileUploadStore.js b/src/store/fileUploadStore.js index b4ecec2f8990..eac5f68c072f 100644 --- a/src/store/fileUploadStore.js +++ b/src/store/fileUploadStore.js @@ -25,6 +25,7 @@ import Vue from 'vue' import { showError } from '@nextcloud/dialogs' import { loadState } from '@nextcloud/initial-state' import moment from '@nextcloud/moment' +import { getUploader } from '@nextcloud/upload' import { getDavClient } from '../services/DavClient.js' import { EventBus } from '../services/EventBus.js' @@ -104,12 +105,8 @@ const getters = { return state.localUrls[referenceId] }, - uploadProgress: (state) => (uploadId, index) => { - if (state.uploads[uploadId]?.files[index]) { - return state.uploads[uploadId].files[index].uploadedSize / state.uploads[uploadId].files[index].totalSize * 100 - } else { - return 0 - } + getUploadFile: (state) => (uploadId, index) => { + return state.uploads[uploadId]?.files[index] }, currentUploadId: (state) => { @@ -142,7 +139,6 @@ const mutations = { file, status: 'initialised', totalSize: file.size, - uploadedSize: 0, temporaryMessage, }) Vue.set(state.localUrls, temporaryMessage.referenceId, localUrl) @@ -194,11 +190,6 @@ const mutations = { state.attachmentFolder = attachmentFolder }, - // Sets uploaded amount of bytes - setUploadedSize(state, { uploadId, index, uploadedSize }) { - state.uploads[uploadId].files[index].uploadedSize = uploadedSize - }, - // Set temporary message for each file setTemporaryMessageForFile(state, { uploadId, index, temporaryMessage }) { console.debug('uploadId: ' + uploadId + ' index: ' + index) @@ -406,23 +397,14 @@ const actions = { * @param {string} data.uploadId The unique uploadId */ async processUpload(context, { token, uploadId }) { - const client = getDavClient() - const userRoot = '/files/' + context.getters.getUserId() - const performUpload = async ([index, uploadedFile]) => { const currentFile = uploadedFile.file const fileName = (currentFile.newName || currentFile.name) try { context.commit('markFileAsUploading', { uploadId, index }) - const currentFileBuffer = await new Blob([currentFile]).arrayBuffer() - await client.putFileContents(userRoot + uploadedFile.sharePath, currentFileBuffer, { - onUploadProgress: progress => { - const uploadedSize = progress.loaded - context.commit('setUploadedSize', { state, uploadId, index, uploadedSize }) - }, - contentLength: currentFile.size, - }) + const uploader = getUploader() + await uploader.upload(uploadedFile.sharePath, currentFile) context.commit('markFileAsSuccessUpload', { uploadId, index }) } catch (exception) { let reason = 'failed-upload' diff --git a/src/store/fileUploadStore.spec.js b/src/store/fileUploadStore.spec.js index 37b6c3d5cbb2..53f4360d89e8 100644 --- a/src/store/fileUploadStore.spec.js +++ b/src/store/fileUploadStore.spec.js @@ -5,6 +5,7 @@ import { createPinia, setActivePinia } from 'pinia' import Vuex from 'vuex' import { showError } from '@nextcloud/dialogs' +import { getUploader } from '@nextcloud/upload' // eslint-disable-next-line no-unused-vars -- required for testing import storeConfig from './storeConfig.js' @@ -84,8 +85,9 @@ describe('fileUploadStore', () => { describe('uploading', () => { let restoreConsole + const uploadMock = jest.fn() const client = { - putFileContents: jest.fn(), + exists: jest.fn(), } beforeEach(() => { @@ -93,6 +95,7 @@ describe('fileUploadStore', () => { store = new Vuex.Store(storeConfig) restoreConsole = mockConsole(['error', 'debug']) getDavClient.mockReturnValue(client) + getUploader.mockReturnValue({ upload: uploadMock }) }) afterEach(() => { @@ -150,7 +153,6 @@ describe('fileUploadStore', () => { size: 123, lastModified: Date.UTC(2021, 3, 27, 15, 30, 0), } - const fileBuffer = await new Blob([file]).arrayBuffer() await store.dispatch('initialiseUpload', { uploadId: 'upload-id1', @@ -162,6 +164,7 @@ describe('fileUploadStore', () => { const uniqueFileName = '/Talk/' + file.name + 'uniq' findUniquePath.mockResolvedValueOnce({ uniquePath: uniqueFileName, suffix: 1 }) + uploadMock.mockResolvedValue() shareFile.mockResolvedValue() await store.dispatch('uploadFiles', { token: 'XXTOKENXX', uploadId: 'upload-id1', caption: 'text-caption', options: { silent: true } }) @@ -169,11 +172,11 @@ describe('fileUploadStore', () => { expect(findUniquePath).toHaveBeenCalledTimes(1) 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()) + expect(uploadMock).toHaveBeenCalledTimes(1) + expect(uploadMock).toHaveBeenCalledWith(uniqueFileName, file) expect(shareFile).toHaveBeenCalledTimes(1) - expect(shareFile).toHaveBeenCalledWith(`/${uniqueFileName}`, 'XXTOKENXX', 'reference-id-1', '{"caption":"text-caption","silent":true}') + expect(shareFile).toHaveBeenCalledWith(uniqueFileName, 'XXTOKENXX', 'reference-id-1', '{"caption":"text-caption","silent":true}') expect(mockedActions.addTemporaryMessage).toHaveBeenCalledTimes(1) expect(store.getters.currentUploadId).not.toBeDefined() @@ -193,10 +196,6 @@ describe('fileUploadStore', () => { lastModified: Date.UTC(2021, 3, 25, 15, 30, 0), } const files = [file1, file2] - const fileBuffers = [ - await new Blob([file1]).arrayBuffer(), - await new Blob([file2]).arrayBuffer(), - ] await store.dispatch('initialiseUpload', { uploadId: 'upload-id1', @@ -216,17 +215,15 @@ describe('fileUploadStore', () => { await store.dispatch('uploadFiles', { token: 'XXTOKENXX', uploadId: 'upload-id1', caption: 'text-caption', options: { silent: false } }) expect(findUniquePath).toHaveBeenCalledTimes(2) - expect(client.putFileContents).toHaveBeenCalledTimes(2) - expect(shareFile).toHaveBeenCalledTimes(2) - + expect(uploadMock).toHaveBeenCalledTimes(2) for (const index in files) { - 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()) + expect(findUniquePath).toHaveBeenNthCalledWith(+index + 1, client, '/files/current-user', '/Talk/' + files[index].name, undefined) + expect(uploadMock).toHaveBeenNthCalledWith(+index + 1, `/Talk/${files[index].name}uniq`, files[index]) } expect(shareFile).toHaveBeenCalledTimes(2) - expect(shareFile).toHaveBeenNthCalledWith(1, '//Talk/' + files[0].name + 'uniq', 'XXTOKENXX', 'reference-id-1', '{}') - expect(shareFile).toHaveBeenNthCalledWith(2, '//Talk/' + files[1].name + 'uniq', 'XXTOKENXX', 'reference-id-2', '{"caption":"text-caption"}') + expect(shareFile).toHaveBeenNthCalledWith(1, '/Talk/' + files[0].name + 'uniq', 'XXTOKENXX', 'reference-id-1', '{}') + expect(shareFile).toHaveBeenNthCalledWith(2, '/Talk/' + files[1].name + 'uniq', 'XXTOKENXX', 'reference-id-2', '{"caption":"text-caption"}') expect(mockedActions.addTemporaryMessage).toHaveBeenCalledTimes(2) expect(store.getters.currentUploadId).not.toBeDefined() @@ -250,15 +247,14 @@ describe('fileUploadStore', () => { findUniquePath .mockResolvedValueOnce({ uniquePath: '/Talk/' + files[0].name + 'uniq', suffix: 1 }) - client.putFileContents.mockRejectedValueOnce({ + uploadMock.mockRejectedValueOnce({ response: { status: 403, }, }) - await store.dispatch('uploadFiles', { token: 'XXTOKENXX', uploadId: 'upload-id1', options: { silent: false } }) - expect(client.putFileContents).toHaveBeenCalledTimes(1) + expect(uploadMock).toHaveBeenCalledTimes(1) expect(shareFile).not.toHaveBeenCalled() expect(mockedActions.addTemporaryMessage).toHaveBeenCalledTimes(1) @@ -299,7 +295,7 @@ describe('fileUploadStore', () => { await store.dispatch('uploadFiles', { token: 'XXTOKENXX', uploadId: 'upload-id1', options: { silent: false } }) - expect(client.putFileContents).toHaveBeenCalledTimes(1) + expect(uploadMock).toHaveBeenCalledTimes(1) expect(shareFile).toHaveBeenCalledTimes(1) expect(mockedActions.addTemporaryMessage).toHaveBeenCalledTimes(1) diff --git a/src/test-setup.js b/src/test-setup.js index 348423aa94e2..07bfd187e48e 100644 --- a/src/test-setup.js +++ b/src/test-setup.js @@ -39,6 +39,10 @@ jest.mock('@nextcloud/initial-state', () => ({ }), })) +jest.mock('@nextcloud/upload', () => ({ + getUploader: jest.fn(), +})) + window.IntersectionObserver = jest.fn(() => ({ observe: jest.fn(), unobserve: jest.fn(),