diff --git a/README.md b/README.md index 64fd9a3..b47ac64 100644 --- a/README.md +++ b/README.md @@ -20,7 +20,7 @@ Automatically download files from urls, even in the background, and keep them lo ## Install First install peer dependencies: -* [@kesha-antonov/react-native-background-downloader](https://github.com/kesha-antonov/react-native-background-downloader#readme). Be sure to follow the sneakily-hidden [extra iOS step for AppDelegate.m](https://github.com/kesha-antonov/react-native-background-downloader#ios---extra-mandatory-step) or else your background tasks will be canceled by the OS. +* [@fivecar/react-native-background-downloader](https://github.com/fivecar/react-native-background-downloader#readme). Be sure to follow the [iOS AppDelegate configuration step](https://github.com/fivecar/react-native-background-downloader#-installation) (or add the Expo config plugin) or else your background tasks will be canceled by the OS. * [react-native-fs](https://github.com/itinance/react-native-fs#readme) * [@react-native-async-storage/async-storage](https://github.com/react-native-async-storage/async-storage#readme) diff --git a/src/index.ts b/src/index.ts index 5ccdc2c..9972f4f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -69,6 +69,10 @@ export interface DownloadQueueStatus { export interface DownloadQueueHandlers { onBegin?: (url: string, totalBytes: number) => void; + /** + * `fractionWritten` is 0 when the total size is unknown (e.g. the server + * didn't send a Content-Length, in which case `totalBytes` is -1 or 0). + */ onProgress?: ( url: string, fractionWritten: number, @@ -265,6 +269,42 @@ export default class DownloadQueue { this.active = startActive; this.isPausedByUser = !startActive; + // Evaluate the network gate before reviving tasks or starting downloads. If + // the current connection isn't allowed (e.g. wifi-only while on cellular), + // onNetInfoChanged() calls pauseAllInternal(), which flips this.active off. + // That way revivals don't resume PAUSED tasks and fresh starts get paused + // in begin() -- so nothing briefly downloads on a forbidden network. + this.wouldAutoPause = false; + + if ( + activeNetworkTypes.length > 0 && + (!netInfoAddEventListener || !netInfoFetchState) + ) { + throw new Error( + "If you pass `activeNetworkTypes`, you must also pass both `netInfoAddEventListener` and `netInfoFetchState`" + ); + } + if (netInfoAddEventListener && !netInfoFetchState) { + throw new Error( + "If you pass `netInfoAddEventListener`, you must also pass `netInfoFetchState`" + ); + } + this.activeNetworkTypes = activeNetworkTypes; + this.netInfoFetchState = netInfoFetchState; + if (netInfoAddEventListener) { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const state = await this.netInfoFetchState!(); + + await this.onNetInfoChanged(state); + this.netInfoUnsubscriber = netInfoAddEventListener( + (state: DownloadQueueNetInfoState) => { + // The caller's listener signature is synchronous (it comes from + // NetInfo.addEventListener), so we can't await this here. + void this.onNetInfoChanged(state); + } + ); + } + // First revive tasks that were working in the background if (existingTasks.length > 0) { await Promise.all(existingTasks.map(task => this.reviveTask(task))); @@ -321,37 +361,6 @@ export default class DownloadQueue { now ); - this.wouldAutoPause = false; - - if ( - activeNetworkTypes.length > 0 && - (!netInfoAddEventListener || !netInfoFetchState) - ) { - throw new Error( - "If you pass `activeNetworkTypes`, you must also pass both `netInfoAddEventListener` and `netInfoFetchState`" - ); - } - if (netInfoAddEventListener && !netInfoFetchState) { - throw new Error( - "If you pass `netInfoAddEventListener`, you must also pass `netInfoFetchState`" - ); - } - this.activeNetworkTypes = activeNetworkTypes; - this.netInfoFetchState = netInfoFetchState; - if (netInfoAddEventListener) { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const state = await this.netInfoFetchState!(); - - await this.onNetInfoChanged(state); - this.netInfoUnsubscriber = netInfoAddEventListener( - (state: DownloadQueueNetInfoState) => { - // The caller's listener signature is synchronous (it comes from - // NetInfo.addEventListener), so we can't await this here. - void this.onNetInfoChanged(state); - } - ); - } - this.inited = true; } @@ -753,7 +762,10 @@ export default class DownloadQueue { if (!this.active) { void task.pause(); } - const fraction = bytesDownloaded / bytesTotal; + // bytesTotal can be -1 (unknown Content-Length) or 0, in which case we + // can't compute a meaningful fraction. Report 0 rather than a negative + // or NaN value, while still passing the raw byte counts through. + const fraction = bytesTotal > 0 ? bytesDownloaded / bytesTotal : 0; this.handlers?.onProgress?.(url, fraction, bytesDownloaded, bytesTotal); }) // eslint-disable-next-line @typescript-eslint/no-misused-promises @@ -939,6 +951,11 @@ export default class DownloadQueue { this.addTask(spec.url, task); if (!this.active && task.state === "DOWNLOADING") { await task.pause(); + } else if (this.active && task.state === "PAUSED") { + // A revived task can come back PAUSED (e.g. paused by the OS while + // backgrounded). If the queue is active, resume it so it doesn't + // stay paused forever -- nothing else in init will restart it. + await task.resume(); } } else { await task.stop(); diff --git a/test/index.spec.ts b/test/index.spec.ts index 8498e9a..684796a 100644 --- a/test/index.spec.ts +++ b/test/index.spec.ts @@ -376,7 +376,7 @@ describe("DownloadQueue", () => { expect(createDownloadTask).not.toHaveBeenCalled(); }); - it("revives paused specs from previous launches", async () => { + it("resumes revived paused specs when the queue is active", async () => { const queue = new DownloadQueue(); const handlers: DownloadQueueHandlers = { onBegin: jest.fn(), @@ -394,9 +394,39 @@ describe("DownloadQueue", () => { }); await queue.init({ domain: "mydomain", handlers }); - // PAUSED tasks are left alone (not auto-resumed) upon revival. + // Since the queue is active, a revived PAUSED task must be resumed -- + // otherwise it would stay paused forever. + expect(task.resume).toHaveBeenCalled(); expect(task.pause).not.toHaveBeenCalled(); + expect(handlers.onBegin).toHaveBeenCalledWith( + "http://foo.com/a.mp3", + task.bytesTotal + ); + expect(createDownloadTask).not.toHaveBeenCalled(); + }); + + it("leaves revived paused specs paused when init'd inactive", async () => { + const queue = new DownloadQueue(); + const handlers: DownloadQueueHandlers = { + onBegin: jest.fn(), + }; + + task.state = "PAUSED"; + task.bytesTotal = 8675309; + (getExistingDownloadTasks as jest.Mock).mockReturnValue([task]); + + await kvfs.write("/mydomain/foo", { + id: "foo", + url: "http://foo.com/a.mp3", + path: `${RNFS.DocumentDirectoryPath}/DownloadQueue/mydomain/foo`, + createTime: Date.now() - 1000, + }); + await queue.init({ domain: "mydomain", handlers, startActive: false }); + + // When init'd inactive, a revived PAUSED task is already paused and must + // be left alone. expect(task.resume).not.toHaveBeenCalled(); + expect(task.pause).not.toHaveBeenCalled(); expect(handlers.onBegin).toHaveBeenCalledWith( "http://foo.com/a.mp3", task.bytesTotal @@ -404,6 +434,42 @@ describe("DownloadQueue", () => { expect(createDownloadTask).not.toHaveBeenCalled(); }); + it("doesn't resume revived paused specs when the network forbids downloads", async () => { + const queue = new DownloadQueue(); + const handlers: DownloadQueueHandlers = { + onBegin: jest.fn(), + }; + + task.state = "PAUSED"; + task.bytesTotal = 8675309; + (getExistingDownloadTasks as jest.Mock).mockReturnValue([task]); + + // We're on cellular, but the queue is wifi-only. init() must evaluate the + // network gate before reviving tasks, so a revived PAUSED task is not + // resumed onto metered data. + (fetch as jest.Mock).mockResolvedValueOnce({ + isConnected: true, + type: "cellular", + }); + + await kvfs.write("/mydomain/foo", { + id: "foo", + url: "http://foo.com/a.mp3", + path: `${RNFS.DocumentDirectoryPath}/DownloadQueue/mydomain/foo`, + createTime: Date.now() - 1000, + }); + await queue.init({ + domain: "mydomain", + handlers, + activeNetworkTypes: ["wifi"], + netInfoAddEventListener: addEventListener, + netInfoFetchState: fetch, + }); + + expect(task.resume).not.toHaveBeenCalled(); + expect(createDownloadTask).not.toHaveBeenCalled(); + }); + it("revives done specs from previous launches", async () => { const queue = new DownloadQueue(); const handlers: DownloadQueueHandlers = { @@ -985,10 +1051,10 @@ describe("DownloadQueue", () => { await relaunchQueue.init({ domain: "mydomain" }); // Revival should reattach to the existing task rather than starting a - // new download, and shouldn't touch pause/resume for an already-paused - // task. + // new download. Since the relaunched queue is active, a revived PAUSED + // task is resumed so it doesn't stay stuck. expect(createDownloadTask).toHaveBeenCalledTimes(1); - expect(task.resume).not.toHaveBeenCalled(); + expect(task.resume).toHaveBeenCalled(); }); it("should pause revived url when relaunched inactive", async () => { @@ -2435,6 +2501,41 @@ describe("DownloadQueue", () => { expect(handlers.onError).toHaveBeenCalledTimes(1); }); + it("reports fraction 0 when total size is unknown", async () => { + const handlers: DownloadQueueHandlers = { + onProgress: jest.fn(), + }; + const queue = new DownloadQueue(); + + Object.assign(task, { + id: "foo", + begin: jest.fn(handler => { + task._begin = handler; + return task; + }), + progress: jest.fn(handler => { + task._progress = handler; + return task; + }), + }); + + await queue.init({ domain: "mydomain", handlers }); + await queue.addUrl("http://foo.com/a.mp3"); + + // The downloader reports bytesTotal === -1 when Content-Length is unknown. + // We must not divide by it, which would yield a negative fraction. + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + task._progress!({ bytesDownloaded: 420, bytesTotal: -1 }); + + expect(handlers.onProgress).toHaveBeenCalledTimes(1); + expect(handlers.onProgress).toHaveBeenCalledWith( + "http://foo.com/a.mp3", + 0, + 420, + -1 + ); + }); + it("should call done handler with local path when finished", async () => { const queue = new DownloadQueue(); const handlers: DownloadQueueHandlers = {