Do not emit progress when pause() is called (#9)

* Do not emit progress events when pause() is called * Fix issues * Optimize test
theogravity · Jul 15, 2024 · 7d5fcb9 · 7d5fcb9
1 parent 2252951
commit 7d5fcb9
Show file tree

Hide file tree

Showing 6 changed files with 99 additions and 41 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,7 @@
+# 3.0.1 (2024-07-13)
+
+Do not emit progress events when `pause()` is called.
+
 # 3.0.0 (2024-04-04)
 
 Adds fixes around `DownloadData` population.

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "electron-dl-manager",
-  "version": "3.0.0",
+  "version": "3.0.1",
   "description": "A library for implementing file downloads in Electron with 'save as' dialog and id support.",
   "main": "dist/cjs/index.js",
   "module": "dist/esm/index.js",

diff --git a/src/DownloadInitiator.ts b/src/DownloadInitiator.ts
@@ -69,6 +69,7 @@ export class DownloadInitiator {
    */
   private downloadData: DownloadData;
   private config: Omit<WillOnDownloadParams, "callbacks">;
+  private onUpdateHandler?: (_event: Event, state: "progressing" | "interrupted") => void;
 
   constructor(config: DownloadInitiatorConstructorParams) {
     this.downloadData = new DownloadData();
@@ -165,7 +166,8 @@ export class DownloadInitiator {
         if (this.downloadData.isDownloadCompleted()) {
           await this.callbackDispatcher.onDownloadCompleted(this.downloadData);
         } else {
-          item.on("updated", this.generateItemOnUpdated());
+          this.onUpdateHandler = this.generateItemOnUpdated();
+          item.on("updated", this.onUpdateHandler);
           item.once("done", this.generateItemOnDone());
         }
 
@@ -191,8 +193,26 @@ export class DownloadInitiator {
     const oldPause = item.pause.bind(item);
     item.pause = () => {
       item["_userInitiatedPause"] = true;
+
+      if (this.onUpdateHandler) {
+        // Don't fire progress updates in a paused state
+        item.off("updated", this.onUpdateHandler);
+        this.onUpdateHandler = undefined;
+      }
+
       oldPause();
     };
+
+    const oldResume = item.resume.bind(item);
+
+    item.resume = () => {
+      if (!this.onUpdateHandler) {
+        this.onUpdateHandler = this.generateItemOnUpdated();
+        item.on("updated", this.onUpdateHandler);
+      }
+
+      oldResume();
+    };
   }
 
   /**
@@ -212,7 +232,8 @@ export class DownloadInitiator {
 
     this.augmentDownloadItem(item);
     await this.callbackDispatcher.onDownloadStarted(this.downloadData);
-    item.on("updated", this.generateItemOnUpdated());
+    this.onUpdateHandler = this.generateItemOnUpdated();
+    item.on("updated", this.onUpdateHandler);
     item.once("done", this.generateItemOnDone());
 
     if (!item["_userInitiatedPause"]) {
@@ -299,5 +320,7 @@ export class DownloadInitiator {
     if (this.onCleanup) {
       this.onCleanup(this.downloadData);
     }
+
+    this.onUpdateHandler = undefined;
   }
 }
diff --git a/src/__mocks__/DownloadData.ts b/src/__mocks__/DownloadData.ts
@@ -25,6 +25,8 @@ export function createMockDownloadData() {
     on: itemEmitter.on.bind(itemEmitter) as DownloadItem["on"],
     // @ts-ignore
     once: itemEmitter.once.bind(itemEmitter) as DownloadItem["once"],
+    // @ts-ignore
+    off: itemEmitter.off.bind(itemEmitter) as DownloadItem["off"],
   };
 
   const downloadData: jest.Mocked<ActualDownloadData> = {

diff --git a/test/DownloadInitiator.test.ts b/test/DownloadInitiator.test.ts
@@ -1,5 +1,8 @@
-import { DownloadInitiator } from "../src";
+import { DownloadInitiator, getFilenameFromMime } from "../src";
 import { createMockDownloadData } from "../src/__mocks__/DownloadData";
+import { determineFilePath } from "../src/utils";
+import path from "node:path";
+import UnusedFilename from "unused-filename";
 
 jest.mock("../src/utils");
 jest.mock("../src/CallbackDispatcher");
@@ -13,6 +16,7 @@ describe("DownloadInitiator", () => {
   let mockDownloadData;
   let mockWebContents;
   let mockEvent;
+  let mockEmitter;
 
   beforeEach(() => {
     jest.clearAllMocks();
@@ -26,6 +30,7 @@ describe("DownloadInitiator", () => {
 
     mockItem = mockedItemData.item;
     mockDownloadData = mockedItemData.downloadData;
+    mockEmitter = mockedItemData.itemEmitter;
   });
 
   describe("generateOnWillDownload", () => {
@@ -63,8 +68,8 @@ describe("DownloadInitiator", () => {
       const downloadInitiator = new DownloadInitiator({});
       downloadInitiator.downloadData = mockDownloadData;
 
-      mockItem.getSavePath.mockReturnValue("");
-      mockDownloadData.isDownloadCancelled.mockReturnValue(true);
+      mockItem.getSavePath.mockReturnValueOnce("");
+      mockDownloadData.isDownloadCancelled.mockReturnValueOnce(true);
 
       await downloadInitiator.generateOnWillDownload({
         saveDialogOptions: {},
@@ -83,8 +88,8 @@ describe("DownloadInitiator", () => {
         downloadInitiator.downloadData = mockDownloadData;
 
         mockItem["_userInitiatedPause"] = true;
-        mockItem.getSavePath.mockReturnValue("");
-        mockDownloadData.isDownloadCancelled.mockReturnValue(true);
+        mockItem.getSavePath.mockReturnValueOnce("");
+        mockDownloadData.isDownloadCancelled.mockReturnValueOnce(true);
 
         await downloadInitiator.generateOnWillDownload({
           saveDialogOptions: {},
@@ -96,13 +101,16 @@ describe("DownloadInitiator", () => {
         expect(mockItem.resume).not.toHaveBeenCalled();
       });
 
-      it("should not resume the download if the did not pause before init", async () => {
+      it("should resume the download if the user *did not* pause before init", async () => {
         const downloadInitiator = new DownloadInitiator({});
         downloadInitiator.downloadData = mockDownloadData;
 
+        determineFilePath.mockReturnValueOnce("/some/path");
+
         mockItem["_userInitiatedPause"] = false;
-        mockItem.getSavePath.mockReturnValue("");
-        mockDownloadData.isDownloadCancelled.mockReturnValue(true);
+        mockItem.getSavePath.mockReturnValueOnce("/some/path");
+
+        const resumeSpy = jest.spyOn(mockItem, "resume");
 
         await downloadInitiator.generateOnWillDownload({
           saveDialogOptions: {},
@@ -111,7 +119,7 @@ describe("DownloadInitiator", () => {
 
         await jest.runAllTimersAsync();
 
-        expect(mockItem.resume).toHaveBeenCalled();
+        expect(resumeSpy).toHaveBeenCalled();
       });
     });
 
@@ -120,7 +128,7 @@ describe("DownloadInitiator", () => {
         const downloadInitiator = new DownloadInitiator({});
         downloadInitiator.downloadData = mockDownloadData;
 
-        mockItem.getSavePath.mockReturnValue("/some/path");
+        mockItem.getSavePath.mockReturnValueOnce("/some/path");
 
         await downloadInitiator.generateOnWillDownload({
           saveDialogOptions: {},
@@ -136,9 +144,9 @@ describe("DownloadInitiator", () => {
         const downloadInitiator = new DownloadInitiator({});
         downloadInitiator.downloadData = mockDownloadData;
 
-        mockItem.getSavePath.mockReturnValue("/some/path");
+        mockItem.getSavePath.mockReturnValueOnce("/some/path");
 
-        mockDownloadData.isDownloadCompleted.mockReturnValue(true);
+        mockDownloadData.isDownloadCompleted.mockReturnValueOnce(true);
 
         await downloadInitiator.generateOnWillDownload({
           saveDialogOptions: {},
@@ -157,24 +165,14 @@ describe("DownloadInitiator", () => {
       const downloadInitiator = new DownloadInitiator({});
       downloadInitiator.downloadData = mockDownloadData;
 
-      await downloadInitiator.generateOnWillDownload({
-        saveAsFilename: "test.txt",
-        callbacks,
-      })(mockEvent, mockItem, mockWebContents);
-
-      expect(mockItem.resolvedFilename).toBe("test.txt");
-      expect(downloadInitiator.callbackDispatcher.onDownloadStarted).toHaveBeenCalled();
-    });
-
-    it("should not require saveAsFilename", async () => {
-      const downloadInitiator = new DownloadInitiator({});
-      downloadInitiator.downloadData = mockDownloadData;
+      determineFilePath.mockReturnValueOnce("/some/path/test.txt");
 
       await downloadInitiator.generateOnWillDownload({
+        saveAsFilename: "test.txt",
         callbacks,
       })(mockEvent, mockItem, mockWebContents);
 
-      expect(mockItem.resolvedFilename).toBe("example.txt");
+      expect(downloadInitiator.getDownloadData().resolvedFilename).toBe("test.txt");
       expect(downloadInitiator.callbackDispatcher.onDownloadStarted).toHaveBeenCalled();
     });
 
@@ -184,27 +182,36 @@ describe("DownloadInitiator", () => {
         downloadInitiator.downloadData = mockDownloadData;
         mockItem["_userInitiatedPause"] = true;
 
+        determineFilePath.mockReturnValueOnce("/some/path/test.txt");
+
         await downloadInitiator.generateOnWillDownload({
           callbacks,
         })(mockEvent, mockItem, mockWebContents);
 
+        const resumeSpy = jest.spyOn(mockItem, "resume");
+
         await jest.runAllTimersAsync();
 
-        expect(mockItem.resume).not.toHaveBeenCalled();
+        expect(resumeSpy).not.toHaveBeenCalled();
       });
 
-      it("should not resume the download if the did not pause before init", async () => {
+      it("should resume the download if the *did not* pause before init", async () => {
         const downloadInitiator = new DownloadInitiator({});
         downloadInitiator.downloadData = mockDownloadData;
         mockItem["_userInitiatedPause"] = true;
 
+        determineFilePath.mockReturnValueOnce("/some/path/test.txt");
+        const resumeSpy = jest.spyOn(mockItem, "resume");
+
         await downloadInitiator.generateOnWillDownload({
           callbacks,
+          directory: "/some/path",
+          saveAsFilename: "test.txt",
         })(mockEvent, mockItem, mockWebContents);
 
         await jest.runAllTimersAsync();
 
-        expect(mockItem.resume).toHaveBeenCalled();
+        expect(resumeSpy).toHaveBeenCalled();
       });
     });
   });
@@ -269,7 +276,7 @@ describe("DownloadInitiator", () => {
       expect(downloadInitiator.cleanup).toHaveBeenCalled();
     });
 
-    it.only("should handle interrupted state", async () => {
+    it("should handle interrupted state", async () => {
       const downloadInitiator = new DownloadInitiator({});
       downloadInitiator.downloadData = mockDownloadData;
       downloadInitiator.callbackDispatcher.onDownloadInterrupted = jest.fn();
@@ -282,5 +289,27 @@ describe("DownloadInitiator", () => {
       expect(mockDownloadData.interruptedVia).toBe("completed");
       expect(downloadInitiator.callbackDispatcher.onDownloadInterrupted).toHaveBeenCalledWith(mockDownloadData);
     });
+
+    it("should call the item updated event if the download was paused and resumed", async () => {
+        const downloadInitiator = new DownloadInitiator({});
+        downloadInitiator.downloadData = mockDownloadData;
+        downloadInitiator.updateProgress = jest.fn();
+
+        determineFilePath.mockReturnValueOnce("/some/path/test.txt");
+
+        await downloadInitiator.generateOnWillDownload({
+            callbacks,
+        })(mockEvent, mockItem, mockWebContents);
+
+        await jest.runAllTimersAsync();
+
+        mockItem.pause();
+        mockEmitter.emit("updated", "", "progressing");
+        expect(downloadInitiator.callbackDispatcher.onDownloadProgress).not.toHaveBeenCalled();
+
+        mockItem.resume();
+        mockEmitter.emit("updated", "", "progressing");
+        expect(downloadInitiator.callbackDispatcher.onDownloadProgress).toHaveBeenCalled();
+    })
   });
 });