mirror of
https://github.com/nimbold/Firelink.git
synced 2026-08-10 03:27:05 +00:00
fix(queue): keep paused items behind pending work
This commit is contained in:
@@ -177,11 +177,13 @@ const startDownloadListeners = async () => {
|
||||
mainStore.updateDownload(payload.id, updates);
|
||||
|
||||
if (status === 'completed' || status === 'failed' || status === 'paused') {
|
||||
mainStore.setPendingOrder(mainStore.pendingOrder.filter(id => id !== payload.id));
|
||||
useDownloadStore.setState(state => ({
|
||||
pendingOrder: state.pendingOrder.filter(id => id !== payload.id)
|
||||
}));
|
||||
} else if (status === 'queued') {
|
||||
if (!mainStore.pendingOrder.includes(payload.id)) {
|
||||
mainStore.setPendingOrder([...mainStore.pendingOrder, payload.id]);
|
||||
}
|
||||
useDownloadStore.setState(state => state.pendingOrder.includes(payload.id)
|
||||
? {}
|
||||
: { pendingOrder: [...state.pendingOrder, payload.id] });
|
||||
}
|
||||
|
||||
if (status === 'queued' || status === 'downloading' || status === 'processing' || status === 'retrying') {
|
||||
|
||||
@@ -523,6 +523,37 @@ describe('useDownloadStore', () => {
|
||||
.toEqual(['00000000-0000-0000-0000-000000000001', '00000000-0000-0000-0000-000000000001']);
|
||||
});
|
||||
|
||||
it('moves persisted paused rows behind runnable rows and assigns contiguous positions', async () => {
|
||||
vi.mocked(ipc.invokeCommand).mockImplementation(async (cmd: string) => {
|
||||
if (cmd === 'db_get_all_queues') {
|
||||
return [JSON.stringify({ id: 'queue-a', name: 'Queue A', isMain: false })];
|
||||
}
|
||||
if (cmd === 'db_get_all_downloads') {
|
||||
return [
|
||||
JSON.stringify({ id: 'active', status: 'downloading', queueId: 'queue-a', queuePosition: 0 }),
|
||||
JSON.stringify({ id: 'queued-one', status: 'queued', queueId: 'queue-a', queuePosition: 1 }),
|
||||
JSON.stringify({ id: 'paused-one', status: 'paused', queueId: 'queue-a', queuePosition: 2 }),
|
||||
JSON.stringify({ id: 'queued-two', status: 'queued', queueId: 'queue-a', queuePosition: 3 }),
|
||||
JSON.stringify({ id: 'legacy-invalid', status: 'queued', queueId: 'queue-a', queuePosition: 'not-a-number' })
|
||||
];
|
||||
}
|
||||
return undefined;
|
||||
});
|
||||
|
||||
await useDownloadStore.getState().initDB();
|
||||
|
||||
expect(useDownloadStore.getState().downloads
|
||||
.filter(download => download.queueId === 'queue-a')
|
||||
.sort((left, right) => (left.queuePosition ?? 0) - (right.queuePosition ?? 0))
|
||||
.map(download => download.id)
|
||||
).toEqual(['active', 'queued-one', 'queued-two', 'legacy-invalid', 'paused-one']);
|
||||
expect(useDownloadStore.getState().downloads
|
||||
.filter(download => download.queueId === 'queue-a')
|
||||
.map(download => download.queuePosition)
|
||||
.sort((left, right) => (left ?? 0) - (right ?? 0))
|
||||
).toEqual([0, 1, 2, 3, 4]);
|
||||
});
|
||||
|
||||
it('removes persisted temporary media estimates that are smaller than downloaded bytes', async () => {
|
||||
vi.mocked(ipc.invokeCommand).mockImplementation(async (cmd: string) => {
|
||||
if (cmd === 'db_get_all_queues') return [];
|
||||
@@ -1052,17 +1083,44 @@ describe('useDownloadStore', () => {
|
||||
downloads: [
|
||||
{ id: 'queued-one', url: 'http://one', fileName: 'one', status: 'queued', category: 'Other', dateAdded: '', queueId: 'pause-queue' },
|
||||
{ id: 'queued-two', url: 'http://two', fileName: 'two', status: 'queued', category: 'Other', dateAdded: '', queueId: 'pause-queue' },
|
||||
{ id: 'staged-one', url: 'http://staged', fileName: 'staged', status: 'staged', category: 'Other', dateAdded: '', queueId: 'pause-queue' },
|
||||
] as any[],
|
||||
});
|
||||
vi.mocked(ipc.invokeCommand).mockResolvedValue(undefined as never);
|
||||
|
||||
await expect(useDownloadStore.getState().pauseAll()).resolves.toBe(2);
|
||||
await expect(useDownloadStore.getState().pauseAll()).resolves.toBe(3);
|
||||
expect(
|
||||
vi.mocked(ipc.invokeCommand).mock.calls.filter(([command]) => command === 'pause_download')
|
||||
).toHaveLength(2);
|
||||
).toHaveLength(3);
|
||||
expect(useDownloadStore.getState().downloads.every(item => item.status === 'paused')).toBe(true);
|
||||
});
|
||||
|
||||
it('moves a paused row behind the remaining runnable queue rows', async () => {
|
||||
useDownloadStore.setState({
|
||||
downloads: [
|
||||
{ id: 'active', status: 'downloading', queueId: 'ordered-pause', queuePosition: 0 },
|
||||
{ id: 'queued-one', status: 'queued', queueId: 'ordered-pause', queuePosition: 1 },
|
||||
{ id: 'pause-target', status: 'queued', queueId: 'ordered-pause', queuePosition: 2 },
|
||||
{ id: 'queued-two', status: 'queued', queueId: 'ordered-pause', queuePosition: 3 }
|
||||
] as any[],
|
||||
pendingOrder: ['queued-one', 'pause-target', 'queued-two']
|
||||
});
|
||||
vi.mocked(ipc.invokeCommand).mockResolvedValue(undefined as never);
|
||||
|
||||
await useDownloadStore.getState().pauseDownload('pause-target');
|
||||
|
||||
const ordered = useDownloadStore.getState().downloads
|
||||
.filter(download => download.queueId === 'ordered-pause')
|
||||
.sort((left, right) => (left.queuePosition ?? 0) - (right.queuePosition ?? 0));
|
||||
expect(ordered.map(download => download.id)).toEqual([
|
||||
'active', 'queued-one', 'queued-two', 'pause-target'
|
||||
]);
|
||||
expect(ordered.map(download => download.queuePosition)).toEqual([0, 1, 2, 3]);
|
||||
expect(useDownloadStore.getState().pendingOrder).toEqual(['queued-one', 'queued-two']);
|
||||
expect(useDownloadStore.getState().downloads.find(download => download.id === 'pause-target')?.status)
|
||||
.toBe('paused');
|
||||
});
|
||||
|
||||
it('does not let a queue pause lose a selected start in flight', async () => {
|
||||
useDownloadStore.setState({
|
||||
downloads: [
|
||||
@@ -1143,6 +1201,35 @@ describe('useDownloadStore', () => {
|
||||
expect(ipc.invokeCommand).not.toHaveBeenCalledWith('enqueue_download', expect.anything());
|
||||
});
|
||||
|
||||
it('inserts a newly staged queue item before paused rows', async () => {
|
||||
useDownloadStore.setState({
|
||||
downloads: [{
|
||||
id: 'already-paused',
|
||||
url: 'https://example.com/paused.bin',
|
||||
fileName: 'paused.bin',
|
||||
category: 'Other',
|
||||
dateAdded: '',
|
||||
status: 'paused',
|
||||
queueId: 'queue-b',
|
||||
queuePosition: 0
|
||||
}] as any[]
|
||||
});
|
||||
|
||||
await useDownloadStore.getState().addDownload({
|
||||
id: 'new-staged',
|
||||
url: 'https://example.com/new.bin',
|
||||
fileName: 'new.bin',
|
||||
category: 'Other',
|
||||
dateAdded: ''
|
||||
}, { type: 'add-to-queue', queueId: 'queue-b' });
|
||||
|
||||
const ordered = useDownloadStore.getState().downloads
|
||||
.filter(item => item.queueId === 'queue-b')
|
||||
.sort((left, right) => (left.queuePosition ?? 0) - (right.queuePosition ?? 0));
|
||||
expect(ordered.map(item => item.id)).toEqual(['new-staged', 'already-paused']);
|
||||
expect(ordered.map(item => item.queuePosition)).toEqual([0, 1]);
|
||||
});
|
||||
|
||||
it('carries a media format estimate into numeric progress state', async () => {
|
||||
await useDownloadStore.getState().addDownload({
|
||||
id: 'media-estimate',
|
||||
@@ -1766,7 +1853,8 @@ describe('useDownloadStore', () => {
|
||||
useDownloadStore.setState({
|
||||
downloads: [
|
||||
{ id: 'ready', status: 'ready', queueId: 'old' },
|
||||
{ id: 'done', status: 'completed', queueId: 'old' }
|
||||
{ id: 'done', status: 'completed', queueId: 'old' },
|
||||
{ id: 'paused', status: 'paused', queueId: 'new', queuePosition: 0 }
|
||||
] as any[]
|
||||
});
|
||||
|
||||
@@ -1774,6 +1862,11 @@ describe('useDownloadStore', () => {
|
||||
|
||||
expect(useDownloadStore.getState().downloads.find(item => item.id === 'ready')?.queueId).toBe('new');
|
||||
expect(useDownloadStore.getState().downloads.find(item => item.id === 'done')?.queueId).toBe('old');
|
||||
expect(useDownloadStore.getState().downloads
|
||||
.filter(item => item.queueId === 'new')
|
||||
.sort((left, right) => (left.queuePosition ?? 0) - (right.queuePosition ?? 0))
|
||||
.map(item => item.id)
|
||||
).toEqual(['ready', 'paused']);
|
||||
});
|
||||
|
||||
it('does not reassign an item that completes while queue assignment is awaiting cancellation', async () => {
|
||||
|
||||
@@ -126,8 +126,15 @@ const advanceQueueControlGeneration = (queueId: string): number => {
|
||||
const isCurrentQueueControlGeneration = (queueId: string, generation: number): boolean =>
|
||||
currentQueueControlGeneration(queueId) === generation;
|
||||
|
||||
const comparableQueuePosition = (download: DownloadItem): number => {
|
||||
const position = download.queuePosition;
|
||||
return typeof position === 'number' && Number.isFinite(position) && position >= 0
|
||||
? position
|
||||
: Number.MAX_SAFE_INTEGER;
|
||||
};
|
||||
|
||||
const queuePositionComparator = (left: DownloadItem, right: DownloadItem): number =>
|
||||
(left.queuePosition ?? Number.MAX_SAFE_INTEGER) - (right.queuePosition ?? Number.MAX_SAFE_INTEGER) ||
|
||||
comparableQueuePosition(left) - comparableQueuePosition(right) ||
|
||||
left.id.localeCompare(right.id);
|
||||
|
||||
const queueItemsForReordering = (downloads: DownloadItem[], queueId: string): DownloadItem[] =>
|
||||
@@ -161,6 +168,17 @@ const applyQueueOrder = (
|
||||
: download);
|
||||
};
|
||||
|
||||
// Paused downloads remain part of the queue, but they must not be interleaved
|
||||
// with the queue's non-paused rows. Keep the partition in the store so
|
||||
// the table, persistence, and later queue operations all observe the same
|
||||
// order instead of each deriving a different one.
|
||||
const reorderQueueWithPausedAtEnd = (downloads: DownloadItem[], queueId: string): DownloadItem[] => {
|
||||
const queueItems = queueItemsForReordering(downloads, queueId);
|
||||
const nonPausedItems = queueItems.filter(download => download.status !== 'paused');
|
||||
const pausedItems = queueItems.filter(download => download.status === 'paused');
|
||||
return applyQueueOrder(downloads, queueId, [...nonPausedItems, ...pausedItems]);
|
||||
};
|
||||
|
||||
const advanceDownloadLifecycle = (id: string): bigint => {
|
||||
const nextGeneration = (downloadLifecycleGenerations.get(id) ?? 0n) + 1n;
|
||||
downloadLifecycleGenerations.set(id, nextGeneration);
|
||||
@@ -519,16 +537,27 @@ const effectiveDestinationForItem = async (
|
||||
|
||||
const normalizeQueuePositions = (downloads: DownloadItem[]): DownloadItem[] => {
|
||||
const nextPosition = new Map<string, number>();
|
||||
return downloads.map(download => {
|
||||
const normalized: DownloadItem[] = downloads.map(download => {
|
||||
const queueId = download.queueId || MAIN_QUEUE_ID;
|
||||
const position = nextPosition.get(queueId) || 0;
|
||||
nextPosition.set(queueId, position + 1);
|
||||
const persistedPosition = download.queuePosition;
|
||||
const queuePosition = typeof persistedPosition === 'number' &&
|
||||
Number.isFinite(persistedPosition) && persistedPosition >= 0
|
||||
? Math.trunc(persistedPosition)
|
||||
: position;
|
||||
return {
|
||||
...download,
|
||||
queueId,
|
||||
queuePosition: download.queuePosition ?? position
|
||||
queuePosition
|
||||
};
|
||||
});
|
||||
|
||||
let ordered = normalized;
|
||||
for (const queueId of new Set(normalized.map(download => download.queueId || MAIN_QUEUE_ID))) {
|
||||
ordered = reorderQueueWithPausedAtEnd(ordered, queueId);
|
||||
}
|
||||
return ordered;
|
||||
};
|
||||
|
||||
const TEMPORARY_MEDIA_ESTIMATE_MAX_BYTES = 1024;
|
||||
@@ -1294,7 +1323,9 @@ export const useDownloadStore = create<DownloadState>((set, get) => {
|
||||
hasBeenDispatched: false
|
||||
};
|
||||
advanceDownloadLifecycle(item.id);
|
||||
set((state) => ({ downloads: [...state.downloads, ownedItem] }));
|
||||
set((state) => ({
|
||||
downloads: reorderQueueWithPausedAtEnd([...state.downloads, ownedItem], queueId)
|
||||
}));
|
||||
|
||||
if (action.type === 'add-to-queue') {
|
||||
info(`Download ${item.id} added to queue ${action.queueId}`);
|
||||
@@ -1334,19 +1365,32 @@ export const useDownloadStore = create<DownloadState>((set, get) => {
|
||||
preemptDispatch
|
||||
),
|
||||
updateDownload: (id, updates) => {
|
||||
set((state) => ({
|
||||
downloads: state.downloads.map(d => {
|
||||
if (d.id === id) {
|
||||
const updated = {
|
||||
...d,
|
||||
set((state) => {
|
||||
const current = state.downloads.find(download => download.id === id);
|
||||
if (!current) return { downloads: state.downloads };
|
||||
|
||||
const downloads = state.downloads.map(d => d.id === id
|
||||
? {
|
||||
...d,
|
||||
...updates,
|
||||
fraction: updates.fraction !== undefined ? updates.fraction : d.fraction
|
||||
};
|
||||
return updated;
|
||||
}
|
||||
return d;
|
||||
})
|
||||
}));
|
||||
}
|
||||
: d
|
||||
);
|
||||
const shouldNormalizeQueue = updates.status === 'paused' ||
|
||||
updates.status === 'queued' ||
|
||||
updates.status === 'staged';
|
||||
const updated = downloads.find(download => download.id === id) ?? current;
|
||||
|
||||
return {
|
||||
downloads: shouldNormalizeQueue
|
||||
? reorderQueueWithPausedAtEnd(downloads, updated.queueId || MAIN_QUEUE_ID)
|
||||
: downloads,
|
||||
...(updates.status === 'paused'
|
||||
? { pendingOrder: state.pendingOrder.filter(value => value !== id) }
|
||||
: {})
|
||||
};
|
||||
});
|
||||
|
||||
if (updates.status && ['completed', 'failed', 'paused'].includes(updates.status)) {
|
||||
info(`Download ${id} status changed to ${updates.status}`);
|
||||
@@ -1753,8 +1797,8 @@ export const useDownloadStore = create<DownloadState>((set, get) => {
|
||||
);
|
||||
const maxPos = queueItems.reduce((max, d) => Math.max(max, d.queuePosition ?? 0), -1);
|
||||
const nextPosition = maxPos + 1;
|
||||
set(state => ({
|
||||
downloads: state.downloads.map(item =>
|
||||
set(state => {
|
||||
const downloads = state.downloads.map(item =>
|
||||
movableIds.has(item.id) && item.status !== 'completed'
|
||||
? {
|
||||
...item,
|
||||
@@ -1764,8 +1808,9 @@ export const useDownloadStore = create<DownloadState>((set, get) => {
|
||||
hasBeenDispatched: false
|
||||
}
|
||||
: item
|
||||
)
|
||||
}));
|
||||
);
|
||||
return { downloads: reorderQueueWithPausedAtEnd(downloads, queueId) };
|
||||
});
|
||||
});
|
||||
},
|
||||
setDownloadSpeedLimit: (id, limit) => runDownloadLifecycleOperation(
|
||||
@@ -2094,11 +2139,11 @@ export const useDownloadStore = create<DownloadState>((set, get) => {
|
||||
|
||||
// Reset interrupted active downloads to queued.
|
||||
set((state) => ({
|
||||
downloads: state.downloads.map(d =>
|
||||
downloads: normalizeQueuePositions(state.downloads.map(d =>
|
||||
isActiveDownloadStatus(d.status) && d.status !== 'queued'
|
||||
? { ...d, status: 'queued' as const }
|
||||
: d
|
||||
)
|
||||
))
|
||||
}));
|
||||
|
||||
} catch (e) {
|
||||
|
||||
@@ -12,13 +12,18 @@ import {
|
||||
} from './downloadActions';
|
||||
|
||||
describe('download action policy', () => {
|
||||
it('keeps start and pause actions mutually exclusive', () => {
|
||||
it('allows queued items to be paused before their first dispatch', () => {
|
||||
expect(canStartDownload('staged')).toBe(true);
|
||||
expect(canPauseDownload('staged')).toBe(true);
|
||||
|
||||
for (const status of ['ready', 'paused', 'failed'] as const) {
|
||||
expect(canStartDownload(status)).toBe(true);
|
||||
expect(canPauseDownload(status)).toBe(false);
|
||||
}
|
||||
for (const status of ['queued', 'downloading', 'processing', 'retrying'] as const) {
|
||||
for (const status of ['staged', 'queued', 'downloading', 'processing', 'retrying'] as const) {
|
||||
expect(canPauseDownload(status)).toBe(true);
|
||||
}
|
||||
for (const status of ['queued', 'downloading', 'processing', 'retrying'] as const) {
|
||||
expect(canStartDownload(status)).toBe(false);
|
||||
}
|
||||
});
|
||||
@@ -37,9 +42,10 @@ describe('download action policy', () => {
|
||||
expect(getPauseResumeAction('retrying')).toBe('pause');
|
||||
expect(getPauseResumeAction('paused')).toBe('resume');
|
||||
|
||||
for (const status of ['ready', 'staged', 'completed', 'failed'] as const) {
|
||||
for (const status of ['ready', 'completed', 'failed'] as const) {
|
||||
expect(getPauseResumeAction(status)).toBeNull();
|
||||
}
|
||||
expect(getPauseResumeAction('staged')).toBe('pause');
|
||||
});
|
||||
|
||||
it('provides consistent labels and edit locks', () => {
|
||||
@@ -62,7 +68,7 @@ describe('download action policy', () => {
|
||||
{ status: 'completed' },
|
||||
]);
|
||||
|
||||
expect(counts).toEqual({ pause: 2, resume: 4 });
|
||||
expect(counts).toEqual({ pause: 3, resume: 4 });
|
||||
});
|
||||
|
||||
it('keeps large action badges compact without changing the accessible count', () => {
|
||||
|
||||
@@ -8,6 +8,7 @@ const STARTABLE_STATUSES: ReadonlySet<DownloadStatus> = new Set([
|
||||
]);
|
||||
|
||||
const PAUSABLE_STATUSES: ReadonlySet<DownloadStatus> = new Set([
|
||||
'staged',
|
||||
'queued',
|
||||
'downloading',
|
||||
'processing',
|
||||
|
||||
Reference in New Issue
Block a user