diff --git a/frontend/src/utils/__tests__/check-conflict.test.ts b/frontend/src/utils/__tests__/check-conflict.test.ts new file mode 100644 index 00000000..e73bd979 --- /dev/null +++ b/frontend/src/utils/__tests__/check-conflict.test.ts @@ -0,0 +1,180 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { checkConflict } from "@/utils/upload"; +import { files as api } from "@/api"; + +vi.mock("@/api", () => ({ + files: { + fetchAll: vi.fn(), + }, +})); + +vi.mock("@/api/utils", () => ({ + removePrefix: (value: string) => value.replace(/^\/files/, ""), +})); + +// upload.ts imports these at module load; they reach window-bound constants +// which don't exist in the node test environment. checkConflict never uses +// them, so empty stubs keep the import graph from blowing up. +vi.mock("@/stores/layout", () => ({ useLayoutStore: vi.fn() })); +vi.mock("@/stores/upload", () => ({ useUploadStore: vi.fn() })); +vi.mock("@/utils/url", () => ({ default: {} })); + +// A move/copy/drag item carries `name` (raw) and `to` (URL-encoded) but no +// `fullPath` — mirroring what Move.vue / Copy.vue / ListingItem.vue build. +function moveItem(name: string, dest: string, size = 12) { + return { + from: `/files/source/${encodeURIComponent(name)}`, + to: dest + encodeURIComponent(name), + name, + size, + isDir: false, + overwrite: false, + rename: false, + }; +} + +describe("checkConflict", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("detects a conflict for a plain filename", async () => { + vi.mocked(api.fetchAll).mockResolvedValue([ + { + path: "/target/file.txt", + name: "file.txt", + size: 10, + modified: "2026-06-04T00:00:00Z", + isDir: false, + }, + ]); + + const conflicts = await checkConflict( + [moveItem("file.txt", "/files/target/")], + "/files/target/" + ); + + expect(conflicts).toHaveLength(1); + expect(conflicts[0].name).toBe("/target/file.txt"); + }); + + // Regression for #5957: names with encodable characters (spaces, "#", + // non-ASCII) were keyed by the URL-encoded `to` value and never matched the + // server's raw path, so the conflict modal was skipped and the backend + // returned a bare 409 instead. + it.each(["my file.txt", "résumé.pdf", "a#b.txt"])( + "detects a conflict for %s (encodable characters)", + async (name) => { + vi.mocked(api.fetchAll).mockResolvedValue([ + { + path: `/target/${name}`, + name, + size: 10, + modified: "2026-06-04T00:00:00Z", + isDir: false, + }, + ]); + + const conflicts = await checkConflict( + [moveItem(name, "/files/target/")], + "/files/target/" + ); + + expect(conflicts).toHaveLength(1); + expect(conflicts[0].name).toBe(`/target/${name}`); + } + ); + + it("reports no conflict when the destination has no matching name", async () => { + vi.mocked(api.fetchAll).mockResolvedValue([ + { + path: "/target/other.txt", + name: "other.txt", + size: 10, + modified: "2026-06-04T00:00:00Z", + isDir: false, + }, + ]); + + const conflicts = await checkConflict( + [moveItem("my file.txt", "/files/target/")], + "/files/target/" + ); + + expect(conflicts).toHaveLength(0); + }); + + it("detects nested conflicts for folder uploads via fullPath", async () => { + vi.mocked(api.fetchAll).mockResolvedValue([ + { + path: "/target/folder", + name: "folder", + size: 0, + modified: "2026-06-04T00:00:00Z", + isDir: true, + }, + { + path: "/target/folder/nested file.txt", + name: "nested file.txt", + size: 10, + modified: "2026-06-04T00:00:00Z", + isDir: false, + }, + ]); + + const files = [ + { name: "folder", size: 0, isDir: true, fullPath: "folder" }, + { + name: "nested file.txt", + size: 12, + isDir: false, + fullPath: "folder/nested file.txt", + }, + ]; + + const conflicts = await checkConflict(files, "/files/target/"); + + expect(conflicts).toHaveLength(1); + expect(conflicts[0].name).toBe("/target/folder/nested file.txt"); + }); + + // The "upload folder" file input pushes only files (with a relative + // fullPath) and no directory entries. Conflict detection must still find a + // nested file even though its parent folder is not in the upload list. + it("detects nested conflicts when no directory entries are present", async () => { + vi.mocked(api.fetchAll).mockResolvedValue([ + { + path: "/target/folder/deep/file.txt", + name: "file.txt", + size: 10, + modified: "2026-06-04T00:00:00Z", + isDir: false, + }, + ]); + + const files = [ + { + name: "file.txt", + size: 12, + isDir: false, + fullPath: "folder/deep/file.txt", + }, + ]; + + const conflicts = await checkConflict(files, "/files/target/"); + + expect(conflicts).toHaveLength(1); + expect(conflicts[0].name).toBe("/target/folder/deep/file.txt"); + }); + + it("returns no conflicts when the recursive listing fails", async () => { + vi.mocked(api.fetchAll).mockRejectedValue(new Error("404")); + + const conflicts = await checkConflict( + [moveItem("file.txt", "/files/target/")], + "/files/target/" + ); + + expect(conflicts).toHaveLength(0); + }); +}); diff --git a/frontend/src/utils/upload.ts b/frontend/src/utils/upload.ts index 060f5d94..994c95be 100644 --- a/frontend/src/utils/upload.ts +++ b/frontend/src/utils/upload.ts @@ -4,159 +4,78 @@ import url from "@/utils/url"; import { files as api } from "@/api"; import { removePrefix } from "@/api/utils"; -interface UploadEntryWithChild extends UploadEntry { - children?: UploadEntryWithChild[]; - originalIndex: number; -} - /** - * Convert UploadList into a forest (array of root nodes). - * This properly handles uploads with multiple top-level files/folders - * instead of assuming a single root. - * @param flatArray - * @param basePath + * The path used to detect conflicts against the server's recursive listing. + * + * It MUST be the raw, un-encoded path relative to the destination: + * - `fullPath` is set for folder uploads and drag & drop (e.g. "sub/file.txt"). + * - `name` is the leaf for every other case (copy/move/paste/single upload), + * which is always a flat top-level entry. + * + * We never key on `item.to`: it is URL-encoded (`dest + encodeURIComponent(name)`) + * and would miss conflicts for any name with encodable characters (spaces, "#", + * non-ASCII, ...), surfacing a raw 409 error instead of the conflict modal. + * @param item */ -function flatToForest( - flatArray: UploadList, - basePath: string -): UploadEntryWithChild[] { - const nodeMap: Record = {}; - - // First pass: create all nodes - flatArray.forEach((item, index) => { - // File list is created from very different action and info available are not always the same. - // By doing a drag and drop or upload a folder (both from the browser or from the OS) we have the fullPath property available - // By uploading a single file using the file input, we only have the "name" property - // By doing drag and drop from filebrowser to filebrowser, we have the "to" property available but not the fullPath - const fullPathOrTo = - item.fullPath || item.to?.replace(basePath, "") || item.name; - nodeMap[fullPathOrTo!] = { - fullPath: fullPathOrTo, - isDir: item.isDir, - name: item.name, - size: item.size, - originalIndex: index, - ...(item.isDir && { children: [] }), - ...(item.file && { file: item.file }), - }; - }); - - const roots: UploadEntryWithChild[] = []; - - // Second pass: build hierarchy - flatArray.forEach((item) => { - // see comment before to explanation - const fullPathOrTo = - item.fullPath || item.to?.replace(basePath, "") || item.name; - - const node = nodeMap[fullPathOrTo!]; - const lastSlash = fullPathOrTo!.lastIndexOf("/"); - - if (lastSlash === -1) { - roots.push(node); - } else { - const parentPath = fullPathOrTo!.substring(0, lastSlash); - const parent = nodeMap[parentPath]; - if (parent?.children) { - parent.children.push(node); - } - } - }); - - return roots; +function conflictKey(item: UploadEntry): string { + return (item.fullPath || item.name).replace(/^\/+/, ""); } /** - * Return conflict files from - * @param files - flat upload list to check - * @param basePath - server destination path (e.g. "/files/uploads/") + * Return the entries from `files` that already exist under `basePath` on the + * server, so the caller can prompt the user to overwrite/rename/skip. + * + * The whole destination tree is fetched once and indexed by path relative to + * the destination, then every file is looked up directly — no need to mirror + * the upload's folder structure. Directories are never reported: an existing + * folder is merged on upload, and a copy/move onto one is rejected server-side. + * + * @param files - flat upload list to check + * @param basePath - server destination path (e.g. "/files/uploads/") */ export async function checkConflict( files: UploadList, basePath: string ): Promise { - console.log( - "Starting conflict check, " + files.length + " possible conflict found." - ); + if (files.length === 0) return []; - const forest = flatToForest(files, basePath); - if (forest.length === 0) return []; - - // Single API call: fetch the entire server tree under basePath. - let serverEntries: RecursiveEntry[] = []; + let serverEntries: RecursiveEntry[]; try { + // Single API call: fetch the entire server tree under basePath. serverEntries = await api.fetchAll(basePath); } catch { - console.error( - `Failed to fetch recursive server listing for ${basePath}. ` + - `Assuming directory doesn't exist and skipping conflict check.` - ); + // The destination doesn't exist yet, so nothing can conflict. return []; } - // Build a lookup map keyed by the normalised server path for O(1) access. - // The server returns paths relative to the user's scope (e.g. "/uploads/foo.txt"). - // We strip the basePath prefix so the key matches the upload entry's fullPath. + // The server returns paths absolute within the user's scope + // (e.g. "/uploads/sub/file.txt"). Strip the basePath prefix so the keys line + // up with each entry's conflictKey, which is relative to the destination. const normBase = removePrefix(basePath).replace(/\/+$/, ""); const serverMap = new Map(); for (const entry of serverEntries) { - // entry.path is absolute from server root, e.g. "/uploads/sub/file.txt" - // We need the relative part after normBase, e.g. "sub/file.txt" - let rel = entry.path; - if (rel.startsWith(normBase)) { - rel = rel.slice(normBase.length); - } - // Strip leading slash so it matches fullPath format ("sub/file.txt") - rel = rel.replace(/^\/+/, ""); - serverMap.set(rel, entry); + const rel = entry.path.startsWith(normBase) + ? entry.path.slice(normBase.length) + : entry.path; + serverMap.set(rel.replace(/^\/+/, ""), entry); } const conflicts: ConflictingResource[] = []; + files.forEach((file, index) => { + if (file.isDir) return; // see directory note above - /** - * Walk the upload tree and compare each file node against the - * pre-fetched server map. Directories only need to be recursed - * when they appear in the map (otherwise no child can conflict). - */ - function recursiveCheckConflict(nodes: UploadEntryWithChild[]): void { - for (const node of nodes) { - if (node.isDir && node.children) { - // Only recurse if this directory exists on the server - const dirKey = node.fullPath!.replace(/^\/+/, ""); - if (serverMap.has(dirKey)) { - recursiveCheckConflict(node.children); - } - } else { - // File – check for a conflict against the server map - const fileKey = node.fullPath!.replace(/^\/+/, ""); - const serverEntry = serverMap.get(fileKey); + const server = serverMap.get(conflictKey(file)); + if (!server) return; - if (serverEntry) { - conflicts.push({ - index: node.originalIndex, - name: serverEntry.path, - origin: { - lastModified: node.file?.lastModified, - size: node.size, - }, - dest: { - lastModified: serverEntry.modified, - size: serverEntry.size, - }, - checked: ["origin"], - isSmallerOnServer: node.size > serverEntry.size, - }); - } - } - } - } - - // Walk all root nodes synchronously against the pre-fetched data - recursiveCheckConflict(forest); - - console.log(conflicts.length + " conflicts found."); - - conflicts.sort((a, b) => a.index - b.index); + conflicts.push({ + index, + name: server.path, + origin: { lastModified: file.file?.lastModified, size: file.size }, + dest: { lastModified: server.modified, size: server.size }, + checked: ["origin"], + isSmallerOnServer: file.size > server.size, + }); + }); return conflicts; }