fix(electron-updater): Electron Updater downloads update multiple times

Close electron-userland#1788

package.json

@@ -47,6 +47,7 @@
     "isbinaryfile": "^3.0.2",
     "js-yaml": "^3.9.0",
     "json5": "^0.5.1",
+    "lodash.isequal": "^4.5.0",
     "mime": "^1.3.6",
     "minimatch": "^3.0.4",
     "node-emoji": "^1.7.0",

packages/electron-updater/package.json

@@ -21,7 +21,8 @@
     "electron-is-dev": "^0.2.0",
     "xelement": "^1.0.16",
     "debug": "^2.6.8",
-    "uuid-1345": "^0.99.6"
+    "uuid-1345": "^0.99.6",
+    "lodash.isequal": "^4.5.0"
   },
   "typings": "./out/electron-updater.d.ts"
 }

packages/electron-updater/src/DownloadedUpdateHelper.ts

@@ -0,0 +1,41 @@
+import { VersionInfo } from "electron-builder-http/out/updateInfo"
+import { FileInfo } from "./main"
+
+let isEqual: any
+
+export class DownloadedUpdateHelper {
+  private setupPath: string | null
+  private versionInfo: VersionInfo | null
+  private fileInfo: FileInfo | null
+
+  get file() {
+    return this.setupPath
+  }
+
+  getDownloadedFile(versionInfo: VersionInfo, fileInfo: FileInfo): string | null {
+    if (this.setupPath == null) {
+      return null
+    }
+
+    if (isEqual == null) {
+      isEqual = require("lodash.isequal")
+    }
+
+    if (isEqual(this.versionInfo, versionInfo) && isEqual(this.fileInfo, fileInfo)) {
+      return this.setupPath
+    }
+    return null
+  }
+
+  setDownloadedFile(file: string, versionInfo: VersionInfo, fileInfo: FileInfo) {
+    this.setupPath = file
+    this.versionInfo = versionInfo
+    this.fileInfo = fileInfo
+  }
+
+  clear() {
+    this.setupPath = null
+    this.versionInfo = null
+    this.fileInfo = null
+  }
+}

packages/electron-updater/src/NsisUpdater.ts

@@ -9,10 +9,12 @@ import { tmpdir } from "os"
 import * as path from "path"
 import "source-map-support/register"
 import { AppUpdater } from "./AppUpdater"
