Merge pull request #8055 from owncloud/retry-failed-uploads

Retry failed uploads on re-upload
owncloud · Dec 2, 2022 · 3722ebe · 3722ebe
2 parents 3be0329 + 995e08c
commit 3722ebe
Show file tree

Hide file tree

Showing 11 changed files with 95 additions and 8 deletions.
diff --git a/changelog/unreleased/enhancement-retry-failed-uploads b/changelog/unreleased/enhancement-retry-failed-uploads
@@ -0,0 +1,6 @@
+Enhancement: Retry failed uploads on re-upload
+
+When re-uploading a file that failed uploading before, the upload is now being retried instead of being started from scratch again. This fixes some issues with the overlay and preserves the upload progress.
+
+https://github.com/owncloud/web/pull/8055
+https://github.com/owncloud/web/issues/7944
diff --git a/packages/web-app-files/src/components/AppBar/CreateAndUpload.vue b/packages/web-app-files/src/components/AppBar/CreateAndUpload.vue
@@ -214,6 +214,7 @@ export default defineComponent({
         uppyService
       }),
       ...useUploadHelpers({
+        uppyService,
         space: computed(() => props.space),
         currentFolder: computed(() => props.item),
         currentFolderId: computed(() => props.itemId)

diff --git a/packages/web-app-files/src/composables/upload/useUploadHelpers.ts b/packages/web-app-files/src/composables/upload/useUploadHelpers.ts
@@ -6,18 +6,20 @@ import * as uuid from 'uuid'
 import path from 'path'
 import { SpaceResource } from 'web-client/src/helpers'
 import { urlJoin } from 'web-client/src/utils'
-
+import { UppyService } from 'web-runtime/src/services/uppyService'
 interface UploadHelpersOptions {
   space: ComputedRef<SpaceResource>
   currentFolder?: ComputedRef<string>
   currentFolderId?: ComputedRef<string | number>
+  uppyService: UppyService
 }
 
 interface UploadHelpersResult {
   inputFilesToUppyFiles(inputFileOptions): UppyResource[]
 }
 
 interface inputFileOptions {
+  uppyService: UppyService
   route: Ref<Route>
   space: Ref<SpaceResource>
   currentFolder: Ref<string>
@@ -27,6 +29,7 @@ interface inputFileOptions {
 export function useUploadHelpers(options: UploadHelpersOptions): UploadHelpersResult {
   return {
     inputFilesToUppyFiles: inputFilesToUppyFiles({
+      uppyService: options.uppyService,
       route: useRoute(),
       space: options.space,
       currentFolder: options.currentFolder,
@@ -49,6 +52,7 @@ const getRelativeFilePath = (file: File): string | undefined => {
 }
 
 const inputFilesToUppyFiles = ({
+  uppyService,
   route,
   space,
   currentFolder,
@@ -86,6 +90,7 @@ const inputFilesToUppyFiles = ({
         source: 'file input',
         name: file.name,
         type: file.type,
+        size: file.size,
         data: file,
         meta: {
           // current path & space
@@ -96,6 +101,7 @@ const inputFilesToUppyFiles = ({
           currentFolder: unref(currentFolder),
           currentFolderId: unref(currentFolderId),
           // upload data
+          uppyId: uppyService.generateUploadId(file),
           relativeFolder: directory,
           relativePath: relativeFilePath, // uppy needs this property to be named relativePath
           tusEndpoint,

diff --git a/packages/web-app-files/src/helpers/resource/actions/upload.ts b/packages/web-app-files/src/helpers/resource/actions/upload.ts
@@ -89,17 +89,42 @@ export class ResourcesUpload extends ConflictDialog {
     this.$uppyService.publish('uploadStarted')
     const result = await this.createDirectoryTree(space, currentFolder, files, this.currentFolderId)
 
+    // filter out files in folders that could not be created
     let filesToUpload = files
     if (result.failed.length) {
       filesToUpload = files.filter(
         (f) => !result.failed.some((r) => f.meta.relativeFolder.startsWith(r))
       )
     }
 
+    // gather failed files to be retried
+    const failedFiles = this.$uppyService.getFailedFiles()
+    const retries = []
+
+    for (const failedFile of failedFiles) {
+      const fileToRetry = filesToUpload.find((f) => f.meta.uppyId === failedFile.meta.uppyId)
+      if (fileToRetry) {
+        // re-use the old uploadId
+        fileToRetry.meta.uploadId = failedFile.meta.uploadId
+        retries.push(fileToRetry)
+      }
+    }
+
     if (filesToUpload.length) {
       this.$uppyService.publish('addedForUpload', filesToUpload)
+    }
+
+    for (const retry of retries) {
+      this.$uppyService.retryUpload(retry.meta.uppyId)
+      // filter out files that have been retried
+      filesToUpload = filesToUpload.filter((f) => f.meta.uppyId !== retry.meta.uppyId)
+    }
+
+    if (filesToUpload.length) {
       this.$uppyService.uploadFiles(filesToUpload)
-    } else {
+    }
+
+    if (!filesToUpload.length && !retries.length) {
       this.$uppyService.publish('uploadCompleted', { successful: [] })
     }
   }

diff --git a/packages/web-app-files/tests/unit/composables/upload/useUploadHelpers.spec.ts b/packages/web-app-files/tests/unit/composables/upload/useUploadHelpers.spec.ts
@@ -3,6 +3,7 @@ import { createWrapper } from './spec'
 import { mockDeep } from 'jest-mock-extended'
 import { SpaceResource } from 'web-client/src/helpers'
 import { ComputedRef, computed } from 'vue'
+import { UppyService } from 'web-runtime/src/services/uppyService'
 
 describe('useUploadHelpers', () => {
   const currentPathMock = 'path'
@@ -17,6 +18,7 @@ describe('useUploadHelpers', () => {
       () => {
         const fileName = 'filename'
         const { inputFilesToUppyFiles } = useUploadHelpers({
+          uppyService: mockDeep<UppyService>(),
           space: mockDeep<ComputedRef<SpaceResource>>(),
           currentFolder: computed(() => '')
         })

diff --git a/packages/web-app-files/tests/unit/helpers/resource/actions/upload.spec.ts b/packages/web-app-files/tests/unit/helpers/resource/actions/upload.spec.ts
@@ -10,7 +10,7 @@ const getResourcesUploadInstance = ({
   currentFiles = [mockDeep<Resource>()],
   spaces = [mockDeep<SpaceResource>()],
   showMessage = jest.fn(),
-  uppyService = mockDeep<UppyService>(),
+  uppyService = mockDeep<UppyService>({ getFailedFiles: jest.fn(() => []) }),
   createDirectoryTree = jest.fn().mockImplementation(() => ({ failed: [], successful: [] }))
 }: {
   space?: SpaceResource
@@ -157,7 +157,8 @@ describe('upload helper', () => {
       const publishStub = jest.fn()
       const uppyService = mockDeep<UppyService>({
         uploadFiles: uploadFilesStub,
-        publish: publishStub
+        publish: publishStub,
+        getFailedFiles: jest.fn(() => [])
       })
       const filesToUpload = [mockDeep<UppyResource>()]
       const resourcesUpload = getResourcesUploadInstance({ uppyService })
@@ -172,7 +173,8 @@ describe('upload helper', () => {
       const publishStub = jest.fn()
       const uppyService = mockDeep<UppyService>({
         uploadFiles: uploadFilesStub,
-        publish: publishStub
+        publish: publishStub,
+        getFailedFiles: jest.fn(() => [])
       })
       const filesToUpload = []
       const resourcesUpload = getResourcesUploadInstance({ uppyService })
@@ -190,7 +192,8 @@ describe('upload helper', () => {
       const publishStub = jest.fn()
       const uppyService = mockDeep<UppyService>({
         uploadFiles: uploadFilesStub,
-        publish: publishStub
+        publish: publishStub,
+        getFailedFiles: jest.fn(() => [])
       })
       const filesToUpload = [mockDeep<UppyResource>({ meta: { relativeFolder: '/parent' } })]
       const resourcesUpload = getResourcesUploadInstance({
@@ -203,6 +206,25 @@ describe('upload helper', () => {
       expect(publishStub).toHaveBeenCalledTimes(2)
       expect(uploadFilesStub).toHaveBeenCalledTimes(0)
     })
+    it('should filter out retries', async () => {
+      const uploadFilesStub = jest.fn()
+      const publishStub = jest.fn()
+      const retryUploadStub = jest.fn()
+      const filesToUpload = [mockDeep<UppyResource>()]
+      const uppyService = mockDeep<UppyService>({
+        uploadFiles: uploadFilesStub,
+        publish: publishStub,
+        retryUpload: retryUploadStub,
+        getFailedFiles: jest.fn(() => filesToUpload)
+      })
+      const resourcesUpload = getResourcesUploadInstance({ uppyService })
+      await resourcesUpload.handleUppyFileUpload(mockDeep<SpaceResource>(), '/', filesToUpload)
+
+      expect(publishStub).toHaveBeenCalledWith('uploadStarted')
+      expect(publishStub).toHaveBeenCalledTimes(2)
+      expect(retryUploadStub).toHaveBeenCalled()
+      expect(uploadFilesStub).not.toHaveBeenCalled()
+    })
   })
 
   describe('method "displayOverwriteDialog"', () => {

diff --git a/packages/web-runtime/package.json b/packages/web-runtime/package.json
@@ -11,6 +11,7 @@
     "@sentry/browser": "^6.19.7",
     "@sentry/integrations": "^6.19.7",
     "@uppy/core": "^3.0.2",
+    "@uppy/utils": "^5.0.2",
     "@uppy/drop-target": "^2.0.0",
     "@uppy/tus": "^3.0.1",
     "@uppy/xhr-upload": "^3.0.1",

diff --git a/packages/web-runtime/src/composables/upload/useUpload.ts b/packages/web-runtime/src/composables/upload/useUpload.ts
@@ -21,6 +21,7 @@ export interface UppyResource {
   source: string
   name: string
   type: string
+  size: number
   data: Blob
   meta: {
     // IMPORTANT: must only contain primitive types, complex types won't be serialized properly!
@@ -33,6 +34,7 @@ export interface UppyResource {
     currentFolderId?: string | number
     fileId?: string | number
     // upload data
+    uppyId?: string
     relativeFolder: string
     relativePath: string
     tusEndpoint: string

diff --git a/packages/web-runtime/src/services/uppyService.ts b/packages/web-runtime/src/services/uppyService.ts
@@ -1,11 +1,13 @@
-import Uppy from '@uppy/core'
+import Uppy, { UppyFile } from '@uppy/core'
 import { TusOptions } from '@uppy/tus'
 import XHRUpload, { XHRUploadOptions } from '@uppy/xhr-upload'
 import { eventBus } from 'web-pkg/src/services/eventBus'
 import { UppyResource } from '../composables/upload'
 import { CustomDropTarget } from '../composables/upload/uppyPlugins/customDropTarget'
 import { CustomTus } from '../composables/upload/uppyPlugins/customTus'
 import { urlJoin } from 'web-client/src/utils'
+import getFileType from '@uppy/utils/lib/getFileType'
+import generateFileID from '@uppy/utils/lib/generateFileID'
 
 type UppyServiceTopics =
   | 'uploadStarted'
@@ -219,6 +221,23 @@ export class UppyService {
     this.uploadInputs = this.uploadInputs.filter((input) => input !== el)
   }
 
+  generateUploadId(file: File): string {
+    return generateFileID({
+      name: file.name,
+      size: file.size,
+      type: getFileType(file as unknown as UppyFile),
+      data: file
+    } as unknown as UppyFile)
+  }
+
+  getFailedFiles(): UppyResource[] {
+    return this.uppy.getFiles() as unknown as UppyResource[]
+  }
+
+  retryUpload(fileId) {
+    this.uppy.retryUpload(fileId)
+  }
+
   uploadFiles(files: UppyResource[]) {
     this.uppy.addFiles(files)
   }

diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
diff --git a/tests/unit/config/jest.config.js b/tests/unit/config/jest.config.js
@@ -24,7 +24,8 @@ module.exports = {
     '@uppy/core': '<rootDir>tests/unit/stubs/uppy',
     '@uppy/xhr-upload': '<rootDir>tests/unit/stubs/uppy',
     '@uppy/drop-target': '<rootDir>tests/unit/stubs/uppy',
-    '@uppy/tus': '<rootDir>tests/unit/stubs/uppy'
+    '@uppy/tus': '<rootDir>tests/unit/stubs/uppy',
+    '@uppy/utils': '<rootDir>tests/unit/stubs/uppy'
   },
   modulePathIgnorePatterns: ['packages/design-system/docs/utils/statusLabels.spec.js'],
   testEnvironment: 'jsdom',