fix(upload): use nextcloud/upload for file uploads, adjust tests

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

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')
 			})
 		})
 

src/components/MessagesList/MessagesGroup/Message/MessagePart/FilePreview.vue

@@ -79,6 +79,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'
@@ -283,6 +284,7 @@ export default {
 		return {
 			isLoading: true,
 			failed: false,
+			uploadManager: null,
 		}
 	},
 	computed: {
@@ -511,14 +513,29 @@ export default {
 			return this.id.startsWith('temp') && this.index && this.uploadId
 		},
 
+		uploadFile() {
+			return this.$store.getters.getUploadFile(this.uploadId, this.index)
+		},
+
+		upload() {
+			return this.uploadManager?.queue.find(item => item._source.includes(this.uploadFile.sharePath))
+		},
+
 		uploadProgress() {
-			if (this.isTemporaryUpload) {
-				if (this.$store.getters.uploadProgress(this.uploadId, this.index)) {
-					return this.$store.getters.uploadProgress(this.uploadId, this.index)
-				}
+			switch (this.uploadFile?.status) {
+			case 'shared':
+			case 'sharing':
+			case 'successUpload':
+				return 100
+			case 'uploading':
+				return this.upload
+					? this.upload._uploaded / this.upload._size * 100
+					: 100 // file was removed from the upload queue, so considering done
+			case 'pendingUpload':
+			case 'initialised':
+			default:
+				return 0
 			}
-			// likely never reached
-			return 0
 		},
 
 		hasTemporaryImageUrl() {
@@ -534,7 +551,19 @@ export default {
 		},
 	},
 
+	watch: {
+		uploadProgress(value) {
+			if (value === 100) {
+				this.uploadManager = null
+			}
+		},
+	},
+
 	mounted() {
+		if (this.isTemporaryUpload && !this.isUploadEditor) {
+			this.uploadManager = getUploader()
+		}
+
 		const img = new Image()
 		img.onerror = () => {
 			this.isLoading = false
@@ -546,6 +575,10 @@ export default {
 		img.src = this.previewUrl
 	},
 
+	beforeDestroy() {
+		this.uploadManager = null
+	},
+
 	methods: {
 		handleClick(event) {
 			if (this.isUploadEditor) {

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'

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,15 +85,17 @@ describe('fileUploadStore', () => {
 
 	describe('uploading', () => {
 		let restoreConsole
+		const uploadMock = jest.fn()
 		const client = {
-			putFileContents: jest.fn(),
+			exists: jest.fn(),
 		}
 
 		beforeEach(() => {
 			storeConfig.getters.getAttachmentFolder = jest.fn().mockReturnValue(() => '/Talk')
 			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,18 +164,19 @@ 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 } })
 
 			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)

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(),