From 013e6ac78a71960d9b8c88a3ff6724ff030fb339 Mon Sep 17 00:00:00 2001 From: Philip Su <39933441+fivecar@users.noreply.github.com> Date: Wed, 8 Jul 2026 16:09:42 -0700 Subject: [PATCH] feat(deps)!: migrate to @fivecar/react-native-background-downloader Replace @kesha-antonov/react-native-background-downloader with @fivecar/react-native-background-downloader (MMKV-free fork), and adapt to its new task API: - checkForExistingDownloads() -> getExistingDownloadTasks() - download() -> createDownloadTask() + explicit task.start() - ensureDownloadsAreRunning() removed; its old pause/resume workaround is gone along with it - task.pause()/resume()/stop() and completeHandler() are now genuinely async (they await the native bridge call instead of firing and forgetting), so internal call sites that have an ordering dependency on completion now await them instead of using `void` BREAKING CHANGE: DownloadQueue#terminate(), #pauseAll(), and #resumeAll() are now async and return Promise instead of void, so that they can correctly await the underlying task stop/pause/resume calls, which are no longer fire-and-forget in the new downloader library. Callers must now await these methods (or otherwise handle the returned promise) instead of calling them synchronously. @fivecar/react-native-background-downloader is now a peerDependency (matching react-native-fs and @react-native-async-storage/async-storage), so consuming apps must install it directly. The separate @types/kesha-antonov__react-native-background-downloader dev dependency is no longer needed since the new package ships its own types. --- README.md | 6 +- package-lock.json | 47 ++---- package.json | 3 +- src/index.ts | 80 +++++---- test/index.spec.ts | 399 +++++++++++++++++++++++---------------------- 5 files changed, 266 insertions(+), 269 deletions(-) diff --git a/README.md b/README.md index ac426ff..64fd9a3 100644 --- a/README.md +++ b/README.md @@ -89,7 +89,7 @@ Here are the optional notification handlers you can pass to be informed of downl |`onWillRemove?: (url: string) => Promise`| Called before any url is removed from the queue. This is async because `removeUrl` (and also `setQueue`, when it needs to remove some urls) will block until you return from this, giving you the opportunity remove any dependencies on any downloaded local file before it's deleted.| |`onError?: (url: string, error: any) => void`| Called when there's been an issue downloading the file. Note that this is mostly for you to communicate something to the user, or to do other housekeeping; DownloadQueue will automatically re-attempt the download every minute (while you're online) until it succeeds.| -### `terminate(): void` +### `async terminate(): Promise` Terminates all pending downloads and stops all activity, including processing lazy-deletes. You can re-init() if you'd like -- but in most cases where you plan to re-init, `pause()` might be what you really meant. @@ -120,11 +120,11 @@ Returns a `DownloadQueueStatus` object reflecting the status of a url's download Returns the status of all urls in the queue, excluding urls marked for lazy deletion. -### `pauseAll(): void` +### `async pauseAll(): Promise` Pauses all active downloads. Note that if you just want to download on certain types of connections, you should instead use `activeNetworkTypes` in `init()`. For instance, to avoid cellular data charges, you might pass `activeNetworkTypes: ["wifi", "ethernet"]`. -### `resumeAll(): void` +### `async resumeAll(): Promise` Resumes all active downloads that were previously paused. If you `init()` with `startActive === false`, you'll want to call this at some point or else downloads will never happen. Also, downloads will only proceed if the network connection type passes the `activeNetworkTypes` filter (which by default allows all connection types). diff --git a/package-lock.json b/package-lock.json index d78f51f..f9d6935 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,6 +9,7 @@ "version": "1.3.2", "license": "MIT", "dependencies": { + "@fivecar/react-native-background-downloader": "^4.5.6", "key-value-file-system": "^1.1.0", "react-uuid": "^2.0.0" }, @@ -17,7 +18,6 @@ "@ryansonshine/commitizen": "^4.2.8", "@ryansonshine/cz-conventional-changelog": "^3.3.4", "@types/jest": "^27.5.2", - "@types/kesha-antonov__react-native-background-downloader": "^2.6.0", "@types/node": "^12.20.11", "@types/react-native-background-downloader": "^2.3.6", "@typescript-eslint/eslint-plugin": "^4.22.0", @@ -43,7 +43,6 @@ "node": ">=12.0" }, "peerDependencies": { - "@kesha-antonov/react-native-background-downloader": "^3.2.6", "@react-native-async-storage/async-storage": "^1.17.11", "react-native-fs": "^2.20.0" } @@ -2136,6 +2135,15 @@ "node": ">= 4" } }, + "node_modules/@fivecar/react-native-background-downloader": { + "version": "4.5.6", + "resolved": "https://registry.npmjs.org/@fivecar/react-native-background-downloader/-/react-native-background-downloader-4.5.6.tgz", + "integrity": "sha512-t9qmNG1fHosJ/m5+TEP58FHydtcWcI4p6dr8AY+f3sq2KnbRjvkdFNcBuqLtGr8Fyd/oRBxykzeiYdaWnrCs3Q==", + "license": "Apache-2.0", + "peerDependencies": { + "react-native": ">=0.57.0" + } + }, "node_modules/@hapi/hoek": { "version": "9.3.0", "resolved": "https://registry.npmjs.org/@hapi/hoek/-/hoek-9.3.0.tgz", @@ -2702,16 +2710,6 @@ "@jridgewell/sourcemap-codec": "1.4.14" } }, - "node_modules/@kesha-antonov/react-native-background-downloader": { - "version": "3.2.6", - "resolved": "https://registry.npmjs.org/@kesha-antonov/react-native-background-downloader/-/react-native-background-downloader-3.2.6.tgz", - "integrity": "sha512-J87PHzBh4knWTQNkCNM4LTMZ85RpMW/QSV+0LGdTxz4JmfLXoeg8R6ratbFU0DP/l8K1eL7r4S1Rc8bmqNJ3Ug==", - "license": "Apache-2.0", - "peer": true, - "peerDependencies": { - "react-native": ">=0.57.0" - } - }, "node_modules/@nodelib/fs.scandir": { "version": "2.1.4", "resolved": "https://registry.npmjs.org/@nodelib/fs.scandir/-/fs.scandir-2.1.4.tgz", @@ -4530,12 +4528,6 @@ "integrity": "sha512-cxWFQVseBm6O9Gbw1IWb8r6OS4OhSt3hPZLkFApLjM8TEXROBuQGLAH2i2gZpcXdLBIrpXuTDhH7Vbm1iXmNGA==", "dev": true }, - "node_modules/@types/kesha-antonov__react-native-background-downloader": { - "version": "2.6.0", - "resolved": "https://registry.npmjs.org/@types/kesha-antonov__react-native-background-downloader/-/kesha-antonov__react-native-background-downloader-2.6.0.tgz", - "integrity": "sha512-AE+Qfg1ofhloPskZSsus63FtB6sGieSq+MtWct+kSanPUBrCA5smHnR/PvU3WqPvqxWzgEpoo7KFyRihAPoYbQ==", - "dev": true - }, "node_modules/@types/minimist": { "version": "1.2.2", "resolved": "https://registry.npmjs.org/@types/minimist/-/minimist-1.2.2.tgz", @@ -19745,6 +19737,12 @@ } } }, + "@fivecar/react-native-background-downloader": { + "version": "4.5.6", + "resolved": "https://registry.npmjs.org/@fivecar/react-native-background-downloader/-/react-native-background-downloader-4.5.6.tgz", + "integrity": "sha512-t9qmNG1fHosJ/m5+TEP58FHydtcWcI4p6dr8AY+f3sq2KnbRjvkdFNcBuqLtGr8Fyd/oRBxykzeiYdaWnrCs3Q==", + "requires": {} + }, "@hapi/hoek": { "version": "9.3.0", "resolved": "https://registry.npmjs.org/@hapi/hoek/-/hoek-9.3.0.tgz", @@ -20204,13 +20202,6 @@ "@jridgewell/sourcemap-codec": "1.4.14" } }, - "@kesha-antonov/react-native-background-downloader": { - "version": "3.2.6", - "resolved": "https://registry.npmjs.org/@kesha-antonov/react-native-background-downloader/-/react-native-background-downloader-3.2.6.tgz", - "integrity": "sha512-J87PHzBh4knWTQNkCNM4LTMZ85RpMW/QSV+0LGdTxz4JmfLXoeg8R6ratbFU0DP/l8K1eL7r4S1Rc8bmqNJ3Ug==", - "peer": true, - "requires": {} - }, "@nodelib/fs.scandir": { "version": "2.1.4", "resolved": "https://registry.npmjs.org/@nodelib/fs.scandir/-/fs.scandir-2.1.4.tgz", @@ -21694,12 +21685,6 @@ "integrity": "sha512-cxWFQVseBm6O9Gbw1IWb8r6OS4OhSt3hPZLkFApLjM8TEXROBuQGLAH2i2gZpcXdLBIrpXuTDhH7Vbm1iXmNGA==", "dev": true }, - "@types/kesha-antonov__react-native-background-downloader": { - "version": "2.6.0", - "resolved": "https://registry.npmjs.org/@types/kesha-antonov__react-native-background-downloader/-/kesha-antonov__react-native-background-downloader-2.6.0.tgz", - "integrity": "sha512-AE+Qfg1ofhloPskZSsus63FtB6sGieSq+MtWct+kSanPUBrCA5smHnR/PvU3WqPvqxWzgEpoo7KFyRihAPoYbQ==", - "dev": true - }, "@types/minimist": { "version": "1.2.2", "resolved": "https://registry.npmjs.org/@types/minimist/-/minimist-1.2.2.tgz", diff --git a/package.json b/package.json index 05f3f76..88ebb19 100644 --- a/package.json +++ b/package.json @@ -47,7 +47,6 @@ "@ryansonshine/commitizen": "^4.2.8", "@ryansonshine/cz-conventional-changelog": "^3.3.4", "@types/jest": "^27.5.2", - "@types/kesha-antonov__react-native-background-downloader": "^2.6.0", "@types/node": "^12.20.11", "@types/react-native-background-downloader": "^2.3.6", "@typescript-eslint/eslint-plugin": "^4.22.0", @@ -70,7 +69,7 @@ "typescript": "^4.2.4" }, "peerDependencies": { - "@kesha-antonov/react-native-background-downloader": "^3.2.6", + "@fivecar/react-native-background-downloader": "^4.5.6", "@react-native-async-storage/async-storage": "^1.17.11", "react-native-fs": "^2.20.0" }, diff --git a/src/index.ts b/src/index.ts index 65e25b6..5ccdc2c 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,10 +1,9 @@ import { - checkForExistingDownloads, completeHandler, - download, + createDownloadTask, DownloadTask, - ensureDownloadsAreRunning, -} from "@kesha-antonov/react-native-background-downloader"; + getExistingDownloadTasks, +} from "@fivecar/react-native-background-downloader"; import AsyncStorage from "@react-native-async-storage/async-storage"; import KeyValueFileSystem from "key-value-file-system"; import { Platform } from "react-native"; @@ -229,7 +228,7 @@ export default class DownloadQueue { const [specData, existingTasks, dirFilenames] = await Promise.all([ this.kvfs.readMulti(`${this.keyFromId("")}*`), - checkForExistingDownloads(), + getExistingDownloadTasks(), this.getDirFilenames(), ]); const now = Date.now(); @@ -269,14 +268,6 @@ export default class DownloadQueue { // First revive tasks that were working in the background if (existingTasks.length > 0) { await Promise.all(existingTasks.map(task => this.reviveTask(task))); - await ensureDownloadsAreRunning(); - if (!this.active) { - // ensureDownloadsAreRunning forces all un-stopped downloads to start. - // So we'll need to stop them again if !active. - existingTasks - .filter(task => task.state === "DOWNLOADING") - .forEach(task => void task.pause()); - } } // Now start downloads for specs that haven't finished @@ -351,10 +342,12 @@ export default class DownloadQueue { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const state = await this.netInfoFetchState!(); - this.onNetInfoChanged(state); + await this.onNetInfoChanged(state); this.netInfoUnsubscriber = netInfoAddEventListener( (state: DownloadQueueNetInfoState) => { - this.onNetInfoChanged(state); + // The caller's listener signature is synchronous (it comes from + // NetInfo.addEventListener), so we can't await this here. + void this.onNetInfoChanged(state); } ); } @@ -367,9 +360,9 @@ export default class DownloadQueue { * processing lazy-deletes. You can re-init() if you'd like -- but in most * cases where you plan to re-init, pause() might be what you really meant. */ - terminate(): void { + async terminate(): Promise { this.active = false; - this.tasks.forEach(task => void task.stop()); + await Promise.all(this.tasks.map(task => task.stop())); this.tasks = []; this.specs = []; this.handlers = undefined; @@ -472,7 +465,7 @@ export default class DownloadQueue { const task = this.removeTask(spec.id); if (task) { - task.stop(); + await task.stop(); } // If it's a lazy delete, just update the spec but don't mess with files. @@ -589,16 +582,16 @@ export default class DownloadQueue { * Pauses all active downloads. Most used to implement wifi-only downloads, * by pausing when NetInfo reports a non-wifi connection. */ - pauseAll(): void { + async pauseAll(): Promise { this.verifyInitialized(); this.isPausedByUser = true; - this.pauseAllInternal(); + await this.pauseAllInternal(); } - private pauseAllInternal(): void { + private async pauseAllInternal(): Promise { this.active = false; - this.tasks.forEach(task => void task.pause()); + await Promise.all(this.tasks.map(task => task.pause())); if (this.errorTimer) { clearInterval(this.errorTimer); @@ -613,20 +606,20 @@ export default class DownloadQueue { * network connection type passes the `activeNetworkTypes` filter (which by * default allows all connection types). */ - resumeAll(): void { + async resumeAll(): Promise { this.verifyInitialized(); this.isPausedByUser = false; if (!this.wouldAutoPause) { // We only resume downloads if we weren't told otherwise to auto-pause // based on network conditions. - this.resumeAllInternal(); + await this.resumeAllInternal(); } } - private resumeAllInternal() { + private async resumeAllInternal(): Promise { this.active = true; - this.tasks.forEach(task => void task.resume()); + await Promise.all(this.tasks.map(task => task.resume())); if (this.erroredIds.size > 0) { this.ensureErrorTimerOn(); @@ -684,13 +677,14 @@ export default class DownloadQueue { void this.kvfs.write(this.keyFromId(spec.id), spec); } - const task = download({ + const task = createDownloadTask({ id: spec.id, url: spec.url, destination: spec.path, }); this.addTask(spec.url, task); + task.start(); // Bug: https://github.com/kesha-antonov/react-native-background-downloader/issues/23 // This is the ideal place to pause, but the downloader has a bug that @@ -720,7 +714,7 @@ export default class DownloadQueue { this.activeNetworkTypes = types; if (this.netInfoFetchState) { const state = await this.netInfoFetchState(); - this.onNetInfoChanged(state); + await this.onNetInfoChanged(state); } else { throw new Error( "Can't `setActiveNetworkType` without having init'd with `netInfoFetchState`" @@ -743,7 +737,7 @@ export default class DownloadQueue { // download sends us this begin() callback. When #23 is fixed, we can // in theory move this into the end of start(), after download(). if (!this.active) { - task.pause(); + void task.pause(); } this.handlers?.onBegin?.(url, data.expectedBytes); }) @@ -757,7 +751,7 @@ export default class DownloadQueue { // See note in begin() above. We can get progress callbacks even without // begin() (e.g. in the case of resuming a background task upon launch). if (!this.active) { - task.pause(); + void task.pause(); } const fraction = bytesDownloaded / bytesTotal; this.handlers?.onProgress?.(url, fraction, bytesDownloaded, bytesTotal); @@ -790,7 +784,7 @@ export default class DownloadQueue { await this.kvfs.write(this.keyFromId(spec.id), spec); if (Platform.OS === "ios") { - completeHandler(task.id); + void completeHandler(task.id); } // Only notify the client once everything has completed successfully and @@ -857,7 +851,9 @@ export default class DownloadQueue { this.specs = this.specs.filter(spec => !delIds.has(spec.id)); } - private onNetInfoChanged(state: DownloadQueueNetInfoState) { + private async onNetInfoChanged( + state: DownloadQueueNetInfoState + ): Promise { const shouldAutoPause = !state.isConnected || (this.activeNetworkTypes.length > 0 && @@ -872,9 +868,9 @@ export default class DownloadQueue { // asked us to pause. If they have, we leave their wishes alone. if (!this.isPausedByUser) { if (shouldAutoPause) { - this.pauseAllInternal(); + await this.pauseAllInternal(); } else { - this.resumeAllInternal(); + await this.resumeAllInternal(); } } } @@ -916,7 +912,7 @@ export default class DownloadQueue { // that we begin the download again. // Downloader docs say every task needs to be paused or stopped. // So we stop here. - task.stop(); + await task.stop(); // Since the file is missing from disk, yet the downloader thinks // it's done, we restart the download. this.start(spec); @@ -940,18 +936,18 @@ export default class DownloadQueue { } if (shouldAddTask) { - // Downloader docs say to pause tasks before reattaching our handlers - task.pause(); this.addTask(spec.url, task); + if (!this.active && task.state === "DOWNLOADING") { + await task.pause(); + } } else { - // According to downloader docs, we must either stop or pause revived - // tasks because ensureTasksRunning() will unpause any download we - // didn't explicitly stop (!) - task.stop(); + await task.stop(); } } else { if (this.isTaskDownloading(task)) { - task.stop(); + // Await this so the unlink() below (for lazy-delete revivals) can't + // race a native write that's still in flight. + await task.stop(); } if (!spec) { diff --git a/test/index.spec.ts b/test/index.spec.ts index 9dec36e..8498e9a 100644 --- a/test/index.spec.ts +++ b/test/index.spec.ts @@ -1,13 +1,12 @@ import { BeginHandler, - checkForExistingDownloads, + createDownloadTask, DoneHandler, - download, DownloadTask, - ensureDownloadsAreRunning, ErrorHandler, + getExistingDownloadTasks, ProgressHandler, -} from "@kesha-antonov/react-native-background-downloader"; +} from "@fivecar/react-native-background-downloader"; import AsyncStorage from "@react-native-async-storage/async-storage"; import { addEventListener, @@ -43,11 +42,10 @@ jest.mock("@react-native-async-storage/async-storage", () => { // Not sure why putting this in /__mocks__ doesn't work, when things like // react-native-fs do. Spent too long researching it. -jest.mock("@kesha-antonov/react-native-background-downloader", () => { +jest.mock("@fivecar/react-native-background-downloader", () => { return { - checkForExistingDownloads: jest.fn(() => []), - ensureDownloadsAreRunning: jest.fn(() => []), - download: jest.fn(() => ({})), + getExistingDownloadTasks: jest.fn(() => []), + createDownloadTask: jest.fn(() => ({})), completeHandler: jest.fn(), DownloadTaskState: { DOWNLOADING: "DOWNLOADING", @@ -148,6 +146,7 @@ function createBasicTask(): TaskWithHandlers { resume: jest.fn(() => baseTask), pause: jest.fn(() => baseTask), stop: jest.fn(() => baseTask), + start: jest.fn(() => baseTask), }); } @@ -158,8 +157,8 @@ async function expectPublicsToFail(queue: DownloadQueue) { await expect(queue.removeUrl("whatevs")).rejects.toThrow(); await expect(queue.setQueue([])).rejects.toThrow(); await expect(queue.getQueueStatus()).rejects.toThrow(); - expect(() => queue.pauseAll()).toThrow(); - expect(() => queue.resumeAll()).toThrow(); + await expect(queue.pauseAll()).rejects.toThrow(); + await expect(queue.resumeAll()).rejects.toThrow(); await expect(queue.getAvailableUrl("whatevs")).rejects.toThrow(); await expect(queue.getStatus("whatevs")).rejects.toThrow(); await expect(queue.setActiveNetworkTypes(["boo"])).rejects.toThrow(); @@ -190,9 +189,8 @@ describe("DownloadQueue", () => { (unlink as jest.Mock).mockImplementation(() => Promise.resolve()); (stat as jest.Mock).mockReturnValue({ size: 8675309 }); - (download as jest.Mock).mockReturnValue(task); - (checkForExistingDownloads as jest.Mock).mockReturnValue([]); - (ensureDownloadsAreRunning as jest.Mock).mockReturnValue(undefined); + (createDownloadTask as jest.Mock).mockReturnValue(task); + (getExistingDownloadTasks as jest.Mock).mockReturnValue([]); }); afterEach(async () => { @@ -343,7 +341,9 @@ describe("DownloadQueue", () => { expect(specs.length).toEqual(2); expect( - specs.some(spec => "id" in spec && spec.id === "foo2") + specs.some( + spec => "id" in spec && (spec as { id?: string }).id === "foo2" + ) ).toBeFalsy(); }); @@ -355,13 +355,7 @@ describe("DownloadQueue", () => { task.state = "DOWNLOADING"; task.bytesTotal = 8675309; - (checkForExistingDownloads as jest.Mock).mockReturnValue([task]); - (ensureDownloadsAreRunning as jest.Mock).mockImplementationOnce(() => { - // This is exactly what the actual implementation does, to work around - // some bug in the library. - task.pause(); - task.resume(); - }); + (getExistingDownloadTasks as jest.Mock).mockReturnValue([task]); await kvfs.write("/mydomain/foo", { id: "foo", @@ -371,12 +365,15 @@ describe("DownloadQueue", () => { }); await queue.init({ domain: "mydomain", handlers }); - expect(task.resume).toHaveBeenCalledTimes(1); + // Since the queue is active, a revived DOWNLOADING task shouldn't be + // paused or resumed -- it's just left running. + expect(task.pause).not.toHaveBeenCalled(); + expect(task.resume).not.toHaveBeenCalled(); expect(handlers.onBegin).toHaveBeenCalledWith( "http://foo.com/a.mp3", task.bytesTotal ); - expect(download).not.toHaveBeenCalled(); + expect(createDownloadTask).not.toHaveBeenCalled(); }); it("revives paused specs from previous launches", async () => { @@ -387,13 +384,7 @@ describe("DownloadQueue", () => { task.state = "PAUSED"; task.bytesTotal = 8675309; - (checkForExistingDownloads as jest.Mock).mockReturnValue([task]); - (ensureDownloadsAreRunning as jest.Mock).mockImplementationOnce(() => { - // This is exactly what the actual implementation does, to work around - // some bug in the library. - task.pause(); - task.resume(); - }); + (getExistingDownloadTasks as jest.Mock).mockReturnValue([task]); await kvfs.write("/mydomain/foo", { id: "foo", @@ -403,12 +394,14 @@ describe("DownloadQueue", () => { }); await queue.init({ domain: "mydomain", handlers }); - expect(task.resume).toHaveBeenCalledTimes(1); + // PAUSED tasks are left alone (not auto-resumed) upon revival. + expect(task.pause).not.toHaveBeenCalled(); + expect(task.resume).not.toHaveBeenCalled(); expect(handlers.onBegin).toHaveBeenCalledWith( "http://foo.com/a.mp3", task.bytesTotal ); - expect(download).not.toHaveBeenCalled(); + expect(createDownloadTask).not.toHaveBeenCalled(); }); it("revives done specs from previous launches", async () => { @@ -420,7 +413,7 @@ describe("DownloadQueue", () => { task.state = "DONE"; task.bytesTotal = 8675309; - (checkForExistingDownloads as jest.Mock).mockReturnValue([task]); + (getExistingDownloadTasks as jest.Mock).mockReturnValue([task]); (exists as jest.Mock).mockReturnValue(true); await kvfs.write("/mydomain/foo", { @@ -441,7 +434,7 @@ describe("DownloadQueue", () => { "http://foo.com/a.mp3", `${RNFS.DocumentDirectoryPath}/DownloadQueue/mydomain/foo` ); - expect(download).not.toHaveBeenCalled(); + expect(createDownloadTask).not.toHaveBeenCalled(); }); it("revives done specs from previous launches with missing files", async () => { @@ -453,7 +446,7 @@ describe("DownloadQueue", () => { task.state = "DONE"; task.bytesTotal = 8675309; - (checkForExistingDownloads as jest.Mock).mockReturnValue([task]); + (getExistingDownloadTasks as jest.Mock).mockReturnValue([task]); (exists as jest.Mock).mockReturnValue(false); await kvfs.write("/mydomain/foo", { @@ -468,7 +461,7 @@ describe("DownloadQueue", () => { expect(task.resume).not.toHaveBeenCalled(); expect(handlers.onBegin).not.toHaveBeenCalled(); expect(handlers.onDone).not.toHaveBeenCalled(); - expect(download).toHaveBeenCalledTimes(1); + expect(createDownloadTask).toHaveBeenCalledTimes(1); }); it("restarts stopped specs from previous launches", async () => { @@ -479,7 +472,7 @@ describe("DownloadQueue", () => { }; task.state = "STOPPED"; - (checkForExistingDownloads as jest.Mock).mockReturnValue([task]); + (getExistingDownloadTasks as jest.Mock).mockReturnValue([task]); await kvfs.write("/mydomain/foo", { id: "foo", @@ -489,7 +482,8 @@ describe("DownloadQueue", () => { }); await queue.init({ domain: "mydomain", handlers }); - expect(download).toHaveBeenCalledTimes(1); + expect(createDownloadTask).toHaveBeenCalledTimes(1); + expect(task.start).toHaveBeenCalledTimes(1); expect(task.resume).not.toHaveBeenCalled(); expect(handlers.onDone).not.toHaveBeenCalled(); }); @@ -503,7 +497,7 @@ describe("DownloadQueue", () => { task.state = "STOPPED"; task.bytesTotal = 8675309; - (checkForExistingDownloads as jest.Mock).mockReturnValue([task]); + (getExistingDownloadTasks as jest.Mock).mockReturnValue([task]); (exists as jest.Mock).mockReturnValue(true); await kvfs.write("/mydomain/foo", { @@ -516,7 +510,7 @@ describe("DownloadQueue", () => { await queue.init({ domain: "mydomain", handlers }); // Should not restart download for finished specs - expect(download).not.toHaveBeenCalled(); + expect(createDownloadTask).not.toHaveBeenCalled(); expect(task.resume).not.toHaveBeenCalled(); // Should call handlers for the finished spec @@ -539,7 +533,7 @@ describe("DownloadQueue", () => { task.state = "PAUSED"; // Use PAUSED so isTaskDownloading returns true task.bytesTotal = 8675309; - (checkForExistingDownloads as jest.Mock).mockReturnValue([task]); + (getExistingDownloadTasks as jest.Mock).mockReturnValue([task]); (exists as jest.Mock).mockReturnValue(true); await kvfs.write("/mydomain/foo", { @@ -559,7 +553,7 @@ describe("DownloadQueue", () => { expect(handlers.onDone).not.toHaveBeenCalled(); // Should not restart download - expect(download).not.toHaveBeenCalled(); + expect(createDownloadTask).not.toHaveBeenCalled(); }); it("restarts failed specs from previous launches", async () => { @@ -570,7 +564,7 @@ describe("DownloadQueue", () => { }; task.state = "FAILED"; - (checkForExistingDownloads as jest.Mock).mockReturnValue([task]); + (getExistingDownloadTasks as jest.Mock).mockReturnValue([task]); await kvfs.write("/mydomain/foo", { id: "foo", @@ -584,7 +578,8 @@ describe("DownloadQueue", () => { expect(jest.getTimerCount()).toEqual(1); // error retry timer await advanceThroughNextTimersAndPromises(); - expect(download).toHaveBeenCalledTimes(1); + expect(createDownloadTask).toHaveBeenCalledTimes(1); + expect(task.start).toHaveBeenCalledTimes(1); expect(task.resume).not.toHaveBeenCalled(); expect(handlers.onDone).not.toHaveBeenCalled(); expect(handlers.onError).toHaveBeenCalledTimes(1); @@ -593,17 +588,17 @@ describe("DownloadQueue", () => { it("stops tasks from previous launches without specs if they're still going", async () => { const queue = new DownloadQueue(); - (checkForExistingDownloads as jest.Mock).mockReturnValue([task]); + (getExistingDownloadTasks as jest.Mock).mockReturnValue([task]); await queue.init({ domain: "mydomain" }); expect(task.stop).not.toHaveBeenCalled(); - queue.terminate(); + await queue.terminate(); task.state = "PAUSED"; await queue.init({ domain: "mydomain" }); expect(task.stop).toHaveBeenCalledTimes(1); - queue.terminate(); + await queue.terminate(); task.state = "DOWNLOADING"; await queue.init({ domain: "mydomain" }); expect(task.stop).toHaveBeenCalledTimes(2); @@ -612,7 +607,7 @@ describe("DownloadQueue", () => { it("stops lazy-deleted tasks from previous launches", async () => { const queue = new DownloadQueue(); - (checkForExistingDownloads as jest.Mock).mockReturnValue([task]); + (getExistingDownloadTasks as jest.Mock).mockReturnValue([task]); await kvfs.write("/mydomain/foo", { id: task.id, url: "http://foo.com/a.mp3", @@ -631,7 +626,7 @@ describe("DownloadQueue", () => { it("starts downloads for unfinished specs without tasks", async () => { const queue = new DownloadQueue(); - (download as jest.Mock).mockReturnValue(task); + (createDownloadTask as jest.Mock).mockReturnValue(task); await kvfs.write("/mydomain/foo", { id: "foo", @@ -642,9 +637,10 @@ describe("DownloadQueue", () => { await queue.init({ domain: "mydomain" }); expect(task.resume).not.toHaveBeenCalled(); - expect(download).toHaveBeenCalledWith( + expect(createDownloadTask).toHaveBeenCalledWith( expect.objectContaining({ id: "foo" }) ); + expect(task.start).toHaveBeenCalledTimes(1); }); // This can happen in cases of TestFlight / XCode installs, somehow, where @@ -653,7 +649,7 @@ describe("DownloadQueue", () => { it("starts downloads for 'finished' specs without tasks and files", async () => { const queue = new DownloadQueue(); - (download as jest.Mock).mockReturnValue(task); + (createDownloadTask as jest.Mock).mockReturnValue(task); await kvfs.write("/mydomain/foo", { id: "foo", @@ -665,7 +661,7 @@ describe("DownloadQueue", () => { await queue.init({ domain: "mydomain" }); expect(task.resume).not.toHaveBeenCalled(); - expect(download).toHaveBeenCalledWith( + expect(createDownloadTask).toHaveBeenCalledWith( expect.objectContaining({ id: "foo" }) ); }); @@ -673,7 +669,7 @@ describe("DownloadQueue", () => { it("doesn't start downloads for 'finished' specs", async () => { const queue = new DownloadQueue(); - (download as jest.Mock).mockReturnValue(task); + (createDownloadTask as jest.Mock).mockReturnValue(task); (exists as jest.Mock).mockImplementation(() => true); await kvfs.write("/mydomain/foo", { @@ -688,13 +684,13 @@ describe("DownloadQueue", () => { // This test protects against a regression. We used to restart downloads // even for finished specs. expect(task.resume).not.toHaveBeenCalled(); - expect(download).not.toHaveBeenCalled(); + expect(createDownloadTask).not.toHaveBeenCalled(); }); it("doesn't start downloads for lazy-deleted specs", async () => { const queue = new DownloadQueue(); - (download as jest.Mock).mockReturnValue(task); + (createDownloadTask as jest.Mock).mockReturnValue(task); (exists as jest.Mock).mockImplementation(() => true); await kvfs.write("/mydomain/foo", { @@ -709,7 +705,7 @@ describe("DownloadQueue", () => { // This test protects against a regression. We used to restart downloads // even for finished specs. expect(task.resume).not.toHaveBeenCalled(); - expect(download).not.toHaveBeenCalled(); + expect(createDownloadTask).not.toHaveBeenCalled(); }); it("should call onDone for already-finished specs without tasks", async () => { @@ -733,7 +729,7 @@ describe("DownloadQueue", () => { await queue.init({ domain: "mydomain", handlers }); // Should not start a new download for finished specs - expect(download).not.toHaveBeenCalled(); + expect(createDownloadTask).not.toHaveBeenCalled(); // Should call handlers for the finished spec expect(handlers.onBegin).toHaveBeenCalledWith( @@ -769,7 +765,7 @@ describe("DownloadQueue", () => { await queue.init({ domain: "mydomain", handlers }); // Should restart download since file is missing - expect(download).toHaveBeenCalledTimes(1); + expect(createDownloadTask).toHaveBeenCalledTimes(1); // Should not call handlers yet since file is missing expect(handlers.onBegin).not.toHaveBeenCalled(); @@ -801,7 +797,7 @@ describe("DownloadQueue", () => { const queue = new DownloadQueue(); task.state = "DOWNLOADING"; - (checkForExistingDownloads as jest.Mock).mockReturnValue([task]); + (getExistingDownloadTasks as jest.Mock).mockReturnValue([task]); await kvfs.write("/mydomain/foo", { id: "foo", @@ -815,7 +811,7 @@ describe("DownloadQueue", () => { expect(task.resume).not.toHaveBeenCalled(); expect(task.stop).not.toHaveBeenCalled(); - queue.terminate(); + await queue.terminate(); expect(task.stop).toHaveBeenCalled(); }); @@ -826,7 +822,7 @@ describe("DownloadQueue", () => { }; task.state = "DOWNLOADING"; - (checkForExistingDownloads as jest.Mock).mockReturnValue([task]); + (getExistingDownloadTasks as jest.Mock).mockReturnValue([task]); await kvfs.write("/mydomain/foo", { id: "foo", @@ -837,7 +833,7 @@ describe("DownloadQueue", () => { await queue.init({ domain: "mydomain", handlers }); expect(handlers.onBegin).toHaveBeenCalledTimes(1); - queue.terminate(); + await queue.terminate(); expect(task.stop).toHaveBeenCalled(); await queue.init({ domain: "mydomain", handlers }); @@ -848,7 +844,7 @@ describe("DownloadQueue", () => { const queue = new DownloadQueue(); await queue.init({ domain: "mydomain" }); - queue.terminate(); + await queue.terminate(); await expectPublicsToFail(queue); @@ -861,7 +857,7 @@ describe("DownloadQueue", () => { it("should add a url while preserving its extension", async () => { const queue = new DownloadQueue(); - (download as jest.Mock).mockImplementation((spec: { id: string }) => + (createDownloadTask as jest.Mock).mockImplementation((spec: { id: string }) => Object.assign(task, { id: spec.id, }) @@ -869,18 +865,19 @@ describe("DownloadQueue", () => { await queue.init({ domain: "mydomain", urlToPath }); await queue.addUrl("http://foo.com/a.mp3"); - expect(download).toHaveBeenCalledWith( + expect(createDownloadTask).toHaveBeenCalledWith( expect.objectContaining({ url: "http://foo.com/a.mp3", destination: `${RNFS.DocumentDirectoryPath}/DownloadQueue/mydomain/${task.id}.mp3`, }) ); + expect(task.start).toHaveBeenCalledTimes(1); }); it("should add a url without an extension", async () => { const queue = new DownloadQueue(); - (download as jest.Mock).mockImplementation((spec: { id: string }) => + (createDownloadTask as jest.Mock).mockImplementation((spec: { id: string }) => Object.assign(task, { id: spec.id, }) @@ -888,47 +885,47 @@ describe("DownloadQueue", () => { await queue.init({ domain: "mydomain" }); await queue.addUrl("http://foo.com/a"); - expect(download).toHaveBeenCalledWith( + expect(createDownloadTask).toHaveBeenCalledWith( expect.objectContaining({ url: "http://foo.com/a", destination: `${RNFS.DocumentDirectoryPath}/DownloadQueue/mydomain/${task.id}`, }) ); - queue.terminate(); + await queue.terminate(); await queue.init({ domain: "mydomain", urlToPath }); - (download as jest.Mock).mockClear(); // Clear out revival of prev url + (createDownloadTask as jest.Mock).mockClear(); // Clear out revival of prev url await queue.addUrl("http://foo.com/a.mp3"); - expect(download).toHaveBeenCalledWith( + expect(createDownloadTask).toHaveBeenCalledWith( expect.objectContaining({ url: "http://foo.com/a.mp3", destination: `${RNFS.DocumentDirectoryPath}/DownloadQueue/mydomain/${task.id}.mp3`, }) ); - (download as jest.Mock).mockClear(); + (createDownloadTask as jest.Mock).mockClear(); // This tests whether paths/extensions are dealt with properly when a url // ends in a slash. await queue.addUrl("http://foo.com/abc/"); - expect(download).toHaveBeenCalledWith( + expect(createDownloadTask).toHaveBeenCalledWith( expect.objectContaining({ url: "http://foo.com/abc/", destination: `${RNFS.DocumentDirectoryPath}/DownloadQueue/mydomain/${task.id}`, }) ); - (download as jest.Mock).mockClear(); + (createDownloadTask as jest.Mock).mockClear(); // Tests whether paths/extensions with terminating periods are handled await queue.addUrl("http://foo.com/a/bc."); - expect(download).toHaveBeenCalledWith( + expect(createDownloadTask).toHaveBeenCalledWith( expect.objectContaining({ url: "http://foo.com/a/bc.", destination: `${RNFS.DocumentDirectoryPath}/DownloadQueue/mydomain/${task.id}`, }) ); - (download as jest.Mock).mockClear(); + (createDownloadTask as jest.Mock).mockClear(); // Tests whether paths with no extensions are handled when urlToPath is // given by caller await queue.addUrl("http://foo.com/a/bc"); - expect(download).toHaveBeenCalledWith( + expect(createDownloadTask).toHaveBeenCalledWith( expect.objectContaining({ url: "http://foo.com/a/bc", destination: `${RNFS.DocumentDirectoryPath}/DownloadQueue/mydomain/${task.id}`, @@ -936,11 +933,11 @@ describe("DownloadQueue", () => { ); // Now make sure termination clears out the urlPath callback - queue.terminate(); + await queue.terminate(); await queue.init({ domain: "mydomain" }); - (download as jest.Mock).mockClear(); // Clear out revival of prev url + (createDownloadTask as jest.Mock).mockClear(); // Clear out revival of prev url await queue.addUrl("http://foo.com/b.mp3"); - expect(download).toHaveBeenCalledWith( + expect(createDownloadTask).toHaveBeenCalledWith( expect.objectContaining({ url: "http://foo.com/b.mp3", destination: `${RNFS.DocumentDirectoryPath}/DownloadQueue/mydomain/${task.id}`, @@ -951,36 +948,31 @@ describe("DownloadQueue", () => { it("shouldn't add the same url twice", async () => { const queue = new DownloadQueue(); - (download as jest.Mock).mockReturnValue(task); + (createDownloadTask as jest.Mock).mockReturnValue(task); await queue.init({ domain: "mydomain" }); await queue.addUrl("http://foo.com/a.mp3"); await queue.addUrl("http://foo.com/a.mp3"); - expect(download).toHaveBeenCalledTimes(1); + expect(createDownloadTask).toHaveBeenCalledTimes(1); + expect(task.start).toHaveBeenCalledTimes(1); }); it("should revive added url upon relaunch", async () => { const queue = new DownloadQueue(); let assignedId = "tbd"; - (download as jest.Mock).mockImplementation((spec: { id: string }) => { + (createDownloadTask as jest.Mock).mockImplementation((spec: { id: string }) => { assignedId = spec.id; return task; }); - (ensureDownloadsAreRunning as jest.Mock).mockImplementation(() => { - // This is exactly what the actual implementation does, to work around - // some bug in the library. - task.pause(); - task.resume(); - }); await queue.init({ domain: "mydomain" }); await queue.addUrl("http://foo.com/a.mp3"); - expect(download).toHaveBeenCalledTimes(1); + expect(createDownloadTask).toHaveBeenCalledTimes(1); expect(task.resume).not.toHaveBeenCalled(); - (checkForExistingDownloads as jest.Mock).mockReturnValue([ + (getExistingDownloadTasks as jest.Mock).mockReturnValue([ Object.assign(task, { id: assignedId, }), @@ -992,62 +984,51 @@ describe("DownloadQueue", () => { const relaunchQueue = new DownloadQueue(); await relaunchQueue.init({ domain: "mydomain" }); - expect(task.resume).toHaveBeenCalledTimes(1); + // Revival should reattach to the existing task rather than starting a + // new download, and shouldn't touch pause/resume for an already-paused + // task. + expect(createDownloadTask).toHaveBeenCalledTimes(1); + expect(task.resume).not.toHaveBeenCalled(); }); it("should pause revived url when relaunched inactive", async () => { const queue = new DownloadQueue(); let assignedId = "tbd"; - let progresser: ProgressHandler; - (download as jest.Mock).mockImplementation((spec: { id: string }) => { + (createDownloadTask as jest.Mock).mockImplementation((spec: { id: string }) => { assignedId = spec.id; return task; }); - (ensureDownloadsAreRunning as jest.Mock).mockImplementation(() => { - // This is exactly what the actual implementation does, to work around - // some bug in the library. - task.pause(); - task.resume(); - }); await queue.init({ domain: "mydomain" }); await queue.addUrl("http://foo.com/a.mp3"); - expect(download).toHaveBeenCalledTimes(1); + expect(createDownloadTask).toHaveBeenCalledTimes(1); expect(task.resume).not.toHaveBeenCalled(); - (checkForExistingDownloads as jest.Mock).mockReturnValue([ + (getExistingDownloadTasks as jest.Mock).mockReturnValue([ Object.assign(task, { id: assignedId, - progress: (handler: ProgressHandler) => { - progresser = handler; - return task; - }, }), ]); - task.state = "PAUSED"; + // Only a DOWNLOADING revived task needs to be explicitly paused when + // relaunched inactive; a PAUSED task is already paused. + task.state = "DOWNLOADING"; // Pretend app got launched again by using another queue const relaunchQueue = new DownloadQueue(); await relaunchQueue.init({ domain: "mydomain", startActive: false }); - expect(task.resume).toHaveBeenCalledTimes(1); - - // We now call the progress() handler because we inserted a workaround for - // https://github.com/kesha-antonov/react-native-background-downloader/issues/23 - // Revived tasks get progress() without begin(), so we also need to pause - // those cases. - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - progresser!({ bytesDownloaded: 42, bytesTotal: 420 }); + expect(task.pause).toHaveBeenCalledTimes(1); + expect(task.resume).not.toHaveBeenCalled(); }); it("shouldn't re-download revived spec if file already downloaded", async () => { const queue = new DownloadQueue(); let doner: DoneHandler | undefined; - (download as jest.Mock).mockImplementation( + (createDownloadTask as jest.Mock).mockImplementation( (spec: { id: string }): TaskWithHandlers => { return Object.assign(task, { id: spec.id, @@ -1061,7 +1042,7 @@ describe("DownloadQueue", () => { await queue.init({ domain: "mydomain" }); await queue.addUrl("http://foo.com/a.mp3"); - expect(download).toHaveBeenCalledTimes(1); + expect(createDownloadTask).toHaveBeenCalledTimes(1); expect(task.resume).not.toHaveBeenCalled(); (exists as jest.Mock).mockImplementation(() => true); @@ -1070,35 +1051,35 @@ describe("DownloadQueue", () => { await queue.addUrl("http://foo.com/a.mp3"); expect(exists).toHaveBeenCalledTimes(1); - expect(download).toHaveBeenCalledTimes(2); // Because it hadn't finished + expect(createDownloadTask).toHaveBeenCalledTimes(2); // Because it hadn't finished // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - doner!({ bytesDownloaded: 8675309, bytesTotal: 8675309 }); + doner!({ location: "whatever", bytesDownloaded: 8675309, bytesTotal: 8675309 }); await queue.removeUrl("http://foo.com/a.mp3", 0); await queue.addUrl("http://foo.com/a.mp3"); // Now that it's been downloaded, removing it for next launch and // re-adding it shouldn't retrigger a download. - expect(download).toHaveBeenCalledTimes(2); + expect(createDownloadTask).toHaveBeenCalledTimes(2); }); it("shouldn't re-download revived spec on relaunch if file already downloaded", async () => { const queue = new DownloadQueue(); let assignedId = "tbd"; - (download as jest.Mock).mockImplementation((spec: { id: string }) => { + (createDownloadTask as jest.Mock).mockImplementation((spec: { id: string }) => { assignedId = spec.id; return task; }); await queue.init({ domain: "mydomain" }); await queue.addUrl("http://foo.com/a.mp3"); - expect(download).toHaveBeenCalledTimes(1); + expect(createDownloadTask).toHaveBeenCalledTimes(1); expect(task.resume).not.toHaveBeenCalled(); await queue.removeUrl("http://foo.com/a.mp3"); - (checkForExistingDownloads as jest.Mock).mockReturnValue([ + (getExistingDownloadTasks as jest.Mock).mockReturnValue([ { ...task, id: assignedId, @@ -1110,7 +1091,7 @@ describe("DownloadQueue", () => { const relaunchQueue = new DownloadQueue(); await relaunchQueue.init({ domain: "mydomain" }); - expect(download).toHaveBeenCalledTimes(1); // Just the first time only + expect(createDownloadTask).toHaveBeenCalledTimes(1); // Just the first time only }); }); @@ -1119,7 +1100,7 @@ describe("DownloadQueue", () => { const queue = new DownloadQueue(); let assignedId = "tbd"; - (download as jest.Mock).mockImplementation((spec: { id: string }) => { + (createDownloadTask as jest.Mock).mockImplementation((spec: { id: string }) => { assignedId = spec.id; return { ...task, @@ -1147,7 +1128,7 @@ describe("DownloadQueue", () => { const queue = new DownloadQueue(); let assignedId = "tbd"; - (download as jest.Mock).mockImplementation((spec: { id: string }) => { + (createDownloadTask as jest.Mock).mockImplementation((spec: { id: string }) => { assignedId = spec.id; return { ...task, @@ -1180,7 +1161,7 @@ describe("DownloadQueue", () => { const queue = new DownloadQueue(); const idMap: { [key: string]: string } = {}; - (download as jest.Mock).mockImplementation( + (createDownloadTask as jest.Mock).mockImplementation( (spec: { id: string; url: string }) => { idMap[spec.url] = spec.id; return { @@ -1206,10 +1187,10 @@ describe("DownloadQueue", () => { "DownloadQueue/mydomain/" + idMap["http://boo.com/a.mp3"], ]); expect(AsyncStorage.multiRemove).toHaveBeenCalledTimes(1); - expect(download).toHaveBeenCalledWith( + expect(createDownloadTask).toHaveBeenCalledWith( expect.objectContaining({ url: "http://shoo.com/a.mp3" }) ); - expect(download).toHaveBeenCalledTimes(4); + expect(createDownloadTask).toHaveBeenCalledTimes(4); }); }); @@ -1217,7 +1198,7 @@ describe("DownloadQueue", () => { it("should return status on a single file", async () => { const queue = new DownloadQueue(); - (download as jest.Mock).mockImplementation( + (createDownloadTask as jest.Mock).mockImplementation( (spec: { id: string; url: string }): Partial => spec.url === "http://boo.com/a.mp3" ? Object.assign(task, { @@ -1245,7 +1226,7 @@ describe("DownloadQueue", () => { ); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - task._done!({ bytesDownloaded: 8675309, bytesTotal: 8675309 }); + task._done!({ location: "whatever", bytesDownloaded: 8675309, bytesTotal: 8675309 }); const resBoo = await queue.getStatus("http://boo.com/a.mp3"); expect(resBoo).toEqual( expect.objectContaining({ url: "http://boo.com/a.mp3", complete: true }) @@ -1321,7 +1302,7 @@ describe("DownloadQueue", () => { const queue = new DownloadQueue(); const idMap: { [url: string]: TaskWithHandlers } = {}; - (download as jest.Mock).mockImplementation( + (createDownloadTask as jest.Mock).mockImplementation( (spec: { id: string; url: string }): Partial => { const baseTask = mock(); @@ -1350,11 +1331,13 @@ describe("DownloadQueue", () => { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion idMap["http://foo.com/a.mp3"]._done!({ + location: "whatever", bytesDownloaded: 8675309, bytesTotal: 8675309, }); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion idMap["http://boo.com/a.mp3"]._done!({ + location: "whatever", bytesDownloaded: 8675309, bytesTotal: 8675309, }); @@ -1383,7 +1366,7 @@ describe("DownloadQueue", () => { const queue = new DownloadQueue(); const idMap: { [url: string]: TaskWithHandlers } = {}; - (download as jest.Mock).mockImplementation( + (createDownloadTask as jest.Mock).mockImplementation( (spec: { id: string; url: string }): Partial => { const baseTask = mock(); @@ -1411,11 +1394,13 @@ describe("DownloadQueue", () => { ]); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion idMap["http://foo.com/a.mp3"]._done!({ + location: "whatever", bytesDownloaded: 8675309, bytesTotal: 8675309, }); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion idMap["http://boo.com/a.mp3"]._done!({ + location: "whatever", bytesDownloaded: 8675309, bytesTotal: 8675309, }); @@ -1454,7 +1439,7 @@ describe("DownloadQueue", () => { const queue = new DownloadQueue(); let assignedId = "tbd"; - (download as jest.Mock).mockImplementation( + (createDownloadTask as jest.Mock).mockImplementation( (spec: { id: string; url: string }) => { if (spec.url === "http://foo.com/a.mp3") { assignedId = spec.id; @@ -1492,7 +1477,7 @@ describe("DownloadQueue", () => { "http://moo.com/a.mp3": "moo", }; - (download as jest.Mock).mockImplementation( + (createDownloadTask as jest.Mock).mockImplementation( (spec: { id: string; url: string }) => { urlsToIds[spec.url] = spec.id; return { @@ -1550,7 +1535,7 @@ describe("DownloadQueue", () => { await queue.init({ domain: "mydomain" }); await queue.addUrl("http://foo.com/a.mp3"); - expect(download).toHaveBeenCalledTimes(1); + expect(createDownloadTask).toHaveBeenCalledTimes(1); await queue.removeUrl("http://foo.com/a.mp3", 0); @@ -1558,7 +1543,7 @@ describe("DownloadQueue", () => { expect(AsyncStorage.multiRemove).toHaveBeenCalledTimes(0); await queue.addUrl("http://foo.com/a.mp3"); - expect(download).toHaveBeenCalledTimes(2); + expect(createDownloadTask).toHaveBeenCalledTimes(2); }); it("should send notifications for revived lazy-deletions", async () => { @@ -1590,7 +1575,7 @@ describe("DownloadQueue", () => { expect(handlers.onBegin).toHaveBeenCalledTimes(1); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/await-thenable - await task._done!({ bytesDownloaded: 8675309, bytesTotal: 8675309 }); + await task._done!({ location: "whatever", bytesDownloaded: 8675309, bytesTotal: 8675309 }); expect(handlers.onDone).toHaveBeenCalledTimes(1); await queue.removeUrl("http://foo.com/a.mp3", 0); @@ -1609,7 +1594,7 @@ describe("DownloadQueue", () => { let beginner: BeginHandler; task.state = "DOWNLOADING"; - (checkForExistingDownloads as jest.Mock).mockReturnValue([task]); + (getExistingDownloadTasks as jest.Mock).mockReturnValue([task]); await kvfs.write("/mydomain/foo", { id: "foo", @@ -1619,13 +1604,12 @@ describe("DownloadQueue", () => { }); await queue.init({ domain: "mydomain", startActive: false }); expect(task.resume).not.toHaveBeenCalled(); - // We expect pause TWO times because, according to downloader docs, we - // should explicitly pause before reattaching handlers; we then pause - // again because we specified !startActive. - expect(task.pause).toHaveBeenCalledTimes(2); - expect(download).not.toHaveBeenCalled(); + // The revived task is DOWNLOADING while the queue is inactive, so it + // gets explicitly paused once upon revival. + expect(task.pause).toHaveBeenCalledTimes(1); + expect(createDownloadTask).not.toHaveBeenCalled(); - (download as jest.Mock).mockImplementation( + (createDownloadTask as jest.Mock).mockImplementation( (spec: { id: string }): TaskWithHandlers => { return Object.assign(task, { id: spec.id, @@ -1644,7 +1628,7 @@ describe("DownloadQueue", () => { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion beginner!({ expectedBytes: 42, headers: {} }); - expect(task.pause).toHaveBeenCalledTimes(3); // for the added download + expect(task.pause).toHaveBeenCalledTimes(2); // for the added download }); it("should start paused if requested on paused background task", async () => { @@ -1652,7 +1636,7 @@ describe("DownloadQueue", () => { let beginner: BeginHandler; task.state = "PAUSED"; - (checkForExistingDownloads as jest.Mock).mockReturnValue([task]); + (getExistingDownloadTasks as jest.Mock).mockReturnValue([task]); await kvfs.write("/mydomain/foo", { id: "foo", @@ -1662,10 +1646,12 @@ describe("DownloadQueue", () => { }); await queue.init({ domain: "mydomain", startActive: false }); expect(task.resume).not.toHaveBeenCalled(); - expect(task.pause).toHaveBeenCalledTimes(1); - expect(download).not.toHaveBeenCalled(); + // A revived PAUSED task is already paused, so revival shouldn't pause + // it again. + expect(task.pause).not.toHaveBeenCalled(); + expect(createDownloadTask).not.toHaveBeenCalled(); - (download as jest.Mock).mockImplementation( + (createDownloadTask as jest.Mock).mockImplementation( (spec: { id: string }): TaskWithHandlers => { return Object.assign(task, { id: spec.id, @@ -1684,7 +1670,36 @@ describe("DownloadQueue", () => { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion beginner!({ expectedBytes: 42, headers: {} }); - expect(task.pause).toHaveBeenCalledTimes(2); // for the added download + expect(task.pause).toHaveBeenCalledTimes(1); // for the added download + }); + + it("should pause on a progress callback when inactive and no begin() fired", async () => { + const queue = new DownloadQueue(); + let progresser: ProgressHandler; + + (createDownloadTask as jest.Mock).mockImplementation( + (spec: { id: string }): TaskWithHandlers => { + return Object.assign(task, { + id: spec.id, + progress: (handler: ProgressHandler) => { + progresser = handler; + return task; + }, + }); + } + ); + + await queue.init({ domain: "mydomain", startActive: false }); + await queue.addUrl("http://foo.com/a.mp3"); + + expect(task.pause).not.toHaveBeenCalled(); + + // Simulate a progress callback arriving without a preceding begin(), + // which can happen when resuming a background task upon launch. + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + progresser!({ bytesDownloaded: 100, bytesTotal: 1000 }); + + expect(task.pause).toHaveBeenCalledTimes(1); }); it("should pause/resume everything when asked", async () => { @@ -1695,19 +1710,19 @@ describe("DownloadQueue", () => { await queue.addUrl("http://boo.com/a.mp3"); await queue.addUrl("http://moo.com/a.mp3"); - expect(download).toHaveBeenCalledTimes(3); + expect(createDownloadTask).toHaveBeenCalledTimes(3); expect(task.resume).not.toHaveBeenCalled(); expect(task.pause).not.toHaveBeenCalled(); - queue.pauseAll(); + await queue.pauseAll(); - expect(download).toHaveBeenCalledTimes(3); // no change expected here + expect(createDownloadTask).toHaveBeenCalledTimes(3); // no change expected here expect(task.pause).toHaveBeenCalledTimes(3); expect(task.resume).not.toHaveBeenCalled(); - queue.resumeAll(); + await queue.resumeAll(); - expect(download).toHaveBeenCalledTimes(3); // no change expected here + expect(createDownloadTask).toHaveBeenCalledTimes(3); // no change expected here expect(task.pause).toHaveBeenCalledTimes(3); // no change here either expect(task.resume).toHaveBeenCalledTimes(3); }); @@ -1789,7 +1804,7 @@ describe("DownloadQueue", () => { }); await queue.addUrl("http://foo.com/a.mp3"); - queue.pauseAll(); + await queue.pauseAll(); expect(task.pause).toHaveBeenCalledTimes(1); netInfoHandler(createNetState(false)); @@ -1800,7 +1815,7 @@ describe("DownloadQueue", () => { expect(task.resume).not.toHaveBeenCalled(); expect(task.pause).toHaveBeenCalledTimes(1); - queue.resumeAll(); + await queue.resumeAll(); expect(task.resume).toHaveBeenCalledTimes(1); expect(task.pause).toHaveBeenCalledTimes(1); }); @@ -1818,14 +1833,14 @@ describe("DownloadQueue", () => { expect(task.resume).not.toHaveBeenCalled(); expect(task.pause).toHaveBeenCalledTimes(1); - queue.pauseAll(); + await queue.pauseAll(); expect(task.pause).toHaveBeenCalledTimes(2); netInfoHandler(createNetState(true)); expect(task.resume).not.toHaveBeenCalled(); expect(task.pause).toHaveBeenCalledTimes(2); - queue.resumeAll(); + await queue.resumeAll(); expect(task.resume).toHaveBeenCalledTimes(1); expect(task.pause).toHaveBeenCalledTimes(2); }); @@ -1841,7 +1856,7 @@ describe("DownloadQueue", () => { startActive: false, }); - (download as jest.Mock).mockImplementation( + (createDownloadTask as jest.Mock).mockImplementation( (spec: { id: string }): TaskWithHandlers => { return Object.assign(task, { id: spec.id, @@ -1865,7 +1880,7 @@ describe("DownloadQueue", () => { expect(task.resume).not.toHaveBeenCalled(); expect(task.pause).toHaveBeenCalledTimes(1); // no change! - queue.resumeAll(); + await queue.resumeAll(); expect(task.resume).toHaveBeenCalledTimes(1); expect(task.pause).toHaveBeenCalledTimes(1); // no change! }); @@ -1888,7 +1903,7 @@ describe("DownloadQueue", () => { expect(customAdder).toHaveBeenCalledTimes(1); expect(unsubscriber).not.toHaveBeenCalled(); - queue.terminate(); + await queue.terminate(); expect(unsubscriber).toHaveBeenCalledTimes(1); }); @@ -1956,11 +1971,11 @@ describe("DownloadQueue", () => { expect(task.pause).toHaveBeenCalledTimes(1); expect(task.resume).not.toHaveBeenCalled(); - queue.pauseAll(); + await queue.pauseAll(); expect(task.pause).toHaveBeenCalledTimes(2); expect(task.resume).not.toHaveBeenCalled(); - queue.resumeAll(); + await queue.resumeAll(); expect(task.pause).toHaveBeenCalledTimes(2); expect(task.resume).not.toHaveBeenCalled(); @@ -2031,7 +2046,7 @@ describe("DownloadQueue", () => { it("should retry errored downloads", async () => { const queue = new DownloadQueue(); - (download as jest.Mock).mockImplementation((spec: { id: string }) => + (createDownloadTask as jest.Mock).mockImplementation((spec: { id: string }) => Object.assign(task, { id: spec.id, done: jest.fn((handler: DoneHandler) => { @@ -2047,7 +2062,7 @@ describe("DownloadQueue", () => { await queue.init({ domain: "mydomain" }); await queue.addUrl("http://foo.com/a.mp3"); - expect(download).toHaveBeenCalledTimes(1); + expect(createDownloadTask).toHaveBeenCalledTimes(1); expect(jest.getTimerCount()).toBe(0); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion @@ -2055,14 +2070,14 @@ describe("DownloadQueue", () => { expect(jest.getTimerCount()).toBe(1); // The interval should be set await advanceThroughNextTimersAndPromises(); - expect(download).toHaveBeenCalledTimes(2); // Should have tried again + expect(createDownloadTask).toHaveBeenCalledTimes(2); // Should have tried again await advanceThroughNextTimersAndPromises(); // Previous task should still be active, so no more download() calls - expect(download).toHaveBeenCalledTimes(2); + expect(createDownloadTask).toHaveBeenCalledTimes(2); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/await-thenable - await task._done!({ bytesDownloaded: 8675309, bytesTotal: 8675309 }); + await task._done!({ location: "whatever", bytesDownloaded: 8675309, bytesTotal: 8675309 }); // The interval should be cleared on successful downloads expect(jest.getTimerCount()).toBe(0); }); @@ -2072,7 +2087,7 @@ describe("DownloadQueue", () => { const doneMap: { [id: string]: DoneHandler } = {}; const errMap: { [id: string]: ErrorHandler } = {}; - (download as jest.Mock).mockImplementation((spec: { id: string }) => { + (createDownloadTask as jest.Mock).mockImplementation((spec: { id: string }) => { // You need local copies to maintain different ids per object const localTask = createBasicTask(); return Object.assign(localTask, { @@ -2112,12 +2127,14 @@ describe("DownloadQueue", () => { // Now pretend to finish one successfully. // eslint-disable-next-line @typescript-eslint/no-non-null-assertion Object.values(doneMap)[0]!({ + location: "whatever", bytesDownloaded: 8675309, bytesTotal: 8675309, }); expect(jest.getTimerCount()).toBe(1); // Still need an interval the other. // eslint-disable-next-line @typescript-eslint/no-non-null-assertion Object.values(doneMap)[1]!({ + location: "whatever", bytesDownloaded: 8675309, bytesTotal: 8675309, }); @@ -2127,7 +2144,7 @@ describe("DownloadQueue", () => { it("should not be retrying while paused", async () => { const queue = new DownloadQueue(); - (download as jest.Mock).mockImplementation((spec: { id: string }) => + (createDownloadTask as jest.Mock).mockImplementation((spec: { id: string }) => Object.assign(task, { id: spec.id, error: (handler: ErrorHandler) => { @@ -2143,19 +2160,19 @@ describe("DownloadQueue", () => { task._error!({ error: "something went wrong", errorCode: 500 }); expect(jest.getTimerCount()).toBe(1); // The interval should be set - queue.pauseAll(); + await queue.pauseAll(); expect(jest.getTimerCount()).toBe(0); - queue.resumeAll(); + await queue.resumeAll(); expect(jest.getTimerCount()).toBe(1); - queue.terminate(); // Don't leave timers floating after this test + await queue.terminate(); // Don't leave timers floating after this test }); it("should cancel retries when terminated", async () => { const queue = new DownloadQueue(); - (download as jest.Mock).mockImplementation((spec: { id: string }) => + (createDownloadTask as jest.Mock).mockImplementation((spec: { id: string }) => Object.assign(task, { id: spec.id, error: (handler: ErrorHandler) => { @@ -2171,7 +2188,7 @@ describe("DownloadQueue", () => { task._error!({ error: "something went wrong", errorCode: 500 }); expect(jest.getTimerCount()).toBe(1); // The interval should be set - queue.terminate(); + await queue.terminate(); expect(jest.getTimerCount()).toBe(0); // The interval should be set }); }); @@ -2189,7 +2206,7 @@ describe("DownloadQueue", () => { it("should handle a case where spec is finished but file is missing", async () => { const queue = new DownloadQueue(); - (download as jest.Mock).mockImplementation(() => + (createDownloadTask as jest.Mock).mockImplementation(() => Object.assign(task, { done: jest.fn((handler: DoneHandler) => { task._done = handler; @@ -2203,7 +2220,7 @@ describe("DownloadQueue", () => { // Mark it finished... but RNFS will say the file's not there. // eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/await-thenable - await task._done!({ bytesDownloaded: 8675309, bytesTotal: 8675309 }); + await task._done!({ location: "whatever", bytesDownloaded: 8675309, bytesTotal: 8675309 }); const url = await queue.getAvailableUrl("http://foo.com/a.mp3"); expect(url).toBe("http://foo.com/a.mp3"); @@ -2213,7 +2230,7 @@ describe("DownloadQueue", () => { Platform.OS = "android"; const queue = new DownloadQueue(); - (download as jest.Mock).mockImplementation(() => + (createDownloadTask as jest.Mock).mockImplementation(() => Object.assign(task, { done: jest.fn((handler: DoneHandler) => { task._done = handler; @@ -2227,7 +2244,7 @@ describe("DownloadQueue", () => { await queue.addUrl("http://foo.com/a.mp3"); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/await-thenable - await task._done!({ bytesDownloaded: 8675309, bytesTotal: 8675309 }); + await task._done!({ location: "whatever", bytesDownloaded: 8675309, bytesTotal: 8675309 }); const statuses = await queue.getQueueStatus(); expect(statuses).toEqual( @@ -2246,7 +2263,7 @@ describe("DownloadQueue", () => { const fooTask = createBasicTask(); let fooPath = "tbd"; - (download as jest.Mock).mockImplementation( + (createDownloadTask as jest.Mock).mockImplementation( (spec: { id: string; url: string; destination: string }) => { if (spec.url === "http://foo.com/a.mp3") { fooPath = spec.destination; @@ -2277,7 +2294,7 @@ describe("DownloadQueue", () => { expect(unfinishedUrl).toBe("http://foo.com/a.mp3"); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/await-thenable - await fooTask._done!({ bytesDownloaded: 8675309, bytesTotal: 8675309 }); + await fooTask._done!({ location: "whatever", bytesDownloaded: 8675309, bytesTotal: 8675309 }); const [fooU, booU] = await Promise.all([ queue.getAvailableUrl("http://foo.com/a.mp3"), @@ -2312,7 +2329,7 @@ describe("DownloadQueue", () => { const fooTask = createBasicTask(); let fooPath = "tbd"; - (download as jest.Mock).mockImplementation( + (createDownloadTask as jest.Mock).mockImplementation( (spec: { id: string; url: string; destination: string }) => { if (spec.url === "http://foo.com/a.mp3") { fooPath = spec.destination; @@ -2339,7 +2356,7 @@ describe("DownloadQueue", () => { (exists as jest.Mock).mockImplementation(path => path === fooPath); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/await-thenable - await fooTask._done!({ bytesDownloaded: 8675309, bytesTotal: 8675309 }); + await fooTask._done!({ location: "whatever", bytesDownloaded: 8675309, bytesTotal: 8675309 }); await queue.removeUrl("http://foo.com/a.mp3", Date.now() + 50000); const [fooU, booU] = await Promise.all([ @@ -2408,7 +2425,7 @@ describe("DownloadQueue", () => { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion task._progress!({ bytesDownloaded: 500, bytesTotal: 1000 }); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/await-thenable - await task._done!({ bytesDownloaded: 8675309, bytesTotal: 8675309 }); + await task._done!({ location: "whatever", bytesDownloaded: 8675309, bytesTotal: 8675309 }); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion task._error!({ error: "foo", errorCode: 500 }); @@ -2426,7 +2443,7 @@ describe("DownloadQueue", () => { let doner: DoneHandler | undefined; - (download as jest.Mock).mockImplementation( + (createDownloadTask as jest.Mock).mockImplementation( (spec: { id: string }): TaskWithHandlers => { return Object.assign(task, { id: spec.id, @@ -2441,7 +2458,7 @@ describe("DownloadQueue", () => { await queue.addUrl("http://foo.com/a.mp3"); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/await-thenable - await doner!({ bytesDownloaded: 8675309, bytesTotal: 8675309 }); + await doner!({ location: "whatever", bytesDownloaded: 8675309, bytesTotal: 8675309 }); expect(handlers.onDone).toHaveBeenCalledWith( "http://foo.com/a.mp3", @@ -2455,7 +2472,7 @@ describe("DownloadQueue", () => { onDone: jest.fn(), }; - (download as jest.Mock).mockImplementation( + (createDownloadTask as jest.Mock).mockImplementation( (spec: { id: string }): TaskWithHandlers => { return Object.assign(task, { id: spec.id, @@ -2471,7 +2488,7 @@ describe("DownloadQueue", () => { await queue.addUrl("http:invalid.url"); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/await-thenable - await task._done!({ bytesDownloaded: 8675309, bytesTotal: 8675309 }); + await task._done!({ location: "whatever", bytesDownloaded: 8675309, bytesTotal: 8675309 }); expect(handlers.onDone).toHaveBeenCalledWith( "http:invalid.url", @@ -2503,7 +2520,7 @@ describe("DownloadQueue", () => { // careful not to crash or falsely say it's done. await expect( // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - doner!({ bytesDownloaded: 8675309, bytesTotal: 8675309 }) + doner!({ location: "whatever", bytesDownloaded: 8675309, bytesTotal: 8675309 }) ).resolves.not.toThrow(); expect(handlers.onDone).not.toHaveBeenCalled(); }); @@ -2516,7 +2533,7 @@ describe("DownloadQueue", () => { }; const queue = new DownloadQueue(); - (download as jest.Mock).mockImplementation( + (createDownloadTask as jest.Mock).mockImplementation( (spec: { id: string }): TaskWithHandlers => { return Object.assign(task, { id: spec.id,