From aa315f7b0e275d72b5f0d8b5f601e79555cad12f Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Wed, 16 Sep 2026 01:18:25 +0200 Subject: [PATCH] fix(upload): propagate folder renames to already initialized child uploads The whole upload tree is initialized upfront, so children are created with the original folder names, while conflicts are resolved later per level. Renaming a directory therefore only updated that node's source and its descendants kept uploading into the conflicting folder. Replace the `source` patching with a `rebase()` method on `Upload`, which `UploadFileTree` overrides to also move all of its children. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Ferdinand Thiessen --- lib/upload/uploader/Upload.spec.ts | 8 +++++ lib/upload/uploader/Upload.ts | 18 ++++++++++ lib/upload/uploader/UploadFile.spec.ts | 11 +++++- lib/upload/uploader/UploadFileTree.spec.ts | 39 +++++++++++++++++++--- lib/upload/uploader/UploadFileTree.ts | 19 ++++++++++- lib/upload/utils/url.ts | 2 +- 6 files changed, 90 insertions(+), 7 deletions(-) diff --git a/lib/upload/uploader/Upload.spec.ts b/lib/upload/uploader/Upload.spec.ts index aaaad11fb..641d5bff3 100644 --- a/lib/upload/uploader/Upload.spec.ts +++ b/lib/upload/uploader/Upload.spec.ts @@ -9,6 +9,8 @@ import { describe, expect, it } from 'vitest' import { Upload } from './Upload.ts' class TestUpload extends Upload { + public source: string = '/destination/file.txt' + public async start(queue: PQueue): Promise { queue.add(() => Promise.resolve()) } @@ -21,4 +23,10 @@ describe('Upload', () => { a.cancel() expect(a.signal.aborted).toBe(true) }) + + it('rebases an upload', () => { + const a = new TestUpload() + a.rebase('/other/renamed.txt') + expect(a.source).toBe('/other/renamed.txt') + }) }) diff --git a/lib/upload/uploader/Upload.ts b/lib/upload/uploader/Upload.ts index 7d3ef607e..c3eed9f4d 100644 --- a/lib/upload/uploader/Upload.ts +++ b/lib/upload/uploader/Upload.ts @@ -78,10 +78,28 @@ export interface IUpload extends TypedEventTarget { export abstract class Upload extends TypedEventTarget implements Partial { #abortController = new AbortController() + /** + * The destination of this upload. + * This is *not* URL encoded, it is encoded when the upload requests are made. + */ + public abstract source: string + get signal(): AbortSignal { return this.#abortController.signal } + /** + * Move this upload to a new destination. + * + * This is needed when a parent folder is renamed while resolving conflicts, + * as the child uploads are already initialized with the previous destination. + * + * @param source - The new destination of this upload + */ + public rebase(source: string): void { + this.source = source + } + /** * Cancels the upload */ diff --git a/lib/upload/uploader/UploadFile.spec.ts b/lib/upload/uploader/UploadFile.spec.ts index 1a1758467..7d8fd387f 100644 --- a/lib/upload/uploader/UploadFile.spec.ts +++ b/lib/upload/uploader/UploadFile.spec.ts @@ -192,7 +192,7 @@ describe('upload status and events', () => { const queue = { add: vi.fn((fn: () => Promise) => fn()) } await uploadFile.start(queue as never) - await queue.add.mock.calls[0][0]() + await Promise.all(queue.add.mock.results.map((r) => r.value)) expect(uploadFile.source).toBe('/destination/a b&c.txt') expect(uploadDataMock).toHaveBeenCalledWith('/destination/a%20b%26c.txt', expect.anything(), expect.anything()) @@ -218,6 +218,15 @@ describe('upload status and events', () => { expect(requestSpy.mock.lastCall![0].headers!.Destination).toBe('/destination/a%20b%26c.txt') }) + it('rebases the upload to a new destination', () => { + isPublicShareMock.mockReturnValue(false) + getMaxChunksSizeMock.mockReturnValue(1024) + + const uploadFile = new UploadFile('/destination/a.txt', new File(['x'], 'a.txt'), { noChunking: true }) + uploadFile.rebase('/destination/folder (2)/a.txt') + expect(uploadFile.source).toBe('/destination/folder (2)/a.txt') + }) + it('scheduled', async () => { isPublicShareMock.mockReturnValue(false) getMaxChunksSizeMock.mockReturnValue(1024) diff --git a/lib/upload/uploader/UploadFileTree.spec.ts b/lib/upload/uploader/UploadFileTree.spec.ts index 51b92f621..a0a615231 100644 --- a/lib/upload/uploader/UploadFileTree.spec.ts +++ b/lib/upload/uploader/UploadFileTree.spec.ts @@ -22,6 +22,7 @@ const uploadFileMocks = vi.hoisted(() => { signal: AbortSignal start: ReturnType cancel: ReturnType + rebase: ReturnType status: number }> = [] @@ -44,6 +45,10 @@ const uploadFileMocks = vi.hoisted(() => { this.status = UploadStatus.CANCELLED }) + public rebase = vi.fn((source: string) => { + this.source = source + }) + // eslint-disable-next-line @typescript-eslint/no-unused-vars public constructor(source: string, _file: File, _options: unknown) { this.source = source @@ -191,8 +196,35 @@ describe('UploadFileTree', () => { expect(tree.status).toBe(UploadStatus.FINISHED) }) + it('rebases already initialized children when a folder is renamed', async () => { + // every MKCOL reports an existing directory so conflicts are resolved on all levels + axiosRequestMock.mockRejectedValue({ response: { status: 405 } }) + isAxiosErrorMock.mockReturnValue(true) + + const conflictCallback = vi.fn(async (nodes: string[]) => Object.fromEntries(nodes.map((node) => [node, node === 'folder' ? 'folder (2)' : node]))) + + const directory = await createDirectoryTree() + const tree = new UploadFileTree('/destination', directory, { callback: conflictCallback }) + tree.initialize() + + const nested = uploadFileMocks.instances[0] + expect(nested.source).toBe('/destination/folder/nested.txt') + + await tree.start(createQueue()) + + expect(tree.children[0].source).toBe('/destination/folder (2)') + // the grandchild was already initialized, but still moved with its parent + expect(nested.source).toBe('/destination/folder (2)/nested.txt') + // so the renamed folder is created - and entered - under its new name + expect(axiosRequestMock.mock.calls.map(([{ url }]) => url)).toEqual([ + '/destination', + '/destination/folder%20(2)', + ]) + expect(conflictCallback).toHaveBeenCalledWith(['nested.txt'], '/destination/folder (2)') + expect(tree.status).toBe(UploadStatus.FINISHED) + }) + it('keeps sources unencoded but encodes them for requests', async () => { - // MKCOL fails with 405 so the directories already exist and conflicts need to be resolved axiosRequestMock.mockRejectedValue({ response: { status: 405 } }) isAxiosErrorMock.mockReturnValue(true) @@ -206,7 +238,7 @@ describe('UploadFileTree', () => { const tree = new UploadFileTree('/destination', directory, { callback: conflictCallback }) const children = tree.initialize() - // the sources are the plain (unencoded) names so they can be matched by the conflict callback + // the sources are the plain names, so the conflict callback can match them expect(children.map((child) => child.source)).toEqual([ '/destination/sub folder', '/destination/a b&c.txt', @@ -215,10 +247,9 @@ describe('UploadFileTree', () => { await tree.start(createQueue()) - // the conflict callback receives plain names, not encoded ones expect(conflictCallback).toHaveBeenCalledWith(['sub folder', 'a b&c.txt'], '/destination') expect(conflictCallback).toHaveBeenCalledWith(['näme #1.txt'], '/destination/sub folder') - // … while the requests use the encoded URLs + // … while the requests use encoded URLs expect(axiosRequestMock.mock.calls.map(([{ url }]) => url)).toEqual([ '/destination', '/destination/sub%20folder', diff --git a/lib/upload/uploader/UploadFileTree.ts b/lib/upload/uploader/UploadFileTree.ts index e1a6f476f..351df522e 100644 --- a/lib/upload/uploader/UploadFileTree.ts +++ b/lib/upload/uploader/UploadFileTree.ts @@ -95,6 +95,22 @@ export class UploadFileTree extends Upload implements IUpload { return [...this.#children] } + /** + * Move this upload - and all already initialized descendants - to a new destination. + * + * The whole tree is initialized upfront, so renaming a directory while resolving + * conflicts also needs to re-base all of its children. + * + * @param source - The new destination of this upload + */ + public rebase(source: string): void { + for (const child of this.#children) { + // read the name before the parent source is updated + child.rebase(concatUrl(source, basename(child.source))) + } + super.rebase(source) + } + /** * Set up all child uploads for this upload tree. */ @@ -152,7 +168,8 @@ export class UploadFileTree extends Upload implements IUpload { if (newName === undefined) { childUpload.cancel() } else if (newName !== originalName) { - Object.defineProperty(childUpload, 'source', { value: concatUrl(this.source, newName) }) + // for directories this also re-bases all of their children + childUpload.rebase(concatUrl(this.source, newName)) } } } diff --git a/lib/upload/utils/url.ts b/lib/upload/utils/url.ts index d6baf6822..b5458fe34 100644 --- a/lib/upload/utils/url.ts +++ b/lib/upload/utils/url.ts @@ -23,7 +23,7 @@ export function concatUrl(base: string, path: string): string { /** * URL encode the path of a decoded URL, leaving a potential origin untouched. - * This must be used whenever a source is used for a request (URL or `Destination` header). + * This must be used whenever a source is used for a request (as URL or `Destination` header). * * @param url - The decoded URL, either absolute ("https://example.com/dav/a b.txt") or a path ("/dav/a b.txt") */