diff --git a/src/index.ts b/src/index.ts index b0066f8..b47796b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -271,19 +271,7 @@ export default class DownloadQueue { if (existingTasks.some(task => task.id === spec.id)) { return; } - if (spec.finished) { - // Once in a while we think the spec is finished but the file isn't - // on disk. This can happen when XCode installs a new build, and - // sometimes through TestFlight. - const exists = await RNFS.exists(spec.path); - - if (exists) { - return; - } - - spec.finished = false; // We're not really finished, it seems. - await this.kvfs.write(this.keyFromId(spec.id), spec); - } + await this.reconcileFinishStateWithFile(spec); return this.start(spec); }) ); @@ -613,7 +601,8 @@ export default class DownloadQueue { /** * Gets a remote or local url, preferring to the local path when possible. If - * the local file hasn't yet been downloaded, returns the remote url. + * the local file hasn't yet been downloaded, returns the remote url. Also + * returns the remote url if the record is being lazy-deleted. * @param url The remote URL to check for local availability * @returns A local file path if the URL has already been downloaded, else url */ @@ -622,7 +611,7 @@ export default class DownloadQueue { const spec = this.specs.find(spec => spec.url === url); - if (!spec || !spec.finished) { + if (!spec || !spec.finished || spec.createTime <= 0) { return url; } @@ -812,6 +801,10 @@ export default class DownloadQueue { private async reviveTask(task: DownloadTask) { const spec = this.specs.find(spec => spec.id === task.id); + if (spec) { + await this.reconcileFinishStateWithFile(spec); + } + // Don't revive finished tasks or ones that already have lazy deletes in // progress. if (spec && !spec.finished && spec.createTime > 0) { @@ -827,9 +820,27 @@ export default class DownloadQueue { this.handlers?.onBegin?.(spec.url, task.totalBytes); break; case "DONE": - this.handlers?.onBegin?.(spec.url, task.totalBytes); - this.handlers?.onDone?.(spec.url, spec.path); - shouldAddTask = false; + { + const exists = await RNFS.exists(spec.path); + + if (exists) { + spec.finished = true; + await this.kvfs.write(this.keyFromId(spec.id), spec); + this.handlers?.onBegin?.(spec.url, task.totalBytes); + this.handlers?.onDone?.(spec.url, spec.path); + shouldAddTask = false; + } else { + // If the spec thinks we're not done but the OS does, yet we + // can't find the file on disk, we'll leave shouldAddTask = true so + // that we begin the download again. + // Downloader docs say every task needs to be paused or stopped. + // So we stop here. + task.stop(); + // Since the file is missing from disk, yet the downloader thinks + // it's done, we restart the download. + this.start(spec); + } + } break; case "STOPPED": this.start(spec); @@ -858,7 +869,7 @@ export default class DownloadQueue { task.stop(); } } else { - if (["DOWNLOADING", "PAUSED"].includes(task.state)) { + if (this.isTaskDownloading(task)) { task.stop(); if (spec && !spec.finished) { @@ -876,6 +887,31 @@ export default class DownloadQueue { } } + /** + * Makes sure, if a spec thinks it's finished, that the file which backs it + * actually exists. If that file doesn't exist, we set finished === false. + * @returns true if spec is finished and the file exists + */ + private async reconcileFinishStateWithFile(spec: Spec) { + if (spec.finished) { + // Once in a while we think the spec is finished but the file isn't + // on disk. This can happen when XCode installs a new build, and + // sometimes through TestFlight. + const exists = await RNFS.exists(spec.path); + + if (exists) { + return; + } + + spec.finished = false; // We're not really finished, it seems. + await this.kvfs.write(this.keyFromId(spec.id), spec); + } + } + + private isTaskDownloading(task: DownloadTask) { + return ["DOWNLOADING", "PAUSED"].includes(task.state); + } + private extensionFromUri(uri: string) { const path = this.urlToPath?.(uri); diff --git a/test/index.spec.ts b/test/index.spec.ts index ec4ba8e..2161e78 100644 --- a/test/index.spec.ts +++ b/test/index.spec.ts @@ -383,6 +383,7 @@ describe("DownloadQueue", () => { task.state = "DONE"; task.totalBytes = 8675309; (checkForExistingDownloads as jest.Mock).mockReturnValue([task]); + (exists as jest.Mock).mockReturnValue(true); await kvfs.write("/mydomain/foo", { id: "foo", @@ -405,6 +406,33 @@ describe("DownloadQueue", () => { expect(download).not.toHaveBeenCalled(); }); + it("revives done specs from previous launches with missing files", async () => { + const queue = new DownloadQueue(); + const handlers: DownloadQueueHandlers = { + onBegin: jest.fn(), + onDone: jest.fn(), + }; + + task.state = "DONE"; + task.totalBytes = 8675309; + (checkForExistingDownloads as jest.Mock).mockReturnValue([task]); + (exists as jest.Mock).mockReturnValue(false); + + 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 }); + + // Because it's done downloading, we don't expect resume() + expect(task.resume).not.toHaveBeenCalled(); + expect(handlers.onBegin).not.toHaveBeenCalled(); + expect(handlers.onDone).not.toHaveBeenCalled(); + expect(download).toHaveBeenCalledTimes(1); + }); + it("restarts stopped specs from previous launches", async () => { const queue = new DownloadQueue(); const handlers: DownloadQueueHandlers = { @@ -1942,6 +1970,70 @@ describe("DownloadQueue", () => { ); }); + it("should give you back the remote url when spec is lazy-deleted", async () => { + const queue = new DownloadQueue(); + const fooTask = createBasicTask(); + let fooPath = "tbd"; + + (download as jest.Mock).mockImplementation( + (spec: { id: string; url: string; destination: string }) => { + if (spec.url === "http://foo.com/a.mp3") { + fooPath = spec.destination; + return Object.assign(fooTask, { + done: jest.fn((handler: DoneHandler) => { + fooTask._done = handler; + return fooTask; + }), + }); + } + return { + ...task, + id: spec.id, + path: spec.destination, + }; + } + ); + + await queue.init({ domain: "mydomain" }); + await queue.addUrl("http://foo.com/a.mp3"); + await queue.addUrl("http://boo.com/a.mp3"); + + // Pretend we've downloaded only foo + (exists as jest.Mock).mockImplementation(path => path === fooPath); + + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + await fooTask._done!(); + await queue.removeUrl("http://foo.com/a.mp3", Date.now() + 50000); + + const [fooU, booU] = await Promise.all([ + queue.getAvailableUrl("http://foo.com/a.mp3"), + queue.getAvailableUrl("http://boo.com/a.mp3"), + ]); + + expect(fooU).toBe("http://foo.com/a.mp3"); // Should give us remote URL + expect(booU).toBe("http://boo.com/a.mp3"); + + const restartedQueue = new DownloadQueue(); + + await restartedQueue.init({ domain: "mydomain" }); + const [fooUR, statuses] = await Promise.all([ + restartedQueue.getAvailableUrl("http://foo.com/a.mp3"), + restartedQueue.getQueueStatus(), + ]); + + // We should be sure that the lazy-deleted status is reported as !complete + expect(fooUR).toBe("http://foo.com/a.mp3"); // Should give us remote URL + expect(statuses.length).toBe(1); // only boo should be left + expect(statuses).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + url: "http://boo.com/a.mp3", + complete: false, + }), + ]) + ); + }); + it("should call handlers for all cases", async () => { const handlers: DownloadQueueHandlers = { onBegin: jest.fn(),