fix(upload): honor root and retries upload options

lib/upload/uploader/Upload.spec.ts

@@ -11,6 +11,10 @@ import { Upload } from './Upload.ts'
 class TestUpload extends Upload {
 	public source: string = '/destination/file.txt'
 
+	constructor() {
+		super({})
+	}
+
 	public async start(queue: PQueue): Promise<void> {
 		queue.add(() => Promise.resolve())
 	}

lib/upload/uploader/Upload.ts

@@ -75,8 +75,25 @@ export interface IUpload extends TypedEventTarget<UploadEvents> {
 	cancel(): void
 }
 
-export abstract class Upload extends TypedEventTarget<UploadEvents> implements Partial<IUpload> {
+export interface IUploadOptions {
+	headers: Record<string, string>
+	noChunking: boolean
+	retries: number
+}
+
+export abstract class Upload<CustomOpts = object> extends TypedEventTarget<UploadEvents> implements Partial<IUpload> {
 	#abortController = new AbortController()
+	protected readonly options: IUploadOptions & CustomOpts
+
+	protected constructor(options: Partial<IUploadOptions> & CustomOpts) {
+		super()
+		this.options = {
+			headers: {},
+			noChunking: false,
+			retries: 5,
+			...options,
+		}
+	}
 
 	/**
 	 * The destination of this upload.

lib/upload/uploader/UploadFile.spec.ts

@@ -96,6 +96,50 @@ describe('chunking', () => {
 	})
 })
 
+describe('retries', () => {
+	it('defaults to 5 retries for a plain upload', async () => {
+		isPublicShareMock.mockReturnValue(false)
+		getMaxChunksSizeMock.mockReturnValue(1024 * 1024)
+		uploadDataMock.mockResolvedValue(undefined)
+
+		const uploadFile = new UploadFile('/destination', new File(['x'.repeat(1024)], 'filename'), {})
+		const queue = { add: vi.fn((fn: () => Promise<void>) => fn()) }
+		await uploadFile.start(queue as never)
+		await queue.add.mock.calls[0][0]()
+
+		expect(uploadDataMock).toHaveBeenLastCalledWith('/destination', expect.anything(), expect.objectContaining({ retries: 5 }))
+	})
+
+	it('forwards the configured retries to the upload request', async () => {
+		isPublicShareMock.mockReturnValue(false)
+		getMaxChunksSizeMock.mockReturnValue(1024 * 1024)
+		uploadDataMock.mockResolvedValue(undefined)
+
+		const uploadFile = new UploadFile('/destination', new File(['x'.repeat(1024)], 'filename'), { retries: 2 })
+		const queue = { add: vi.fn((fn: () => Promise<void>) => fn()) }
+		await uploadFile.start(queue as never)
+		await queue.add.mock.calls[0][0]()
+
+		expect(uploadDataMock).toHaveBeenLastCalledWith('/destination', expect.anything(), expect.objectContaining({ retries: 2 }))
+	})
+
+	it('forwards the configured retries to chunked uploads and the workspace creation', async () => {
+		isPublicShareMock.mockReturnValue(false)
+		getMaxChunksSizeMock.mockReturnValue(1024)
+		initChunkWorkspaceMock.mockResolvedValue('/tmp/temporary')
+		uploadDataMock.mockResolvedValue(undefined)
+		vi.spyOn(axios, 'request').mockResolvedValueOnce({})
+
+		const uploadFile = new UploadFile('/destination', new File(['x'.repeat(4096)], 'bigfile'), { retries: 2 })
+		const queue = { add: vi.fn((fn: () => Promise<void>) => fn()) }
+		await uploadFile.start(queue as never)
+		await Promise.all(queue.add.mock.results.map((r) => r.value))
+
+		expect(initChunkWorkspaceMock).toHaveBeenLastCalledWith('/destination', 2, false, {})
+		expect(uploadDataMock).toHaveBeenLastCalledWith(expect.any(String), expect.anything(), expect.objectContaining({ retries: 2 }))
+	})
+})
+
 describe('upload status and events', () => {
 	it('initialized', () => {
 		const uploadFile = new UploadFile('/destination', new File(['x'.repeat(2048)], 'filename'), { noChunking: false })

lib/upload/uploader/UploadFile.ts

@@ -4,7 +4,7 @@
  */
 
 import type PQueue from 'p-queue'
-import type { IUpload, TUploadStatus } from './Upload.ts'
+import type { IUpload, IUploadOptions, TUploadStatus } from './Upload.ts'
 
 import axios from '@nextcloud/axios'
 import { join } from '@nextcloud/paths'
@@ -21,10 +21,8 @@ import { Upload, UploadStatus } from './Upload.ts'
  * A class representing a single file to be uploaded
  */
 export class UploadFile extends Upload implements IUpload {
-	#customHeaders: Record<string, string>
 	#fileHandle: File | FileSystemFileEntry
 	#file?: File
-	#noChunking: boolean
 
 	public source: string
 	public status: TUploadStatus = UploadStatus.INITIALIZED
@@ -36,22 +34,16 @@ export class UploadFile extends Upload implements IUpload {
 	constructor(
 		destination: string,
 		fileHandle: File | FileSystemFileEntry,
-		options: { headers?: Record<string, string>, noChunking?: boolean },
+		options: Partial<IUploadOptions> = {},
 	) {
-		super()
-		const {
-			headers = {},
-			noChunking = false,
-		} = options
+		super(options)
 
 		// exposed state
 		this.source = destination
 		this.totalBytes = 'size' in fileHandle ? fileHandle.size : -1
 
 		// private state
 		this.#fileHandle = fileHandle
-		this.#customHeaders = headers
-		this.#noChunking = noChunking
 		this.signal.addEventListener('abort', () => {
 			if (this.status !== UploadStatus.FAILED) {
 				this.status = UploadStatus.CANCELLED
@@ -61,7 +53,7 @@ export class UploadFile extends Upload implements IUpload {
 
 	get isChunked(): boolean {
 		const maxChunkSize = getMaxChunksSize('size' in this.#fileHandle ? this.#fileHandle.size : undefined)
-		return !this.#noChunking
+		return !this.options.noChunking
 			&& maxChunkSize > 0
 			&& this.totalBytes > maxChunkSize
 			&& (!isPublicShare() || supportsPublicChunking())
@@ -121,7 +113,7 @@ export class UploadFile extends Upload implements IUpload {
 		this.status = UploadStatus.UPLOADING
 		// The `Destination` header must be a URI, so the source has to be encoded here
 		const destination = encodeUrl(this.source)
-		const temporaryUrl = await initChunkWorkspace(destination, 5, isPublicShare(), this.#customHeaders)
+		const temporaryUrl = await initChunkWorkspace(destination, this.options.retries, isPublicShare(), this.options.headers)
 
 		const promises: Promise<void>[] = []
 		const chunkSize = Math.floor(this.totalBytes / this.numberOfChunks)
@@ -149,7 +141,7 @@ export class UploadFile extends Upload implements IUpload {
 					method: 'MOVE',
 					url: `${temporaryUrl}/.file`,
 					headers: {
-						...this.#customHeaders,
+						...this.options.headers,
 						...getMtimeHeader(this.#file!),
 						'OC-Total-Length': this.totalBytes,
 						Destination: destination,
@@ -183,6 +175,7 @@ export class UploadFile extends Upload implements IUpload {
 				chunk,
 				{
 					signal: this.signal,
+					retries: this.options.retries,
 					onUploadProgress: ({ bytes }) => {
 						// As this is only the sent bytes not the processed ones we only count 90%.
 						// When the upload is finished (server acknowledged the upload) the remaining 10% will be correctly set.
@@ -193,7 +186,7 @@ export class UploadFile extends Upload implements IUpload {
 						this.uploadedBytes = 0
 					},
 					headers: {
-						...this.#customHeaders,
+						...this.options.headers,
 						...getMtimeHeader(this.#file!),
 						'Content-Type': this.#file!.type,
 					},

lib/upload/uploader/UploadFileTree.spec.ts

@@ -20,6 +20,7 @@ const uploadFileMocks = vi.hoisted(() => {
 	const instances: Array<{
 		source: string
 		signal: AbortSignal
+		options: unknown
 		start: ReturnType<typeof vi.fn>
 		cancel: ReturnType<typeof vi.fn>
 		rebase: ReturnType<typeof vi.fn>
@@ -28,6 +29,7 @@ const uploadFileMocks = vi.hoisted(() => {
 
 	class MockUploadFile {
 		public source: string
+		public options: unknown
 		public status: number = UploadStatus.INITIALIZED
 
 		#abortController = new AbortController()
@@ -49,9 +51,9 @@ const uploadFileMocks = vi.hoisted(() => {
 			this.source = source
 		})
 
-		// eslint-disable-next-line @typescript-eslint/no-unused-vars
-		public constructor(source: string, _file: File, _options: unknown) {
+		public constructor(source: string, _file: File, options: unknown) {
 			this.source = source
+			this.options = options
 			instances.push(this)
 		}
 	}
@@ -131,6 +133,26 @@ describe('UploadFileTree', () => {
 		expect(snapshot[2].source).toBe('/destination/folder/nested.txt')
 	})
 
+	it('passes the configured retries down to nested child uploads', async () => {
+		const directory = await createDirectoryTree()
+		const tree = new UploadFileTree('/destination', directory, { retries: 2 })
+		tree.initialize()
+
+		// both the direct child file and the one nested in a sub directory
+		expect(uploadFileMocks.instances).toHaveLength(2)
+		for (const instance of uploadFileMocks.instances) {
+			expect(instance.options).toMatchObject({ retries: 2 })
+		}
+	})
+
+	it('defaults to 5 retries for child uploads', async () => {
+		const directory = await createDirectoryTree()
+		const tree = new UploadFileTree('/destination', directory, {})
+		tree.initialize()
+
+		expect(uploadFileMocks.instances[0].options).toMatchObject({ retries: 5 })
+	})
+
 	it('cancels child uploads when aborted', async () => {
 		const directory = await createDirectoryTree()
 		const tree = new UploadFileTree('/destination', directory, {})

lib/upload/uploader/UploadFileTree.ts

@@ -4,7 +4,7 @@
  */
 
 import type PQueue from 'p-queue'
-import type { IUpload, TUploadStatus } from './Upload.ts'
+import type { IUpload, IUploadOptions, TUploadStatus } from './Upload.ts'
 
 import axios, { isAxiosError } from '@nextcloud/axios'
 import { basename } from '@nextcloud/paths'
@@ -31,20 +31,19 @@ import { UploadFile } from './UploadFile.ts'
  */
 export type ConflictsCallback = (nodes: string[], currentPath: string) => Promise<false | Record<string, string>>
 
+interface IUploadFileTreeOptions {
+	/** The callback to handle conflicts */
+	callback?: ConflictsCallback
+}
+
 /**
  * A class representing a single file to be uploaded
  */
-export class UploadFileTree extends Upload implements IUpload {
-	/** Customer headers passed */
-	#customHeaders: Record<string, string>
+export class UploadFileTree extends Upload<IUploadFileTreeOptions> implements IUpload {
 	/** The current file tree to upload */
 	#directory: FileTree
-	/** Whether chunking is disabled */
-	#noChunking: boolean
 	/** Children uploads of this parent folder upload */
 	#children: (Upload & IUpload)[] = []
-	/** The callback to handle conflicts */
-	#conflictsCallback?: ConflictsCallback
 
 	/** Whether we need to check for conflicts or not (newly created parent folders = no conflict resolution needed) */
 	protected needConflictResolution = true
@@ -58,24 +57,13 @@ export class UploadFileTree extends Upload implements IUpload {
 	constructor(
 		destination: string,
 		directory: FileTree,
-		options: {
-			callback?: ConflictsCallback
-			headers?: Record<string, string>
-			noChunking?: boolean
-		},
+		options: Partial<IUploadOptions & IUploadFileTreeOptions> = {},
 	) {
-		super()
-		const {
-			headers = {},
-			noChunking = false,
-		} = options
+		super(options)
 
 		// exposed state
 		this.source = destination
 		this.#directory = directory
-		this.#customHeaders = headers
-		this.#noChunking = noChunking
-		this.#conflictsCallback = options.callback
 
 		this.signal.addEventListener('abort', () => {
 			for (const child of this.#children) {
@@ -121,19 +109,15 @@ export class UploadFileTree extends Upload implements IUpload {
 				const upload = new UploadFileTree(
 					concatUrl(this.source, child.originalName),
 					child,
-					{
-						callback: this.#conflictsCallback,
-						headers: this.#customHeaders,
-						noChunking: this.#noChunking,
-					},
+					this.options,
 				)
 				this.#children.push(upload)
 				grandchildren.push(...upload.initialize())
 			} else {
 				const upload = new UploadFile(
 					concatUrl(this.source, child.name),
 					child,
-					{ headers: this.#customHeaders, noChunking: this.#noChunking },
+					this.options,
 				)
 				this.#children.push(upload)
 			}
@@ -152,8 +136,8 @@ export class UploadFileTree extends Upload implements IUpload {
 		this.status = UploadStatus.UPLOADING
 		await this.#createDirectory(queue)
 
-		if (this.needConflictResolution && this.#conflictsCallback) {
-			const nodes = await this.#conflictsCallback(
+		if (this.needConflictResolution && this.options.callback) {
+			const nodes = await this.options.callback(
 				this.#directory.children.map((node) => basename(node.name)),
 				this.source,
 			)
@@ -219,7 +203,7 @@ export class UploadFileTree extends Upload implements IUpload {
 				await axios.head(encodeUrl(this.source), {
 					signal: this.signal,
 					headers: {
-						...this.#customHeaders,
+						...this.options.headers,
 					},
 				})
 				return // directory already exists, no need to create it
@@ -236,7 +220,7 @@ export class UploadFileTree extends Upload implements IUpload {
 					method: 'MKCOL',
 					url: encodeUrl(this.source),
 					headers: {
-						...this.#customHeaders,
+						...this.options.headers,
 						...getMtimeHeader(this.#directory),
 					},
 					signal: this.signal,