Skip to content

Commit

Permalink
fix(upload): use nextcloud/upload for file uploads, adjust tests
Browse files Browse the repository at this point in the history
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
  • Loading branch information
Antreesy committed Jan 22, 2024
1 parent ff9c03b commit 0cd7591
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down Expand Up @@ -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,
Expand All @@ -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')
})
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -286,6 +287,8 @@ export default {
return {
isLoading: true,
failed: false,
uploadManager: null,
uploadProgress: 0,
}
},
computed: {
Expand Down Expand Up @@ -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() {
Expand All @@ -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 = () => {
Expand Down
28 changes: 5 additions & 23 deletions src/store/fileUploadStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -142,7 +139,6 @@ const mutations = {
file,
status: 'initialised',
totalSize: file.size,
uploadedSize: 0,
temporaryMessage,
})
Vue.set(state.localUrls, temporaryMessage.referenceId, localUrl)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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'
Expand Down
36 changes: 16 additions & 20 deletions src/store/fileUploadStore.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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',
Expand All @@ -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()
Expand All @@ -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',
Expand All @@ -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()
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions src/test-setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down

0 comments on commit 0cd7591

Please sign in to comment.