From 2372d007eb06ea4726a3c11a4e1c602d831b1131 Mon Sep 17 00:00:00 2001 From: Tony Anziano Date: Wed, 6 May 2020 11:46:45 -0700 Subject: [PATCH] fix: Improved Electron auto update UX (#2925) * More auto update polish. * Added more tests. Co-authored-by: Chris Whitten --- .../src/components/AppUpdater/index.tsx | 15 ++- .../electron-server/__tests__/appMenu.test.ts | 4 +- .../__tests__/appUpdater.test.ts | 75 ++++++++++++++- .../packages/electron-server/package.json | 3 +- .../packages/electron-server/src/appMenu.ts | 8 ++ .../electron-server/src/appUpdater.ts | 93 +++++++++++++++++-- Composer/packages/electron-server/src/main.ts | 6 +- Composer/yarn.lock | 5 + 8 files changed, 184 insertions(+), 25 deletions(-) diff --git a/Composer/packages/client/src/components/AppUpdater/index.tsx b/Composer/packages/client/src/components/AppUpdater/index.tsx index 3159ae38ea..a8728400a2 100644 --- a/Composer/packages/client/src/components/AppUpdater/index.tsx +++ b/Composer/packages/client/src/components/AppUpdater/index.tsx @@ -80,11 +80,16 @@ export const AppUpdater: React.FC<{}> = _props => { break; } - case 'update-not-available': - // TODO: re-enable once we have implemented explicit "check for updates" - // setAppUpdateStatus({ status: AppUpdaterStatus.UPDATE_UNAVAILABLE }); - // setAppUpdateShowing(true); + case 'update-not-available': { + const explicit = payload; + if (explicit) { + // the user has explicitly checked for an update via the Help menu; + // we should display some UI feedback if there are no updates available + setAppUpdateStatus({ status: AppUpdaterStatus.UPDATE_UNAVAILABLE }); + setAppUpdateShowing(true); + } break; + } case 'update-downloaded': setAppUpdateStatus({ status: AppUpdaterStatus.UPDATE_SUCCEEDED }); @@ -188,7 +193,7 @@ export const AppUpdater: React.FC<{}> = _props => { default: return undefined; } - }, [status, progressPercent]); + }, [status, progressPercent, error]); const footer = useMemo(() => { switch (status) { diff --git a/Composer/packages/electron-server/__tests__/appMenu.test.ts b/Composer/packages/electron-server/__tests__/appMenu.test.ts index 9585275363..d619a68150 100644 --- a/Composer/packages/electron-server/__tests__/appMenu.test.ts +++ b/Composer/packages/electron-server/__tests__/appMenu.test.ts @@ -52,7 +52,7 @@ describe('App menu', () => { // Help expect(menuTemplate[4].label).toBe('Help'); - expect(menuTemplate[4].submenu.length).toBe(10); + expect(menuTemplate[4].submenu.length).toBe(12); expect(mockSetApplicationMenu).toHaveBeenCalledTimes(1); }); @@ -87,7 +87,7 @@ describe('App menu', () => { // Help expect(menuTemplate[5].label).toBe('Help'); - expect(menuTemplate[5].submenu.length).toBe(10); + expect(menuTemplate[5].submenu.length).toBe(12); expect(mockSetApplicationMenu).toHaveBeenCalledTimes(1); }); diff --git a/Composer/packages/electron-server/__tests__/appUpdater.test.ts b/Composer/packages/electron-server/__tests__/appUpdater.test.ts index dc7d0e888b..c539005432 100644 --- a/Composer/packages/electron-server/__tests__/appUpdater.test.ts +++ b/Composer/packages/electron-server/__tests__/appUpdater.test.ts @@ -2,6 +2,7 @@ // Licensed under the MIT License. const mockAutoUpdater = { + allowDowngrade: false, checkForUpdates: jest.fn(), downloadUpdate: jest.fn(), on: jest.fn(), @@ -14,24 +15,59 @@ jest.mock('electron-updater', () => ({ }, })); +const mockGetVersion = jest.fn(() => '0.0.1'); +jest.mock('electron', () => ({ + app: { + getVersion: () => mockGetVersion(), + }, +})); + import { AppUpdater } from '../src/appUpdater'; describe('App updater', () => { let appUpdater: AppUpdater; beforeEach(() => { - appUpdater = new AppUpdater(); + mockAutoUpdater.allowDowngrade = false; + appUpdater = AppUpdater.getInstance(); + (appUpdater as any).checkingForUpdate = false; + (appUpdater as any).downloadingUpdate = false; mockAutoUpdater.checkForUpdates.mockClear(); mockAutoUpdater.checkForUpdates.mockClear(); mockAutoUpdater.downloadUpdate.mockClear(); mockAutoUpdater.on.mockClear(); mockAutoUpdater.quitAndInstall.mockClear(); mockAutoUpdater.setFeedURL.mockClear(); + mockGetVersion.mockClear(); }); - it('should check for updates', () => { - appUpdater.checkForUpdates(); + it('should check for updates from the nightly repo', () => { + const getSettingsSpy = jest.spyOn(appUpdater as any, 'getSettings').mockReturnValue({ useNightly: true }); + appUpdater.checkForUpdates(true); + + expect(mockAutoUpdater.checkForUpdates).toHaveBeenCalled(); + expect((appUpdater as any).explicitCheck).toBe(true); + expect(mockAutoUpdater.setFeedURL).toHaveBeenCalledWith({ + provider: 'github', + repo: 'BotFramework-Composer-Nightlies', + owner: 'microsoft', + vPrefixedTagName: true, + }); + getSettingsSpy.mockRestore(); + }); + + it('should check for updates from the stable repo', () => { + const getSettingsSpy = jest.spyOn(appUpdater as any, 'getSettings').mockReturnValue({ useNightly: false }); + appUpdater.checkForUpdates(true); expect(mockAutoUpdater.checkForUpdates).toHaveBeenCalled(); + expect((appUpdater as any).explicitCheck).toBe(true); + expect(mockAutoUpdater.setFeedURL).toHaveBeenCalledWith({ + provider: 'github', + repo: 'BotFramework-Composer', + owner: 'microsoft', + vPrefixedTagName: true, + }); + getSettingsSpy.mockRestore(); }); it('should not check for updates if it is already checking for an update', () => { @@ -48,6 +84,34 @@ describe('App updater', () => { expect(mockAutoUpdater.checkForUpdates).not.toHaveBeenCalled(); }); + it('should not allow a downgrade when checking for updates from nightly to (stable or nightly)', () => { + mockGetVersion.mockReturnValueOnce('0.0.1-nightly.12345.abcdef'); + mockAutoUpdater.allowDowngrade = true; + appUpdater.checkForUpdates(); + + expect(mockAutoUpdater.allowDowngrade).toBe(false); + }); + + it('should not allow a downgrade when checking for updates from stable to stable', () => { + mockGetVersion.mockReturnValueOnce('0.0.1'); + const getSettingsSpy = jest.spyOn(appUpdater as any, 'getSettings').mockReturnValue({ useNightly: false }); + mockAutoUpdater.allowDowngrade = true; + appUpdater.checkForUpdates(); + + expect(mockAutoUpdater.allowDowngrade).toBe(false); + getSettingsSpy.mockRestore(); + }); + + it('should allow a downgrade when checking for updates from stable to nightly', () => { + mockGetVersion.mockReturnValueOnce('0.0.1'); + const getSettingsSpy = jest.spyOn(appUpdater as any, 'getSettings').mockReturnValue({ useNightly: true }); + mockAutoUpdater.allowDowngrade = false; + appUpdater.checkForUpdates(); + + expect(mockAutoUpdater.allowDowngrade).toBe(true); + getSettingsSpy.mockRestore(); + }); + it('should download an update', () => { appUpdater.downloadUpdate(); @@ -86,11 +150,14 @@ describe('App updater', () => { it('should handle no available update', () => { const emitSpy = jest.spyOn(appUpdater, 'emit'); + const explicit = true; (appUpdater as any).checkingForUpdate = true; + (appUpdater as any).explicitCheck = explicit; (appUpdater as any).onUpdateNotAvailable('update info'); expect((appUpdater as any).checkingForUpdate).toBe(false); - expect(emitSpy).toHaveBeenCalledWith('update-not-available', 'update info'); + expect((appUpdater as any).explicitCheck).toBe(false); + expect(emitSpy).toHaveBeenCalledWith('update-not-available', explicit); }); it('should handle download progress', () => { diff --git a/Composer/packages/electron-server/package.json b/Composer/packages/electron-server/package.json index aa81c998ef..dacd72a416 100644 --- a/Composer/packages/electron-server/package.json +++ b/Composer/packages/electron-server/package.json @@ -59,6 +59,7 @@ "debug": "4.1.1", "electron-updater": "4.2.5", "fix-path": "^3.0.0", - "lodash": "^4.17.15" + "lodash": "^4.17.15", + "semver": "7.3.2" } } diff --git a/Composer/packages/electron-server/src/appMenu.ts b/Composer/packages/electron-server/src/appMenu.ts index 1a6d0cd0d7..dc22c24da6 100644 --- a/Composer/packages/electron-server/src/appMenu.ts +++ b/Composer/packages/electron-server/src/appMenu.ts @@ -4,6 +4,7 @@ import { app, dialog, Menu, MenuItemConstructorOptions, shell } from 'electron'; import { isMac } from './utility/platform'; +import { AppUpdater } from './appUpdater'; function getAppMenu(): MenuItemConstructorOptions[] { if (isMac()) { @@ -128,6 +129,13 @@ export function initAppMenu() { }, }, { type: 'separator' }, + { + label: 'Check for Updates', + click: () => { + AppUpdater.getInstance().checkForUpdates(true); + }, + }, + { type: 'separator' }, { label: 'About', click: async () => { diff --git a/Composer/packages/electron-server/src/appUpdater.ts b/Composer/packages/electron-server/src/appUpdater.ts index 3eeefda7f6..0f92df64c1 100644 --- a/Composer/packages/electron-server/src/appUpdater.ts +++ b/Composer/packages/electron-server/src/appUpdater.ts @@ -4,28 +4,32 @@ import { EventEmitter } from 'events'; import { autoUpdater, UpdateInfo } from 'electron-updater'; +import { app } from 'electron'; +import { prerelease as isNightly } from 'semver'; import logger from './utility/logger'; const log = logger.extend('app-updater'); +interface AppUpdaterSettings { + autoDownload: boolean; + useNightly: boolean; +} + +let appUpdater: AppUpdater | undefined; export class AppUpdater extends EventEmitter { private checkingForUpdate = false; private downloadingUpdate = false; + private explicitCheck = false; constructor() { super(); - const settings = { autoDownload: false, useNightly: false }; // TODO: implement and load these settings from disk / memory + const settings = this.getSettings(); autoUpdater.allowDowngrade = false; autoUpdater.allowPrerelease = true; autoUpdater.autoDownload = settings.autoDownload; - autoUpdater.setFeedURL({ - provider: 'github', - repo: 'BotFramework-Composer', - owner: 'microsoft', - }); - autoUpdater.on('error', this.onError); + autoUpdater.on('error', this.onError.bind(this)); autoUpdater.on('checking-for-update', this.onCheckingForUpdate.bind(this)); autoUpdater.on('update-available', this.onUpdateAvailable.bind(this)); autoUpdater.on('update-not-available', this.onUpdateNotAvailable.bind(this)); @@ -34,8 +38,23 @@ export class AppUpdater extends EventEmitter { logger('Initialized'); } - public checkForUpdates() { + public static getInstance(): AppUpdater { + if (!appUpdater) { + appUpdater = new AppUpdater(); + } + return appUpdater; + } + + /** + * Checks GitHub for Composer updates + * @param explicit If true, the user explicitly checked for an update via the Help menu, + * and we will show UI if there are no available updates. + */ + public checkForUpdates(explicit = false) { if (!(this.checkingForUpdate || this.downloadingUpdate)) { + this.explicitCheck = explicit; + this.setFeedURL(); + this.determineUpdatePath(); autoUpdater.checkForUpdates(); } } @@ -72,8 +91,8 @@ export class AppUpdater extends EventEmitter { private onUpdateNotAvailable(updateInfo: UpdateInfo) { log('Update not available: %O', updateInfo); - this.checkingForUpdate = false; - this.emit('update-not-available', updateInfo); + this.emit('update-not-available', this.explicitCheck); + this.resetToIdle(); } private onDownloadProgress(progress: any) { @@ -91,5 +110,59 @@ export class AppUpdater extends EventEmitter { log('Resetting to idle...'); this.checkingForUpdate = false; this.downloadingUpdate = false; + this.explicitCheck = false; + } + + private setFeedURL() { + const settings = this.getSettings(); + if (settings.useNightly) { + log('Updates set to be retrieved from nightly repo.'); + autoUpdater.setFeedURL({ + provider: 'github', + repo: 'BotFramework-Composer-Nightlies', + owner: 'microsoft', + vPrefixedTagName: true, // whether our release tags are prefixed with v or not + }); + } else { + log('Updates set to be retrieved from stable repo.'); + autoUpdater.setFeedURL({ + provider: 'github', + repo: 'BotFramework-Composer', + owner: 'microsoft', + vPrefixedTagName: true, + }); + } + } + + private determineUpdatePath() { + const currentVersion = app.getVersion(); + + // The following paths don't need to allow downgrade: + // nightly -> stable (1.0.1-nightly.x.x -> 1.0.2) + // nightly -> nightly (1.0.1-nightly.x.x -> 1.0.1-nightly.y.x) + if (isNightly(currentVersion)) { + log(`Updating from nightly to (stable | nightly). Not allowing downgrade.`); + autoUpdater.allowDowngrade = false; + return; + } + + // https://github.com/npm/node-semver/blob/v7.3.2/classes/semver.js#L127 + // The following path needs to allow downgrade to work: + // stable -> nightly (1.0.1 -> 1.0.1-nightly.x.x) + const settings = this.getSettings(); + if (!isNightly(currentVersion) && settings.useNightly) { + log(`Updating from stable to nightly. Allowing downgrade.`); + autoUpdater.allowDowngrade = true; + return; + } + + // stable -> stable + log('Updating from stable to stable. Not allowing downgrade.'); + autoUpdater.allowDowngrade = false; + } + + private getSettings(): AppUpdaterSettings { + // TODO: replace with actual implementation that fetches settings from disk + return { autoDownload: false, useNightly: true }; } } diff --git a/Composer/packages/electron-server/src/main.ts b/Composer/packages/electron-server/src/main.ts index 2b69106004..9ee47a4dca 100644 --- a/Composer/packages/electron-server/src/main.ts +++ b/Composer/packages/electron-server/src/main.ts @@ -63,7 +63,7 @@ function initializeAppUpdater() { log('Initializing app updater...'); const mainWindow = ElectronWindow.getInstance().browserWindow; if (mainWindow) { - const appUpdater = new AppUpdater(); + const appUpdater = AppUpdater.getInstance(); appUpdater.on('update-available', (updateInfo: UpdateInfo) => { // TODO: if auto/silent download is enabled in settings, don't send this event. // instead, just download silently @@ -72,8 +72,8 @@ function initializeAppUpdater() { appUpdater.on('progress', progress => { mainWindow.webContents.send('app-update', 'progress', progress); }); - appUpdater.on('update-not-available', () => { - mainWindow.webContents.send('app-update', 'update-not-available'); + appUpdater.on('update-not-available', (explicitCheck: boolean) => { + mainWindow.webContents.send('app-update', 'update-not-available', explicitCheck); }); appUpdater.on('update-downloaded', () => { mainWindow.webContents.send('app-update', 'update-downloaded'); diff --git a/Composer/yarn.lock b/Composer/yarn.lock index 8bacf3a47e..6ada943164 100644 --- a/Composer/yarn.lock +++ b/Composer/yarn.lock @@ -16167,6 +16167,11 @@ semver@7.0.0: resolved "https://registry.yarnpkg.com/semver/-/semver-7.0.0.tgz#5f3ca35761e47e05b206c6daff2cf814f0316b8e" integrity sha512-+GB6zVA9LWh6zovYQLALHwv5rb2PHGlJi3lfiqIHxR0uuwCgefcOJc59v9fv1w8GbStwxuuqqAjI9NMAOOgq1A== +semver@7.3.2: + version "7.3.2" + resolved "https://registry.yarnpkg.com/semver/-/semver-7.3.2.tgz#604962b052b81ed0786aae84389ffba70ffd3938" + integrity sha512-OrOb32TeeambH6UrhtShmF7CRDqhL6/5XpPNp2DuRH6+9QLw/orhp72j87v8Qa1ScDkvrrBNpZcDejAirJmfXQ== + semver@^5.0.1: version "5.7.0" resolved "https://registry.yarnpkg.com/semver/-/semver-5.7.0.tgz#790a7cf6fea5459bac96110b29b60412dc8ff96b"