Reduce the number of writes to the config (#2498)

* Add some logging, refactor setting config items

* Move active team to setMultiple, reduce serverInfos calls if the data is already the same

* Fix some logging

* Lint fix

src/common/config/index.ts

@@ -102,7 +102,7 @@ export class Config extends EventEmitter {
         ipcMain.handle(GET_CONFIGURATION, this.handleGetConfiguration);
         ipcMain.handle(GET_LOCAL_CONFIGURATION, this.handleGetLocalConfiguration);
         ipcMain.handle(UPDATE_TEAMS, this.handleUpdateTeams);
-        ipcMain.on(UPDATE_CONFIGURATION, this.setMultiple);
+        ipcMain.on(UPDATE_CONFIGURATION, this.updateConfiguration);
         if (process.platform === 'darwin' || process.platform === 'win32') {
             nativeTheme.on('updated', this.handleUpdateTheme);
         }
@@ -151,16 +151,22 @@ export class Config extends EventEmitter {
      * @param {*} data value to save for provided key
      */
     set = (key: keyof ConfigType, data: ConfigType[keyof ConfigType]): void => {
-        if (key && this.localConfigData) {
+        log.debug('Config.set');
-            if (key === 'teams') {
+        this.setMultiple({[key]: data});
-                this.localConfigData.teams = this.filterOutPredefinedTeams(data as TeamWithTabs[]);
+    }
-                this.predefinedTeams = this.filterInPredefinedTeams(data as TeamWithTabs[]);
+
-            } else {
+    updateConfiguration = (event: Electron.IpcMainEvent, properties: Array<{key: keyof ConfigType; data: ConfigType[keyof ConfigType]}> = []): Partial<ConfigType> | undefined => {
-                this.localConfigData = Object.assign({}, this.localConfigData, {[key]: data});
+        log.debug('Config.updateConfiguration', properties);
-            }
+
-            this.regenerateCombinedConfigData();
+        if (properties.length) {
-            this.saveLocalConfigData();
+            const newData = properties.reduce((obj, data) => {
+                (obj as any)[data.key] = data.data;
+                return obj;
+            }, {} as Partial<ConfigType>);
+            this.setMultiple(newData);
         }
+
+        return this.localConfigData;
     }
 
     /**
@@ -168,14 +174,16 @@ export class Config extends EventEmitter {
      *
      * @param {array} properties an array of config properties to save
      */
-    setMultiple = (event: Electron.IpcMainEvent, properties: Array<{key: keyof ConfigType; data: ConfigType[keyof ConfigType]}> = []): Partial<ConfigType> | undefined => {
+    setMultiple = (newData: Partial<ConfigType>) => {
-        log.debug('Config.setMultiple', properties);
+        log.debug('Config.setMultiple', newData);
 
-        if (properties.length) {
+        this.localConfigData = Object.assign({}, this.localConfigData, newData);
-            this.localConfigData = Object.assign({}, this.localConfigData, ...properties.map(({key, data}) => ({[key]: data})));
+        if (newData.teams && this.localConfigData) {
-            this.regenerateCombinedConfigData();
+            this.localConfigData.teams = this.filterOutPredefinedTeams(newData.teams as TeamWithTabs[]);
-            this.saveLocalConfigData();
+            this.predefinedTeams = this.filterInPredefinedTeams(newData.teams as TeamWithTabs[]);
         }
+        this.regenerateCombinedConfigData();
+        this.saveLocalConfigData();
 
         return this.localConfigData; //this is the only part that changes
     }
@@ -211,6 +219,8 @@ export class Config extends EventEmitter {
             return;
         }
 
+        log.info('Saving config data to file...');
+
         try {
             this.writeFile(this.configFilePath, this.localConfigData, (error: NodeJS.ErrnoException | null) => {
                 if (error) {

src/main/app/config.ts

@@ -24,6 +24,13 @@ let didCheckForAddServerModal = false;
 //
 
 export function handleConfigUpdate(newConfig: CombinedConfig) {
+    if (log.transports.file.level !== newConfig.logLevel) {
+        log.error('Log level set to:', newConfig.logLevel);
+    }
+    if (newConfig.logLevel) {
+        setLoggingLevel(newConfig.logLevel as LogLevel);
+    }
+
     log.debug('App.Config.handleConfigUpdate');
     log.silly('App.Config.handleConfigUpdate', newConfig);
 
@@ -63,9 +70,6 @@ export function handleConfigUpdate(newConfig: CombinedConfig) {
         handleMainWindowIsShown();
     }
 
-    log.info('Log level set to:', newConfig.logLevel);
-    setLoggingLevel(newConfig.logLevel as LogLevel);
-
     handleUpdateMenuEvent();
     if (newConfig.trayIconTheme) {
         refreshTrayImages(newConfig.trayIconTheme);

src/main/app/intercom.ts

@@ -321,8 +321,10 @@ export function handleUpdateLastActive(event: IpcMainEvent, serverName: string,
             team.lastActiveTab = viewOrder;
         }
     });
-    Config.set('teams', teams);
+    Config.setMultiple({
-    Config.set('lastActiveTeam', teams.find((team) => team.name === serverName)?.order || 0);
+        teams,
+        lastActiveTeam: teams.find((team) => team.name === serverName)?.order || 0,
+    });
 }
 
 export function handlePingDomain(event: IpcMainInvokeEvent, url: string): Promise<string> {

src/main/app/utils.ts

@@ -50,6 +50,7 @@ export function updateSpellCheckerLocales() {
 }
 
 export function updateServerInfos(teams: TeamWithTabs[]) {
+    log.silly('app.utils.updateServerInfos');
     const serverInfos: Array<Promise<RemoteInfo | string | undefined>> = [];
     teams.forEach((team) => {
         const serverInfo = new ServerInfo(new MattermostServer(team.name, team.url));
@@ -57,11 +58,14 @@ export function updateServerInfos(teams: TeamWithTabs[]) {
     });
     Promise.all(serverInfos).then((data: Array<RemoteInfo | string | undefined>) => {
         const teams = Config.teams;
+        let hasUpdates = false;
         teams.forEach((team) => {
-            updateServerURL(data, team);
+            hasUpdates = hasUpdates || updateServerURL(data, team);
-            openExtraTabs(data, team);
+            hasUpdates = hasUpdates || openExtraTabs(data, team);
         });
-        Config.set('teams', teams);
+        if (hasUpdates) {
+            Config.set('teams', teams);
+        }
     }).catch((reason: any) => {
         log.error('Error getting server infos', reason);
     });
@@ -69,27 +73,33 @@ export function updateServerInfos(teams: TeamWithTabs[]) {
 
 function updateServerURL(data: Array<RemoteInfo | string | undefined>, team: TeamWithTabs) {
     const remoteInfo = data.find((info) => info && typeof info !== 'string' && info.name === team.name) as RemoteInfo;
-    if (remoteInfo && remoteInfo.siteURL) {
+    if (remoteInfo && remoteInfo.siteURL && team.url !== remoteInfo.siteURL) {
         team.url = remoteInfo.siteURL;
+        return true;
     }
+    return false;
 }
 
 function openExtraTabs(data: Array<RemoteInfo | string | undefined>, team: TeamWithTabs) {
+    let hasUpdates = false;
     const remoteInfo = data.find((info) => info && typeof info !== 'string' && info.name === team.name) as RemoteInfo;
     if (remoteInfo) {
         team.tabs.forEach((tab) => {
             if (tab.name !== TAB_MESSAGING && remoteInfo.serverVersion && Utils.isVersionGreaterThanOrEqualTo(remoteInfo.serverVersion, '6.0.0')) {
-                if (tab.name === TAB_PLAYBOOKS && remoteInfo.hasPlaybooks && tab.isOpen !== false) {
+                if (tab.name === TAB_PLAYBOOKS && remoteInfo.hasPlaybooks && typeof tab.isOpen === 'undefined') {
                     log.info(`opening ${team.name}___${tab.name} on hasPlaybooks`);
                     tab.isOpen = true;
+                    hasUpdates = true;
                 }
-                if (tab.name === TAB_FOCALBOARD && remoteInfo.hasFocalboard && tab.isOpen !== false) {
+                if (tab.name === TAB_FOCALBOARD && remoteInfo.hasFocalboard && typeof tab.isOpen === 'undefined') {
                     log.info(`opening ${team.name}___${tab.name} on hasFocalboard`);
                     tab.isOpen = true;
+                    hasUpdates = true;
                 }
             }
         });
     }
+    return hasUpdates;
 }
 
 export function handleUpdateMenuEvent() {

src/main/views/MattermostView.ts

@@ -85,7 +85,7 @@ export class MattermostView extends EventEmitter {
         this.view = new BrowserView(this.options);
         this.resetLoadingStatus();
 
-        log.info(`BrowserView created for server ${this.tab.name}`);
+        log.verbose(`BrowserView created for server ${this.tab.name}`);
 
         this.hasBeenShown = false;
 
@@ -164,7 +164,7 @@ export class MattermostView extends EventEmitter {
         } else {
             loadURL = this.tab.url.toString();
         }
-        log.info(`[${Util.shorten(this.tab.name)}] Loading ${loadURL}`);
+        log.verbose(`[${Util.shorten(this.tab.name)}] Loading ${loadURL}`);
         const loading = this.view.webContents.loadURL(loadURL, {userAgent: composeUserAgent()});
         loading.then(this.loadSuccess(loadURL)).catch((err) => {
             if (err.code && err.code.startsWith('ERR_CERT')) {
@@ -220,7 +220,7 @@ export class MattermostView extends EventEmitter {
 
     loadSuccess = (loadURL: string) => {
         return () => {
-            log.info(`[${Util.shorten(this.tab.name)}] finished loading ${loadURL}`);
+            log.verbose(`[${Util.shorten(this.tab.name)}] finished loading ${loadURL}`);
             WindowManager.sendToRenderer(LOAD_SUCCESS, this.tab.name);
             this.maxRetries = MAX_SERVER_RETRIES;
             if (this.status === Status.LOADING) {

src/main/views/viewManager.ts

@@ -127,6 +127,8 @@ export class ViewManager {
      * close, open, or reload tabs, taking care to reuse tabs and
      * preserve focus on the currently selected tab. */
     reloadConfiguration = (configServers: TeamWithTabs[]) => {
+        log.debug('viewManager.reloadConfiguration');
+
         const focusedTuple: TabTuple | undefined = this.views.get(this.currentView as string)?.urlTypeTuple;
 
         const current: Map<TabTuple, MattermostView> = new Map();
@@ -199,6 +201,8 @@ export class ViewManager {
     }
 
     showInitial = () => {
+        log.verbose('viewManager.showInitial');
+
         const servers = this.getServers();
         if (servers.length) {
             const element = servers.find((e) => e.order === this.lastActiveServer) || servers.find((e) => e.order === 0);

src/main/windows/windowManager.ts

@@ -590,6 +590,7 @@ export class WindowManager {
     }
 
     switchServer = (serverName: string, waitForViewToExist = false) => {
+        log.debug('windowManager.switchServer');
         this.showMainWindow();
         const server = Config.teams.find((team) => team.name === serverName);
         if (!server) {
@@ -617,6 +618,7 @@ export class WindowManager {
     }
 
     switchTab = (serverName: string, tabName: string) => {
+        log.debug('windowManager.switchTab');
         this.showMainWindow();
         const tabViewName = getTabViewName(serverName, tabName);
         this.viewManager?.showByName(tabViewName);