From bc4c7e63180eb577423b7ef848704a29c470b1e1 Mon Sep 17 00:00:00 2001 From: 4gray Date: Sat, 1 Aug 2026 17:01:05 +0200 Subject: [PATCH] perf(downloads): avoid blocking file availability probes --- .changes/downloads-manager-mvp.md | 1 + .../database/download-file-availability.ts | 107 ++++++++++++++++++ ...downloads-file-availability.events.spec.ts | 101 ++++++++++++++++- .../app/events/database/downloads.events.ts | 6 +- .../events/database/downloads.test-helpers.ts | 8 ++ docs/architecture/download-manager.md | 1 + 6 files changed, 217 insertions(+), 7 deletions(-) diff --git a/.changes/downloads-manager-mvp.md b/.changes/downloads-manager-mvp.md index cc3983754..30613e4d9 100644 --- a/.changes/downloads-manager-mvp.md +++ b/.changes/downloads-manager-mvp.md @@ -8,3 +8,4 @@ missing files to Needs attention with Download again, and open movies and series in focused offline details with saved metadata. Series show only local episodes; View in portal returns to provider playback when the source can be recovered. +Completed-file checks stay responsive on slow or unavailable storage. diff --git a/apps/electron-backend/src/app/events/database/download-file-availability.ts b/apps/electron-backend/src/app/events/database/download-file-availability.ts index 08e9e8eab..a269c11b0 100644 --- a/apps/electron-backend/src/app/events/database/download-file-availability.ts +++ b/apps/electron-backend/src/app/events/database/download-file-availability.ts @@ -4,6 +4,7 @@ import { type ElectronDownloadFileAvailability, } from '@iptvnator/shared/interfaces'; import { lstatSync, type Stats } from 'node:fs'; +import { lstat } from 'node:fs/promises'; import { decodeDownloadMetadataSnapshot } from './download-metadata-snapshot'; interface DownloadFileRow { @@ -16,6 +17,76 @@ export type DownloadLstat = ( filePath: string ) => Pick; +type DownloadAsyncLstat = ( + filePath: string +) => Promise>; + +type DownloadFileAvailabilityProbe = ( + filePath: string +) => Promise; + +const DEFAULT_MAX_CONCURRENT_FILE_PROBES = 4; + +function createDownloadFileAvailabilityProbe( + asyncLstat: DownloadAsyncLstat = lstat, + maxConcurrent = DEFAULT_MAX_CONCURRENT_FILE_PROBES +): DownloadFileAvailabilityProbe { + const concurrency = Math.max(1, Math.floor(maxConcurrent)); + const pending: Array<() => void> = []; + // Coalesce only active probes. Completed results are discarded so an + // externally removed file is visible on the next list refresh. + const inFlight = new Map>(); + let active = 0; + + const acquire = (): Promise => { + if (active < concurrency) { + active += 1; + return Promise.resolve(); + } + return new Promise((resolve) => pending.push(resolve)); + }; + + const release = (): void => { + const next = pending.shift(); + if (next) { + next(); + } else { + active -= 1; + } + }; + + const inspect = async (filePath: string): Promise => { + await acquire(); + try { + const stats = await asyncLstat(filePath); + return stats.isFile() && !stats.isSymbolicLink(); + } catch { + return false; + } finally { + release(); + } + }; + + return (filePath: string) => { + const existing = inFlight.get(filePath); + if (existing) { + return existing; + } + + const probe = inspect(filePath); + inFlight.set(filePath, probe); + const clear = () => { + if (inFlight.get(filePath) === probe) { + inFlight.delete(filePath); + } + }; + void probe.then(clear, clear); + return probe; + }; +} + +const probeDownloadFileAvailability = createDownloadFileAvailabilityProbe(); + export function isAvailableDownloadFile( filePath: string | null | undefined, lstat: DownloadLstat = lstatSync @@ -45,6 +116,21 @@ export function getDownloadFileAvailability( : 'missing'; } +export async function getDownloadFileAvailabilityAsync( + download: DownloadFileRow, + probe: DownloadFileAvailabilityProbe = probeDownloadFileAvailability +): Promise { + if (download.status !== 'completed') { + return 'not-applicable'; + } + + if (!download.filePath) { + return 'missing'; + } + + return (await probe(download.filePath)) ? 'available' : 'missing'; +} + export function decorateDownloadItem( download: T, lstat: DownloadLstat = lstatSync @@ -60,3 +146,24 @@ export function decorateDownloadItem( fileAvailability: getDownloadFileAvailability(download, lstat), }; } + +export async function decorateDownloadItemAsync( + download: T, + probe: DownloadFileAvailabilityProbe = probeDownloadFileAvailability +): Promise< + Omit & { + metadataSnapshot: DownloadMetadataSnapshot | undefined; + fileAvailability: ElectronDownloadFileAvailability; + } +> { + return { + ...download, + metadataSnapshot: decodeDownloadMetadataSnapshot( + download.metadataSnapshot + ), + fileAvailability: await getDownloadFileAvailabilityAsync( + download, + probe + ), + }; +} diff --git a/apps/electron-backend/src/app/events/database/downloads-file-availability.events.spec.ts b/apps/electron-backend/src/app/events/database/downloads-file-availability.events.spec.ts index 9a2c3ae8d..f6f0ba8d1 100644 --- a/apps/electron-backend/src/app/events/database/downloads-file-availability.events.spec.ts +++ b/apps/electron-backend/src/app/events/database/downloads-file-availability.events.spec.ts @@ -1,6 +1,7 @@ import { getHandler, mockGetDatabase, + mockLstat, mockLstatSync, setupDownloadsEventsHarness, } from './downloads.test-helpers'; @@ -12,11 +13,103 @@ function regularFile() { }; } +function nextEventLoopTurn(): Promise { + return new Promise((resolve) => setImmediate(resolve)); +} + describe('downloads events: file availability', () => { beforeEach(async () => { await setupDownloadsEventsHarness(); }); + it('loads list availability without synchronous filesystem probes', async () => { + const row = { + filePath: '/downloads/network-volume/movie.mp4', + id: 1, + status: 'completed', + }; + const orderBy = jest.fn().mockResolvedValue([row]); + mockGetDatabase.mockResolvedValue({ + select: jest.fn(() => ({ + from: jest.fn(() => ({ orderBy })), + })), + }); + mockLstat.mockResolvedValue(regularFile()); + mockLstatSync.mockImplementation(() => { + throw new Error('synchronous probe must not run'); + }); + + await expect(getHandler('DOWNLOADS_GET_LIST')(null)).resolves.toEqual([ + { + ...row, + metadataSnapshot: undefined, + fileAvailability: 'available', + }, + ]); + expect(mockLstat).toHaveBeenCalledWith(row.filePath); + expect(mockLstatSync).not.toHaveBeenCalled(); + }); + + it('deduplicates concurrent probes for the same completed file', async () => { + const rows = [1, 2].map((id) => ({ + filePath: '/downloads/shared/movie.mp4', + id, + status: 'completed', + })); + const orderBy = jest.fn().mockResolvedValue(rows); + mockGetDatabase.mockResolvedValue({ + select: jest.fn(() => ({ + from: jest.fn(() => ({ orderBy })), + })), + }); + let finishProbe!: (value: ReturnType) => void; + mockLstat.mockReturnValue( + new Promise((resolve) => { + finishProbe = resolve; + }) + ); + + const response = getHandler('DOWNLOADS_GET_LIST')(null); + await nextEventLoopTurn(); + + expect(mockLstat).toHaveBeenCalledTimes(1); + finishProbe(regularFile()); + await expect(response).resolves.toHaveLength(2); + }); + + it('starts at most four completed-file probes concurrently', async () => { + const rows = Array.from({ length: 6 }, (_, index) => ({ + filePath: `/downloads/network/movie-${index}.mp4`, + id: index + 1, + status: 'completed', + })); + const orderBy = jest.fn().mockResolvedValue(rows); + mockGetDatabase.mockResolvedValue({ + select: jest.fn(() => ({ + from: jest.fn(() => ({ orderBy })), + })), + }); + const finishProbes: Array< + (value: ReturnType) => void + > = []; + mockLstat.mockImplementation( + () => + new Promise((resolve) => { + finishProbes.push(resolve); + }) + ); + + const response = getHandler('DOWNLOADS_GET_LIST')(null); + await nextEventLoopTurn(); + + expect(mockLstat).toHaveBeenCalledTimes(4); + finishProbes.slice(0, 4).forEach((finish) => finish(regularFile())); + await nextEventLoopTurn(); + expect(mockLstat).toHaveBeenCalledTimes(6); + finishProbes.slice(4).forEach((finish) => finish(regularFile())); + await expect(response).resolves.toHaveLength(6); + }); + it('decorates every download in the list from the current filesystem state', async () => { const metadataSnapshot = { version: 1, @@ -48,7 +141,7 @@ describe('downloads events: file availability', () => { from: jest.fn(() => ({ orderBy })), })), }); - mockLstatSync.mockImplementation((filePath) => { + mockLstat.mockImplementation(async (filePath) => { if (filePath === '/downloads/missing.mp4') { throw new Error('ENOENT'); } @@ -72,7 +165,7 @@ describe('downloads events: file availability', () => { fileAvailability: 'not-applicable', }, ]); - expect(mockLstatSync).toHaveBeenCalledTimes(2); + expect(mockLstat).toHaveBeenCalledTimes(2); }); it('decorates an individual download from the current filesystem state', async () => { @@ -91,7 +184,7 @@ describe('downloads events: file availability', () => { })), })), }); - mockLstatSync.mockImplementation(() => { + mockLstat.mockImplementation(async () => { throw new Error('ENOENT'); }); @@ -116,6 +209,6 @@ describe('downloads events: file availability', () => { await expect( getHandler('DOWNLOADS_GET')(null, 404) ).resolves.toBeNull(); - expect(mockLstatSync).not.toHaveBeenCalled(); + expect(mockLstat).not.toHaveBeenCalled(); }); }); diff --git a/apps/electron-backend/src/app/events/database/downloads.events.ts b/apps/electron-backend/src/app/events/database/downloads.events.ts index d5ce12070..bdd86ebb6 100644 --- a/apps/electron-backend/src/app/events/database/downloads.events.ts +++ b/apps/electron-backend/src/app/events/database/downloads.events.ts @@ -7,7 +7,7 @@ import { getDatabase } from '../../database/connection'; import * as schema from '../../database/schema'; import { DownloadDirectoryAuthorizer } from './download-directory-authorization'; import { - decorateDownloadItem, + decorateDownloadItemAsync, isAvailableDownloadFile, } from './download-file-availability'; import { removePartialDownloadFile } from './download-file-path'; @@ -229,7 +229,7 @@ ipcMain.handle('DOWNLOADS_GET_LIST', async (_event, playlistId?: string) => { .where(eq(schema.downloads.playlistId, playlistId)) .orderBy(schema.downloads.createdAt) : query.orderBy(schema.downloads.createdAt)); - return rows.map((row) => decorateDownloadItem(row)); + return Promise.all(rows.map((row) => decorateDownloadItemAsync(row))); } catch (error) { console.error('[Downloads] Error getting download list:', error); throw error; @@ -244,7 +244,7 @@ ipcMain.handle('DOWNLOADS_GET', async (_event, downloadId: number) => { .from(schema.downloads) .where(eq(schema.downloads.id, downloadId)) .limit(1); - return result[0] ? decorateDownloadItem(result[0]) : null; + return result[0] ? await decorateDownloadItemAsync(result[0]) : null; } catch (error) { console.error('[Downloads] Error getting download:', error); throw error; diff --git a/apps/electron-backend/src/app/events/database/downloads.test-helpers.ts b/apps/electron-backend/src/app/events/database/downloads.test-helpers.ts index 5161cf7c1..a112697ba 100644 --- a/apps/electron-backend/src/app/events/database/downloads.test-helpers.ts +++ b/apps/electron-backend/src/app/events/database/downloads.test-helpers.ts @@ -16,6 +16,7 @@ export const mockRemovePartialDownloadFile = jest.fn(); export const mockPauseDownload = jest.fn(); export const mockRedownloadMissingRequest = jest.fn(); export const mockResumeDownloadRequest = jest.fn(); +export const mockLstat = jest.fn(); export const mockLstatSync = jest.fn(); export const mockOpenPath = jest.fn(); export const mockShowItemInFolder = jest.fn(); @@ -57,6 +58,7 @@ export async function setupDownloadsEventsHarness(): Promise { mockPauseDownload.mockReset(); mockRedownloadMissingRequest.mockReset(); mockResumeDownloadRequest.mockReset(); + mockLstat.mockReset(); mockLstatSync.mockReset(); mockOpenPath.mockReset().mockResolvedValue(''); mockShowItemInFolder.mockReset(); @@ -66,6 +68,12 @@ export async function setupDownloadsEventsHarness(): Promise { ...jest.requireActual('node:fs'), lstatSync: mockLstatSync, })); + jest.doMock('node:fs/promises', () => ({ + ...jest.requireActual( + 'node:fs/promises' + ), + lstat: mockLstat, + })); jest.doMock('drizzle-orm', () => { const actual = jest.requireActual('drizzle-orm'); diff --git a/docs/architecture/download-manager.md b/docs/architecture/download-manager.md index d9af552b4..42825f6fc 100644 --- a/docs/architecture/download-manager.md +++ b/docs/architecture/download-manager.md @@ -178,6 +178,7 @@ variants, contextual buttons, and theme-aware styling. ## Queuing, persistence, and UX notes - Every download row writes to the shared `downloads` table with statuses (`queued`, `downloading`, `paused`, `completed`, `failed`, `canceled`) plus metadata such as `bytesDownloaded`, `totalBytes`, `errorMessage`, `requestHeaders`, `resumeValidator`, the offline-detail metadata snapshot, and Xtream identifiers. Downloads are locally owned records rather than playlist children: deleting a source retains its rows and local files, keeps them visible in the global library, and disables provider handoff until that source exists again. Startup rebuilds older tables that still carry the playlist foreign key, while additive columns use the idempotent column migrations. +- Download-list and focused-detail reads verify completed files with asynchronous `lstat` probes in the main process. One shared probe queue permits at most four filesystem calls at once and coalesces only in-flight checks for the same path; it does not cache completed results, so external file deletion is visible on the next refresh without letting frequent progress broadcasts block Electron's main thread. - On startup, `download-recovery.ts` converts stale `downloading` rows with a non-empty `.part` file to `paused`, converts stale `queued` rows to `paused` while keeping any retained `.part` (a resumed download waiting behind an active one persists as `queued` with its partial), and marks stale `downloading` rows without recoverable partial bytes as `failed`. - Queue cancellation removes a queued task or records an active cancellation request and aborts the request when available. Pausing follows the same abort path but persists `paused` and keeps the `.part`. Retries reuse the same database entry: a failed row with a retained `filePath` resumes its `.part` through HTTP Range, otherwise the retry starts from zero. Resume appends to the existing `.part` through HTTP Range with `If-Range` validation. - A `.part` that cannot be deleted (locked, permission denied) never loses its database path: cancel persists `canceled` while retaining `filePath` for later cleanup, and `DOWNLOADS_REMOVE` keeps the row and answers `success: false` (surfaced as a snackbar) so retrying the remove re-attempts the deletion once the lock is released.