Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[full-ci] Switch upload logic to Uppy #6202

Merged
merged 23 commits into from
Apr 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
d5f663e
WIP
pascalwengerter Apr 4, 2022
093f9f7
WIP uppy
JammingBen Apr 11, 2022
2475b93
Refactor uppy code
JammingBen Apr 13, 2022
b75b450
Update changelog item, move uppy dependencies to web-runtime package
JammingBen Apr 13, 2022
c885318
Fix unit tests
JammingBen Apr 13, 2022
640f236
Remove logs and unnecessary events
JammingBen Apr 13, 2022
5951d9d
Minor fixes and improvements
JammingBen Apr 14, 2022
648da58
Fix ocis e2e tests
JammingBen Apr 14, 2022
39f8ff4
Fix more e2e tests
JammingBen Apr 14, 2022
7f1b2ea
Prevent form fields from being added to uploaded file contents
JammingBen Apr 25, 2022
435ad36
Properly encode file name for XHR uploads
JammingBen Apr 25, 2022
c162002
update fileDrop acceptance tests
saw-jan Apr 25, 2022
fedb11b
Remove Uppy drop target plugin on unmount
JammingBen Apr 25, 2022
0684dff
Fix issues when uploading nested folders
JammingBen Apr 25, 2022
5182cba
change rollup plugin order to apply injections for ts files also
fschade Apr 25, 2022
52934d6
Fix issue with missing slashes when uploading
JammingBen Apr 26, 2022
99aba33
Ensure that uppy uses only one upload plugin at a time
JammingBen Apr 26, 2022
495aa91
Fix issue when updating the store for newly created folders
JammingBen Apr 26, 2022
ae25b61
Remove Uppy debug state
JammingBen Apr 27, 2022
f887105
Small fixes and polishing
JammingBen Apr 27, 2022
af1bc4b
Fix tests, polishing
JammingBen Apr 27, 2022
e38c3a2
Undo change in sunday,monday.txt file
JammingBen Apr 27, 2022
9198b07
Bump owncloud-test-middleware docker image version to 1.5.0
JammingBen Apr 27, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .drone.star
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ OC_CI_HUGO = "owncloudci/hugo:0.89.4"
OC_CI_NODEJS = "owncloudci/nodejs:14"
OC_CI_PHP = "owncloudci/php:7.4"
OC_CI_WAIT_FOR = "owncloudci/wait-for:latest"
OC_TESTING_MIDDLEWARE = "owncloud/owncloud-test-middleware:1.4.2"
OC_TESTING_MIDDLEWARE = "owncloud/owncloud-test-middleware:1.5.0"
OC_UBUNTU = "owncloud/ubuntu:20.04"
PLUGINS_DOCKER = "plugins/docker:18.09"
PLUGINS_DOWNSTREAM = "plugins/downstream"
Expand Down
12 changes: 12 additions & 0 deletions changelog/unreleased/enhancement-resumeable-uploads
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
Enhancement: Resumable uploads

We've implemented Uppy as a library for handling uploads. This concludes the following features and changes:

- Resumable uploads when the backend supports the Tus-protocol
- A nice looking overview for all files that have been uploaded successfully or failed to upload
- Navigation across Web while uploads are in progress
- Improved rendering of uploadProgress-visualization
- Removed `vue2-dropzone` and `vue-drag-drop` libraries

https://github.com/owncloud/web/pull/6202
https://github.com/owncloud/web/issues/6268
3 changes: 1 addition & 2 deletions packages/web-app-files/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
"description": "ownCloud web files",
"license": "AGPL-3.0",
"dependencies": {
"copy-to-clipboard": "^3.3.1",
"vue2-dropzone": "^3.6.0"
"copy-to-clipboard": "^3.3.1"
}
}
16 changes: 3 additions & 13 deletions packages/web-app-files/src/App.vue
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
<template>
<main id="files" class="oc-flex oc-height-1-1">
<div
ref="filesListWrapper"
tabindex="-1"
class="files-list-wrapper oc-width-expand"
@dragover="$_ocApp_dragOver"
>
<div ref="filesListWrapper" tabindex="-1" class="files-list-wrapper oc-width-expand">
<router-view id="files-view" />
</div>
<side-bar
Expand All @@ -25,7 +20,7 @@
</template>
<script lang="ts">
import Mixins from './mixins'
import { mapActions, mapGetters, mapState } from 'vuex'
import { mapActions, mapState } from 'vuex'
import SideBar from './components/SideBar/SideBar.vue'
import { defineComponent } from '@vue/composition-api'

