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

Retry failed uploads on re-upload #8055

Merged
merged 2 commits into from
Dec 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions changelog/unreleased/enhancement-retry-failed-uploads
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ export default defineComponent({
uppyService
}),
...useUploadHelpers({
uppyService,
space: computed(() => props.space),
currentFolder: computed(() => props.item),
currentFolderId: computed(() => props.itemId)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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,
Expand All @@ -49,6 +52,7 @@ const getRelativeFilePath = (file: File): string | undefined => {
}

const inputFilesToUppyFiles = ({
uppyService,
route,
space,
currentFolder,
Expand Down Expand Up @@ -86,6 +90,7 @@ const inputFilesToUppyFiles = ({
source: 'file input',
name: file.name,
type: file.type,
size: file.size,
data: file,
meta: {
// current path & space
Expand All @@ -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,
Expand Down
27 changes: 26 additions & 1 deletion packages/web-app-files/src/helpers/resource/actions/upload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: [] })
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -17,6 +18,7 @@ describe('useUploadHelpers', () => {
() => {
const fileName = 'filename'
const { inputFilesToUppyFiles } = useUploadHelpers({
uppyService: mockDeep<UppyService>(),
space: mockDeep<ComputedRef<SpaceResource>>(),
currentFolder: computed(() => '')
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 })
Expand All @@ -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 })
Expand All @@ -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({
Expand All @@ -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"', () => {
Expand Down
1 change: 1 addition & 0 deletions packages/web-runtime/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 2 additions & 0 deletions packages/web-runtime/src/composables/upload/useUpload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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!
Expand All @@ -33,6 +34,7 @@ export interface UppyResource {
currentFolderId?: string | number
fileId?: string | number
// upload data
uppyId?: string
relativeFolder: string
relativePath: string
tusEndpoint: string
Expand Down
21 changes: 20 additions & 1 deletion packages/web-runtime/src/services/uppyService.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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)
}
Expand Down
2 changes: 2 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion tests/unit/config/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down