diff --git a/src/backend/services/share/ShareService.test.ts b/src/backend/services/share/ShareService.test.ts index 4045ac808..829f43eb4 100644 --- a/src/backend/services/share/ShareService.test.ts +++ b/src/backend/services/share/ShareService.test.ts @@ -1530,8 +1530,7 @@ describe('ShareService', () => { mode: 'read', }); - // Nothing was shared at the file, so the revoke reports no holder - // for it — the audience has to be found through the folder. + // No share on the file, so the audience is only found upward. const payload = await capturePayload( 'outer.gui.item.removed', recipient.user.id, @@ -1561,8 +1560,7 @@ describe('ShareService', () => { }); } - // The revoke reports them for the file, and the folder reaches it - // too — one delete must not arrive as two removals. + // Both passes reach them; one delete must not arrive as two. const audiences = await captureAudiences( 'outer.gui.item.removed', file.uuid, @@ -1592,8 +1590,7 @@ describe('ShareService', () => { mode: 'read', }); - // What the GUI's Delete does. `item.moved` would name a path the - // recipient cannot see, leaving the file on their screen. + // What Delete does; `item.moved` would name a path they can't see. const audiences = await captureAudiences( 'outer.gui.item.removed', file.uuid, @@ -1715,6 +1712,86 @@ describe('ShareService', () => { expect(payload?.path).toBe(`${masked}/renamed-${file.name}`); }); + it('tells a recipient when the shared item itself is trashed', async () => { + const owner = await makeUser(); + const recipient = await makeUser(); + const file = await makeFile(owner.user); + const before = file.path; + const trashName = uuidv4(); + const trashed = `/${owner.user.username}/Trash/${trashName}`; + + await share(owner.actor, { + uid: file.uuid, + recipient: { email: recipient.email }, + mode: 'read', + }); + + // The grant follows it into Trash, so both ends would resolve. + const payload = await capturePayload( + 'outer.gui.item.removed', + recipient.user.id, + async () => { + await server.clients.event.emitAndWait( + 'fs.move.node', + { + node: { + ...file, + path: trashed, + name: trashName, + }, + fromPath: before, + toPath: trashed, + }, + {}, + ); + }, + ); + + // Named as they knew it, never the GUID that Trash gave it. + expect(payload?.path).toBe( + `/${owner.user.username}/${file.uuid}/${file.name}`, + ); + }); + + it('stays quiet when a move leaves the recipient address alone', async () => { + const owner = await makeUser(); + const recipient = await makeUser(); + const file = await makeFile(owner.user); + const elsewhere = `/${owner.user.username}/Documents/${file.name}`; + + await share(owner.actor, { + uid: file.uuid, + recipient: { email: recipient.email }, + mode: 'read', + }); + + const seen: unknown[] = []; + const listener = (_key: string, data: unknown) => { + const payload = data as { user_id_list?: number[] }; + if (!payload.user_id_list?.includes(recipient.user.id)) return; + seen.push(payload); + }; + server.clients.event.on('outer.gui.item.moved', listener); + + try { + // Masked at its own root, so their path holds wherever it goes. + await server.clients.event.emitAndWait( + 'fs.move.node', + { + node: { ...file, path: elsewhere }, + fromPath: file.path, + toPath: elsewhere, + }, + {}, + ); + await new Promise((resolve) => setTimeout(resolve, 100)); + } finally { + server.clients.event.off('outer.gui.item.moved', listener); + } + + expect(seen).toEqual([]); + }); + it('tells a folder recipient when a file inside it changes', async () => { const owner = await makeUser(); const recipient = await makeUser(); @@ -1953,8 +2030,7 @@ describe('ShareService', () => { reaching.mockRestore(); } - // Siblings share a parent, so they share the audience lookup — - // a burst settles in one or two flushes, never one per file. + // Siblings share a parent, so a burst settles in a flush or two. expect(seen).toHaveLength(8); expect(lookups).toBeLessThanOrEqual(2); }); @@ -2015,7 +2091,11 @@ describe('ShareService', () => { await server.clients.event.emitAndWait( 'fs.move.node', { - node: { ...file, path: `${before}-moved` }, + node: { + ...file, + path: `${before}-moved`, + name: `${file.name}-moved`, + }, fromPath: before, toPath: `${before}-moved`, }, @@ -2112,7 +2192,11 @@ describe('ShareService', () => { await server.clients.event.emitAndWait( 'fs.move.node', { - node: file, + node: { + ...file, + path: `${file.path}-moved`, + name: `${file.name}-moved`, + }, fromPath: file.path, toPath: `${file.path}-moved`, }, diff --git a/src/backend/services/share/ShareService.ts b/src/backend/services/share/ShareService.ts index 044a068ac..ae085ebf1 100644 --- a/src/backend/services/share/ShareService.ts +++ b/src/backend/services/share/ShareService.ts @@ -244,6 +244,10 @@ type HolderGuiEvent = | 'outer.gui.item.updated'; /** `///` for a path in the owner's tree. */ +/** Inside some owner's top-level Trash, which is where Delete puts things. */ +const isTrashedPath = (path: string): boolean => + /^\/[^/]+\/Trash(\/|$)/u.test(path); + const maskedSelfPath = (entry: FSEntry, realPath: string): string => { const owner = realPath.split('/')[1]; const name = realPath.split('/').pop(); @@ -527,10 +531,9 @@ export class ShareService extends PuterService { } /** - * Tell whoever reached these through a folder above them. Their grant is on - * that folder, not on what was deleted, so the revoke reports no holder for - * them — and the file stays in their window, 404ing when they open it. - * Coalesced by parent like creates, so a subtree stays a few queries. + * Tell whoever reached these through a folder above them — their grant is + * on that folder, so the revoke reports no holder for them. Coalesced by + * parent like creates, so a subtree stays a few queries. */ async #fanOutRetiredToAncestors( entries: FSEntry[], @@ -545,8 +548,7 @@ export class ShareService extends PuterService { for (const siblings of byParent.values()) { const first = siblings[0]; if (!first) continue; - // Ancestors only — a share on the entry itself is the revoke's to - // report, and it already has. + // Ancestors only; a share on the entry is the revoke's to report. const groups = (await this.#reachingRoots(first)).filter( ({ root }) => root.uuid !== first.uuid, ); @@ -567,11 +569,9 @@ export class ShareService extends PuterService { } /** - * A move seen from both ends, because it can take an item out of someone's - * share as easily as into it. Reaching both ends is a move, only the - * destination an arrival, only the origin a removal — that last being the - * GUI's Delete, a move to the owner's Trash where no recipient has a - * share. + * A move seen from both ends, since it can take an item out of a share as + * easily as into it: both ends is a move, only the destination an arrival, + * only the origin a removal. */ async #fanOutMove(entry: FSEntry, fromPath?: string): Promise { const destination = await this.#reachingRoots(entry); @@ -591,8 +591,7 @@ export class ShareService extends PuterService { const originRootOf = rootsBySide(origin); const destinationRootOf = rootsBySide(destination); - // Keyed on both shares: the paths a holder is told depend on the one - // they saw each end through, and those need not be the same share. + // Keyed on both: a holder can see each end through a different share. const batches = new Map< string, { @@ -614,16 +613,29 @@ export class ShareService extends PuterService { batches.set(key, batch); }; + // A grant follows its entry into Trash, so Delete would read as a move + // and rename their item to the GUID Trash gave it. Listings omit it. + const nowTrashed = isTrashedPath(entry.path); + const wasTrashed = fromPath ? isTrashedPath(fromPath) : false; + for (const [holder, to] of destinationRootOf) { + if (nowTrashed) continue; const from = originRootOf.get(holder); place( holder, - from ? 'outer.gui.item.moved' : 'outer.gui.item.added', + from && !wasTrashed + ? 'outer.gui.item.moved' + : 'outer.gui.item.added', from, to, ); } for (const [holder, from] of originRootOf) { + if (nowTrashed) { + // Already gone from their view if it was trashed before. + if (!wasTrashed) place(holder, 'outer.gui.item.removed', from); + continue; + } if (destinationRootOf.has(holder)) continue; place(holder, 'outer.gui.item.removed', from); } @@ -638,12 +650,15 @@ export class ShareService extends PuterService { ); continue; } + const payload = holderPayload(entry, to as FSEntry); const formerPath = from && fromPath ? maskedFormerPath(from, entry, fromPath) : null; + // A share masks its own root, so this move is invisible to them. + if (formerPath && formerPath === payload.path) continue; await this.#emitGui(event, holders, { - ...holderPayload(entry, to as FSEntry), + ...payload, // The GUI rewrites the item it already has by this. ...(formerPath ? { from_path: formerPath } : {}), }); @@ -2029,7 +2044,7 @@ export class ShareService extends PuterService { } #isTrashed(entry: FSEntry): boolean { - return /^\/[^/]+\/Trash(\/|$)/u.test(entry.path); + return isTrashedPath(entry.path); } /**