From 76334b808dedaff5e6ec122c4740ce0b6685c08e Mon Sep 17 00:00:00 2001
From: Hexxa <hexxa@outlook.com>
Date: Tue, 2 Feb 2021 12:02:32 +0800
Subject: [PATCH] fix(client/worker): incorrect uploading list filtering logic
 (#39)

---
 .../src/worker/__test__/upload.worker.test.ts | 27 +++++++++++--------
 .../web/src/worker/upload.baseworker.ts       | 24 ++++++++---------
 src/client/web/src/worker/upload_mgr.ts       | 10 +++----
 3 files changed, 32 insertions(+), 29 deletions(-)

diff --git a/src/client/web/src/worker/__test__/upload.worker.test.ts b/src/client/web/src/worker/__test__/upload.worker.test.ts
index 157da2e..b1b2d39 100644
--- a/src/client/web/src/worker/__test__/upload.worker.test.ts
+++ b/src/client/web/src/worker/__test__/upload.worker.test.ts
@@ -31,12 +31,8 @@ describe("upload.worker", () => {
     );
     when(mockUploaderClass.stop()).thenCall(() => {});
 
-    let currentUploader: FileUploader = undefined;
-    let uploaderFile: File = undefined;
-    let uploaderFilePath: string = undefined;
-    let uploaderStopFilePath: string = undefined;
-
     interface TestCase {
+      desc: string;
       infos: Array<UploadEntry>;
       expectedUploadingFile: string;
       expectedUploaderStartInput: string;
@@ -45,33 +41,41 @@ describe("upload.worker", () => {
 
     const tcs: Array<TestCase> = [
       {
+        desc: "add new uploadings when worker is in idle",
         infos: [makeEntry("file1", true), makeEntry("file2", true)],
+        currentFilePath: "",
         expectedUploadingFile: "file1",
         expectedUploaderStartInput: "file1",
-        currentFilePath: "",
       },
       {
+        desc: "add new uploadings when worker is in idle and skip some stopped files",
         infos: [makeEntry("file1", false), makeEntry("file2", true)],
+        currentFilePath: "",
         expectedUploadingFile: "file2",
         expectedUploaderStartInput: "file2",
-        currentFilePath: "",
       },
       {
-        infos: [makeEntry("file1", true), makeEntry("file0", true)],
+        desc: "current file should be stopped and start new uploading",
+        infos: [makeEntry("file0", false), makeEntry("file1", true)],
+        currentFilePath: "file0",
         expectedUploadingFile: "file1",
         expectedUploaderStartInput: "file1",
-        currentFilePath: "file0",
       },
       {
+        desc: "uploader should keep uploading if the first uploadable file is not changed",
         infos: [makeEntry("file1", true)],
         expectedUploadingFile: "file1",
-        expectedUploaderStartInput: "file1",
+        expectedUploaderStartInput: undefined,
         currentFilePath: "file1",
       },
     ];
 
     for (let i = 0; i < tcs.length; i++) {
       const uploadWorker = new UploadWorker();
+      let currentUploader: FileUploader = undefined;
+      let uploaderFile: File = undefined;
+      let uploaderFilePath: string = undefined;
+      let uploaderStopFilePath: string = undefined;
       uploadWorker.sendEvent = (_: FileWorkerResp) => {};
       uploadWorker.makeUploader = (
         file: File,
@@ -82,7 +86,6 @@ describe("upload.worker", () => {
         currentUploader = instance(mockUploaderClass);
         return currentUploader;
       };
-
       if (tcs[i].currentFilePath !== "") {
         uploadWorker.setFilePath(tcs[i].currentFilePath);
       }
@@ -96,6 +99,8 @@ describe("upload.worker", () => {
           data: req,
         })
       );
+
+      console.log(tcs[i].desc);
       expect(uploadWorker.getFilePath()).toEqual(tcs[i].expectedUploadingFile);
       expect(uploaderFilePath).toEqual(tcs[i].expectedUploaderStartInput);
     }
diff --git a/src/client/web/src/worker/upload.baseworker.ts b/src/client/web/src/worker/upload.baseworker.ts
index 50968d6..2d00c5b 100644
--- a/src/client/web/src/worker/upload.baseworker.ts
+++ b/src/client/web/src/worker/upload.baseworker.ts
@@ -53,20 +53,18 @@ export class UploadWorker {
         const infoArray = syncReq.infos;
 
         for (let i = 0; i < infoArray.length; i++) {
-          if (
-            infoArray[i].runnable &&
-            infoArray[i].uploaded < infoArray[i].size
-          ) {
-            // infoArray[i].filePath !== this.filePath, it may re-uploading a deleted file
-            // and it will stuck or the file will be renamed in the future
-            this.stopUploader();
-            this.startUploader(infoArray[i].file, infoArray[i].filePath);
+          if (infoArray[i].runnable) {
+            if (infoArray[i].filePath === this.filePath) {
+              // in uploading, do nothing
+            } else {
+              this.stopUploader();
+              this.startUploader(infoArray[i].file, infoArray[i].filePath);
+            }
             break;
-          } else if (
-            !infoArray[i].runnable &&
-            infoArray[i].filePath == this.filePath
-          ) {
-            this.stopUploader();
+          } else {
+            if (infoArray[i].filePath === this.filePath) {
+              this.stopUploader();
+            }
           }
         }
         break;
diff --git a/src/client/web/src/worker/upload_mgr.ts b/src/client/web/src/worker/upload_mgr.ts
index 03bd474..d3005ca 100644
--- a/src/client/web/src/worker/upload_mgr.ts
+++ b/src/client/web/src/worker/upload_mgr.ts
@@ -1,4 +1,4 @@
-import { Map } from "immutable";
+import { Map, OrderedMap } from "immutable";
 
 import {
   FileWorkerReq,
@@ -20,7 +20,7 @@ export interface IWorker {
 }
 
 export class UploadMgr {
-  private infos = Map<string, UploadEntry>();
+  private infos = OrderedMap<string, UploadEntry>();
   private worker: IWorker;
   private intervalID: number;
   private cycle: number = 500;
@@ -44,7 +44,7 @@ export class UploadMgr {
     win.clearInterval(this.intervalID);
   };
 
-  _setInfos = (infos: Map<string, UploadEntry>) => {
+  _setInfos = (infos: OrderedMap<string, UploadEntry>) => {
     this.infos = infos;
   };
 
@@ -98,7 +98,7 @@ export class UploadMgr {
     this.infos = this.infos.delete(filePath);
   };
 
-  list = (): Map<string, UploadEntry> => {
+  list = (): OrderedMap<string, UploadEntry> => {
     return this.infos;
   };
 
@@ -128,7 +128,7 @@ export class UploadMgr {
           }
 
           // call back to update the info
-          this.statusCb(this.infos);
+          this.statusCb(this.infos.toMap());
         } else {
           // TODO: refine this
           console.error(