mirror of
https://github.com/filebrowser/filebrowser.git
synced 2026-07-17 16:36:49 +00:00
fix: avoid recursive conflict checks for copy and move (#6009)
This commit is contained in:
parent
aac2516637
commit
dfc2e887e1
3 changed files with 97 additions and 44 deletions
|
|
@ -71,7 +71,9 @@ const event = {
|
|||
describe("copy and move conflict prompts", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
vi.mocked(checkConflict).mockResolvedValue(conflict);
|
||||
vi.mocked(checkConflict).mockResolvedValue(
|
||||
conflict as ConflictingResource[]
|
||||
);
|
||||
});
|
||||
|
||||
it("waits for copy conflict detection before calling the copy API", async () => {
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import { files as api } from "@/api";
|
|||
|
||||
vi.mock("@/api", () => ({
|
||||
files: {
|
||||
fetch: vi.fn(),
|
||||
fetchAll: vi.fn(),
|
||||
},
|
||||
}));
|
||||
|
|
@ -19,8 +20,8 @@ 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.
|
||||
// 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)}`,
|
||||
|
|
@ -167,29 +168,46 @@ describe("checkConflict", () => {
|
|||
expect(conflicts[0].name).toBe("/target/folder/deep/file.txt");
|
||||
});
|
||||
|
||||
// Copy/move stats the whole destination, so a same-named directory is a
|
||||
// conflict in its own right (regression for the directory case of #5957).
|
||||
it("reports a directory conflict for copy/move (includeDirectories)", async () => {
|
||||
vi.mocked(api.fetchAll).mockResolvedValue([
|
||||
{
|
||||
path: "/target/folder",
|
||||
name: "folder",
|
||||
size: 0,
|
||||
modified: "2026-06-04T00:00:00Z",
|
||||
isDir: true,
|
||||
},
|
||||
]);
|
||||
// Copy/move only needs the target directory's direct children. A recursive
|
||||
// walk can make the UI look frozen on large destinations (regression #6005).
|
||||
it("checks only the direct destination listing for copy/move", async () => {
|
||||
vi.mocked(api.fetch).mockResolvedValue({
|
||||
items: [
|
||||
{
|
||||
path: "/target/file.txt",
|
||||
name: "file.txt",
|
||||
size: 10,
|
||||
modified: "2026-06-04T00:00:00Z",
|
||||
isDir: false,
|
||||
},
|
||||
{
|
||||
path: "/target/folder",
|
||||
name: "folder",
|
||||
size: 0,
|
||||
modified: "2026-06-04T00:00:00Z",
|
||||
isDir: true,
|
||||
},
|
||||
],
|
||||
} as Resource);
|
||||
|
||||
const items = [{ ...moveItem("folder", "/files/target/", 0), isDir: true }];
|
||||
const items = [
|
||||
moveItem("file.txt", "/files/target/"),
|
||||
{ ...moveItem("folder", "/files/target/", 0), isDir: true },
|
||||
];
|
||||
|
||||
const conflicts = await checkConflict(items, "/files/target/", true);
|
||||
|
||||
expect(conflicts).toHaveLength(1);
|
||||
expect(conflicts[0].name).toBe("/target/folder");
|
||||
expect(api.fetch).toHaveBeenCalledWith("/files/target/");
|
||||
expect(api.fetchAll).not.toHaveBeenCalled();
|
||||
expect(conflicts).toHaveLength(2);
|
||||
expect(conflicts.map((conflict) => conflict.name)).toEqual([
|
||||
"/target/file.txt",
|
||||
"/target/folder",
|
||||
]);
|
||||
});
|
||||
|
||||
// Uploads merge into an existing folder, so the directory itself must not be
|
||||
// reported — only the files inside it can conflict.
|
||||
// reported - only the files inside it can conflict.
|
||||
it("ignores a directory conflict for uploads (default)", async () => {
|
||||
vi.mocked(api.fetchAll).mockResolvedValue([
|
||||
{
|
||||
|
|
|
|||
|
|
@ -21,20 +21,61 @@ function conflictKey(item: UploadEntry): string {
|
|||
return (item.fullPath || item.name).replace(/^\/+/, "");
|
||||
}
|
||||
|
||||
type ServerConflictEntry = {
|
||||
path: string;
|
||||
name: string;
|
||||
size: number;
|
||||
modified: string;
|
||||
};
|
||||
|
||||
async function fetchConflictEntries(
|
||||
basePath: string,
|
||||
includeDirectories: boolean
|
||||
): Promise<ServerConflictEntry[]> {
|
||||
if (!includeDirectories) {
|
||||
return await api.fetchAll(basePath);
|
||||
}
|
||||
|
||||
const destination = await api.fetch(basePath);
|
||||
return destination.items ?? [];
|
||||
}
|
||||
|
||||
function conflictPath(entry: ServerConflictEntry): string {
|
||||
return entry.path.replace(/\\/g, "/");
|
||||
}
|
||||
|
||||
function buildConflictMap(
|
||||
serverEntries: ServerConflictEntry[],
|
||||
basePath: string,
|
||||
includeDirectories: boolean
|
||||
): Map<string, ServerConflictEntry> {
|
||||
const serverMap = new Map<string, ServerConflictEntry>();
|
||||
const normBase = removePrefix(basePath).replace(/\/+$/, "");
|
||||
for (const entry of serverEntries) {
|
||||
// A Windows server may return OS-native backslash separators; normalize to
|
||||
// forward slashes so the prefix strip and key lookup line up.
|
||||
const path = conflictPath(entry);
|
||||
const key = includeDirectories
|
||||
? entry.name
|
||||
: path.startsWith(normBase)
|
||||
? path.slice(normBase.length)
|
||||
: path;
|
||||
serverMap.set(key.replace(/^\/+/, ""), entry);
|
||||
}
|
||||
|
||||
return serverMap;
|
||||
}
|
||||
|
||||
/**
|
||||
* 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 entry is looked up directly — no need to mirror
|
||||
* the upload's folder structure.
|
||||
*
|
||||
* Directory handling differs by action, hence `includeDirectories`:
|
||||
* - Upload (false): an existing folder is silently merged, so only the
|
||||
* individual files inside it can conflict.
|
||||
* - Copy/move (true): the server stats the destination and rejects it whole if
|
||||
* a same-named entry exists, so the directory itself is a conflict. The list
|
||||
* only holds the top-level items being moved, so each is reported once.
|
||||
* - Upload (false): the destination tree is fetched recursively so nested
|
||||
* file uploads can be checked; existing folders are silently merged.
|
||||
* - Copy/move (true): only the destination directory itself is fetched. These
|
||||
* operations move flat top-level selections, so a same-named direct child is
|
||||
* the only preflight conflict the backend will reject.
|
||||
*
|
||||
* @param files - flat upload list to check
|
||||
* @param basePath - server destination path (e.g. "/files/uploads/")
|
||||
|
|
@ -47,27 +88,19 @@ export async function checkConflict(
|
|||
): Promise<ConflictingResource[]> {
|
||||
if (files.length === 0) return [];
|
||||
|
||||
let serverEntries: RecursiveEntry[];
|
||||
let serverEntries: ServerConflictEntry[];
|
||||
try {
|
||||
// Single API call: fetch the entire server tree under basePath.
|
||||
serverEntries = await api.fetchAll(basePath);
|
||||
serverEntries = await fetchConflictEntries(basePath, includeDirectories);
|
||||
} catch {
|
||||
// The destination doesn't exist yet, so nothing can conflict.
|
||||
return [];
|
||||
}
|
||||
|
||||
// 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) {
|
||||
// A Windows server may return OS-native backslash separators; normalize to
|
||||
// forward slashes so the prefix strip and key lookup line up.
|
||||
const path = entry.path.replace(/\\/g, "/");
|
||||
const rel = path.startsWith(normBase) ? path.slice(normBase.length) : path;
|
||||
serverMap.set(rel.replace(/^\/+/, ""), entry);
|
||||
}
|
||||
const serverMap = buildConflictMap(
|
||||
serverEntries,
|
||||
basePath,
|
||||
includeDirectories
|
||||
);
|
||||
|
||||
const conflicts: ConflictingResource[] = [];
|
||||
files.forEach((file, index) => {
|
||||
|
|
@ -78,7 +111,7 @@ export async function checkConflict(
|
|||
|
||||
conflicts.push({
|
||||
index,
|
||||
name: server.path,
|
||||
name: conflictPath(server),
|
||||
origin: { lastModified: file.file?.lastModified, size: file.size },
|
||||
dest: { lastModified: server.modified, size: server.size },
|
||||
checked: ["origin"],
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue