diff --git a/src/backend/controllers/fs/LegacyFSController.test.ts b/src/backend/controllers/fs/LegacyFSController.test.ts index 4954c58f1..0b02dd530 100644 --- a/src/backend/controllers/fs/LegacyFSController.test.ts +++ b/src/backend/controllers/fs/LegacyFSController.test.ts @@ -928,6 +928,87 @@ describe('LegacyFSController.copy', () => { // The entry must still exist — the ghost handler must NOT have run. expect(await server.stores.fsEntry.getEntryByPath(src)).not.toBeNull(); }); + + it('surfaces a name collision, then reports and removes the replaced entry on overwrite', async () => { + const { actor } = await makeUser(); + const username = actor.user!.username!; + const src = `/${username}/Documents/dup.txt`; + const existing = `/${username}/Pictures/dup.txt`; + for ( const p of [src, existing] ) { + await withActor(actor, () => + controller.touch( + makeReq({ body: { path: p }, actor }), + makeRes().res, + ), + ); + } + + // Without overwrite: the v1 conflict contract the GUI's + // replace/skip prompts key on. + await expect( + withActor(actor, () => + controller.copy( + makeReq({ + body: { + source: src, + destination: `/${username}/Pictures`, + }, + actor, + }), + makeRes().res, + ), + ), + ).rejects.toMatchObject({ + statusCode: 409, + legacyCode: 'item_with_same_name_exists', + fields: { entry_name: 'dup.txt' }, + }); + + const replaced = (await server.stores.fsEntry.getEntryByPath( + existing, + ))!; + + // With overwrite: the replaced entry rides along in the response + // (so the caller can drop its row) and item.removed tells every + // other client to do the same — without it they keep a ghost row + // until the directory is re-listed. + const emitSpy = vi.spyOn(server.clients.event, 'emit'); + let body: Array<{ + copied: { path: string }; + overwritten?: { id: string }; + }>; + let removedCall: (typeof emitSpy.mock.calls)[number] | undefined; + try { + const { res, captured } = makeRes(); + await withActor(actor, () => + controller.copy( + makeReq({ + body: { + source: src, + destination: `/${username}/Pictures`, + overwrite: true, + }, + actor, + }), + res, + ), + ); + body = captured.body as typeof body; + removedCall = emitSpy.mock.calls.find( + ([eventName]) => eventName === 'outer.gui.item.removed', + ); + } finally { + emitSpy.mockRestore(); + } + + expect(body[0].copied.path).toBe(existing); + expect(body[0].overwritten?.id).toBe(replaced.uuid); + expect(removedCall).toBeTruthy(); + const removedPayload = removedCall?.[1] as { + response?: { uid?: string }; + }; + expect(removedPayload.response?.uid).toBe(replaced.uuid); + }); }); // ── move ──────────────────────────────────────────────────────────── @@ -1004,6 +1085,89 @@ describe('LegacyFSController.move', () => { const body = captured.body as { moved: { path: string } }; expect(body.moved.path).toBe(`/${username}/Pictures/bar`); }); + + it('surfaces a name collision, then reports and removes the replaced entry on overwrite', async () => { + const { actor } = await makeUser(); + const username = actor.user!.username!; + const src = `/${username}/Documents/clash.txt`; + const existing = `/${username}/Pictures/clash.txt`; + for ( const p of [src, existing] ) { + await withActor(actor, () => + controller.touch( + makeReq({ body: { path: p }, actor }), + makeRes().res, + ), + ); + } + + // Without overwrite: the v1 conflict contract the GUI's + // replace/skip prompts key on. + await expect( + withActor(actor, () => + controller.move( + makeReq({ + body: { + source: src, + destination: `/${username}/Pictures`, + }, + actor, + }), + makeRes().res, + ), + ), + ).rejects.toMatchObject({ + statusCode: 409, + legacyCode: 'item_with_same_name_exists', + fields: { entry_name: 'clash.txt' }, + }); + + const replaced = (await server.stores.fsEntry.getEntryByPath( + existing, + ))!; + + // With overwrite: the replaced entry rides along in the response + // (so the caller can drop its row) and item.removed tells every + // other client to do the same — without it they keep a ghost row + // until the directory is re-listed. + const emitSpy = vi.spyOn(server.clients.event, 'emit'); + let body: { + moved: { path: string }; + old_path: string; + overwritten?: { id: string }; + }; + let removedCall: (typeof emitSpy.mock.calls)[number] | undefined; + try { + const { res, captured } = makeRes(); + await withActor(actor, () => + controller.move( + makeReq({ + body: { + source: src, + destination: `/${username}/Pictures`, + overwrite: true, + }, + actor, + }), + res, + ), + ); + body = captured.body as typeof body; + removedCall = emitSpy.mock.calls.find( + ([eventName]) => eventName === 'outer.gui.item.removed', + ); + } finally { + emitSpy.mockRestore(); + } + + expect(body.old_path).toBe(src); + expect(body.moved.path).toBe(existing); + expect(body.overwritten?.id).toBe(replaced.uuid); + expect(removedCall).toBeTruthy(); + const removedPayload = removedCall?.[1] as { + response?: { uid?: string }; + }; + expect(removedPayload.response?.uid).toBe(replaced.uuid); + }); }); // ── search ────────────────────────────────────────────────────────── diff --git a/src/backend/controllers/fs/LegacyFSController.ts b/src/backend/controllers/fs/LegacyFSController.ts index dc9d78255..b7cb346ea 100644 --- a/src/backend/controllers/fs/LegacyFSController.ts +++ b/src/backend/controllers/fs/LegacyFSController.ts @@ -166,8 +166,7 @@ export class LegacyFSController extends PuterController { router.get('/get-launch-apps', apiOptions, async (req, res) => { const recommendedSvc = this.services.recommendedApps as unknown as - | { getRecommendedApps?: () => Promise } - | undefined; + { getRecommendedApps?: () => Promise } | undefined; const recommended = recommendedSvc?.getRecommendedApps ? await recommendedSvc.getRecommendedApps() : []; @@ -589,26 +588,61 @@ export class LegacyFSController extends PuterController { 'write', ); + // The v1 wire contract reports the entry an overwrite replaced + // (`overwritten`) so clients can drop its stale row/icon. The copy + // deletes that entry, so resolve it beforehand. + const overwriteRequested = getBoolean(body, 'overwrite') ?? false; + let overwrittenEntry = null; + if (overwriteRequested) { + const targetName = getString(body, 'new_name') ?? source.name; + const targetPath = + destinationParent.path === '/' + ? `/${targetName}` + : `${destinationParent.path}/${targetName}`; + overwrittenEntry = + await this.stores.fsEntry.getEntryByPath(targetPath); + } + const copy = await this.services.fs.copy(userId, { source, destinationParent, newName: getString(body, 'new_name'), - overwrite: getBoolean(body, 'overwrite') ?? false, + overwrite: overwriteRequested, dedupeName: getBoolean(body, 'dedupe_name', 'change_name') ?? false, }); await this.#emitGuiEvent('outer.gui.item.added', copy); + // Without this, every other client keeps a ghost row for the + // replaced entry until the directory is re-listed. + if (overwrittenEntry) { + await this.#emitGuiEvent( + 'outer.gui.item.removed', + overwrittenEntry, + ); + } // Legacy response shape: `[{copied: fsentry, overwritten?}]`. // Array is historical — originally supported bulk copy. - const copied = await toLegacyEntry(this.clients.event, copy, { + const legacyEntryOpts = { fsEntryStore: this.stores.fsEntry, userStore: this.stores.user as unknown as { getById: ( id: number, ) => Promise | null>; }, - }); - res.json([{ copied }]); + }; + const copied = await toLegacyEntry( + this.clients.event, + copy, + legacyEntryOpts, + ); + const overwritten = overwrittenEntry + ? await toLegacyEntry( + this.clients.event, + overwrittenEntry, + legacyEntryOpts, + ) + : undefined; + res.json([{ copied, ...(overwritten ? { overwritten } : {}) }]); }; move = async (req: Request, res: Response): Promise => { @@ -640,36 +674,77 @@ export class LegacyFSController extends PuterController { 'write', ); + // The v1 wire contract reports the entry an overwrite replaced + // (`overwritten`) so clients can drop its stale row/icon. The move + // deletes that entry, so resolve it beforehand. + const overwriteRequested = getBoolean(body, 'overwrite') ?? false; + let overwrittenEntry = null; + if (overwriteRequested) { + const targetName = getString(body, 'new_name') ?? source.name; + const targetPath = + destinationParent.path === '/' + ? `/${targetName}` + : `${destinationParent.path}/${targetName}`; + const existing = + await this.stores.fsEntry.getEntryByPath(targetPath); + // Moving an entry onto its own path is not an overwrite. + if (existing && existing.uuid !== source.uuid) { + overwrittenEntry = existing; + } + } + const moved = await this.services.fs.move(userId, { source, destinationParent, newName: getString(body, 'new_name'), - overwrite: getBoolean(body, 'overwrite') ?? false, + overwrite: overwriteRequested, dedupeName: getBoolean(body, 'dedupe_name', 'change_name') ?? false, // Trash/restore rides on this: GUI sends // `{ original_name, original_path, trashed_ts }` when moving into // Trash, and `null`/`{}` when restoring. See // `src/gui/src/helpers.js` → `window.move_items`. newMetadata: (body.new_metadata ?? undefined) as - | Record - | null - | undefined, + Record | null | undefined, }); const oldPath = source.path; await this.#emitGuiEvent('outer.gui.item.moved', moved, { old_path: oldPath, }); + // Without this, every other client keeps a ghost row for the + // replaced entry until the directory is re-listed. + if (overwrittenEntry) { + await this.#emitGuiEvent( + 'outer.gui.item.removed', + overwrittenEntry, + ); + } - // Legacy response shape: `{moved: fsentry, old_path}`. - const movedEntry = await toLegacyEntry(this.clients.event, moved, { + // Legacy response shape: `{moved: fsentry, old_path, overwritten?}`. + const legacyEntryOpts = { fsEntryStore: this.stores.fsEntry, userStore: this.stores.user as unknown as { getById: ( id: number, ) => Promise | null>; }, + }; + const movedEntry = await toLegacyEntry( + this.clients.event, + moved, + legacyEntryOpts, + ); + const overwritten = overwrittenEntry + ? await toLegacyEntry( + this.clients.event, + overwrittenEntry, + legacyEntryOpts, + ) + : undefined; + res.json({ + moved: movedEntry, + old_path: oldPath, + ...(overwritten ? { overwritten } : {}), }); - res.json({ moved: movedEntry, old_path: oldPath }); }; delete = async (req: Request, res: Response): Promise => { @@ -1090,8 +1165,7 @@ export class LegacyFSController extends PuterController { } type SignedOrEmpty = - | (SignedFile & { path?: string }) - | Record; + (SignedFile & { path?: string }) | Record; const result: { signatures: SignedOrEmpty[]; token?: string } = { signatures: [], }; @@ -1631,10 +1705,7 @@ export class LegacyFSController extends PuterController { const subjectRef = body.subject; const appRef = body.app; const mode = (getString(body, 'mode') ?? 'read') as - | 'see' - | 'list' - | 'read' - | 'write'; + 'see' | 'list' | 'read' | 'write'; if (!subjectRef || !appRef) throw new HttpError(400, '`subject` and `app` are required', { legacyCode: 'bad_request', diff --git a/src/backend/services/fs/FSService.test.ts b/src/backend/services/fs/FSService.test.ts index 0ea28c5bd..ad39d2d57 100644 --- a/src/backend/services/fs/FSService.test.ts +++ b/src/backend/services/fs/FSService.test.ts @@ -2248,6 +2248,10 @@ describe('FSService move', () => { fs.move(user.userId, { source, destinationParent: destination }), ); expect(conflict.statusCode).toBe(409); + // v1 wire contract: the GUI's replace/skip prompts key on this + // code + entry_name; a generic 'conflict' makes them fail silently. + expect(conflict.legacyCode).toBe('item_with_same_name_exists'); + expect(conflict.fields).toMatchObject({ entry_name: 'coll.txt' }); const deduped = await fs.move(user.userId, { source, @@ -2461,16 +2465,17 @@ describe('FSService copy', () => { ); await writeFile(user, `${user.home}/Desktop/cp-coll.txt`, 'existing'); - expect( - ( - await caught(() => - fs.copy(user.userId, { - source, - destinationParent: destination, - }), - ) - ).statusCode, - ).toBe(409); + const conflict = await caught(() => + fs.copy(user.userId, { + source, + destinationParent: destination, + }), + ); + expect(conflict.statusCode).toBe(409); + // v1 wire contract: the GUI's replace/skip prompts key on this + // code + entry_name; a generic 'conflict' makes them fail silently. + expect(conflict.legacyCode).toBe('item_with_same_name_exists'); + expect(conflict.fields).toMatchObject({ entry_name: 'cp-coll.txt' }); const deduped = await fs.copy(user.userId, { source, diff --git a/src/gui/src/helpers.js b/src/gui/src/helpers.js index 615006bc1..ca9cfa270 100644 --- a/src/gui/src/helpers.js +++ b/src/gui/src/helpers.js @@ -1255,14 +1255,21 @@ window.copy_clipboard_items = async function (dest_path, dest_container_element) // only show progress window if it takes longer than 2s to copy let progwin; - let progwin_timeout = setTimeout(async () => { + let latest_status; + const arm_progwin = () => setTimeout(async () => { progwin = await UIWindowProgress({ operation_id: copy_op_id, on_cancel: () => { window.operation_cancelled[copy_op_id] = true; }, }); - }, 0); + // Opened mid-operation: show the file being copied rather than + // the default "Preparing..." status. + if ( latest_status ) { + progwin.set_status(latest_status); + } + }, 2000); + let progwin_timeout = arm_progwin(); const copied_item_paths = []; @@ -1270,7 +1277,8 @@ window.copy_clipboard_items = async function (dest_path, dest_container_element) let copy_path = window.clipboard[i].path; let item_with_same_name_already_exists = true; let overwrite = overwrite_all; - progwin?.set_status(i18n('copying_file', copy_path)); + latest_status = i18n('copying_file', copy_path); + progwin?.set_status(latest_status); do { if ( overwrite ) @@ -1296,7 +1304,7 @@ window.copy_clipboard_items = async function (dest_path, dest_container_element) // remove overwritten item from the DOM if ( resp[0].overwritten?.id ) { - $(`.item[data-uid=${resp[0].overwritten.id}]`).removeItems(); + $(`.item[data-uid='${resp[0].overwritten.id}']`).removeItems(); } // copy new path for undo copy @@ -1306,6 +1314,10 @@ window.copy_clipboard_items = async function (dest_path, dest_container_element) break; } catch ( err ) { if ( err.code === 'item_with_same_name_exists' ) { + // The operation is paused on user input, so pause the + // progress-window timer too — otherwise "Preparing..." + // pops up on top of the dialog while it waits. + clearTimeout(progwin_timeout); const alert_resp = await UIAlert({ message: `${html_encode(err.entry_name)} already exists.`, buttons: [ @@ -1314,6 +1326,7 @@ window.copy_clipboard_items = async function (dest_path, dest_container_element) ... (window.clipboard.length > 1) ? [{ label: i18n('skip'), value: 'skip' }] : [{ label: i18n('cancel'), value: 'cancel' }], ], }); + progwin_timeout = arm_progwin(); if ( alert_resp === 'replace' ) { overwrite = true; } else if ( alert_resp === 'replace_all' ) { @@ -1371,14 +1384,21 @@ window.copy_items = function (el_items, dest_path) { // only show progress window if it takes longer than 2s to copy let progwin; - let progwin_timeout = setTimeout(async () => { + let latest_status; + const arm_progwin = () => setTimeout(async () => { progwin = await UIWindowProgress({ operation_id: copy_op_id, on_cancel: () => { window.operation_cancelled[copy_op_id] = true; }, }); + // Opened mid-operation: show the file being copied rather than + // the default "Preparing..." status. + if ( latest_status ) { + progwin.set_status(latest_status); + } }, 2000); + let progwin_timeout = arm_progwin(); const copied_item_paths = []; @@ -1386,7 +1406,8 @@ window.copy_items = function (el_items, dest_path) { let copy_path = $(el_items[i]).attr('data-path'); let item_with_same_name_already_exists = true; let overwrite = overwrite_all; - progwin?.set_status(i18n('copying_file', copy_path)); + latest_status = i18n('copying_file', copy_path); + progwin?.set_status(latest_status); do { if ( overwrite ) @@ -1409,7 +1430,7 @@ window.copy_items = function (el_items, dest_path) { // remove overwritten item from the DOM if ( resp[0].overwritten?.id ) { - $(`.item[data-uid=${resp.overwritten.id}]`).removeItems(); + $(`.item[data-uid='${resp[0].overwritten.id}']`).removeItems(); } // copy new path for undo copy @@ -1419,6 +1440,10 @@ window.copy_items = function (el_items, dest_path) { item_with_same_name_already_exists = false; } catch ( err ) { if ( err.code === 'item_with_same_name_exists' ) { + // The operation is paused on user input, so pause the + // progress-window timer too — otherwise "Preparing..." + // pops up on top of the dialog while it waits. + clearTimeout(progwin_timeout); const alert_resp = await UIAlert({ message: `${html_encode(err.entry_name)} already exists.`, buttons: [ @@ -1427,6 +1452,7 @@ window.copy_items = function (el_items, dest_path) { ... (el_items.length > 1) ? [{ label: i18n('skip'), value: 'skip' }] : [{ label: i18n('cancel'), value: 'cancel' }], ], }); + progwin_timeout = arm_progwin(); if ( alert_resp === 'replace' ) { overwrite = true; } else if ( alert_resp === 'replace_all' ) { @@ -1690,14 +1716,21 @@ window.move_items = async function (el_items, dest_path, is_undo = false) { // only show progress window if it takes longer than 2s to move let progwin; - let progwin_timeout = setTimeout(async () => { + let latest_status; + const arm_progwin = () => setTimeout(async () => { progwin = await UIWindowProgress({ operation_id: move_op_id, on_cancel: () => { window.operation_cancelled[move_op_id] = true; }, }); + // Opened mid-operation: show the file being moved rather than the + // default "Preparing..." status. + if ( latest_status ) { + progwin.set_status(latest_status); + } }, 2000); + let progwin_timeout = arm_progwin(); // storing moved items for undo ability const moved_items = []; @@ -1721,7 +1754,10 @@ window.move_items = async function (el_items, dest_path, is_undo = false) { // cannot move item to its own path, skip it if ( path.dirname($(el_item).attr('data-path')) === dest_path ) { + // pause the progress-window timer while waiting for the user + clearTimeout(progwin_timeout); await UIAlert(`

Moving ${html_encode($(el_item).attr('data-name'))}

Cannot move item to its current location.`); + progwin_timeout = arm_progwin(); continue; } @@ -1794,6 +1830,9 @@ window.move_items = async function (el_items, dest_path, is_undo = false) { // moving an item into a trashed directory? deny. else if ( dest_path.startsWith(window.trash_path) ) { + // the pending timer would otherwise open an orphan + // progress window after the operation already bailed + clearTimeout(progwin_timeout); progwin?.close(); UIAlert('Cannot move items into a deleted folder.'); return; @@ -1813,7 +1852,8 @@ window.move_items = async function (el_items, dest_path, is_undo = false) { // -------------------------------------------------------- // update progress window with current item being moved // -------------------------------------------------------- - progwin?.set_status(i18n(status_i18n_string, path_to_show_on_progwin)); + latest_status = i18n(status_i18n_string, path_to_show_on_progwin); + progwin?.set_status(latest_status); // execute move let resp = await puter.fs.move({ @@ -1895,7 +1935,7 @@ window.move_items = async function (el_items, dest_path, is_undo = false) { // if replacing an existing item, remove the old item that was just replaced if ( resp.overwritten?.id ) { - $(`.item[data-uid=${resp.overwritten.id}]`).removeItems(); + $(`.item[data-uid='${resp.overwritten.id}']`).removeItems(); } // if this is trash, get original name from item metadata @@ -1971,6 +2011,10 @@ window.move_items = async function (el_items, dest_path, is_undo = false) { if ( err.code === 'item_with_same_name_exists' ) { item_with_same_name_already_exists = true; + // The operation is paused on user input, so pause the + // progress-window timer too — otherwise "Preparing..." + // pops up on top of the dialog while it waits. + clearTimeout(progwin_timeout); const alert_resp = await UIAlert({ message: `${html_encode(err.entry_name)} already exists.`, buttons: [ @@ -1979,6 +2023,7 @@ window.move_items = async function (el_items, dest_path, is_undo = false) { ... (el_items.length > 1) ? [{ label: i18n('skip'), value: 'skip' }] : [{ label: i18n('cancel'), value: 'cancel' }], ], }); + progwin_timeout = arm_progwin(); if ( alert_resp === 'replace' ) { overwrite = true; } else if ( alert_resp === 'replace_all' ) {