From f217cb6d8547ef7f79606fdfd26c5a5ff9420648 Mon Sep 17 00:00:00 2001 From: Philip Su <39933441+fivecar@users.noreply.github.com> Date: Fri, 13 Dec 2024 11:59:20 -0800 Subject: [PATCH] feat: removes duplicate spec records with same URL --- src/index.ts | 22 +++++++++++++++++++--- test/index.spec.ts | 44 +++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 60 insertions(+), 6 deletions(-) diff --git a/src/index.ts b/src/index.ts index 4eb3f5b..7dd3c13 100644 --- a/src/index.ts +++ b/src/index.ts @@ -233,11 +233,25 @@ export default class DownloadQueue { this.getDirFilenames(), ]); const now = Date.now(); - const loadedSpecs = specData.map(spec => spec.value as Spec); + const seenUrls = new Set(); + const dupeSpecIds = new Set(); + const loadedSpecs = specData.map(data => { + const spec = data.value as Spec; + + // This deduplicates specs that might have been written multiple times, + // which has happened in the past based on client use mistakes. + if (seenUrls.has(spec.url)) { + dupeSpecIds.add(spec.id); + } else { + seenUrls.add(spec.url); + } + return spec; + }); const deletes = loadedSpecs.filter( spec => spec.createTime === 0 || - (spec.createTime < 0 && -spec.createTime <= now) + (spec.createTime < 0 && -spec.createTime <= now) || + dupeSpecIds.has(spec.id) ); const deleteIds = new Set(deletes.map(spec => spec.id)); @@ -482,7 +496,9 @@ export default class DownloadQueue { const liveUrls = new Set( this.specs.filter(spec => spec.createTime > 0).map(spec => spec.url) ); - const urlsToAdd = urls.filter(url => !liveUrls.has(url)); + // Using urlSet instead of urls in the filter automatically deduplicates + // any URLs the caller might have duplicated. + const urlsToAdd = [...urlSet].filter(url => !liveUrls.has(url)); const specsToRemove = this.specs.filter( spec => !urlSet.has(spec.url) && spec.createTime > 0 ); diff --git a/test/index.spec.ts b/test/index.spec.ts index 0ac5714..5098305 100644 --- a/test/index.spec.ts +++ b/test/index.spec.ts @@ -6,14 +6,14 @@ import { DownloadTask, ensureDownloadsAreRunning, ErrorHandler, - ProgressHandler + ProgressHandler, } from "@kesha-antonov/react-native-background-downloader"; import AsyncStorage from "@react-native-async-storage/async-storage"; import { addEventListener, fetch, NetInfoState, - NetInfoStateType + NetInfoStateType, } from "@react-native-community/netinfo"; import { mock } from "jest-mock-extended"; import KVFS from "key-value-file-system"; @@ -309,6 +309,44 @@ describe("DownloadQueue", () => { expect(unlink).toHaveBeenCalledTimes(1); }); + it("removes duplicate specs with the same URLs", async () => { + const queue = new DownloadQueue(); + + (readdir as jest.Mock).mockImplementation(() => [ + "foo.mp3", + "bar.mp3", + "foo.mp3", + ]); + + await kvfs.write("/mydomain/foo", { + id: "foo", + url: "http://foo.com/a.mp3", + path: `${RNFS.DocumentDirectoryPath}/DownloadQueue/mydomain/foo.mp3`, + createTime: Date.now() - 1000, + }); + await kvfs.write("/mydomain/boo", { + id: "boo", + url: "http://foo.com/b.mp3", + path: `${RNFS.DocumentDirectoryPath}/DownloadQueue/mydomain/boo.mp3`, + createTime: Date.now() - 1000, + }); + await kvfs.write("/mydomain/foo2", { + id: "foo2", + url: "http://foo.com/a.mp3", + path: `${RNFS.DocumentDirectoryPath}/DownloadQueue/mydomain/foo.mp3`, + createTime: Date.now() - 800, + }); + expect((await kvfs.readMulti("/mydomain/*")).length).toEqual(3); + + await queue.init({ domain: "mydomain" }); + const specs = await kvfs.readMulti("/mydomain/*"); + + expect(specs.length).toEqual(2); + expect( + specs.some(spec => "id" in spec && spec.id === "foo2") + ).toBeFalsy(); + }); + it("revives still-downloading specs from previous launches", async () => { const queue = new DownloadQueue(); const handlers: DownloadQueueHandlers = { @@ -853,7 +891,7 @@ describe("DownloadQueue", () => { progress: (handler: ProgressHandler) => { progresser = handler; return task; - } + }, }), ]);