mirror of
https://github.com/filebrowser/filebrowser.git
synced 2026-07-17 16:36:49 +00:00
refactor: cleanup and simplify upload.ts
This commit is contained in:
Henrique Dias
parent
998bd95bfa
commit
5f7311d324
2 changed files with 227 additions and 128 deletions
180
frontend/src/utils/__tests__/check-conflict.test.ts
Normal file
180
frontend/src/utils/__tests__/check-conflict.test.ts
Normal file
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
|
|
@ -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<string, UploadEntryWithChild> = {};
|
||||
|
||||
// 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<ConflictingResource[]> {
|
||||
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<string, RecursiveEntry>();
|
||||
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;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue