From 6e3581fddd7f03a6ceee2449a692499305d47ad0 Mon Sep 17 00:00:00 2001 From: develar Date: Wed, 12 Jul 2017 08:23:26 +0200 Subject: [PATCH] fix(electron-updater): Electron Updater downloads update multiple times Close #1788 --- package.json | 1 + packages/electron-updater/package.json | 3 +- .../src/DownloadedUpdateHelper.ts | 41 +++++++++ packages/electron-updater/src/NsisUpdater.ts | 26 +++--- .../out/__snapshots__/nsisUpdaterTest.js.snap | 11 +++ test/src/ExtraBuildTest.ts | 1 + test/src/nsisUpdaterTest.ts | 88 +++++++++++-------- test/src/windows/oneClickInstallerTest.ts | 14 +-- yarn.lock | 4 + 9 files changed, 135 insertions(+), 54 deletions(-) create mode 100644 packages/electron-updater/src/DownloadedUpdateHelper.ts diff --git a/package.json b/package.json index b89f51b445a..f58ad45d762 100644 --- a/package.json +++ b/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", diff --git a/packages/electron-updater/package.json b/packages/electron-updater/package.json index ea27486812c..70b8da94d6a 100644 --- a/packages/electron-updater/package.json +++ b/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" } diff --git a/packages/electron-updater/src/DownloadedUpdateHelper.ts b/packages/electron-updater/src/DownloadedUpdateHelper.ts new file mode 100644 index 00000000000..9b427af8cf0 --- /dev/null +++ b/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 + } +} \ No newline at end of file diff --git a/packages/electron-updater/src/NsisUpdater.ts b/packages/electron-updater/src/NsisUpdater.ts index ddada2714b1..b57a05dba14 100644 --- a/packages/electron-updater/src/NsisUpdater.ts +++ b/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,6 +32,11 @@ 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) } @@ -37,13 +44,12 @@ export class NsisUpdater extends AppUpdater { 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) diff --git a/test/out/__snapshots__/nsisUpdaterTest.js.snap b/test/out/__snapshots__/nsisUpdaterTest.js.snap index 753a871edaa..099600ce1b0 100644 --- a/test/out/__snapshots__/nsisUpdaterTest.js.snap +++ b/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", ] `; diff --git a/test/src/ExtraBuildTest.ts b/test/src/ExtraBuildTest.ts index d229599c5fb..a0837ceca9e 100644 --- a/test/src/ExtraBuildTest.ts +++ b/test/src/ExtraBuildTest.ts @@ -96,6 +96,7 @@ test.ifAll.ifDevOrWinCi("override targets in the config - only arch", app({ }, // https://github.com/electron-userland/electron-builder/issues/1348 win: { + // tslint:disable:no-invalid-template-strings artifactName: "${channel}-${name}.exe", target: [ "nsis", diff --git a/test/src/nsisUpdaterTest.ts b/test/src/nsisUpdaterTest.ts index f92d976c418..4c88d3d106d 100644 --- a/test/src/nsisUpdaterTest.ts +++ b/test/src/nsisUpdaterTest.ts @@ -19,40 +19,47 @@ 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 = (global) +const g = (global as any) g.__test_app = createTestApp("0.0.1") process.env.TEST_UPDATER_PLATFORM = "win32" function tuneNsisUpdater(updater: NsisUpdater) { - (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({ + updater.setFeedURL({ provider: "bintray", owner: "actperepo", package: "no-versions", - }) + } as BintrayOptions) await assertThat(updater.checkForUpdates()).throws() }) @@ -60,11 +67,11 @@ test("check updates - no versions at all", async () => { async function testUpdateFromBintray(app: any) { const updater = new NsisUpdater(null, app) updater.allowDowngrade = true - updater.updateConfigPath = await writeUpdateConfig({ + updater.updateConfigPath = await writeUpdateConfig({ provider: "bintray", owner: "actperepo", package: "TestApp", - }) + } as BintrayOptions) tuneNsisUpdater(updater) const actualEvents: Array = [] @@ -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({ + updater.updateConfigPath = await writeUpdateConfig({ provider: "bintray", owner: "actperepo", package: "TestApp", - }) + } as BintrayOptions) tuneNsisUpdater(updater) const actualEvents: Array = [] @@ -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({ + updater.updateConfigPath = await writeUpdateConfig({ provider: "github", owner: "develar", repo: "__test_nsis_release", - }) + } as GithubOptions) tuneNsisUpdater(updater) const actualEvents: Array = [] @@ -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({ + 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({ + 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({ + 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({ + updater.updateConfigPath = await writeUpdateConfig({ provider: "generic", url: "https://develar.s3.amazonaws.com/test", - }) + } as GenericServerOptions) tuneNsisUpdater(updater) const actualEvents = trackEvents(updater) @@ -202,30 +209,37 @@ 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({ + 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({ + updater.updateConfigPath = await writeUpdateConfig({ provider: "github", owner: "develar", repo: "__test_nsis_release", - }) + } as GithubOptions) const updateCheckResult = await validateDownload(updater) expect(updateCheckResult.versionInfo).toMatchSnapshot() @@ -233,11 +247,11 @@ test("file url github pre-release", async () => { test.skip("file url github private", async () => { const updater = new NsisUpdater() - updater.updateConfigPath = await writeUpdateConfig({ + 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 = >checkResult.downloadPromise + const downloadPromise = checkResult.downloadPromise as BluebirdPromise await assertThat(downloadPromise).throws() expect(downloadPromise.isRejected()).toBe(true) expect(cancelled).toBe(true) diff --git a/test/src/windows/oneClickInstallerTest.ts b/test/src/windows/oneClickInstallerTest.ts index 2573b06644d..b8b6cb5fd37 100644 --- a/test/src/windows/oneClickInstallerTest.ts +++ b/test/src/windows/oneClickInstallerTest.ts @@ -24,7 +24,7 @@ test("one-click", app({ } }, { signedWin: true, - packed: async (context) => { + packed: async context => { await checkHelpers(context.getResources(Platform.WINDOWS, Arch.ia32), false) await doTest(context.outDir, true, "TestApp Setup", "TestApp", null, false) await expectUpdateMetadata(context, Arch.ia32, true) @@ -68,6 +68,7 @@ test.ifDevOrLinuxCi("perMachine, no run after finish", app({ }, publish: { provider: "generic", + // tslint:disable:no-invalid-template-strings url: "https://develar.s3.amazonaws.com/test/${os}/${arch}", }, }, @@ -78,7 +79,7 @@ test.ifDevOrLinuxCi("perMachine, no run after finish", app({ copyTestAsset("license.txt", path.join(projectDir, "build", "license.txt")), ]) }, - packed: async(context) => { + packed: async context => { await expectUpdateMetadata(context) const updateInfo = safeLoad(await readFile(path.join(context.outDir, "latest.yml"), "utf-8")) expect(updateInfo.sha512).not.toEqual("") @@ -96,7 +97,7 @@ test.ifNotCiMac("installerHeaderIcon", () => { let headerIconPath: string | null = null return assertPack("test-app-one", { targets: nsisTarget, - effectiveOptionComputed: async (it) => { + effectiveOptionComputed: async it => { const defines = it[0] expect(defines.HEADER_ICO).toEqual(headerIconPath) return false @@ -142,7 +143,7 @@ test.ifAll.ifNotCiMac("menuCategory", app({ projectDirCreated: projectDir => modifyPackageJson(projectDir, data => { data.name = "test-menu-category" }), - packed: async(context) => { + packed: async context => { await doTest(context.outDir, false, "Test Menu Category", "test-menu-category", "Foo Bar") } })) @@ -158,6 +159,7 @@ test.ifAll.ifNotCiMac("string menuCategory", app({ nsis: { oneClick: false, menuCategory: "Foo/Bar", + // tslint:disable:no-invalid-template-strings artifactName: "${productName} CustomName ${version}.${ext}" }, } @@ -165,7 +167,7 @@ test.ifAll.ifNotCiMac("string menuCategory", app({ projectDirCreated: projectDir => modifyPackageJson(projectDir, data => { data.name = "test-menu-category" }), - packed: async(context) => { + packed: async context => { await doTest(context.outDir, false, "Test Menu Category", "test-menu-category", "Foo Bar") } })) @@ -185,7 +187,7 @@ test.ifDevOrLinuxCi("file associations only perMachine", appThrows({ test.ifNotCiMac("web installer", app({ targets: Platform.WINDOWS.createTarget(["nsis-web"], Arch.x64), config: { - compression: process.env.COMPRESSION || "store", + compression: process.env.COMPRESSION as any || "store", publish: { provider: "s3", bucket: "develar", diff --git a/yarn.lock b/yarn.lock index 79187f10814..ff5cbc0c748 100644 --- a/yarn.lock +++ b/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"