cache config files when read from disk (#139)

* cache config files when read from disk

* config updates from code review

js/config.js

@@ -9,6 +9,11 @@ const isMac = require('./utils/misc.js').isMac;
 const getRegistry = require('./utils/getRegistry.js');
 const configFileName = 'Symphony.config';
 
+// cached config when first reading files. initially undefined and will be
+// updated when read from disk.
+let userConfig;
+let globalConfig;
+
 /**
  * Tries to read given field from user config file, if field doesn't exist
  * then tries reading from global config. User config is stord in directory:
@@ -45,21 +50,25 @@ function getUserConfigField(fieldName) {
 
 function readUserConfig() {
     return new Promise(function(resolve, reject) {
+        if (userConfig) {
+            resolve(userConfig);
+            return;
+        }
+
         let configPath = path.join(app.getPath('userData'), configFileName);
 
         fs.readFile(configPath, 'utf8', function(err, data) {
             if (err) {
                 reject('cannot open user config file: ' + configPath + ', error: ' + err);
             } else {
-                let config = {};
                 try {
                     // data is the contents of the text file we just read
-                    config = JSON.parse(data);
+                    userConfig = JSON.parse(data);
                 } catch (e) {
                     reject('can not parse user config file data: ' + data + ', error: ' + err);
+                    return;
                 }
-
+                resolve(userConfig);
-                resolve(config);
             }
         });
     });
@@ -85,6 +94,11 @@ function getGlobalConfigField(fieldName) {
  */
 function readGlobalConfig() {
     return new Promise(function(resolve, reject) {
+        if (globalConfig) {
+            resolve(globalConfig);
+            return;
+        }
+
         let configPath;
         let globalConfigFileName = path.join('config', configFileName);
         if (isDevEnv) {
@@ -103,19 +117,18 @@ function readGlobalConfig() {
             if (err) {
                 reject('cannot open global config file: ' + configPath + ', error: ' + err);
             } else {
-                let config = {};
                 try {
                     // data is the contents of the text file we just read
-                    config = JSON.parse(data);
+                    globalConfig = JSON.parse(data);
                 } catch (e) {
                     reject('can not parse config file data: ' + data + ', error: ' + err);
                 }
                 getRegistry('PodUrl')
-                .then(function(url){
+                .then(function(url) {
-                    config.url = url;
+                    globalConfig.url = url;
-                    resolve(config);
+                    resolve(globalConfig);
-                }).catch(function (){
+                }).catch(function () {
-                    resolve(config);
+                    resolve(globalConfig);
                 });
             }
         });
@@ -124,9 +137,9 @@ function readGlobalConfig() {
 
 /**
  * Updates user config with given field with new value
- * @param  {String} fieldName [description]
+ * @param  {String} fieldName  Name of field in config to be added/changed.
- * @param  {Object} newValue  object to replace given value
+ * @param  {Object} newValue   Object to replace given value
- * @return {[type]}           [description]
+ * @return {Promise}           Promise that resolves/rejects when file write is complete.
  */
 function updateConfigField(fieldName, newValue) {
     return readUserConfig()
@@ -135,7 +148,11 @@ function updateConfigField(fieldName, newValue) {
     },
     function() {
         // in case config doesn't exist, can't read or is corrupted.
-        return saveUserConfig(fieldName, newValue, {});
+        // add configVersion - just in case in future we need to provide
+        // upgrade capabilities.
+        return saveUserConfig(fieldName, newValue, {
+            configVersion: '1.0.0'
+        });
     });
 }
 
@@ -158,15 +175,24 @@ function saveUserConfig(fieldName, newValue, oldConfig) {
             if (err) {
                 reject(err);
             } else {
+                userConfig = newConfig;
                 resolve(newConfig);
             }
         });
     });
 }
 
+function clearCachedConfigs() {
+    userConfig = null;
+    globalConfig = null;
+}
+
 module.exports = {
     getConfigField,
     updateConfigField,
     configFileName,
-    saveUserConfig // Exporting this for unit tests
+
+    // items below here are only exported for testing, do NOT use!
+    saveUserConfig,
+    clearCachedConfigs
 };

tests/config.test.js

@@ -1,4 +1,4 @@
-const { getConfigField, updateConfigField, configFileName, saveUserConfig } = require('../js/config');
+const { clearCachedConfigs, getConfigField, updateConfigField, configFileName, saveUserConfig } = require('../js/config');
 const fs = require('fs');
 const path = require('path');
 const os = require('os');
@@ -33,12 +33,15 @@ function mockedGetPath(type) {
     return '';
 }
 
-describe('getConfigField tests', function() {
+describe('read/write config tests', function() {
 
     beforeEach(function() {
         /// reset module vars between running tests.
         globalConfigDir = null;
         userConfigDir = null;
+
+        // reset module values so each test starts clean.
+        clearCachedConfigs();
     });
 
     afterEach(function() {
@@ -137,7 +140,6 @@ describe('getConfigField tests', function() {
         });
 
         it('should fail when global config path is invalid', function() {
-
             var globalConfig = {
                 url: 'something-else'
             };
@@ -150,11 +152,9 @@ describe('getConfigField tests', function() {
                 globalConfigDir = correctConfigDir;
                 expect(err).toBeTruthy();
             });
-
         });
 
         it('should fail when user config path is invalid', function() {
-
             var userConfig = {
                 url: 'something'
             };
@@ -167,8 +167,54 @@ describe('getConfigField tests', function() {
                 userConfigDir = correctConfigDir;
                 expect(err).toBeTruthy();
             });
-
         });
+
+        it('should read cached user config value rather than reading file from disk again', function(done) {
+            var userConfig = {
+                url: 'qa4.symphony.com'
+            };
+            createTempUserConfig(userConfig);
+
+            var userConfig2 = {
+                url: 'qa5.symphony.com'
+            };
+
+            return getConfigField('url')
+                .then(function() {
+                    createTempUserConfig(userConfig2);
+                })
+                .then(function() {
+                    return getConfigField('url')
+                })
+                .then(function(url) {
+                    expect(url).toBe('qa4.symphony.com');
+                    done();
+                });
+        });
+
+        it('should read cache global config value rather than reading file from disk again', function(done) {
+            var globalConfig = {
+                url: 'qa8.symphony.com'
+            };
+            createTempGlobalConfig(globalConfig);
+
+            var globalConfig2 = {
+                url: 'qa9.symphony.com'
+            };
+
+            return getConfigField('url')
+                .then(function() {
+                    createTempGlobalConfig(globalConfig2);
+                })
+                .then(function() {
+                    return getConfigField('url')
+                })
+                .then(function(url) {
+                    expect(url).toBe('qa8.symphony.com');
+                    done();
+                });
+        });
+
     });
 
     describe('updateConfigField tests', function() {