Expand All @@ -35,7 +30,6 @@ export default defineComponent({
},
mixins: [Mixins],
computed: {
...mapGetters('Files', ['dropzone']),
...mapState('Files/sidebar', {
sidebarClosed: 'closed',
sidebarActivePanel: 'activePanel'
Expand All @@ -62,7 +56,7 @@ export default defineComponent({
},

methods: {
...mapActions('Files', ['dragOver', 'resetFileSelection']),
...mapActions('Files', ['resetFileSelection']),
...mapActions('Files/sidebar', {
closeSidebar: 'close',
setActiveSidebarPanel: 'setActivePanel'
Expand All @@ -75,10 +69,6 @@ export default defineComponent({
to: this.$refs.filesSidebar?.$el,
revert: event === 'beforeDestroy'
})
},
$_ocApp_dragOver(event) {
const hasfileInEvent = (event.dataTransfer.types || []).some((e) => e === 'Files')
this.dragOver(hasfileInEvent)
}
}
})
Expand Down
217 changes: 143 additions & 74 deletions packages/web-app-files/src/components/AppBar/CreateAndUpload.vue
Original file line number Diff line number Diff line change
Expand Up @@ -70,15 +70,6 @@
<translate>New folder</translate>
</oc-button>
</template>
<file-drop
v-if="!uploadOrFileCreationBlocked"
:root-path="currentPath"
:path="currentPath"
:headers="headers"
@success="onFileSuccess"
@error="onFileError"
@progress="onFileProgress"
/>
<oc-button
id="upload-menu-btn"
key="upload-menu-btn-enabled"
Expand All @@ -99,34 +90,20 @@
>
<oc-list id="upload-list">
<li>
<folder-upload
:root-path="currentPath"
:path="currentPath"
:headers="headers"
@success="onFileSuccess"
@error="onFileError"
@progress="onFileProgress"
/>
<folder-upload ref="folder-upload" />
</li>
<li>
<file-upload
:path="currentPath"
:headers="headers"
@success="onFileSuccess"
@error="onFileError"
@progress="onFileProgress"
/>
<file-upload ref="file-upload" btn-class="oc-width-1-1" />
</li>
</oc-list>
</oc-drop>
</div>
</template>

<script>
<script lang="ts">
import { mapActions, mapGetters, mapMutations, mapState } from 'vuex'
import pathUtil from 'path'

import Mixins from '../../mixins'
import MixinFileActions, { EDITOR_MODE_CREATE } from '../../mixins/fileActions'
import { buildResource, buildWebDavFilesPath, buildWebDavSpacesPath } from '../../helpers/resources'
import { isLocationPublicActive, isLocationSpacesActive } from '../../router'
Expand All @@ -135,19 +112,46 @@ import { useAppDefaults } from 'web-pkg/src/composables'

import { DavProperties, DavProperty } from 'web-pkg/src/constants'

import FileDrop from './Upload/FileDrop.vue'
// TODO: Simplify to one UploadButton component and fill from here
pascalwengerter marked this conversation as resolved.
Show resolved Hide resolved
import FileUpload from './Upload/FileUpload.vue'
import FolderUpload from './Upload/FolderUpload.vue'
import { defineComponent, getCurrentInstance, onMounted, onUnmounted } from '@vue/composition-api'
import { UppyResource, useUpload } from 'web-runtime/src/composables/upload'
import { useUploadHelpers } from '../../composables/upload'

export default {
export default defineComponent({
components: {
FileDrop,
FileUpload,
FolderUpload
},
mixins: [Mixins, MixinFileActions],
mixins: [MixinFileActions],
setup() {
const instance = getCurrentInstance().proxy
const uppyService = instance.$uppyService

onMounted(() => {
uppyService.$on('filesSelected', instance.onFilesSelected)
uppyService.$on('uploadSuccess', instance.onFileSuccess)
uppyService.$on('uploadError', instance.onFileError)

uppyService.useDropTarget({
targetSelector: '#files-view',
uppyService
})
})

onUnmounted(() => {
uppyService.$off('filesSelected', instance.onFilesSelected)
uppyService.$off('uploadSuccess', instance.onFileSuccess)
uppyService.$off('uploadError', instance.onFileError)
uppyService.removeDropTarget()
})

return {
...useUpload({
uppyService
}),
...useUploadHelpers(),
isPersonalLocation: useActiveLocation(isLocationSpacesActive, 'files-spaces-personal-home'),
isPublicLocation: useActiveLocation(isLocationPublicActive, 'files-public-files'),
isSpacesProjectsLocation: useActiveLocation(isLocationSpacesActive, 'files-spaces-projects'),
Expand Down Expand Up @@ -176,29 +180,6 @@ export default {
return mimeTypes.filter((mimetype) => mimetype.allow_creation) || []
},

currentPath() {
const path = this.$route.params.item || ''
if (path.endsWith('/')) {
return path
}
return path + '/'
},

headers() {
if (this.isPublicLocation) {
const password = this.publicLinkPassword

if (password) {
return { Authorization: 'Basic ' + Buffer.from('public:' + password).toString('base64') }
}

return {}
}
return {
Authorization: 'Bearer ' + this.getToken
}
},

showActions() {
return !(this.uploadOrFileCreationBlocked && this.isPublicLocation)
},
Expand Down Expand Up @@ -265,20 +246,23 @@ export default {
}
},
methods: {
...mapActions('Files', ['updateFileProgress', 'loadIndicators']),
...mapActions(['showMessage', 'createModal', 'setModalInputErrorMessage']),
...mapActions('Files', ['loadPreview', 'loadIndicators']),
...mapActions(['openFile', 'showMessage', 'createModal', 'setModalInputErrorMessage']),
...mapMutations('Files', ['UPSERT_RESOURCE']),
...mapMutations(['SET_QUOTA']),

async onFileSuccess(event, file) {
async onFileSuccess(file: UppyResource) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somewhere in here on public views (and interestingly only for files) UPSERT fails with Cannot read properties of undefined (reading '0')

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you provide some steps to reproduce this? I tried several times, but I couldn't get this error.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me check, it was fairly obvious (since the uploaded file didn't get upserted) but maybe was fixed by the formData config change?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm could be. I noticed another strange behavior though: When you upload file.txt to a public page, reload, then upload the same file again and overwrite it, it will appear twice in the file list. It happens on master as well

try {
if (file.name) {
file = file.name
let pathFileWasUploadedTo = file.meta.currentFolder
if (file.meta.relativeFolder) {
pathFileWasUploadedTo += file.meta.relativeFolder
}

const fileIsInCurrentPath = pathFileWasUploadedTo === this.currentPath

await this.$nextTick()

let path = pathUtil.join(this.currentPath, file)
let path = pathUtil.join(pathFileWasUploadedTo, file.name)
let resource

if (this.isPersonalLocation) {
Expand All @@ -297,14 +281,17 @@ export default {

resource = buildResource(resource)

this.UPSERT_RESOURCE(resource)

if (this.isPersonalLocation) {
this.loadIndicators({
client: this.$client,
currentFolder: this.currentFolder.path,
encodePath: this.encodePath
})
// Update table only if the file was uploaded to the current directory
if (fileIsInCurrentPath) {
this.UPSERT_RESOURCE(resource)

if (this.isPersonalLocation) {
this.loadIndicators({
client: this.$client,
currentFolder: this.currentFolder.path,
encodePath: this.encodePath
})
}
}

const user = await this.$client.users.getUser(this.user.id)
Expand All @@ -315,18 +302,13 @@ export default {
}
},

onFileError(error) {
console.error(error)
onFileError(file) {
this.showMessage({
title: this.$gettext('Failed to upload'),
status: 'danger'
})
},

onFileProgress(progress) {
this.updateFileProgress(progress)
},

showCreateResourceModal(
isFolder = true,
ext = 'txt',
Expand Down Expand Up @@ -623,9 +605,96 @@ export default {
}

return null
},

onFilesSelected(files: File[]) {
const conflicts = []
const uppyResources: UppyResource[] = this.inputFilesToUppyFiles(files)
for (const file of uppyResources) {
const relativeFilePath = file.meta.relativePath
if (relativeFilePath) {
const rootFolder = relativeFilePath.replace(/^\/+/, '').split('/')[0]
const exists = this.files.find((f) => f.name === rootFolder)
if (exists) {
this.showMessage({
title: this.$gettextInterpolate(
this.$gettext('Folder "%{folder}" already exists.'),
{ folder: rootFolder },
true
),
status: 'danger'
})
return
}

// Folder does not exist, no need to care about files inside -> continue
continue
}
const exists = this.files.find((f) => f.name === file.name)
if (exists) {
conflicts.push(file)
}
}

if (conflicts.length) {
this.displayOverwriteDialog(uppyResources, conflicts)
} else {
this.handleUppyFileUpload(uppyResources)
}
},

async handleUppyFileUpload(files: UppyResource[]) {
await this.createDirectoryTree(files)
await this.updateStoreForCreatedFolders(files)
this.$uppyService.uploadFiles(files)
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not so happy with this, it kind of feels the service should handle this - but we have so many dependencies here that are weird to get in a service... totally unsure about that.

At least we can move this function to the composable.


displayOverwriteDialog(files: UppyResource[], conflicts) {
const title = this.$ngettext(
'File already exists',
'Some files already exist',
conflicts.length
)

const isVersioningEnabled =
!this.publicPage() && this.capabilities.files && this.capabilities.files.versioning

let translatedMsg
if (isVersioningEnabled) {
translatedMsg = this.$ngettext(
'The following resource already exists: %{resources}. Do you want to create a new version for it?',
'The following resources already exist: %{resources}. Do you want to create a new version for them?',
conflicts.length
)
} else {
translatedMsg = this.$ngettext(
'The following resource already exists: %{resources}. Do you want to overwrite it?',
'The following resources already exist: %{resources}. Do you want to overwrite them?',
conflicts.length
)
}
const message = this.$gettextInterpolate(translatedMsg, {
resources: conflicts.map((f) => `${f.name}`).join(', ')
})

const modal = {
variation: isVersioningEnabled ? 'passive' : 'danger',
icon: 'upload-cloud',
title,
message,
cancelText: this.$gettext('Cancel'),
confirmText: isVersioningEnabled ? this.$gettext('Create') : this.$gettext('Overwrite'),
onCancel: this.hideModal,
onConfirm: () => {
this.hideModal()
this.handleUppyFileUpload(files)
}
}

this.createModal(modal)
}
}
}
})
</script>
<style lang="scss" scoped>
#create-list {
Expand Down
Loading