+import { DownloadedUpdateHelper } from "./DownloadedUpdateHelper"
 import { DOWNLOAD_PROGRESS, FileInfo, UPDATE_DOWNLOADED } from "./main"
 
 export class NsisUpdater extends AppUpdater {
-  private setupPath: string | null
+  private readonly downloadedUpdateHelper = new DownloadedUpdateHelper()
+
   private quitAndInstallCalled = false
   private quitHandlerAdded = false
 
@@ -30,20 +32,24 @@ export class NsisUpdater extends AppUpdater {
       sha512: fileInfo == null ? null : fileInfo.sha512,
     }
 
+    const downloadedFile = this.downloadedUpdateHelper.getDownloadedFile(versionInfo, fileInfo)
+    if (downloadedFile != null) {
+      return downloadedFile
+    }
+
     if (this.listenerCount(DOWNLOAD_PROGRESS) > 0) {
       downloadOptions.onProgress = it => this.emit(DOWNLOAD_PROGRESS, it)
     }
 
     const tempDir = await mkdtemp(`${path.join(tmpdir(), "up")}-`)
     const tempFile = path.join(tempDir, fileInfo.name)
 
-    const removeTempDirIfAny = async () => {
-      try {
-        await remove(tempDir)
-      }
-      catch (ignored) {
-        // ignored
-      }
+    const removeTempDirIfAny = () => {
+      this.downloadedUpdateHelper.clear()
+      return remove(tempDir)
+        .catch(error => {
+          // ignored
+        })
     }
 
     let signatureVerificationStatus
@@ -68,7 +74,7 @@ export class NsisUpdater extends AppUpdater {
     }
 
     this._logger.info(`New version ${this.versionInfo!.version} has been downloaded to ${tempFile}`)
-    this.setupPath = tempFile
+    this.downloadedUpdateHelper.setDownloadedFile(tempFile, versionInfo, fileInfo)
     this.addQuitHandler()
     this.emit(UPDATE_DOWNLOADED, this.versionInfo)
     return tempFile
@@ -170,7 +176,7 @@ export class NsisUpdater extends AppUpdater {
       return false
     }
 
-    const setupPath = this.setupPath
+    const setupPath = this.downloadedUpdateHelper.file
     if (!this.updateAvailable || setupPath == null) {
       const message = "No update available, can't quit and install"
       this.emit("error", new Error(message), message)

test/out/__snapshots__/nsisUpdaterTest.js.snap

@@ -39,10 +39,21 @@ Object {
 `;
 
 exports[`checkForUpdates several times 2`] = `
+Object {
+  "name": "TestApp Setup 1.1.0.exe",
+  "sha2": undefined,
+  "sha512": "Dj51I0q8aPQ3ioaz9LMqGYujAYRbDNblAQbodDRXAMxmY6hsHqEl3F6SvhfJj5oPhcqdX1ldsgEvfMNXGUXBIw==",
+  "url": "https://develar.s3.amazonaws.com/test/TestApp Setup 1.1.0.exe",
+}
+`;
+
+exports[`checkForUpdates several times 3`] = `
 Array [
   "checking-for-update",
   "update-available",
   "update-downloaded",
+  "checking-for-update",
+  "update-available",
 ]
 `;
 

test/src/nsisUpdaterTest.ts

@@ -19,52 +19,59 @@ if (process.env.ELECTRON_BUILDER_OFFLINE === "true") {
 const tmpDir = new TmpDir()
 
 function createTestApp(version: string) {
-  return {
-    getVersion: () => version,
-
-    getAppPath: function () {
-    },
+  class MockApp {
+    // noinspection JSMethodCanBeStatic,JSUnusedGlobalSymbols
+    getVersion() {
+      return version
+    }
+
+    // noinspection JSMethodCanBeStatic,JSUnusedGlobalSymbols
+    getAppPath() {
+      // ignored
+    }
 
-    getPath: function (type: string) {
+    // noinspection JSMethodCanBeStatic,JSUnusedGlobalSymbols
+    getPath(type: string) {
       return path.join(tmpdir(), "electron-updater-test", type)
-    },
+    }
 
-    on: function () {
+    on() {
       // ignored
-    },
+    }
   }
+  return new MockApp()
 }
 
-const g = (<any>global)
+const g = (global as any)
 g.__test_app = createTestApp("0.0.1")
 
 process.env.TEST_UPDATER_PLATFORM = "win32"
 
 function tuneNsisUpdater(updater: NsisUpdater) {
-  (<any>updater).httpExecutor = httpExecutor
+  (updater as any).httpExecutor = httpExecutor
   updater.logger = new NoOpLogger()
 }
 
 test("check updates - no versions at all", async () => {
   const updater = new NsisUpdater()
   tuneNsisUpdater(updater)
-  updater.setFeedURL(<BintrayOptions>{
+  updater.setFeedURL({
     provider: "bintray",
     owner: "actperepo",
     package: "no-versions",
-  })
+  } as BintrayOptions)
 
   await assertThat(updater.checkForUpdates()).throws()
 })
 
 async function testUpdateFromBintray(app: any) {
   const updater = new NsisUpdater(null, app)
   updater.allowDowngrade = true
-  updater.updateConfigPath = await writeUpdateConfig(<BintrayOptions>{
+  updater.updateConfigPath = await writeUpdateConfig({
     provider: "bintray",
     owner: "actperepo",
     package: "TestApp",
-  })
+  } as BintrayOptions)
   tuneNsisUpdater(updater)
 
   const actualEvents: Array<string> = []
@@ -85,11 +92,11 @@ test("file url", () => testUpdateFromBintray(null))
 
 test("downgrade (disallowed)", async () => {
   const updater = new NsisUpdater(null, createTestApp("2.0.0"))
-  updater.updateConfigPath = await writeUpdateConfig(<BintrayOptions>{
+  updater.updateConfigPath = await writeUpdateConfig({
     provider: "bintray",
     owner: "actperepo",
     package: "TestApp",
-  })
+  } as BintrayOptions)
   tuneNsisUpdater(updater)
 
   const actualEvents: Array<string> = []
@@ -109,11 +116,11 @@ test("downgrade (disallowed)", async () => {
 
 test("downgrade (disallowed, beta)", async () => {
   const updater = new NsisUpdater(null, createTestApp("1.5.2-beta.4"))
-  updater.updateConfigPath = await writeUpdateConfig(<GithubOptions>{
+  updater.updateConfigPath = await writeUpdateConfig({
     provider: "github",
     owner: "develar",
     repo: "__test_nsis_release",
-  })
+  } as GithubOptions)
   tuneNsisUpdater(updater)
 
   const actualEvents: Array<string> = []
@@ -135,10 +142,10 @@ test("downgrade (allowed)", () => testUpdateFromBintray(createTestApp("2.0.0-bet
 
 test("file url generic", async () => {
   const updater = new NsisUpdater()
-  updater.updateConfigPath = await writeUpdateConfig(<GenericServerOptions>{
+  updater.updateConfigPath = await writeUpdateConfig({
     provider: "generic",
     url: "https://develar.s3.amazonaws.com/test",
-  })
+  } as GenericServerOptions)
   tuneNsisUpdater(updater)
 
   const actualEvents = trackEvents(updater)
@@ -152,11 +159,11 @@ test("file url generic", async () => {
 
 test.ifNotCiWin("sha512 mismatch error event", async () => {
   const updater = new NsisUpdater()
-  updater.updateConfigPath = await writeUpdateConfig(<GenericServerOptions>{
+  updater.updateConfigPath = await writeUpdateConfig({
     provider: "generic",
     url: "https://develar.s3.amazonaws.com/test",
     channel: "beta",
-  })
+  } as GenericServerOptions)
   tuneNsisUpdater(updater)
 
   const actualEvents = trackEvents(updater)
@@ -170,10 +177,10 @@ test.ifNotCiWin("sha512 mismatch error event", async () => {
 
 test("file url generic - manual download", async () => {
   const updater = new NsisUpdater()
-  updater.updateConfigPath = await writeUpdateConfig(<GenericServerOptions>{
+  updater.updateConfigPath = await writeUpdateConfig({
     provider: "generic",
     url: "https://develar.s3.amazonaws.com/test",
-  })
+  } as GenericServerOptions)
   tuneNsisUpdater(updater)
   updater.autoDownload = false
 
@@ -190,10 +197,10 @@ test("file url generic - manual download", async () => {
 // https://github.com/electron-userland/electron-builder/issues/1045
 test("checkForUpdates several times", async () => {
   const updater = new NsisUpdater()
-  updater.updateConfigPath = await writeUpdateConfig(<GenericServerOptions>{
+  updater.updateConfigPath = await writeUpdateConfig({
     provider: "generic",
     url: "https://develar.s3.amazonaws.com/test",
-  })
+  } as GenericServerOptions)
   tuneNsisUpdater(updater)
 
   const actualEvents = trackEvents(updater)
@@ -202,42 +209,49 @@ test("checkForUpdates several times", async () => {
     //noinspection JSIgnoredPromiseFromCall
     updater.checkForUpdates()
   }
-  const updateCheckResult = await updater.checkForUpdates()
-  expect(updateCheckResult.fileInfo).toMatchSnapshot()
-  await assertThat(path.join(await updateCheckResult.downloadPromise)).isFile()
+
+  async function checkForUpdates() {
+    const updateCheckResult = await updater.checkForUpdates()
+    expect(updateCheckResult.fileInfo).toMatchSnapshot()
+    await assertThat(path.join(await updateCheckResult.downloadPromise)).isFile()
+  }
+
+  await checkForUpdates()
+  // we must not download the same file again
+  await checkForUpdates()
 
   expect(actualEvents).toMatchSnapshot()
 })
 
 test("file url github", async () => {
   const updater = new NsisUpdater()
-  updater.updateConfigPath = await writeUpdateConfig(<GithubOptions>{
+  updater.updateConfigPath = await writeUpdateConfig({
     provider: "github",
     owner: "develar",
     repo: "__test_nsis_release",
-  })
+  } as GithubOptions)
   await validateDownload(updater)
 })
 
 test("file url github pre-release", async () => {
   const updater = new NsisUpdater(null, createTestApp("1.5.0-beta.1"))
-  updater.updateConfigPath = await writeUpdateConfig(<GithubOptions>{
+  updater.updateConfigPath = await writeUpdateConfig({
     provider: "github",
     owner: "develar",
     repo: "__test_nsis_release",
-  })
+  } as GithubOptions)
 
   const updateCheckResult = await validateDownload(updater)
   expect(updateCheckResult.versionInfo).toMatchSnapshot()
 })
 
 test.skip("file url github private", async () => {
   const updater = new NsisUpdater()
-  updater.updateConfigPath = await writeUpdateConfig(<GithubOptions>{
+  updater.updateConfigPath = await writeUpdateConfig({
     provider: "github",
     owner: "develar",
     repo: "__test_nsis_release_private",
-  })
+  } as GithubOptions)
   await validateDownload(updater)
 })
 
@@ -369,7 +383,7 @@ test("cancel download with progress", async () => {
     expect(lastEvent.transferred).not.toBe(lastEvent.total)
   }
 
-  const downloadPromise = <BluebirdPromise<any>>checkResult.downloadPromise
+  const downloadPromise = checkResult.downloadPromise as BluebirdPromise<any>
   await assertThat(downloadPromise).throws()
   expect(downloadPromise.isRejected()).toBe(true)
   expect(cancelled).toBe(true)

yarn.lock

@@ -2222,6 +2222,10 @@ locate-path@^2.0.0:
     p-locate "^2.0.0"
     path-exists "^3.0.0"
 
+lodash.isequal@^4.5.0:
+  version "4.5.0"
+  resolved "https://registry.yarnpkg.com/lodash.isequal/-/lodash.isequal-4.5.0.tgz#415c4478f2bcc30120c22ce10ed3226f7d3e18e0"
+
 lodash.omit@^4.5.0:
   version "4.5.0"
   resolved "https://registry.yarnpkg.com/lodash.omit/-/lodash.omit-4.5.0.tgz#6eb19ae5a1ee1dd9df0b969e66ce0b7fa30b5e60"