fix: accept urlToPath callback for people without URL.pathname polyfill (#25)

RN's default URL polyfill might not provide `.pathname`, which we use to extract file extnensions.
Instead of requiring everyone to use a polyfill in their app, we instead now take an optional
`urlToPath` callback which is used to extract file extensions. If it's not provided, then files
will be saved without extensions.

BREAKING CHANGE: The default behavior is now changed to not save files with extensions. To make this
change not-breaking would have required that this package accept and use a polyfill for URL, which
feels like it could cause greater problems for people who already use other polyfills.

fix #23

Author: fivecar

README.md

@@ -71,6 +71,7 @@ You can pass any of the following options, or nothing at all:
 |---|---|---|---|
 |handlers|DownloadQueueHandlers|undefined|For any events you'd like to receive notifications about, you'll need to pass a handler here. More details in the next table.|
 |domain|string|"main"|By default, AsyncStorage keys and RNFS filenames are with DownloadQueue/main". If you want to use something other than "main", pass it here. This is commonly used to manage different queues for different users (e.g. you can use userId as the domain).|
+|urlToPath|(url:string) => string|undefined (i.e. files will be saved without extensions)|Callback used to get a pathname from a URL. By default, files are saved without any particular extension. But if you need the server extension to be preserved (e.g. you pass the file to a media player that uses the extension to determine its data format), pass a function here that returns a path given a URL (e.g. for `https://foo.com/baz/moo.mp3?q=song`, returns  `baz/moo.mp3`). The easiest way to implement this if you already have a React Native URL polyfill is: `(url) => new URL(url).pathname`. If you don't have a polyfill, you can use something like  https://www.npmjs.com/package/react-native-url-polyfill|
 |startActive|boolean|true|Whether to start the queue in an active state where downloads will be started. If false, no downloads will begin until you call resumeAll().|
 |netInfoAddEventListener|(listener: (state: {isConnected: boolean \| null, type: string}) => void) => ()=> void|undefined|If you'd like DownloadQueue to pause downloads when the device is offline, pass this. Usually easiest to literally pass `NetInfo.addEventListener`.|
 |activeNetworkTypes| string[] | [] |The NetInfoStateType values for which downloads will be allowed. If you pass undefined or [], downloads will happen on all connection types. A common practice is to pass ["wifi", "ethernet"] if you want to help users avoid cellular data charges. As of @react-native-community/[email protected], valid values are "unknown", "none", "wifi", "cellular", "bluetooth", "ethernet", "wimax", "vpn", "other", "mixed".|

src/index.ts

@@ -120,6 +120,24 @@ export interface DownloadQueueOptions {
    * started. If false, no downloads will begin until you call resumeAll().
    */
   startActive?: boolean;
+  /**
+   * Callback used to get a pathname from a URL. By default, files are saved
+   * without any particular extension. But if you need the server extension to
+   * be preserved (e.g. you pass the file to a media player that uses the
+   * extension to determine its data format), pass a function here that returns
+   * a path given a URL (e.g. for `https://foo.com/baz/moo.mp3?q=song`, returns
+   * `baz/moo.mp3`). The easiest way to implement this, if you already have
+   * a React Native polyfill for URL, is:
+   *
+   * function urlToPath(url) {
+   *  const parsed = new URL(url);
+   *  return parsed.pathname;
+   * }
+   *
+   * If you don't have a polyfill, you can use something like
+   * https://www.npmjs.com/package/react-native-url-polyfill
+   */
+  urlToPath?: (url: string) => string;
 }
 
 /**
@@ -139,6 +157,7 @@ export default class DownloadQueue {
   );
   private handlers?: DownloadQueueHandlers = undefined;
   private active = true;
+  private urlToPath?: (url: string) => string = undefined;
   private erroredIds = new Set<string>();
   private errorTimer: NodeJS.Timeout | null = null;
   private netInfoUnsubscriber?: () => void;
@@ -178,13 +197,15 @@ export default class DownloadQueue {
     netInfoAddEventListener = undefined,
     activeNetworkTypes = [],
     startActive = true,
+    urlToPath = undefined,
   }: DownloadQueueOptions = {}): Promise<void> {
     if (this.inited) {
       throw new Error("DownloadQueue already initialized");
     }
 
     this.domain = domain;
     this.handlers = handlers;
+    this.urlToPath = urlToPath;
 
     // This is safe to call even if it already exists. It'll also create all
     // necessary parent directories.
@@ -279,6 +300,7 @@ export default class DownloadQueue {
     this.tasks = [];
     this.specs = [];
     this.handlers = undefined;
+    this.urlToPath = undefined;
     this.inited = false;
     this.erroredIds.clear();
     if (this.errorTimer) {
@@ -308,7 +330,7 @@ export default class DownloadQueue {
 
         const [fileExists] = await Promise.all([
           RNFS.exists(
-            this.pathFromId(curSpec.id, extensionFromUri(curSpec.url))
+            this.pathFromId(curSpec.id, this.extensionFromUri(curSpec.url))
           ),
           this.kvfs.write(this.keyFromId(curSpec.id), curSpec),
         ]);
@@ -331,7 +353,7 @@ export default class DownloadQueue {
     const spec: Spec = {
       id,
       url,
-      path: this.pathFromId(id, extensionFromUri(url)),
+      path: this.pathFromId(id, this.extensionFromUri(url)),
       createTime: Date.now(),
       finished: false,
     };
@@ -760,6 +782,24 @@ export default class DownloadQueue {
     }
   }
 
+  private extensionFromUri(uri: string) {
+    const path = this.urlToPath?.(uri);
+
+    if (path) {
+      const filename = path.split("/").pop();
+
+      if (filename) {
+        const parts = filename.split(".");
+
+        if (parts.length > 1) {
+          // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+          return parts.pop()!;
+        }
+      }
+    }
+    return "";
+  }
+
   private async getDirFilenames() {
     try {
       return await RNFS.readdir(this.getDomainedBasePath());
@@ -802,17 +842,3 @@ function basePath() {
   return `${RNFS.DocumentDirectoryPath}/DownloadQueue`;
 }
 
-function extensionFromUri(uri: string) {
-  const url = new URL(uri);
-  const filename = url.pathname.split("/").pop();
-
-  if (filename) {
-    const parts = filename.split(".");
-
-    if (parts.length > 1) {
-      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-      return parts.pop()!;
-    }
-  }
-  return "";
-}

test/index.spec.ts

@@ -170,6 +170,10 @@ jest.mock("@react-native-community/netinfo", () => ({
   }),
 }));
 
+function urlToPath(url: string): string {
+  return new URL(url).pathname;
+}
+
 describe("DownloadQueue", () => {
   beforeEach(() => {
     // restore a few commonly-used functions between tests to avoid unexpected
@@ -526,7 +530,7 @@ describe("DownloadQueue", () => {
         })
       );
 
-      await queue.init({ domain: "mydomain" });
+      await queue.init({ domain: "mydomain", urlToPath });
       await queue.addUrl("http://foo.com/a.mp3");
       expect(download).toHaveBeenCalledWith(
         expect.objectContaining({
@@ -553,6 +557,58 @@ describe("DownloadQueue", () => {
           destination: `${RNFS.DocumentDirectoryPath}/DownloadQueue/mydomain/${task.id}`,
         })
       );
+
+      queue.terminate();
+      await queue.init({ domain: "mydomain", urlToPath });
+      (download as jest.Mock).mockClear(); // Clear out revival of prev url
+      await queue.addUrl("http://foo.com/a.mp3");
+      expect(download).toHaveBeenCalledWith(
+        expect.objectContaining({
+          url: "http://foo.com/a.mp3",
+          destination: `${RNFS.DocumentDirectoryPath}/DownloadQueue/mydomain/${task.id}.mp3`,
+        })
+      );
+      (download 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.objectContaining({
+          url: "http://foo.com/abc/",
+          destination: `${RNFS.DocumentDirectoryPath}/DownloadQueue/mydomain/${task.id}`,
+        })
+      );
+      (download 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.objectContaining({
+          url: "http://foo.com/a/bc.",
+          destination: `${RNFS.DocumentDirectoryPath}/DownloadQueue/mydomain/${task.id}`,
+        })
+      );
+      (download 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.objectContaining({
+          url: "http://foo.com/a/bc",
+          destination: `${RNFS.DocumentDirectoryPath}/DownloadQueue/mydomain/${task.id}`,
+        })
+      );
+
+      // Now make sure termination clears out the urlPath callback
+      queue.terminate();
+      await queue.init({ domain: "mydomain" });
+      (download as jest.Mock).mockClear(); // Clear out revival of prev url
+      await queue.addUrl("http://foo.com/b.mp3");
+      expect(download).toHaveBeenCalledWith(
+        expect.objectContaining({
+          url: "http://foo.com/b.mp3",
+          destination: `${RNFS.DocumentDirectoryPath}/DownloadQueue/mydomain/${task.id}`,
+        })
+      );
     });
 
     it("shouldn't add the same url twice", async () => {
@@ -1780,7 +1836,7 @@ describe("DownloadQueue", () => {
           });
         }
       );
-      await queue.init({ domain: "mydomain", handlers });
+      await queue.init({ domain: "mydomain", handlers, urlToPath });
       await queue.addUrl("http://foo.com/a.mp3");
 
       // eslint-disable-next-line @typescript-eslint/no-non-null-assertion