Merge pull request #133 from qiniu/fix/region-config-and-mkdir

Jessie-shsh · web-flow · commit 080f607366e1 · 2022-09-14T20:09:10.000+08:00
Fix bugs with transfer jobs and fix ci linux wine
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -16,8 +16,10 @@ jobs:
           sudo dpkg --add-architecture i386
           sudo apt-key adv --keyserver keyserver.ubuntu.com --recv-keys 86B72ED9
           sudo add-apt-repository 'deb [arch=amd64] https://mirror.mxe.cc/repos/apt focal main'
+          sudo rm -f /etc/apt/sources.list.d/microsoft-prod.list
           sudo apt -qq update
-          sudo apt install -y --allow-downgrades libpcre2-8-0=10.34-7
+          sudo apt install -yqq --allow-downgrades libgd3/focal libpcre2-8-0/focal libpcre2-16-0/focal libpcre2-32-0/focal libpcre2-posix2/focal
+          sudo apt purge -yqq libmono* moby* mono* php* libgdiplus libpcre2-posix3
           sudo apt install -y wine32 wine64
       - name: test
         run: make i test
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "kodo-browser",
-  "version": "1.0.19.1",
+  "version": "1.0.19.2",
   "license": "Apache-2.0",
   "author": {
     "name": "Rong Zhou",
diff --git a/src/common/models/job/download-job.ts b/src/common/models/job/download-job.ts
@@ -250,6 +250,9 @@ export default class DownloadJob extends TransferJob {
     private async startDownload(client: Adapter) {
         client.storageClasses = this.options.storageClasses;
 
+        // prevent the base directory has gone by unexpected reason.
+        await this.checkAndCreateBaseDir(this.options.to.path);
+
         // check overwrite
         // the reason for overwrite when `this.prog.loaded > 0` is to allow
         // download from breakpoint to work properly.
@@ -379,4 +382,27 @@ export default class DownloadJob extends TransferJob {
         }
         // useless?
     }
+
+    private async checkAndCreateBaseDir(filePath: string) {
+        const baseDirPath = path.dirname(filePath);
+        const isBaseDirValid: boolean = await fsPromises.access(
+            baseDirPath,
+            fsConstants.R_OK | fsConstants.W_OK | fsConstants.X_OK,
+        )
+            .then(() => true)
+            .catch(() => false);
+        if (isBaseDirValid) {
+            return;
+        }
+        const isBaseDirExists: boolean = await fsPromises.access(
+            baseDirPath,
+            fsConstants.F_OK,
+        )
+            .then(() => true)
+            .catch(() => false);
+        if (isBaseDirExists) {
+            throw Error(`Can't download to ${this.options.to}: Permission denied`);
+        }
+        await fsPromises.mkdir(baseDirPath, {recursive: true})
+    }
 }
diff --git a/src/main/transfer-managers/download-manager.ts b/src/main/transfer-managers/download-manager.ts
@@ -1,5 +1,5 @@
 import path from "path";
-import fs, {promises as fsPromises} from "fs";
+import fs, {promises as fsPromises, constants as fsConstants} from "fs";
 
 import lodash from "lodash";
 import sanitizeFilename from "sanitize-filename";
@@ -232,22 +232,20 @@ export default class DownloadManager extends TransferManager<DownloadJob, Config
 
     private async createDirectory(path: string): Promise<void> {
         try {
-            await fsPromises.stat(path)
-                .then(stat => {
-                    if (stat.isDirectory()) {
-                        return;
-                    }
-                })
-                .catch(err => {
-                    if (err.message.indexOf("EEXIST: file already exists") > -1) {
-                        return Promise.resolve();
-                    }
-                    if (err.message.indexOf("ENOENT: no such file or directory") > -1) {
-                        return fsPromises.mkdir(path);
-                    }
-
-                    return Promise.reject(err);
-                });
+            const isDirectoryExists: boolean = await fsPromises.access(
+                path,
+                fsConstants.F_OK,
+            )
+                .then(() => true)
+                .catch(() => false);
+            if (isDirectoryExists) {
+                return;
+            }
+            const stat = await fsPromises.stat(path);
+            if (stat.isDirectory()) {
+                return;
+            }
+            await fsPromises.mkdir(path);
         } catch (err: any) {
             this.config.onError?.(err);
         }
diff --git a/src/main/transfer-managers/single-flight.test.ts b/src/main/transfer-managers/single-flight.test.ts
@@ -0,0 +1,80 @@
+import singleFlight from "./single-flight";
+
+describe("test transfer-managers/single-flight.ts", () => {
+    function makeWait(t: number) {
+        return async (): Promise<void> => {
+            return new Promise(resolve => {
+                setTimeout(() => {
+                    resolve();
+                }, t);
+            });
+        };
+    }
+
+    function makeProcessWithTimestamp(t: number) {
+        return async (): Promise<number> => {
+            return new Promise(resolve => {
+                setTimeout(() => {
+                    resolve(Date.now());
+                }, t);
+            });
+        };
+    }
+
+    function groupFlights(doneResults: number[]): number[] {
+        return [
+            ...doneResults
+                .reduce((acc: Map<number, number>, cur) => {
+                    const times = acc.get(cur);
+                    if (times) {
+                        acc.set(cur, times + 1);
+                    } else {
+                        acc.set(cur, 1);
+                    }
+                    return acc;
+                }, new Map())
+                .values()
+        ];
+    }
+
+    const asyncProcess = makeProcessWithTimestamp(2000);
+    const wait = makeWait(300);
+
+    it("with single flight", async () => {
+        const singleFlightProcess = singleFlight(asyncProcess);
+        const processPromises: Promise<number>[] = []
+        for (let i = 0; i < 5; i++) {
+            processPromises.push(singleFlightProcess(`key-${i % 2}`))
+            await wait();
+        }
+        const doneResults = await Promise.all(processPromises);
+        const actual = groupFlights(doneResults);
+        // we call 5 processes.
+        // 2 of them have same flightKey, the others 3 hava same flightKey.
+        // so expect [2, 3]
+        expect(actual).toEqual(expect.arrayContaining([2, 3]))
+    });
+
+    it("without single flight", async () => {
+        const processPromises: Promise<number>[] = []
+        for (let i = 0; i < 5; i++) {
+            processPromises.push(asyncProcess())
+            await wait();
+        }
+        const doneResults = await Promise.all(processPromises);
+        const actual = groupFlights(doneResults);
+        expect(actual).toEqual(expect.arrayContaining([1, 1, 1, 1, 1]))
+    });
+
+    it("with single flight but should tow flight with same key", async () => {
+        const singleFlightProcess = singleFlight(asyncProcess);
+        const processResults: number[] = []
+        for (let i = 0; i < 2; i++) {
+            const result = await singleFlightProcess(`key`);
+            processResults.push(result);
+            await wait();
+        }
+        const actual = groupFlights(processResults);
+        expect(actual).toEqual(expect.arrayContaining([1, 1]))
+    });
+});
diff --git a/src/main/transfer-managers/single-flight.ts b/src/main/transfer-managers/single-flight.ts
@@ -0,0 +1,27 @@
+type AsyncFunction = (...args: any[]) => Promise<any>;
+
+// TODO: Upgrade TypeScript to ≥4.5 to get a build-in `Awaited`.
+type Awaited<T> = T extends PromiseLike<infer U> ? U : T
+
+// `Promise<Awaited<ReturnType<T>>>` is ugly to get the right type because of
+// the deferred resolving of conditional types involving unbound type parameters in TypeScript.
+// There are many issue with it, such as
+// https://github.com/microsoft/TypeScript/issues/43702
+// https://github.com/microsoft/TypeScript/issues/50251
+export default function singleFlight<T extends AsyncFunction>(
+    fn: T,
+): (key: string, ...args: Parameters<T>) => Promise<Awaited<ReturnType<T>>> {
+    const flightMap = new Map<string, Promise<Awaited<ReturnType<T>>>>();
+    return function (key: string, ...args: Parameters<T>): Promise<Awaited<ReturnType<T>>> {
+        const flight = flightMap.get(key);
+        if (flight) {
+            return flight;
+        }
+        const p = fn(...args);
+        flightMap.set(key, p);
+        p.then(() => {
+            flightMap.delete(key);
+        });
+        return p;
+    }
+}
diff --git a/src/main/transfer-managers/transfer-manager.ts b/src/main/transfer-managers/transfer-manager.ts
@@ -206,15 +206,12 @@ export default abstract class TransferManager<Job extends TransferJob, Opt = {}>
     }: {
         matchStatus: Status[],
     } = {
-        matchStatus: [],
+        matchStatus: [Status.Running, Status.Waiting],
     }): void {
         this.jobIds
             .map(id => this.jobs.get(id))
             .forEach(job => {
-                if (!job || ![Status.Running, Status.Waiting].includes(job.status)) {
-                    return;
-                }
-                if (!matchStatus.includes(job.status)){
+                if (!job || !matchStatus.includes(job.status)){
                     return;
                 }
                 job.stop();
diff --git a/src/main/transfer-managers/upload-manager.ts b/src/main/transfer-managers/upload-manager.ts
@@ -13,6 +13,7 @@ import {Status} from "@common/models/job/types";
 
 import {MAX_MULTIPART_COUNT, MIN_MULTIPART_SIZE} from "./boundary-const";
 import TransferManager, {TransferManagerConfig} from "./transfer-manager";
+import singleFlight from "./single-flight";
 
 // for walk
 interface StatsWithName extends Stats {
@@ -77,7 +78,9 @@ export default class UploadManager extends TransferManager<UploadJob, Config> {
                     // if enable skip empty directory upload.
                     const remoteDirectoryKey = path.dirname(remoteKey) + "/";
                     if (remoteDirectoryKey !== "./" && !directoryToCreate.get(remoteDirectoryKey)) {
-                        this.createDirectory(
+                        const flightKey = destInfo.regionId + destInfo.bucketName + remoteDirectoryKey;
+                        this.createDirectoryWithSingleFlight(
+                            flightKey,
                             qiniuClient,
                             {
                                 region: destInfo.regionId,
@@ -95,9 +98,11 @@ export default class UploadManager extends TransferManager<UploadJob, Config> {
                         //  and in this electron version(nodejs v10.x) read the directory again
                         //  is too waste. so we create later.
                         //  if we are nodejs > v12.12，use opendir API to determine empty.
-                        if (!this.config.isSkipEmptyDirectory) {
+                        if (!this.config.isSkipEmptyDirectory && !directoryToCreate.get(remoteDirectoryKey)) {
                             const remoteDirectoryKey = remoteKey + "/";
-                            this.createDirectory(
+                            const flightKey = destInfo.regionId + destInfo.bucketName + remoteDirectoryKey;
+                            this.createDirectoryWithSingleFlight(
+                                flightKey,
                                 qiniuClient,
                                 {
                                     region: destInfo.regionId,
@@ -134,6 +139,9 @@ export default class UploadManager extends TransferManager<UploadJob, Config> {
         hooks?.jobsAdded?.();
     }
 
+    /**
+     *  best to call {@link createDirectoryWithSingleFlight}
+     */
     private async createDirectory(
         client: Adapter,
         options: {
@@ -144,6 +152,16 @@ export default class UploadManager extends TransferManager<UploadJob, Config> {
         },
     ) {
         await client.enter("createFolder", async client => {
+            const isDirectoryExists = await client.isExists(
+                options.region,
+                {
+                    bucket: options.bucketName,
+                    key: options.key,
+                },
+            );
+            if (isDirectoryExists) {
+                return
+            }
             await client.putObject(
                 options.region,
                 {
@@ -163,6 +181,8 @@ export default class UploadManager extends TransferManager<UploadJob, Config> {
         };
     }
 
+    private createDirectoryWithSingleFlight = singleFlight(this.createDirectory)
+
     private createUploadJob(
         from: Required<UploadJob["options"]["from"]>,
         to: UploadJob["options"]["to"],
diff --git a/src/renderer/i18n/en-US.js b/src/renderer/i18n/en-US.js
@@ -224,6 +224,7 @@ export default {
 
   //files
   upload: "Upload",
+  "upload.duplicated.basename": "Duplicated name in selected files or directories",
   "folder.create": "Directory",
   "folder.create.success": "Directory created successfully",
   "folder.name": "Name",
diff --git a/src/renderer/i18n/ja-JP.js b/src/renderer/i18n/ja-JP.js
@@ -223,6 +223,7 @@ export default {
 
   //files
   upload: "アップロード",
+  "upload.duplicated.basename": "選択したファイルやディレクトリの名前が重複している",
   "folder.create": "ディレクトリ",
   "folder.create.success": "ディレクトリが正常に作成されました",
   "folder.name": "名前",
diff --git a/src/renderer/i18n/zh-CN.js b/src/renderer/i18n/zh-CN.js
@@ -223,6 +223,7 @@ export default {
 
   //files
   upload: "上传",
+  "upload.duplicated.basename": "所选项目存在同名文件或目录",
   "folder.create": "创建目录",
   "folder.create.success": "创建目录成功",
   "folder.name": "目录名",
diff --git a/src/renderer/main/files/files.js b/src/renderer/main/files/files.js
@@ -1,4 +1,5 @@
 import angular from 'angular'
+import lodash from 'lodash'
 import deepEqual from 'fast-deep-equal'
 import { Base64 } from 'js-base64'
 import { S3_MODE } from 'kodo-s3-adapter-sdk'
@@ -338,19 +339,17 @@ webModule.controller(FILES_CONTROLLER_NAME, [
     }
 
     /////////////////////////////////
-    var refreshTid;
-
-    $scope.$on("refreshFilesList", (e) => {
-      $timeout.cancel(refreshTid);
+    const refreshFilesListThrottled = lodash.throttle((bucketName, key) => {
+      if (bucketName === $scope.currentInfo.bucketName &&
+          key === $scope.currentInfo.key) {
+        gotoAddress(bucketName, key);
+      }
+    }, 5000);
 
-      const bucketName = $scope.currentInfo.bucketName,
-            key = $scope.currentInfo.key;
-      refreshTid = $timeout(() => {
-        if (bucketName === $scope.currentInfo.bucketName &&
-            key === $scope.currentInfo.key) {
-          gotoAddress(bucketName, key);
-        }
-      }, 600);
+    $scope.$on("refreshFilesList", () => {
+      const bucketName = $scope.currentInfo.bucketName;
+      const key = $scope.currentInfo.key;
+      refreshFilesListThrottled(bucketName, key);
     });
 
     var searchTid;
@@ -1305,9 +1304,17 @@ webModule.controller(FILES_CONTROLLER_NAME, [
     }
 
     /**
-    * @param filePaths { string }
+    * @param filePaths { string[] }
     */
     function showUploadConfirmModal(filePaths) {
+      // check is duplicated basename to prevent create same directory
+      const fileNames = filePaths.map(p => path.basename(p));
+      const isSomeDuplicatedBasename = fileNames.some((p, i) => fileNames.indexOf(p) < i);
+      if (isSomeDuplicatedBasename) {
+        Toast.error(T('upload.duplicated.basename'))
+        return;
+      }
+
       $modal.open({
         templateUrl: uploadConfirmModalHtmlMapping.path,
         controller: uploadConfirmModalCtrl,
diff --git a/src/renderer/main/files/transfer/frame.js b/src/renderer/main/files/transfer/frame.js
@@ -410,7 +410,7 @@ webModule.controller(TRANSFER_FRAME_CONTROLLER_NAME, [
           accessKey: AuthInfo.get().id,
           secretKey: AuthInfo.get().secret,
           ucUrl: ngConfig.load().ucUrl || "",
-          regions: ngConfig.load().regionId || [],
+          regions: ngConfig.load().regions || [],
           backendMode: $scope.selectedDomain.domain.qiniuBackendMode(),
         },
       });

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "kodo-browser",`
`3`		`- "version": "1.0.19.1",`
	`3`	`+ "version": "1.0.19.2",`
`4`	`4`	`"license": "Apache-2.0",`
`5`	`5`	`"author": {`
`6`	`6`	`"name": "Rong Zhou",`