fix: update.json files ignored (#1861)

* Fix update.json parsing

* Isolate configuration merging strategy

* Add unit tests

src/updates/common.ts

@@ -5,3 +5,15 @@ export type UpdateConfiguration = {
   isUpdatingEnabled: boolean;
   skippedUpdateVersion: string | null;
 };
+
+export type AppLevelUpdateConfiguration = {
+  forced?: boolean;
+  canUpdate?: boolean;
+  autoUpdate?: boolean;
+  skip?: string | null;
+};
+
+export type UserLevelUpdateConfiguration = {
+  autoUpdate?: boolean;
+  skip?: string | null;
+};

src/updates/main.spec.ts

@@ -0,0 +1,111 @@
+import {
+  AppLevelUpdateConfiguration,
+  UpdateConfiguration,
+  UserLevelUpdateConfiguration,
+} from './common';
+import { mergeConfigurations } from './main';
+
+describe('mergeConfigurations', () => {
+  it('keeps default configuration', () => {
+    const defaultConfiguration: UpdateConfiguration = {
+      doCheckForUpdatesOnStartup: true,
+      isEachUpdatesSettingConfigurable: true,
+      isUpdatingAllowed: true,
+      isUpdatingEnabled: true,
+      skippedUpdateVersion: null,
+    };
+    const appConfiguration: AppLevelUpdateConfiguration = {};
+    const userConfiguration: UserLevelUpdateConfiguration = {};
+
+    expect(mergeConfigurations(
+      defaultConfiguration,
+      appConfiguration,
+      userConfiguration,
+    )).toStrictEqual(defaultConfiguration);
+  });
+
+  it('merges app configuration', () => {
+    const defaultConfiguration: UpdateConfiguration = {
+      doCheckForUpdatesOnStartup: true,
+      isEachUpdatesSettingConfigurable: true,
+      isUpdatingAllowed: true,
+      isUpdatingEnabled: true,
+      skippedUpdateVersion: null,
+    };
+    const appConfiguration: AppLevelUpdateConfiguration = {
+      autoUpdate: false,
+      canUpdate: false,
+    };
+    const userConfiguration: UserLevelUpdateConfiguration = {};
+
+    expect(mergeConfigurations(
+      defaultConfiguration,
+      appConfiguration,
+      userConfiguration,
+    )).toStrictEqual({
+      ...defaultConfiguration,
+      doCheckForUpdatesOnStartup: false,
+      isUpdatingEnabled: false,
+    });
+  });
+
+  it('merges user configuration', () => {
+    const defaultConfiguration: UpdateConfiguration = {
+      doCheckForUpdatesOnStartup: true,
+      isEachUpdatesSettingConfigurable: true,
+      isUpdatingAllowed: true,
+      isUpdatingEnabled: true,
+      skippedUpdateVersion: null,
+    };
+    const appConfiguration: AppLevelUpdateConfiguration = {
+      autoUpdate: false,
+      canUpdate: false,
+    };
+    const userConfiguration: UserLevelUpdateConfiguration = {
+      autoUpdate: true,
+      skip: 'x.y.z',
+    };
+
+    expect(mergeConfigurations(
+      defaultConfiguration,
+      appConfiguration,
+      userConfiguration,
+    )).toStrictEqual({
+      ...defaultConfiguration,
+      doCheckForUpdatesOnStartup: true,
+      isUpdatingEnabled: false,
+      skippedUpdateVersion: 'x.y.z',
+    });
+  });
+
+  it('may force app configuration', () => {
+    const defaultConfiguration: UpdateConfiguration = {
+      doCheckForUpdatesOnStartup: true,
+      isEachUpdatesSettingConfigurable: true,
+      isUpdatingAllowed: true,
+      isUpdatingEnabled: true,
+      skippedUpdateVersion: null,
+    };
+    const appConfiguration: AppLevelUpdateConfiguration = {
+      forced: true,
+      autoUpdate: false,
+      canUpdate: false,
+    };
+    const userConfiguration: UserLevelUpdateConfiguration = {
+      autoUpdate: true,
+      skip: 'x.y.z',
+    };
+
+    expect(mergeConfigurations(
+      defaultConfiguration,
+      appConfiguration,
+      userConfiguration,
+    )).toStrictEqual({
+      ...defaultConfiguration,
+      isEachUpdatesSettingConfigurable: false,
+      doCheckForUpdatesOnStartup: false,
+      isUpdatingEnabled: false,
+      skippedUpdateVersion: 'x.y.z',
+    });
+  });
+});

src/updates/main.ts

@@ -6,7 +6,10 @@ import { autoUpdater } from 'electron-updater';
 
 import { listen, dispatch, select } from '../store';
 import { RootState } from '../store/rootReducer';
-import { UPDATE_DIALOG_SKIP_UPDATE_CLICKED, UPDATE_DIALOG_INSTALL_BUTTON_CLICKED } from '../ui/actions';
+import {
+  UPDATE_DIALOG_SKIP_UPDATE_CLICKED,
+  UPDATE_DIALOG_INSTALL_BUTTON_CLICKED,
+} from '../ui/actions';
 import {
   askUpdateInstall,
   AskUpdateInstallResponse,
@@ -23,40 +26,67 @@ import {
   UPDATES_NEW_VERSION_NOT_AVAILABLE,
   UPDATES_READY,
 } from './actions';
-import { UpdateConfiguration } from './common';
+import { AppLevelUpdateConfiguration, UpdateConfiguration, UserLevelUpdateConfiguration } from './common';
 
-const loadAppConfiguration = async (): Promise<Record<string, unknown>> => {
+const readJsonObject = async (filePath: string): Promise<Record<string, unknown>> => {
   try {
-    const filePath = path.join(
-      app.getAppPath(),
-      app.getAppPath().endsWith('app.asar') ? '..' : '.',
-      'update.json',
-    );
     const content = await fs.promises.readFile(filePath, 'utf8');
     const json = JSON.parse(content);
 
-    return json && typeof json === 'object' ? json : {};
+    return json && typeof json === 'object' && !Array.isArray(json) ? json : {};
   } catch (error) {
     return {};
   }
 };
 
-const loadUserConfiguration = async (): Promise<Record<string, unknown>> => {
-  try {
-    const filePath = path.join(app.getPath('userData'), 'update.json');
-    const content = await fs.promises.readFile(filePath, 'utf8');
-    const json = JSON.parse(content);
-    await fs.promises.unlink(filePath);
+const readAppJsonObject = async (basename: string): Promise<Record<string, unknown>> => {
+  const filePath = path.join(
+    app.getAppPath(),
+    app.getAppPath().endsWith('app.asar') ? '..' : '.',
+    basename,
+  );
+  return readJsonObject(filePath);
+};
 
-    return json && typeof json === 'object' ? json : {};
-  } catch (error) {
-    return {};
+const readUserJsonObject = async (basename: string): Promise<Record<string, unknown>> => {
+  const filePath = path.join(app.getPath('userData'), basename);
+  return readJsonObject(filePath);
+};
+
+const loadAppConfiguration = async (): Promise<AppLevelUpdateConfiguration> =>
+  readAppJsonObject('update.json');
+
+const loadUserConfiguration = async (): Promise<UserLevelUpdateConfiguration> =>
+  readUserJsonObject('update.json');
+
+export const mergeConfigurations = (
+  defaultConfiguration: UpdateConfiguration,
+  appConfiguration: AppLevelUpdateConfiguration,
+  userConfiguration: UserLevelUpdateConfiguration,
+): UpdateConfiguration => {
+  const configuration = {
+    ...defaultConfiguration,
+    ...typeof appConfiguration.forced === 'boolean' && { isEachUpdatesSettingConfigurable: !appConfiguration.forced },
+    ...typeof appConfiguration.canUpdate === 'boolean' && { isUpdatingEnabled: appConfiguration.canUpdate },
+    ...typeof appConfiguration.autoUpdate === 'boolean' && { doCheckForUpdatesOnStartup: appConfiguration.autoUpdate },
+    ...typeof appConfiguration.skip === 'string' && { skippedUpdateVersion: appConfiguration.skip },
+  };
+
+  if (typeof userConfiguration.autoUpdate === 'boolean'
+    && (configuration.isEachUpdatesSettingConfigurable || typeof appConfiguration.autoUpdate === 'undefined')) {
+    configuration.doCheckForUpdatesOnStartup = userConfiguration.autoUpdate;
+  }
+
+  if (typeof userConfiguration.skip === 'string'
+    && (configuration.isEachUpdatesSettingConfigurable || typeof appConfiguration.skip === 'undefined')) {
+    configuration.skippedUpdateVersion = userConfiguration.skip;
   }
+
+  return configuration;
 };
 
 const loadConfiguration = async (): Promise<UpdateConfiguration> => {
   const defaultConfiguration = select(({
-    isEachUpdatesSettingConfigurable,
     isUpdatingEnabled,
     doCheckForUpdatesOnStartup,
     skippedUpdateVersion,
@@ -65,34 +95,19 @@ const loadConfiguration = async (): Promise<UpdateConfiguration> => {
       (process.platform === 'linux' && !!process.env.APPIMAGE)
         || (process.platform === 'win32' && !process.windowsStore)
         || (process.platform === 'darwin' && !process.mas),
-    isEachUpdatesSettingConfigurable,
+    isEachUpdatesSettingConfigurable: true,
     isUpdatingEnabled,
     doCheckForUpdatesOnStartup,
     skippedUpdateVersion,
   }));
   const appConfiguration = await loadAppConfiguration();
+  const userConfiguration = await loadUserConfiguration();
 
-  const configuration = {
-    ...defaultConfiguration,
-    ...appConfiguration.forced ? { isEachUpdatesSettingConfigurable: false } : {},
-    ...appConfiguration.canUpdate ? { doCheckForUpdatesOnStartup: true } : {},
-    ...appConfiguration.autoUpdate ? { doCheckForUpdatesOnStartup: true } : {},
-    ...appConfiguration.skip ? { skippedUpdateVersion: String(appConfiguration.skip) } : {},
-  };
-
-  if (configuration.isEachUpdatesSettingConfigurable) {
-    const userConfiguration = await loadUserConfiguration();
-
-    if (userConfiguration.autoUpdate) {
-      configuration.doCheckForUpdatesOnStartup = true;
-    }
-
-    if (userConfiguration.skip) {
-      configuration.skippedUpdateVersion = String(userConfiguration.skip);
-    }
-  }
-
-  return configuration;
+  return mergeConfigurations(
+    defaultConfiguration,
+    appConfiguration,
+    userConfiguration,
+  );
 };
 
 export const setupUpdates = async (): Promise<void> => {