diff --git a/src/store/useDownloadStore.test.ts b/src/store/useDownloadStore.test.ts index b07a319..e498ba0 100644 --- a/src/store/useDownloadStore.test.ts +++ b/src/store/useDownloadStore.test.ts @@ -608,6 +608,48 @@ describe('useDownloadStore', () => { expect(useDownloadStore.getState().backendRegisteredIds.has('resume-generation')).toBe(true); // Re-registered by dispatchItem }); + it('coalesces duplicate resume actions while the backend transition is in flight', async () => { + useDownloadStore.setState({ + downloads: [{ + id: 'resume-double-submit', + url: 'http://test1', + fileName: 'f1', + destination: '/tmp', + status: 'paused', + category: 'Other', + dateAdded: '', + hasBeenDispatched: true + }] as any[], + backendRegisteredIds: new Set(['resume-double-submit']) + }); + + let releaseResume!: () => void; + const resumeReleased = new Promise(resolve => { + releaseResume = resolve; + }); + vi.mocked(ipc.invokeCommand).mockImplementation(async (cmd: string) => { + if (cmd === 'resume_download') { + await resumeReleased; + return true; + } + return undefined; + }); + + const first = useDownloadStore.getState().resumeDownload('resume-double-submit'); + await vi.waitFor(() => { + expect(vi.mocked(ipc.invokeCommand).mock.calls.filter(([command]) => command === 'resume_download')) + .toHaveLength(1); + }); + const second = useDownloadStore.getState().resumeDownload('resume-double-submit'); + + expect(second).toBe(first); + releaseResume(); + await Promise.all([first, second]); + expect( + vi.mocked(ipc.invokeCommand).mock.calls.filter(([command]) => command === 'resume_download') + ).toHaveLength(1); + }); + it('does not re-enqueue when the existing resume RPC fails', async () => { useDownloadStore.setState({ downloads: [ @@ -1185,6 +1227,48 @@ describe('useDownloadStore', () => { ).toBe(false); }); + it('coalesces duplicate redownloads and serializes a destructive removal behind them', async () => { + useDownloadStore.setState({ + downloads: [{ + id: 'redownload-double-submit', + url: 'https://example.com/file.bin', + fileName: 'file.bin', + destination: '/tmp', + status: 'paused', + category: 'Other', + dateAdded: '' + }] as any[] + }); + + let releaseRedownloadRemoval!: () => void; + const redownloadRemovalReleased = new Promise(resolve => { + releaseRedownloadRemoval = resolve; + }); + let removeCallCount = 0; + vi.mocked(ipc.invokeCommand).mockImplementation(async (cmd: string) => { + if (cmd === 'remove_download') { + removeCallCount += 1; + if (removeCallCount === 1) await redownloadRemovalReleased; + return undefined; + } + if (cmd === 'get_pending_order') return []; + return undefined; + }); + + const first = useDownloadStore.getState().redownload('redownload-double-submit'); + await vi.waitFor(() => expect(removeCallCount).toBe(1)); + const second = useDownloadStore.getState().redownload('redownload-double-submit'); + const remove = useDownloadStore.getState().removeDownload('redownload-double-submit'); + + expect(second).toBe(first); + expect(removeCallCount).toBe(1); + releaseRedownloadRemoval(); + await first; + await remove; + expect(removeCallCount).toBe(2); + expect(useDownloadStore.getState().downloads).toEqual([]); + }); + it('starts and pauses all items regardless of legacy missing queue ids', async () => { useDownloadStore.setState({ downloads: [ diff --git a/src/store/useDownloadStore.ts b/src/store/useDownloadStore.ts index 44d0230..e4a691b 100644 --- a/src/store/useDownloadStore.ts +++ b/src/store/useDownloadStore.ts @@ -23,6 +23,12 @@ const downloadLifecycleGenerations = new Map(); const queueReorderPromises = new Map>(); const queueStartPromises = new Map>(); const queueControlGenerations = new Map(); +type DownloadLifecycleOperation = { + kind: string; + promise: Promise; +}; +const downloadLifecycleOperations = new Map(); +const preemptDispatch = ['dispatch'] as const; let pendingStartupResume: Promise | null = null; type DownloadControlIntent = 'pause' | 'resume'; @@ -48,6 +54,49 @@ export const clearDownloadControlIntents = (): void => { downloadControlIntents.clear(); }; +const runDownloadLifecycleOperation = ( + id: string, + kind: string, + operation: () => Promise, + coalesce = true, + preemptKinds: readonly string[] = [] +): Promise => { + const current = downloadLifecycleOperations.get(id); + if (coalesce && current?.kind === kind) { + return current.promise as Promise; + } + + const operationPromise = current && !preemptKinds.includes(current.kind) + ? current.promise.catch(() => undefined).then(operation) + : operation(); + const trackedOperation = operationPromise.finally(() => { + if (downloadLifecycleOperations.get(id)?.promise === trackedOperation) { + downloadLifecycleOperations.delete(id); + } + }); + downloadLifecycleOperations.set(id, { kind, promise: trackedOperation }); + return trackedOperation; +}; + +const runDownloadLifecycleOperations = ( + ids: string[], + kind: string, + operation: () => Promise +): Promise => { + const orderedIds = [...new Set(ids)].sort(); + const runNext = (index: number): Promise => { + if (index === orderedIds.length) return operation(); + return runDownloadLifecycleOperation( + orderedIds[index], + kind, + () => runNext(index + 1), + false, + preemptDispatch + ); + }; + return runNext(0); +}; + const waitForPendingStartupResume = async (): Promise => { const pending = pendingStartupResume; if (pending) await pending.catch(() => undefined); @@ -193,7 +242,7 @@ const speedLimitForDispatch = ( return normalizeSpeedLimitForBackend(globalSpeedLimit); }; -export async function dispatchItem(id: string, proxyOverride?: string | null): Promise { +async function dispatchItemInternal(id: string, proxyOverride?: string | null): Promise { await waitForPendingStartupResume(); if (backendDispatchPromises.has(id)) return backendDispatchPromises.get(id)!; @@ -205,6 +254,7 @@ export async function dispatchItem(id: string, proxyOverride?: string | null): P const item = state.downloads.find(d => d.id === id); if (!item) return false; if (state.backendRegisteredIds.has(id)) return true; + if (!['ready', 'staged', 'failed', 'queued'].includes(item.status)) return false; lifecycleGeneration = currentDownloadLifecycle(id); const settings = useSettingsStore.getState(); @@ -304,6 +354,18 @@ export async function dispatchItem(id: string, proxyOverride?: string | null): P return promise; } +export function dispatchItem(id: string, proxyOverride?: string | null): Promise { + const current = downloadLifecycleOperations.get(id); + if (current && current.kind !== 'dispatch') { + return current.promise.then(() => false, () => false); + } + return runDownloadLifecycleOperation( + id, + 'dispatch', + () => dispatchItemInternal(id, proxyOverride) + ); +} + export const normalizeCustomProxy = (host: string, port: number): string | null => { const trimmedHost = host.trim(); const normalizedPort = Number.isFinite(port) ? Math.trunc(port) : NaN; @@ -889,7 +951,7 @@ export const useDownloadStore = create((set, get) => ({ } return false; }, - applyProperties: async (id, updates) => { + applyProperties: (id, updates) => runDownloadLifecycleOperation(id, 'properties', async () => { await waitForPendingStartupResume(); const wasDispatching = await invalidateAndWaitForDispatch(id); const state = get(); @@ -916,7 +978,7 @@ export const useDownloadStore = create((set, get) => ({ } state.updateDownload(id, updates); if (isRegistered || wasDispatching) { - const dispatched = await dispatchItem(id); + const dispatched = await dispatchItemInternal(id); if (dispatched) { state.updateDownload(id, { hasBeenDispatched: true }); } else { @@ -935,7 +997,7 @@ export const useDownloadStore = create((set, get) => ({ } state.updateDownload(id, updates); } - }, + }, false, preemptDispatch), updateDownload: (id, updates) => { set((state) => ({ downloads: state.downloads.map(d => { @@ -959,7 +1021,10 @@ export const useDownloadStore = create((set, get) => ({ syncSystemIntegrations(); } }, - removeDownload: async (id, deleteFile = false, preserveResumable = false) => { + removeDownload: (id, deleteFile = false, preserveResumable = false) => runDownloadLifecycleOperation( + id, + `remove:${deleteFile}:${preserveResumable}`, + async () => { await waitForPendingStartupResume(); clearDownloadControlIntent(id); const { pendingDispatch } = await invalidateDispatch(id); @@ -986,8 +1051,11 @@ export const useDownloadStore = create((set, get) => ({ useDownloadProgressStore.getState().clearDownloadProgress(id); info(`Download ${id} removed`); syncSystemIntegrations(); - }, - pauseDownload: async (id) => { + }, + true, + preemptDispatch + ), + pauseDownload: (id) => runDownloadLifecycleOperation(id, 'pause', async () => { await waitForPendingStartupResume(); if (!get().downloads.some(download => download.id === id)) return; setDownloadControlIntent(id, 'pause'); @@ -1007,8 +1075,8 @@ export const useDownloadStore = create((set, get) => ({ } finally { clearDownloadControlIntent(id, 'pause'); } - }, - redownload: async (id) => { + }, true, preemptDispatch), + redownload: (id) => runDownloadLifecycleOperation(id, 'redownload', async () => { await waitForPendingStartupResume(); const targetItem = get().downloads.find(d => d.id === id); if (!targetItem) { @@ -1049,7 +1117,7 @@ export const useDownloadStore = create((set, get) => ({ dateAdded: new Date().toISOString() }); - if (!await dispatchItem(id)) { + if (!await dispatchItemInternal(id)) { console.error("Failed to enqueue redownload"); get().updateDownload(id, { status: 'failed' }); clearDownloadControlIntent(id, 'resume'); @@ -1057,8 +1125,8 @@ export const useDownloadStore = create((set, get) => ({ get().updateDownload(id, { hasBeenDispatched: true }); info(`Download ${id} redownloaded (queued)`); } - }, - resumeDownload: async (id) => { + }, true, preemptDispatch), + resumeDownload: (id) => runDownloadLifecycleOperation(id, 'resume', async () => { await waitForPendingStartupResume(); const targetItem = get().downloads.find(d => d.id === id); if (!targetItem) return false; @@ -1067,7 +1135,7 @@ export const useDownloadStore = create((set, get) => ({ try { if (targetItem.status === 'ready' || targetItem.status === 'staged') { get().updateDownload(id, { status: 'queued', hasBeenDispatched: true }); - if (await dispatchItem(id)) { + if (await dispatchItemInternal(id)) { return true; } get().updateDownload(id, { status: targetItem.status }); @@ -1098,7 +1166,7 @@ export const useDownloadStore = create((set, get) => ({ // lifecycle. Advance and cancel the old generation before dispatching // so QueueManager does not reject the legitimate user retry as stale. await invalidateAndWaitForDispatch(id); - dispatchSucceeded = await dispatchItem(id); + dispatchSucceeded = await dispatchItemInternal(id); } if (dispatchSucceeded) { @@ -1115,7 +1183,7 @@ export const useDownloadStore = create((set, get) => ({ clearDownloadControlIntent(id, 'resume'); return false; } - }, + }, true, preemptDispatch), startQueue: (queueId) => { const requestedGeneration = currentQueueControlGeneration(queueId); const previousOperation = queueStartPromises.get(queueId) ?? Promise.resolve([]); @@ -1282,46 +1350,49 @@ export const useDownloadStore = create((set, get) => ({ }, assignToQueue: async (ids, queueId) => { await waitForPendingStartupResume(); - const selectedIds = new Set(ids); - const selected = get().downloads.filter(item => selectedIds.has(item.id)); - const locked = selected.find(item => isActiveDownloadStatus(item.status) && item.status !== 'queued'); - if (locked) { - throw new Error(i18n.t($ => $.downloadTable.pauseBeforeMove, { file: locked.fileName })); - } + const lockedIds = ids.filter(id => get().downloads.some(item => item.id === id)); + return runDownloadLifecycleOperations(lockedIds, 'assign-to-queue', async () => { + const selectedIds = new Set(ids); + const selected = get().downloads.filter(item => selectedIds.has(item.id)); + const locked = selected.find(item => isActiveDownloadStatus(item.status) && item.status !== 'queued'); + if (locked) { + throw new Error(i18n.t($ => $.downloadTable.pauseBeforeMove, { file: locked.fileName })); + } - const movableSelected = selected.filter(item => item.status !== 'completed'); - const movableIds = new Set(movableSelected.map(item => item.id)); - await Promise.all(movableSelected.map(item => invalidateAndWaitForDispatch(item.id))); + const movableSelected = selected.filter(item => item.status !== 'completed'); + const movableIds = new Set(movableSelected.map(item => item.id)); + await Promise.all(movableSelected.map(item => invalidateAndWaitForDispatch(item.id))); - for (const item of get().downloads.filter(item => movableIds.has(item.id))) { - if (!get().backendRegisteredIds.has(item.id)) continue; - // The UI can still say queued while a dispatch has already reached - // Aria2/media. Detach through the backend lifecycle owner for every - // registered item; remove_from_queue only handles the pending list. - await invoke('detach_download_for_reconfigure', { id: item.id }); - get().unregisterBackendIds([item.id]); - set(state => ({ pendingOrder: state.pendingOrder.filter(value => value !== item.id) })); - } + for (const item of get().downloads.filter(item => movableIds.has(item.id))) { + if (!get().backendRegisteredIds.has(item.id)) continue; + // The UI can still say queued while a dispatch has already reached + // Aria2/media. Detach through the backend lifecycle owner for every + // registered item; remove_from_queue only handles the pending list. + await invoke('detach_download_for_reconfigure', { id: item.id }); + get().unregisterBackendIds([item.id]); + set(state => ({ pendingOrder: state.pendingOrder.filter(value => value !== item.id) })); + } - const queueItems = get().downloads.filter(item => - !movableIds.has(item.id) && - (item.queueId || MAIN_QUEUE_ID) === queueId - ); - const maxPos = queueItems.reduce((max, d) => Math.max(max, d.queuePosition ?? 0), -1); - const nextPosition = maxPos + 1; - set(state => ({ - downloads: state.downloads.map(item => - movableIds.has(item.id) && item.status !== 'completed' - ? { - ...item, - queueId, - queuePosition: nextPosition + movableSelected.findIndex(selectedItem => selectedItem.id === item.id), - status: 'staged' as const, - hasBeenDispatched: false - } - : item - ) - })); + const queueItems = get().downloads.filter(item => + !movableIds.has(item.id) && + (item.queueId || MAIN_QUEUE_ID) === queueId + ); + const maxPos = queueItems.reduce((max, d) => Math.max(max, d.queuePosition ?? 0), -1); + const nextPosition = maxPos + 1; + set(state => ({ + downloads: state.downloads.map(item => + movableIds.has(item.id) && item.status !== 'completed' + ? { + ...item, + queueId, + queuePosition: nextPosition + movableSelected.findIndex(selectedItem => selectedItem.id === item.id), + status: 'staged' as const, + hasBeenDispatched: false + } + : item + ) + })); + }); }, addQueue: (name) => { const normalizedName = name.trim();