Upload (performance) improvements round 2

changelog/unreleased/enhancement-upload-improvements

@@ -1,12 +1,19 @@
 Enhancement: Upload progress & overlay improvements
 
-* Remove fetching of newly uploaded files to improve performance
+* Remove fetching of newly uploaded files and folders to improve performance
 * Redesign the upload overlay
 * Show uploading files in the upload overlay
 * Immediately show the upload overlay when uploading folders to tell the user that the upload is starting
 * Only show top level folders in the upload overlay when uploading folders
 * Remove the Uppy StatusBar plugin
+* Use `uppy.addFiles()` instead of calling `uppy.addFile()` for each file
+* The upload overlay now shows a more proper time estimation when uploading many files
+* The user feedback of the upload overlay during upload preparation has been improved
+* Several router bugs have been fixed in the upload overlay
+* Temporarily disabled `uploadDataDuringCreation` to prevent the local storage from getting to full
+* Some more refactoring under the hood has been made
 
 https://github.com/owncloud/web/pull/7067
+https://github.com/owncloud/web/pull/7127
 https://github.com/owncloud/web/issues/7066
 https://github.com/owncloud/web/issues/7069

packages/web-app-files/src/components/AppBar/CreateAndUpload.vue

@@ -732,7 +732,12 @@ export default defineComponent({
       this.$uppyService.publish('uploadStarted')
       await this.createDirectoryTree(files)
       this.$uppyService.publish('addedForUpload', files)
-      await this.updateStoreForCreatedFolders(files)
+
+      const reloadRequired = !!files.find((f) => f.meta.currentFolder === this.currentPath)
+      if (reloadRequired) {
+        bus.publish('app.files.list.load')
+      }
+
       this.$uppyService.uploadFiles(files)
     },
 

packages/web-app-files/src/composables/upload/useUploadHelpers.ts

@@ -1,10 +1,7 @@
 import { Route } from 'vue-router'
 import { UppyResource } from 'web-runtime/src/composables/upload'
-import { buildResource, buildWebDavFilesPath, buildWebDavSpacesPath } from '../../helpers/resources'
+import { buildWebDavFilesPath, buildWebDavSpacesPath } from '../../helpers/resources'
 import { User } from '../../helpers/user'
-import { DavProperties } from 'web-pkg/src/constants'
-import { ClientService } from 'web-pkg/src/services'
-import { Store } from 'vuex'
 import {
   useCapabilityShareJailEnabled,
   useClientService,
@@ -16,10 +13,10 @@ import { isLocationPublicActive, isLocationSpacesActive } from '../../router'
 import { computed, onMounted, ref, Ref, unref } from '@vue/composition-api'
 import { SHARE_JAIL_ID } from '../../services/folder'
 import * as uuid from 'uuid'
+import path from 'path'
 
 interface UploadHelpersResult {
   inputFilesToUppyFiles(inputFileOptions): UppyResource[]
-  updateStoreForCreatedFolders(files: UppyResource[]): void
   currentPath: Ref<string>
   uploadPath: Ref<string>
   personalDriveId: Ref<string>
@@ -38,7 +35,6 @@ export function useUploadHelpers(): UploadHelpersResult {
   const getToken = computed((): string => store.getters.getToken)
   const server = computed((): string => store.getters.configuration.server)
   const hasShareJail = useCapabilityShareJailEnabled()
-  const publicLinkPassword = computed((): string => store.getters['Files/publicLinkPassword'])
   const isPublicLocation = useActiveLocation(isLocationPublicActive, 'files-public-files')
   const isSpacesProjectLocation = useActiveLocation(isLocationSpacesActive, 'files-spaces-project')
   const isSpacesShareLocation = useActiveLocation(isLocationSpacesActive, 'files-spaces-share')
@@ -103,12 +99,6 @@ export function useUploadHelpers(): UploadHelpersResult {
       currentPath,
       webDavBasePath
     }),
-    updateStoreForCreatedFolders: updateStoreForCreatedFolders({
-      clientService,
-      store,
-      isPublicLocation,
-      publicLinkPassword
-    }),
     currentPath,
     uploadPath,
     personalDriveId
@@ -125,52 +115,6 @@ const getPersonalDriveId = async (clientService, server: string, getToken: strin
   return drivesResponse.data.value[0].id
 }
 
-const updateStoreForCreatedFolders = ({
-  clientService,
-  store,
-  isPublicLocation,
-  publicLinkPassword
-}: {
-  clientService: ClientService
-  store: Store<any>
-  isPublicLocation: Ref<boolean>
-  publicLinkPassword: Ref<string>
-}) => {
-  return async (files: UppyResource[]) => {
-    const { owncloudSdk: client } = clientService
-    const fetchedFolders = []
-    for (const file of files) {
-      if (!file.meta.relativeFolder) {
-        continue
-      }
-
-      const relativeFolder = `/${file.meta.relativeFolder.replace(/^\/+/, '')}`
-      // Only care about the root folders, no need to fetch nested folders
-      const rootFolder = relativeFolder.split('/').slice(0, 2).join('/')
-      const rootFolderPath = `${file.meta.webDavBasePath}/${rootFolder}`
-
-      if (fetchedFolders.includes(rootFolderPath)) {
-        continue
-      }
-
-      let resource
-      if (unref(isPublicLocation)) {
-        resource = await client.publicFiles.getFileInfo(
-          `${file.meta.currentFolder}${rootFolder}`,
-          unref(publicLinkPassword),
-          DavProperties.PublicLink
-        )
-      } else {
-        resource = await client.files.fileInfo(rootFolderPath, DavProperties.Default)
-      }
-
-      resource = buildResource(resource)
-      store.commit('Files/UPSERT_RESOURCE', resource)
-      fetchedFolders.push(rootFolderPath)
-    }
-  }
-}
-
 const inputFilesToUppyFiles = ({
   route,
   uploadPath,
@@ -180,33 +124,25 @@ const inputFilesToUppyFiles = ({
   return (files: File[]): UppyResource[] => {
     const uppyFiles: UppyResource[] = []
 
-    const { params } = unref(route)
+    const { name, params, query } = unref(route)
     const currentFolder = unref(currentPath)
+    const trimmedUploadPath = unref(uploadPath).replace(/\/+$/, '')
     const topLevelFolderIds = {}
 
     for (const file of files) {
       // Get the relative path of the file when the file was inside a directory on the client computer
       const relativeFilePath = file.webkitRelativePath || (file as any).relativePath || null
       // Directory without filename
-      const directory = relativeFilePath
-        ? relativeFilePath.substring(0, relativeFilePath.lastIndexOf('/'))
-        : ''
+      const directory =
+        !relativeFilePath || path.dirname(relativeFilePath) === '.'
+          ? ''
+          : path.dirname(relativeFilePath)
 
       // Build tus endpoint to dynamically set it on file upload.
       // Looks something like: https://localhost:9200/remote.php/dav/files/admin
-      let tusEndpoint
-      if (directory) {
-        tusEndpoint = `${unref(uploadPath).replace(/\/+$/, '')}/${directory.replace(/^\/+/, '')}`
-      } else {
-        tusEndpoint = unref(uploadPath)
-      }
-
-      // Build the route object for this file. This is used later by the upload-info-box
-      const item = params.item ? `${params.item}/${directory}` : directory
-      const fileRoute = {
-        ...unref(route),
-        params: { ...params, item }
-      }
+      const tusEndpoint = directory
+        ? `${trimmedUploadPath}/${directory.replace(/^\/+/, '')}`
+        : unref(uploadPath)
 
       let topLevelFolderId
       if (relativeFilePath) {
@@ -230,12 +166,13 @@ const inputFilesToUppyFiles = ({
           webDavBasePath: unref(webDavBasePath), // WebDAV base path where the files will be uploaded to
           uploadId: uuid.v4(),
           topLevelFolderId,
-          routeName: fileRoute.name,
-          routeItem: fileRoute.params?.item || '',
-          routeShareName: (fileRoute.params as any)?.shareName || '',
-          routeShareId: (fileRoute.query as any)?.shareId || '',
-          routeStorage: (fileRoute.params as any)?.storage || '',
-          routeStorageId: (fileRoute.params as any)?.storageId || ''
+          routeName: name,
+          routeItem: params.item ? `${params.item}/${directory}` : directory,
+          routeShareName: (params as any)?.shareName || '',
+          routeShareId: (query as any)?.shareId || '',
+          routeStorage: (params as any)?.storage || '',
+          routeStorageId: (params as any)?.storageId || '',
+          routeParamName: (params as any)?.name || ''
         }
       })
     }

packages/web-app-files/tests/unit/composables/upload/useUploadHelpers.spec.ts

@@ -45,50 +45,4 @@ describe('useUploadHelpers', () => {
       { currentItem: currentPathMock, fileUrl: uploadPathMock }
     )
   })
-
-  it('should update the store for newly created folders', async () => {
-    const wrapper = createWrapper(
-      () => {
-        const { updateStoreForCreatedFolders } = useUploadHelpers()
-        return { updateStoreForCreatedFolders }
-      },
-      { currentItem: currentPathMock, fileUrl: uploadPathMock }
-    )
-
-    const spyStoreCommit = jest.spyOn(wrapper.vm.$store, 'commit')
-
-    const uppyResources = [
-      {
-        source: 'source',
-        name: 'file1',
-        type: 'type',
-        data: new Blob(),
-        meta: {
-          currentFolder: 'currentFolder',
-          relativeFolder: 'l1/l2/l3',
-          relativePath: 'relativePath',
-          route: { name: 'files-personal' },
-          tusEndpoint: 'tusEndpoint',
-          webDavBasePath: 'webDavBasePath'
-        }
-      },
-      {
-        source: 'source',
-        name: 'file2',
-        type: 'type',
-        data: new Blob(),
-        meta: {
-          currentFolder: 'currentFolder',
-          relativeFolder: 'l1/l2/l3',
-          relativePath: 'relativePath',
-          route: { name: 'files-personal' },
-          tusEndpoint: 'tusEndpoint',
-          webDavBasePath: 'webDavBasePath'
-        }
-      }
-    ]
-
-    await wrapper.vm.updateStoreForCreatedFolders(uppyResources)
-    expect(spyStoreCommit).toHaveBeenCalledTimes(1)
-  })
 })

packages/web-runtime/src/components/UploadInfo.vue

@@ -56,7 +56,7 @@
           <oc-icon name="restart" fill-type="line" />
         </oc-button>
         <oc-button
-          v-if="runningUploads && uploadsPausable"
+          v-if="runningUploads && uploadsPausable && !inPreparation"
           id="pause-upload-info-btn"
           v-oc-tooltip="uploadsPaused ? $gettext('Resume upload') : $gettext('Pause upload')"
           class="oc-ml-s"
@@ -66,7 +66,7 @@
           <oc-icon :name="uploadsPaused ? 'play-circle' : 'pause-circle'" fill-type="line" />
         </oc-button>
         <oc-button
-          v-if="runningUploads"
+          v-if="runningUploads && !inPreparation"
           id="cancel-upload-info-btn"
           v-oc-tooltip="$gettext('Cancel upload')"
           class="oc-ml-s"
@@ -154,6 +154,7 @@ export default {
     totalProgress: 0, // current uploads progress (0-100)
     uploadsPaused: false, // all uploads paused?
     uploadsCancelled: false, // all uploads cancelled?
+    inPreparation: true, // preparation before upload
     runningUploads: 0, // all uploads (not files!) that are in progress currently
     bytesTotal: 0,
     bytesUploaded: 0,
@@ -165,7 +166,7 @@ export default {
     ...mapGetters(['configuration']),
 
     uploadInfoTitle() {
-      if (this.filesInProgressCount) {
+      if (this.filesInProgressCount && !this.inPreparation) {
         return this.$gettextInterpolate(
           this.$ngettext(
             '%{ filesInProgressCount } item uploading...',
@@ -232,6 +233,10 @@ export default {
       this.filesInProgressCount += files.filter((f) => !f.isFolder).length
 
       for (const file of files) {
+        if (file.data?.size) {
+          this.bytesTotal += file.data.size
+        }
+
         const { relativeFolder, uploadId, topLevelFolderId } = file.meta
         const isTopLevelItem = !relativeFolder
         // only add top level items to this.uploads because we only show those
@@ -264,11 +269,11 @@ export default {
     this.$uppyService.subscribe('upload-progress', ({ file, progress }) => {
       if (!this.timeStarted) {
         this.timeStarted = new Date()
+        this.inPreparation = false
       }
 
       if (this.filesInEstimation[file.meta.uploadId] === undefined) {
         this.filesInEstimation[file.meta.uploadId] = 0
-        this.bytesTotal += progress.bytesTotal
       }
 
       const byteIncrease = progress.bytesUploaded - this.filesInEstimation[file.meta.uploadId]
@@ -418,6 +423,7 @@ export default {
       this.filesInEstimation = {}
       this.timeStarted = null
       this.remainingTime = undefined
+      this.inPreparation = true
     },
     displayFileAsResource(file) {
       return !!file.targetRoute
@@ -426,10 +432,10 @@ export default {
       return file.isFolder === true
     },
     folderLink(file) {
-      return this.createFolderLink(file.path, file.storageId, file.targetRoute)
+      return this.createFolderLink(file.path, file.targetRoute)
     },
     parentFolderLink(file) {
-      return this.createFolderLink(path.dirname(file.path), file.storageId, file.targetRoute)
+      return this.createFolderLink(path.dirname(file.path), file.targetRoute)
     },
     buildRouteFromUppyResource(resource) {
       if (!resource.meta.routeName) {
@@ -445,7 +451,8 @@ export default {
           item: resource.meta.routeItem,
           shareName: resource.meta.routeShareName,
           storage: resource.meta.routeStorage,
-          storageId: resource.meta.routeStorageId
+          storageId: resource.meta.routeStorageId,
+          name: resource.meta.routeParamName
         }
       }
     },
@@ -465,7 +472,7 @@ export default {
       }
       return this.hasShareJail ? this.$gettext('Personal') : this.$gettext('All files and folders')
     },
-    createFolderLink(path, storageId, targetRoute) {
+    createFolderLink(path, targetRoute) {
       if (!targetRoute) {
         return {}
       }
@@ -475,7 +482,8 @@ export default {
         name: targetRoute.name,
         query: targetRoute.query,
         params: {
-          ...(storageId && path && { storageId }),
+          ...(targetRoute.params?.storageId &&
+            path && { storageId: targetRoute.params?.storageId }),
           ...(targetRoute.params?.storage && { storage: targetRoute.params?.storage }),
           ...(targetRoute.params?.shareName && { shareName: targetRoute.params?.shareName })
         }

packages/web-runtime/src/composables/upload/useUpload.ts

@@ -33,6 +33,7 @@ export interface UppyResource {
     routeShareId?: string
     routeStorage?: string
     routeStorageId?: string
+    routeParamName?: string
   }
 }
 
@@ -163,8 +164,10 @@ const createDirectoryTree = ({
             routeName: file.meta.routeName,
             routeItem: file.meta.routeItem,
             routeShareName: file.meta.routeShareName,
+            routeShareId: file.meta.routeShareId,
             routeStorage: file.meta.routeStorage,
-            routeStorageId: file.meta.routeStorageId
+            routeStorageId: file.meta.routeStorageId,
+            routeParamName: file.meta.routeParamName
           }
         }