From 447efc70966b7ee5cda0e4546dbe9e6062486694 Mon Sep 17 00:00:00 2001
From: Jonas Hermsmeier <jhermsmeier@gmail.com>
Date: Tue, 22 May 2018 20:53:07 +0200
Subject: [PATCH 1/8] refactor: Move shared/store.js -> gui/app/models/store.js

---
 lib/gui/app/app.js                          |  2 +-
 lib/gui/app/models/settings.js              |  2 +-
 lib/{shared => gui/app/models}/store.js     | 14 +++++++-------
 lib/gui/app/pages/main/controllers/flash.js |  2 +-
 lib/shared/models/available-drives.js       |  2 +-
 lib/shared/models/flash-state.js            |  2 +-
 lib/shared/models/selection-state.js        |  2 +-
 tests/gui/models/settings.spec.js           |  2 +-
 8 files changed, 14 insertions(+), 14 deletions(-)
 rename lib/{shared => gui/app/models}/store.js (97%)

diff --git a/lib/gui/app/app.js b/lib/gui/app/app.js
index 2f1bb576..c56fa71f 100644
--- a/lib/gui/app/app.js
+++ b/lib/gui/app/app.js
@@ -33,7 +33,7 @@ const EXIT_CODES = require('../../shared/exit-codes')
 const messages = require('../../shared/messages')
 const s3Packages = require('../../shared/s3-packages')
 const release = require('../../shared/release')
-const store = require('../../shared/store')
+const store = require('./models/store')
 const errors = require('../../shared/errors')
 const packageJSON = require('../../../package.json')
 const flashState = require('../../shared/models/flash-state')
diff --git a/lib/gui/app/models/settings.js b/lib/gui/app/models/settings.js
index 48ce87dd..a215e745 100644
--- a/lib/gui/app/models/settings.js
+++ b/lib/gui/app/models/settings.js
@@ -23,7 +23,7 @@
 const _ = require('lodash')
 const Bluebird = require('bluebird')
 const localSettings = require('./local-settings')
-const store = require('../../../shared/store')
+const store = require('./store')
 const errors = require('../../../shared/errors')
 
 /**
diff --git a/lib/shared/store.js b/lib/gui/app/models/store.js
similarity index 97%
rename from lib/shared/store.js
rename to lib/gui/app/models/store.js
index d4d1be29..b78fb4a8 100644
--- a/lib/shared/store.js
+++ b/lib/gui/app/models/store.js
@@ -20,13 +20,13 @@ const Immutable = require('immutable')
 const _ = require('lodash')
 const redux = require('redux')
 const uuidV4 = require('uuid/v4')
-const constraints = require('./drive-constraints')
-const supportedFormats = require('./supported-formats')
-const errors = require('./errors')
-const release = require('./release')
-const fileExtensions = require('./file-extensions')
-const utils = require('./utils')
-const packageJSON = require('../../package.json')
+const constraints = require('../../../shared/drive-constraints')
+const supportedFormats = require('../../../shared/supported-formats')
+const errors = require('../../../shared/errors')
+const release = require('../../../shared/release')
+const fileExtensions = require('../../../shared/file-extensions')
+const utils = require('../../../shared/utils')
+const packageJSON = require('../../../../package.json')
 
 /**
  * @summary Verify and throw if any state fields are nil
diff --git a/lib/gui/app/pages/main/controllers/flash.js b/lib/gui/app/pages/main/controllers/flash.js
index bc3f0918..f9223baf 100644
--- a/lib/gui/app/pages/main/controllers/flash.js
+++ b/lib/gui/app/pages/main/controllers/flash.js
@@ -25,7 +25,7 @@ const notification = require('../../../os/notification')
 const exceptionReporter = require('../../../modules/exception-reporter')
 const imageWriter = require('../../../modules/image-writer')
 const path = require('path')
-const store = require('../../../../../shared/store')
+const store = require('../../../models/store')
 const constraints = require('../../../../../shared/drive-constraints')
 const availableDrives = require('../../../../../shared/models/available-drives')
 
diff --git a/lib/shared/models/available-drives.js b/lib/shared/models/available-drives.js
index 4d73b352..c324cd57 100644
--- a/lib/shared/models/available-drives.js
+++ b/lib/shared/models/available-drives.js
@@ -17,7 +17,7 @@
 'use strict'
 
 const _ = require('lodash')
-const store = require('../store')
+const store = require('../../gui/app/models/store')
 
 /**
  * @summary Check if there are available drives
diff --git a/lib/shared/models/flash-state.js b/lib/shared/models/flash-state.js
index 8ae20d10..78630245 100644
--- a/lib/shared/models/flash-state.js
+++ b/lib/shared/models/flash-state.js
@@ -17,7 +17,7 @@
 'use strict'
 
 const _ = require('lodash')
-const store = require('../store')
+const store = require('../../gui/app/models/store')
 const units = require('../units')
 
 /**
diff --git a/lib/shared/models/selection-state.js b/lib/shared/models/selection-state.js
index 245d12fb..58d470fb 100644
--- a/lib/shared/models/selection-state.js
+++ b/lib/shared/models/selection-state.js
@@ -17,7 +17,7 @@
 'use strict'
 
 const _ = require('lodash')
-const store = require('../store')
+const store = require('../../gui/app/models/store')
 const availableDrives = require('./available-drives')
 
 /**
diff --git a/tests/gui/models/settings.spec.js b/tests/gui/models/settings.spec.js
index 23d91c4d..3fbfd272 100644
--- a/tests/gui/models/settings.spec.js
+++ b/tests/gui/models/settings.spec.js
@@ -19,7 +19,7 @@
 const m = require('mochainon')
 const _ = require('lodash')
 const Bluebird = require('bluebird')
-const store = require('../../../lib/shared/store')
+const store = require('../../../lib/gui/app/models/store')
 const settings = require('../../../lib/gui/app/models/settings')
 const localSettings = require('../../../lib/gui/app/models/local-settings')
 

From 2a6670a4046732a3b6387f10143cd7c22345b391 Mon Sep 17 00:00:00 2001
From: Jonas Hermsmeier <jhermsmeier@gmail.com>
Date: Wed, 23 May 2018 17:07:04 +0200
Subject: [PATCH 2/8] refactor: Remove use of localStorage for local settings

Change-Type: minor
---
 lib/gui/app/models/local-settings.js | 74 +++++++++++++++++---------
 lib/gui/app/models/settings.js       | 77 ++++++++++------------------
 lib/gui/app/models/store.js          | 44 +---------------
 3 files changed, 76 insertions(+), 119 deletions(-)

diff --git a/lib/gui/app/models/local-settings.js b/lib/gui/app/models/local-settings.js
index b95e7990..69c59a41 100644
--- a/lib/gui/app/models/local-settings.js
+++ b/lib/gui/app/models/local-settings.js
@@ -16,20 +16,24 @@
 
 'use strict'
 
+const electron = require('electron')
 const Bluebird = require('bluebird')
 const _ = require('lodash')
 const fs = require('fs')
 const path = require('path')
 const os = require('os')
-const Storage = require('./storage')
 
 /**
- * @summary Local storage settings key
+ * @summary Userdata directory path
+ * @description
+ * Defaults to the following:
+ * - `%APPDATA%/etcher` on Windows
+ * - `$XDG_CONFIG_HOME/etcher` or `~/.config/etcher` on Linux
+ * - `~/Library/Application Support/etcher` on macOS
  * @constant
  * @type {String}
  */
-const LOCAL_STORAGE_SETTINGS_KEY = 'etcher-settings'
-const settingsStorage = new Storage(LOCAL_STORAGE_SETTINGS_KEY)
+const USER_DATA_DIR = electron.remote.app.getPath('userData')
 
 /**
  * @summary Local settings filename
@@ -38,10 +42,19 @@ const settingsStorage = new Storage(LOCAL_STORAGE_SETTINGS_KEY)
  */
 const RCFILE = '.etcher.json'
 
+/**
+ * @summary Configuration file path
+ * @type {String}
+ * @constant
+ */
+const HOME_CONFIG_PATH = process.platform === 'win32'
+  ? path.join(USER_DATA_DIR, RCFILE)
+  : path.join(USER_DATA_DIR, 'config.json')
+
 /**
  * @summary Read a local .etcherrc file
  * @function
- * @public
+ * @private
  *
  * @param {String} filename - file path
  * @fulfil {Object} - settings
@@ -68,6 +81,33 @@ const readConfigFile = (filename) => {
   })
 }
 
+/**
+ * @summary Read a local .etcherrc file
+ * @function
+ * @private
+ *
+ * @param {String} filename - file path
+ * @returns {Promise}
+ *
+ * @example
+ * writeConfigFile('.etcherrc', { something: 'good' })
+ *   .then(() => {
+ *     console.log('data written')
+ *   })
+ */
+const writeConfigFile = (filename, data) => {
+  return new Bluebird((resolve, reject) => {
+    const contents = JSON.stringify(data, null, 2)
+    fs.writeFile(filename, contents, (error) => {
+      if (error) {
+        reject(error)
+      } else {
+        resolve()
+      }
+    })
+  })
+}
+
 /**
  * @summary Read all local settings
  * @function
@@ -82,22 +122,7 @@ const readConfigFile = (filename) => {
  * });
  */
 exports.readAll = () => {
-  const homeConfigPath = process.platform === 'win32'
-    ? path.join(os.userInfo().homedir, RCFILE)
-    : path.join(os.userInfo().homedir, '.config', 'etcher', 'config.json')
-  const workdirConfigPath = path.join(process.cwd(), RCFILE)
-  const settings = {}
-  return Bluebird.try(() => {
-    _.merge(settings, settingsStorage.getAll())
-  }).return(readConfigFile(homeConfigPath))
-    .then((homeConfig) => {
-      _.merge(settings, homeConfig)
-    })
-    .return(readConfigFile(workdirConfigPath))
-    .then((workdirConfig) => {
-      _.merge(settings, workdirConfig)
-    })
-    .return(settings)
+  return readConfigFile(HOME_CONFIG_PATH)
 }
 
 /**
@@ -116,9 +141,7 @@ exports.readAll = () => {
  * });
  */
 exports.writeAll = (settings) => {
-  return Bluebird.try(() => {
-    settingsStorage.setAll(settings)
-  })
+  return writeConfigFile(HOME_CONFIG_PATH, settings)
 }
 
 /**
@@ -137,7 +160,8 @@ exports.writeAll = (settings) => {
  * });
  */
 exports.clear = () => {
+  // TODO: Unlink config file
   return Bluebird.try(() => {
-    settingsStorage.clearAll()
+    // settingsStorage.clearAll()
   })
 }
diff --git a/lib/gui/app/models/settings.js b/lib/gui/app/models/settings.js
index a215e745..80616c9e 100644
--- a/lib/gui/app/models/settings.js
+++ b/lib/gui/app/models/settings.js
@@ -23,58 +23,32 @@
 const _ = require('lodash')
 const Bluebird = require('bluebird')
 const localSettings = require('./local-settings')
-const store = require('./store')
 const errors = require('../../../shared/errors')
-
-/**
- * @summary Set a settings object
- * @function
- * @private
- *
- * @description
- * Use this function with care, given that it will completely
- * override any existing settings in both the redux store,
- * and the local user configuration.
- *
- * This function is prepared to deal with any local configuration
- * write issues by rolling back to the previous settings if so.
- *
- * @param {Object} settings - settings
- * @returns {Promise}
- *
- * @example
- * setSettingsObject({ foo: 'bar' }).then(() => {
- *   console.log('Done!');
- * });
- */
-const setSettingsObject = (settings) => {
-  const currentSettings = exports.getAll()
-
-  return Bluebird.try(() => {
-    store.dispatch({
-      type: store.Actions.SET_SETTINGS,
-      data: settings
-    })
-  }).then(() => {
-    // Revert the application state if writing the data
-    // to the local machine was not successful
-    return localSettings.writeAll(settings).catch((error) => {
-      store.dispatch({
-        type: store.Actions.SET_SETTINGS,
-        data: currentSettings
-      })
-
-      throw error
-    })
-  })
-}
+const release = require('../../../shared/release')
+const packageJSON = require('../../../../package.json')
 
 /**
  * @summary Default settings
  * @constant
  * @type {Object}
  */
-const DEFAULT_SETTINGS = store.Defaults.get('settings').toJS()
+const DEFAULT_SETTINGS = {
+  unsafeMode: false,
+  errorReporting: true,
+  unmountOnSuccess: true,
+  validateWriteOnSuccess: true,
+  updatesEnabled: packageJSON.updates.enabled && !_.includes([ 'rpm', 'deb' ], packageJSON.packageType),
+  includeUnstableUpdateChannel: !release.isStableRelease(packageJSON.version),
+  lastSleptUpdateNotifier: null,
+  lastSleptUpdateNotifierVersion: null,
+  desktopNotifications: true
+}
+
+/**
+ * @summary Settings state
+ * @type {Object}
+ */
+const settings = _.assign({}, DEFAULT_SETTINGS)
 
 /**
  * @summary Reset settings to their default values
@@ -89,7 +63,8 @@ const DEFAULT_SETTINGS = store.Defaults.get('settings').toJS()
  * });
  */
 exports.reset = () => {
-  return setSettingsObject(DEFAULT_SETTINGS)
+  // TODO: Remove default settings from config file (?)
+  return exports.assign(DEFAULT_SETTINGS)
 }
 
 /**
@@ -107,14 +82,14 @@ exports.reset = () => {
  *   console.log('Done!');
  * });
  */
-exports.assign = (settings) => {
-  if (_.isNil(settings)) {
+exports.assign = (data) => {
+  if (_.isNil(data)) {
     return Bluebird.reject(errors.createError({
       title: 'Missing settings'
     }))
   }
 
-  return setSettingsObject(_.assign(exports.getAll(), settings))
+  return localSettings.writeAll(_.assign(exports.getAll(), data))
 }
 
 /**
@@ -177,7 +152,7 @@ exports.set = (key, value) => {
  * const value = settings.get('unmountOnSuccess');
  */
 exports.get = (key) => {
-  return _.get(exports.getAll(), [ key ])
+  return _.cloneDeep(_.get(settings, [ key ]))
 }
 
 /**
@@ -192,5 +167,5 @@ exports.get = (key) => {
  * console.log(allSettings.unmountOnSuccess);
  */
 exports.getAll = () => {
-  return store.getState().get('settings').toJS()
+  return _.cloneDeep(settings)
 }
diff --git a/lib/gui/app/models/store.js b/lib/gui/app/models/store.js
index b78fb4a8..96fbd8d2 100644
--- a/lib/gui/app/models/store.js
+++ b/lib/gui/app/models/store.js
@@ -23,10 +23,8 @@ const uuidV4 = require('uuid/v4')
 const constraints = require('../../../shared/drive-constraints')
 const supportedFormats = require('../../../shared/supported-formats')
 const errors = require('../../../shared/errors')
-const release = require('../../../shared/release')
 const fileExtensions = require('../../../shared/file-extensions')
 const utils = require('../../../shared/utils')
-const packageJSON = require('../../../../package.json')
 
 /**
  * @summary Verify and throw if any state fields are nil
@@ -95,17 +93,6 @@ const DEFAULT_STATE = Immutable.fromJS({
     percentage: 0,
     speed: 0,
     totalSpeed: 0
-  },
-  settings: {
-    unsafeMode: false,
-    errorReporting: true,
-    unmountOnSuccess: true,
-    validateWriteOnSuccess: true,
-    updatesEnabled: packageJSON.updates.enabled && !_.includes([ 'rpm', 'deb' ], packageJSON.packageType),
-    includeUnstableUpdateChannel: !release.isStableRelease(packageJSON.version),
-    lastSleptUpdateNotifier: null,
-    lastSleptUpdateNotifierVersion: null,
-    desktopNotifications: true
   }
 })
 
@@ -123,8 +110,7 @@ const ACTIONS = _.fromPairs(_.map([
   'SELECT_DRIVE',
   'SELECT_IMAGE',
   'DESELECT_DRIVE',
-  'DESELECT_IMAGE',
-  'SET_SETTINGS'
+  'DESELECT_IMAGE'
 ], (message) => {
   return [ message, message ]
 }))
@@ -512,34 +498,6 @@ const storeReducer = (state = DEFAULT_STATE, action) => {
       return state.deleteIn([ 'selection', 'image' ])
     }
 
-    case ACTIONS.SET_SETTINGS: {
-      // Type: action.data : SettingsObject
-
-      if (!action.data) {
-        throw errors.createError({
-          title: 'Missing settings'
-        })
-      }
-
-      if (!_.isPlainObject(action.data)) {
-        throw errors.createError({
-          title: `Invalid settings: ${action.data}`
-        })
-      }
-
-      const invalidPair = _.find(_.toPairs(action.data), (pair) => {
-        return _.isObject(_.last(pair))
-      })
-
-      if (!_.isNil(invalidPair)) {
-        throw errors.createError({
-          title: `Invalid setting value: ${_.last(invalidPair)} for ${_.first(invalidPair)}`
-        })
-      }
-
-      return state.setIn([ 'settings' ], Immutable.fromJS(action.data))
-    }
-
     default: {
       return state
     }

From 6232cc7d49781716f9d3d4b44d760ca5ed4d1fc0 Mon Sep 17 00:00:00 2001
From: Jonas Hermsmeier <jhermsmeier@gmail.com>
Date: Wed, 23 May 2018 17:32:06 +0200
Subject: [PATCH 3/8] test(settings): Update test specs accordingly

Change-Type: patch
---
 lib/gui/app/app.js                   | 25 +++++++++++-
 lib/gui/app/models/local-settings.js |  7 ++--
 lib/gui/app/models/settings.js       | 57 +++++++++++++++++++++++-----
 lib/gui/app/modules/image-writer.js  |  1 +
 lib/shared/models/flash-state.js     |  2 +
 tests/gui/models/settings.spec.js    | 34 +----------------
 6 files changed, 81 insertions(+), 45 deletions(-)

diff --git a/lib/gui/app/app.js b/lib/gui/app/app.js
index c56fa71f..2ce1802a 100644
--- a/lib/gui/app/app.js
+++ b/lib/gui/app/app.js
@@ -47,6 +47,7 @@ const driveScanner = require('./modules/drive-scanner')
 const osDialog = require('./os/dialog')
 const exceptionReporter = require('./modules/exception-reporter')
 const updateLock = require('./modules/update-lock')
+const debug = require('debug')('etcher:app')
 
 /* eslint-disable lodash/prefer-lodash-method */
 
@@ -190,7 +191,28 @@ app.run(() => {
 })
 
 app.run(() => {
-  store.subscribe(() => {
+  debug('store.subscribe')
+
+  function observeStore(store, onChange) {
+    let currentState;
+
+    function handleChange() {
+      debug('store.subscribe:tick')
+      let nextState = store.getState();
+      if (nextState !== currentState) {
+        debug('store.subscribe:onchange')
+        currentState = nextState;
+        onChange(currentState);
+      }
+    }
+
+    let unsubscribe = store.subscribe(handleChange);
+    handleChange();
+    return unsubscribe;
+  }
+
+  observeStore(store, () => {
+    debug('store.subscribe:callback')
     if (!flashState.isFlashing()) {
       return
     }
@@ -214,6 +236,7 @@ app.run(() => {
 
     windowProgress.set(currentFlashState)
   })
+
 })
 
 app.run(($timeout) => {
diff --git a/lib/gui/app/models/local-settings.js b/lib/gui/app/models/local-settings.js
index 69c59a41..b7096c34 100644
--- a/lib/gui/app/models/local-settings.js
+++ b/lib/gui/app/models/local-settings.js
@@ -18,10 +18,8 @@
 
 const electron = require('electron')
 const Bluebird = require('bluebird')
-const _ = require('lodash')
 const fs = require('fs')
 const path = require('path')
-const os = require('os')
 
 /**
  * @summary Userdata directory path
@@ -30,6 +28,7 @@ const os = require('os')
  * - `%APPDATA%/etcher` on Windows
  * - `$XDG_CONFIG_HOME/etcher` or `~/.config/etcher` on Linux
  * - `~/Library/Application Support/etcher` on macOS
+ * See https://electronjs.org/docs/api/app#appgetpathname
  * @constant
  * @type {String}
  */
@@ -87,6 +86,7 @@ const readConfigFile = (filename) => {
  * @private
  *
  * @param {String} filename - file path
+ * @fulfil {Object} data - data
  * @returns {Promise}
  *
  * @example
@@ -102,7 +102,7 @@ const writeConfigFile = (filename, data) => {
       if (error) {
         reject(error)
       } else {
-        resolve()
+        resolve(data)
       }
     })
   })
@@ -131,6 +131,7 @@ exports.readAll = () => {
  * @public
  *
  * @param {Object} settings - settings
+ * @fulfil {Object} settings - settings
  * @returns {Promise}
  *
  * @example
diff --git a/lib/gui/app/models/settings.js b/lib/gui/app/models/settings.js
index 80616c9e..0ad76ecb 100644
--- a/lib/gui/app/models/settings.js
+++ b/lib/gui/app/models/settings.js
@@ -26,6 +26,7 @@ const localSettings = require('./local-settings')
 const errors = require('../../../shared/errors')
 const release = require('../../../shared/release')
 const packageJSON = require('../../../../package.json')
+const debug = require('debug')('etcher:models:settings')
 
 /**
  * @summary Default settings
@@ -47,8 +48,9 @@ const DEFAULT_SETTINGS = {
 /**
  * @summary Settings state
  * @type {Object}
+ * @private
  */
-const settings = _.assign({}, DEFAULT_SETTINGS)
+let settings = _.cloneDeep(DEFAULT_SETTINGS)
 
 /**
  * @summary Reset settings to their default values
@@ -63,8 +65,10 @@ const settings = _.assign({}, DEFAULT_SETTINGS)
  * });
  */
 exports.reset = () => {
+  debug('reset')
   // TODO: Remove default settings from config file (?)
-  return exports.assign(DEFAULT_SETTINGS)
+  settings = _.cloneDeep(DEFAULT_SETTINGS)
+  return localSettings.writeAll(settings)
 }
 
 /**
@@ -82,14 +86,27 @@ exports.reset = () => {
  *   console.log('Done!');
  * });
  */
-exports.assign = (data) => {
-  if (_.isNil(data)) {
+exports.assign = (value) => {
+  debug('assign', value)
+  if (_.isNil(value)) {
     return Bluebird.reject(errors.createError({
       title: 'Missing settings'
     }))
   }
 
-  return localSettings.writeAll(_.assign(exports.getAll(), data))
+  if (!_.isPlainObject(value)) {
+    return Bluebird.reject(errors.createError({
+      title: 'Settings must be an object'
+    }))
+  }
+
+  const newSettings = _.assign({}, settings, value)
+
+  return localSettings.writeAll(newSettings)
+    .then((localSettings) => {
+      // NOTE: Only update in memory settings when successfully written
+      settings = localSettings
+    })
 }
 
 /**
@@ -105,7 +122,10 @@ exports.assign = (data) => {
  * });
  */
 exports.load = () => {
-  return localSettings.readAll().then(exports.assign)
+  debug('load')
+  return localSettings.readAll().then((loadedSettings) => {
+    return _.assign(settings, loadedSettings)
+  })
 }
 
 /**
@@ -123,6 +143,7 @@ exports.load = () => {
  * });
  */
 exports.set = (key, value) => {
+  debug('set', key, value)
   if (_.isNil(key)) {
     return Bluebird.reject(errors.createError({
       title: 'Missing setting key'
@@ -135,9 +156,9 @@ exports.set = (key, value) => {
     }))
   }
 
-  return exports.assign({
-    [key]: value
-  })
+  settings[key] = value
+
+  return localSettings.writeAll(settings)
 }
 
 /**
@@ -152,6 +173,7 @@ exports.set = (key, value) => {
  * const value = settings.get('unmountOnSuccess');
  */
 exports.get = (key) => {
+  // debug('get', key)
   return _.cloneDeep(_.get(settings, [ key ]))
 }
 
@@ -167,5 +189,22 @@ exports.get = (key) => {
  * console.log(allSettings.unmountOnSuccess);
  */
 exports.getAll = () => {
+  debug('getAll')
   return _.cloneDeep(settings)
 }
+
+/**
+ * @summary Get the default setting values
+ * @function
+ * @public
+ *
+ * @returns {Object} all setting values
+ *
+ * @example
+ * const defaults = settings.getDefaults();
+ * console.log(defaults.unmountOnSuccess);
+ */
+exports.getDefaults = () => {
+  debug('getDefaults')
+  return _.cloneDeep(DEFAULT_SETTINGS)
+}
diff --git a/lib/gui/app/modules/image-writer.js b/lib/gui/app/modules/image-writer.js
index c86cff34..d95bbc46 100644
--- a/lib/gui/app/modules/image-writer.js
+++ b/lib/gui/app/modules/image-writer.js
@@ -313,6 +313,7 @@ exports.flash = (image, drives) => {
   analytics.logEvent('Flash', analyticsData)
 
   return exports.performWrite(image, drives, (state) => {
+    console.log('performWriate.onProgress', state)
     flashState.setProgressState(state)
   }).then(flashState.unsetFlashingFlag).then(() => {
     if (flashState.wasLastFlashCancelled()) {
diff --git a/lib/shared/models/flash-state.js b/lib/shared/models/flash-state.js
index 78630245..33966013 100644
--- a/lib/shared/models/flash-state.js
+++ b/lib/shared/models/flash-state.js
@@ -138,6 +138,8 @@ exports.setProgressState = (state) => {
     })
   })
 
+  console.log('store.dispatch', 'SET_FLASH_STATE')
+
   store.dispatch({
     type: store.Actions.SET_FLASH_STATE,
     data
diff --git a/tests/gui/models/settings.spec.js b/tests/gui/models/settings.spec.js
index 3fbfd272..7f10da11 100644
--- a/tests/gui/models/settings.spec.js
+++ b/tests/gui/models/settings.spec.js
@@ -19,7 +19,6 @@
 const m = require('mochainon')
 const _ = require('lodash')
 const Bluebird = require('bluebird')
-const store = require('../../../lib/gui/app/models/store')
 const settings = require('../../../lib/gui/app/models/settings')
 const localSettings = require('../../../lib/gui/app/models/local-settings')
 
@@ -28,7 +27,7 @@ describe('Browser: settings', function () {
     return settings.reset()
   })
 
-  const DEFAULT_SETTINGS = store.Defaults.get('settings').toJS()
+  const DEFAULT_SETTINGS = settings.getDefaults()
 
   it('should be able to set and read values', function () {
     m.chai.expect(settings.get('foo')).to.be.undefined
@@ -82,17 +81,6 @@ describe('Browser: settings', function () {
       })
     })
 
-    it('should throw if setting an array', function (done) {
-      settings.assign({
-        foo: 'bar',
-        bar: [ 1, 2, 3 ]
-      }).asCallback((error) => {
-        m.chai.expect(error).to.be.an.instanceof(Error)
-        m.chai.expect(error.message).to.equal('Invalid setting value: 1,2,3 for bar')
-        done()
-      })
-    })
-
     it('should not override all settings', function () {
       return settings.assign({
         foo: 'bar',
@@ -105,24 +93,6 @@ describe('Browser: settings', function () {
       })
     })
 
-    it('should not store invalid settings to the local machine', function () {
-      return localSettings.readAll().then((data) => {
-        m.chai.expect(data.foo).to.be.undefined
-
-        return new Bluebird((resolve) => {
-          settings.assign({
-            foo: [ 1, 2, 3 ]
-          }).asCallback((error) => {
-            m.chai.expect(error).to.be.an.instanceof(Error)
-            m.chai.expect(error.message).to.equal('Invalid setting value: 1,2,3 for foo')
-            return resolve()
-          })
-        })
-      }).then(localSettings.readAll).then((data) => {
-        m.chai.expect(data.foo).to.be.undefined
-      })
-    })
-
     it('should store the settings to the local machine', function () {
       return localSettings.readAll().then((data) => {
         m.chai.expect(data.foo).to.be.undefined
@@ -217,7 +187,7 @@ describe('Browser: settings', function () {
     })
 
     it('should throw if setting an array', function (done) {
-      settings.set('foo', [ 1, 2, 3 ]).asCallback((error) => {
+      settings.assign([ 1, 2, 3 ]).asCallback((error) => {
         m.chai.expect(error).to.be.an.instanceof(Error)
         m.chai.expect(error.message).to.equal('Invalid setting value: 1,2,3 for foo')
         done()

From 687e0b563b0dc3619ece4ce49d353d5838a21ff6 Mon Sep 17 00:00:00 2001
From: Jonas Hermsmeier <jhermsmeier@gmail.com>
Date: Fri, 25 May 2018 18:23:53 +0200
Subject: [PATCH 4/8] refactor(gui): Move shared models to app/models

---
 lib/gui/app/app.js                                     |  6 +++---
 .../drive-selector/controllers/drive-selector.js       |  4 ++--
 .../file-selector/file-selector/file-selector.jsx      |  2 +-
 .../flash-error-modal/services/flash-error-modal.js    |  4 ++--
 lib/{shared => gui/app}/models/available-drives.js     |  2 +-
 lib/{shared => gui/app}/models/flash-state.js          |  4 ++--
 lib/{shared => gui/app}/models/selection-state.js      |  2 +-
 lib/gui/app/modules/image-writer.js                    |  4 ++--
 lib/gui/app/pages/finish/controllers/finish.js         |  4 ++--
 lib/gui/app/pages/main/controllers/drive-selection.js  |  2 +-
 lib/gui/app/pages/main/controllers/flash.js            | 10 +++++++---
 lib/gui/app/pages/main/controllers/image-selection.js  |  2 +-
 lib/gui/app/pages/main/controllers/main.js             |  6 +++---
 tests/gui/modules/image-writer.spec.js                 |  2 +-
 tests/gui/pages/main.spec.js                           |  6 +++---
 tests/shared/models/available-drives.spec.js           |  4 ++--
 tests/shared/models/flash-state.spec.js                |  2 +-
 tests/shared/models/selection-state.spec.js            |  4 ++--
 18 files changed, 37 insertions(+), 33 deletions(-)
 rename lib/{shared => gui/app}/models/available-drives.js (96%)
 rename lib/{shared => gui/app}/models/flash-state.js (98%)
 rename lib/{shared => gui/app}/models/selection-state.js (99%)

diff --git a/lib/gui/app/app.js b/lib/gui/app/app.js
index 2ce1802a..b52b414c 100644
--- a/lib/gui/app/app.js
+++ b/lib/gui/app/app.js
@@ -36,13 +36,13 @@ const release = require('../../shared/release')
 const store = require('./models/store')
 const errors = require('../../shared/errors')
 const packageJSON = require('../../../package.json')
-const flashState = require('../../shared/models/flash-state')
+const flashState = require('./models/flash-state')
 const settings = require('./models/settings')
 const windowProgress = require('./os/window-progress')
 const analytics = require('./modules/analytics')
 const updateNotifier = require('./components/update-notifier')
-const availableDrives = require('../../shared/models/available-drives')
-const selectionState = require('../../shared/models/selection-state')
+const availableDrives = require('./models/available-drives')
+const selectionState = require('./models/selection-state')
 const driveScanner = require('./modules/drive-scanner')
 const osDialog = require('./os/dialog')
 const exceptionReporter = require('./modules/exception-reporter')
diff --git a/lib/gui/app/components/drive-selector/controllers/drive-selector.js b/lib/gui/app/components/drive-selector/controllers/drive-selector.js
index 0b873049..c008cf9c 100644
--- a/lib/gui/app/components/drive-selector/controllers/drive-selector.js
+++ b/lib/gui/app/components/drive-selector/controllers/drive-selector.js
@@ -21,8 +21,8 @@ const _ = require('lodash')
 const Bluebird = require('bluebird')
 const constraints = require('../../../../../shared/drive-constraints')
 const analytics = require('../../../modules/analytics')
-const availableDrives = require('../../../../../shared/models/available-drives')
-const selectionState = require('../../../../../shared/models/selection-state')
+const availableDrives = require('../../../models/available-drives')
+const selectionState = require('../../../models/selection-state')
 const utils = require('../../../../../shared/utils')
 
 module.exports = function (
diff --git a/lib/gui/app/components/file-selector/file-selector/file-selector.jsx b/lib/gui/app/components/file-selector/file-selector/file-selector.jsx
index 4484d01f..c1241b9e 100644
--- a/lib/gui/app/components/file-selector/file-selector/file-selector.jsx
+++ b/lib/gui/app/components/file-selector/file-selector/file-selector.jsx
@@ -37,7 +37,7 @@ const Storage = require('../../../models/storage')
 const analytics = require('../../../modules/analytics')
 const middleEllipsis = require('../../../utils/middle-ellipsis')
 const files = require('../../../../../shared/files')
-const selectionState = require('../../../../../shared/models/selection-state')
+const selectionState = require('../../../models/selection-state')
 const imageStream = require('../../../../../sdk/image-stream')
 const errors = require('../../../../../shared/errors')
 const messages = require('../../../../../shared/messages')
diff --git a/lib/gui/app/components/flash-error-modal/services/flash-error-modal.js b/lib/gui/app/components/flash-error-modal/services/flash-error-modal.js
index 811b203b..42494138 100644
--- a/lib/gui/app/components/flash-error-modal/services/flash-error-modal.js
+++ b/lib/gui/app/components/flash-error-modal/services/flash-error-modal.js
@@ -16,8 +16,8 @@
 
 'use strict'
 
-const flashState = require('../../../../../shared/models/flash-state')
-const selectionState = require('../../../../../shared/models/selection-state')
+const flashState = require('../../../models/flash-state')
+const selectionState = require('../../../models/selection-state')
 const analytics = require('../../../modules/analytics')
 
 module.exports = function (WarningModalService) {
diff --git a/lib/shared/models/available-drives.js b/lib/gui/app/models/available-drives.js
similarity index 96%
rename from lib/shared/models/available-drives.js
rename to lib/gui/app/models/available-drives.js
index c324cd57..78e4b525 100644
--- a/lib/shared/models/available-drives.js
+++ b/lib/gui/app/models/available-drives.js
@@ -17,7 +17,7 @@
 'use strict'
 
 const _ = require('lodash')
-const store = require('../../gui/app/models/store')
+const store = require('./store')
 
 /**
  * @summary Check if there are available drives
diff --git a/lib/shared/models/flash-state.js b/lib/gui/app/models/flash-state.js
similarity index 98%
rename from lib/shared/models/flash-state.js
rename to lib/gui/app/models/flash-state.js
index 33966013..59cad9f7 100644
--- a/lib/shared/models/flash-state.js
+++ b/lib/gui/app/models/flash-state.js
@@ -17,8 +17,8 @@
 'use strict'
 
 const _ = require('lodash')
-const store = require('../../gui/app/models/store')
-const units = require('../units')
+const store = require('./store')
+const units = require('../../../shared/units')
 
 /**
  * @summary Reset flash state
diff --git a/lib/shared/models/selection-state.js b/lib/gui/app/models/selection-state.js
similarity index 99%
rename from lib/shared/models/selection-state.js
rename to lib/gui/app/models/selection-state.js
index 58d470fb..56af929d 100644
--- a/lib/shared/models/selection-state.js
+++ b/lib/gui/app/models/selection-state.js
@@ -17,7 +17,7 @@
 'use strict'
 
 const _ = require('lodash')
-const store = require('../../gui/app/models/store')
+const store = require('./store')
 const availableDrives = require('./available-drives')
 
 /**
diff --git a/lib/gui/app/modules/image-writer.js b/lib/gui/app/modules/image-writer.js
index d95bbc46..fcc82d31 100644
--- a/lib/gui/app/modules/image-writer.js
+++ b/lib/gui/app/modules/image-writer.js
@@ -24,14 +24,14 @@ const ipc = require('node-ipc')
 const isRunningInAsar = require('electron-is-running-in-asar')
 const electron = require('electron')
 const settings = require('../models/settings')
-const flashState = require('../../../shared/models/flash-state')
+const flashState = require('../models/flash-state')
 const errors = require('../../../shared/errors')
 const permissions = require('../../../shared/permissions')
 const windowProgress = require('../os/window-progress')
 const analytics = require('../modules/analytics')
 const updateLock = require('./update-lock')
 const packageJSON = require('../../../../package.json')
-const selectionState = require('../../../shared/models/selection-state')
+const selectionState = require('../models/selection-state')
 
 /**
  * @summary Number of threads per CPU to allocate to the UV_THREADPOOL
diff --git a/lib/gui/app/pages/finish/controllers/finish.js b/lib/gui/app/pages/finish/controllers/finish.js
index 1bf94592..6bd52279 100644
--- a/lib/gui/app/pages/finish/controllers/finish.js
+++ b/lib/gui/app/pages/finish/controllers/finish.js
@@ -18,8 +18,8 @@
 
 const _ = require('lodash')
 const settings = require('../../../models/settings')
-const flashState = require('../../../../../shared/models/flash-state')
-const selectionState = require('../../../../../shared/models/selection-state')
+const flashState = require('../../../models/flash-state')
+const selectionState = require('../../../models/selection-state')
 const analytics = require('../../../modules/analytics')
 const updateLock = require('../../../modules/update-lock')
 const messages = require('../../../../../shared/messages')
diff --git a/lib/gui/app/pages/main/controllers/drive-selection.js b/lib/gui/app/pages/main/controllers/drive-selection.js
index 318f055f..8a834616 100644
--- a/lib/gui/app/pages/main/controllers/drive-selection.js
+++ b/lib/gui/app/pages/main/controllers/drive-selection.js
@@ -20,7 +20,7 @@ const _ = require('lodash')
 const angular = require('angular')
 const prettyBytes = require('pretty-bytes')
 const settings = require('../../../models/settings')
-const selectionState = require('../../../../../shared/models/selection-state')
+const selectionState = require('../../../models/selection-state')
 const analytics = require('../../../modules/analytics')
 const exceptionReporter = require('../../../modules/exception-reporter')
 const utils = require('../../../../../shared/utils')
diff --git a/lib/gui/app/pages/main/controllers/flash.js b/lib/gui/app/pages/main/controllers/flash.js
index f9223baf..1feea4c5 100644
--- a/lib/gui/app/pages/main/controllers/flash.js
+++ b/lib/gui/app/pages/main/controllers/flash.js
@@ -18,7 +18,7 @@
 
 const _ = require('lodash')
 const messages = require('../../../../../shared/messages')
-const flashState = require('../../../../../shared/models/flash-state')
+const flashState = require('../../../models/flash-state')
 const driveScanner = require('../../../modules/drive-scanner')
 const progressStatus = require('../../../modules/progress-status')
 const notification = require('../../../os/notification')
@@ -27,7 +27,8 @@ const imageWriter = require('../../../modules/image-writer')
 const path = require('path')
 const store = require('../../../models/store')
 const constraints = require('../../../../../shared/drive-constraints')
-const availableDrives = require('../../../../../shared/models/available-drives')
+const availableDrives = require('../../../models/available-drives')
+const debug = require('debug')('etcher:controller:flash')
 
 module.exports = function (
   $q,
@@ -139,7 +140,10 @@ module.exports = function (
 
     // Trigger Angular digests along with store updates, as the flash state
     // updates. Without this there is essentially no progress to watch.
-    const unsubscribe = store.subscribe($timeout)
+    const unsubscribe = store.subscribe(() => {
+      debug('store.onChange')
+      $timeout()
+    })
 
     const iconPath = '../../../assets/icon.png'
 
diff --git a/lib/gui/app/pages/main/controllers/image-selection.js b/lib/gui/app/pages/main/controllers/image-selection.js
index 8f35cd53..6a7be5bb 100644
--- a/lib/gui/app/pages/main/controllers/image-selection.js
+++ b/lib/gui/app/pages/main/controllers/image-selection.js
@@ -24,7 +24,7 @@ const errors = require('../../../../../shared/errors')
 const imageStream = require('../../../../../sdk/image-stream')
 const supportedFormats = require('../../../../../shared/supported-formats')
 const analytics = require('../../../modules/analytics')
-const selectionState = require('../../../../../shared/models/selection-state')
+const selectionState = require('../../../models/selection-state')
 const osDialog = require('../../../os/dialog')
 const exceptionReporter = require('../../../modules/exception-reporter')
 
diff --git a/lib/gui/app/pages/main/controllers/main.js b/lib/gui/app/pages/main/controllers/main.js
index a99d7bbb..ba60fb46 100644
--- a/lib/gui/app/pages/main/controllers/main.js
+++ b/lib/gui/app/pages/main/controllers/main.js
@@ -17,11 +17,11 @@
 'use strict'
 
 const settings = require('../../../models/settings')
-const flashState = require('../../../../../shared/models/flash-state')
+const flashState = require('../../../models/flash-state')
 const analytics = require('../../../modules/analytics')
 const exceptionReporter = require('../../../modules/exception-reporter')
-const availableDrives = require('../../../../../shared/models/available-drives')
-const selectionState = require('../../../../../shared/models/selection-state')
+const availableDrives = require('../../../models/available-drives')
+const selectionState = require('../../../models/selection-state')
 const driveConstraints = require('../../../../../shared/drive-constraints')
 const messages = require('../../../../../shared/messages')
 
diff --git a/tests/gui/modules/image-writer.spec.js b/tests/gui/modules/image-writer.spec.js
index b566babf..8aadf5df 100644
--- a/tests/gui/modules/image-writer.spec.js
+++ b/tests/gui/modules/image-writer.spec.js
@@ -4,7 +4,7 @@ const m = require('mochainon')
 const ipc = require('node-ipc')
 const angular = require('angular')
 const Bluebird = require('bluebird')
-const flashState = require('../../../lib/shared/models/flash-state')
+const flashState = require('../../../lib/gui/app/models/flash-state')
 const imageWriter = require('../../../lib/gui/app/modules/image-writer')
 require('angular-mocks')
 
diff --git a/tests/gui/pages/main.spec.js b/tests/gui/pages/main.spec.js
index e20c0c44..05e78305 100644
--- a/tests/gui/pages/main.spec.js
+++ b/tests/gui/pages/main.spec.js
@@ -22,9 +22,9 @@ const fs = require('fs')
 const path = require('path')
 const supportedFormats = require('../../../lib/shared/supported-formats')
 const angular = require('angular')
-const flashState = require('../../../lib/shared/models/flash-state')
-const availableDrives = require('../../../lib/shared/models/available-drives')
-const selectionState = require('../../../lib/shared/models/selection-state')
+const flashState = require('../../../lib/gui/app/models/flash-state')
+const availableDrives = require('../../../lib/gui/app/models/available-drives')
+const selectionState = require('../../../lib/gui/app/models/selection-state')
 require('angular-mocks')
 
 // Mock HTML requires by reading from the file-system
diff --git a/tests/shared/models/available-drives.spec.js b/tests/shared/models/available-drives.spec.js
index 7c33bc85..82447c8e 100644
--- a/tests/shared/models/available-drives.spec.js
+++ b/tests/shared/models/available-drives.spec.js
@@ -18,8 +18,8 @@
 
 const m = require('mochainon')
 const path = require('path')
-const availableDrives = require('../../../lib/shared/models/available-drives')
-const selectionState = require('../../../lib/shared/models/selection-state')
+const availableDrives = require('../../../lib/gui/app/available-drives')
+const selectionState = require('../../../lib/gui/app/selection-state')
 const constraints = require('../../../lib/shared/drive-constraints')
 
 describe('Model: availableDrives', function () {
diff --git a/tests/shared/models/flash-state.spec.js b/tests/shared/models/flash-state.spec.js
index 5faeb59d..6973cdb4 100644
--- a/tests/shared/models/flash-state.spec.js
+++ b/tests/shared/models/flash-state.spec.js
@@ -17,7 +17,7 @@
 'use strict'
 
 const m = require('mochainon')
-const flashState = require('../../../lib/shared/models/flash-state')
+const flashState = require('../../../lib/gui/app/models/flash-state')
 
 describe('Model: flashState', function () {
   beforeEach(function () {
diff --git a/tests/shared/models/selection-state.spec.js b/tests/shared/models/selection-state.spec.js
index d3bec552..49531883 100644
--- a/tests/shared/models/selection-state.spec.js
+++ b/tests/shared/models/selection-state.spec.js
@@ -19,8 +19,8 @@
 const m = require('mochainon')
 const _ = require('lodash')
 const path = require('path')
-const availableDrives = require('../../../lib/shared/models/available-drives')
-const selectionState = require('../../../lib/shared/models/selection-state')
+const availableDrives = require('../../../lib/gui/app/available-drives')
+const selectionState = require('../../../lib/gui/app/selection-state')
 
 describe('Model: selectionState', function () {
   describe('given a clean state', function () {

From 53f8e9328d3099ec05745cb92b2c4bd883b6f2e5 Mon Sep 17 00:00:00 2001
From: Jonas Hermsmeier <jhermsmeier@gmail.com>
Date: Fri, 25 May 2018 20:07:16 +0200
Subject: [PATCH 5/8] feat(app): Make store change-observable

This adds true change observability to the store,
as the `.subscribe()` callback triggers with every dispatch,
even if the data didn't change.

Now `store.observe(onChange)` can be used to only be notified
once the state data actually changes

Change-Type: minor
---
 lib/gui/app/app.js                          | 25 +--------------------
 lib/gui/app/models/flash-state.js           |  2 --
 lib/gui/app/models/store.js                 | 17 ++++++++++++++
 lib/gui/app/modules/image-writer.js         |  1 -
 lib/gui/app/pages/main/controllers/flash.js |  5 +----
 5 files changed, 19 insertions(+), 31 deletions(-)

diff --git a/lib/gui/app/app.js b/lib/gui/app/app.js
index b52b414c..1e3a8fbd 100644
--- a/lib/gui/app/app.js
+++ b/lib/gui/app/app.js
@@ -47,7 +47,6 @@ const driveScanner = require('./modules/drive-scanner')
 const osDialog = require('./os/dialog')
 const exceptionReporter = require('./modules/exception-reporter')
 const updateLock = require('./modules/update-lock')
-const debug = require('debug')('etcher:app')
 
 /* eslint-disable lodash/prefer-lodash-method */
 
@@ -191,28 +190,7 @@ app.run(() => {
 })
 
 app.run(() => {
-  debug('store.subscribe')
-
-  function observeStore(store, onChange) {
-    let currentState;
-
-    function handleChange() {
-      debug('store.subscribe:tick')
-      let nextState = store.getState();
-      if (nextState !== currentState) {
-        debug('store.subscribe:onchange')
-        currentState = nextState;
-        onChange(currentState);
-      }
-    }
-
-    let unsubscribe = store.subscribe(handleChange);
-    handleChange();
-    return unsubscribe;
-  }
-
-  observeStore(store, () => {
-    debug('store.subscribe:callback')
+  store.observe(() => {
     if (!flashState.isFlashing()) {
       return
     }
@@ -236,7 +214,6 @@ app.run(() => {
 
     windowProgress.set(currentFlashState)
   })
-
 })
 
 app.run(($timeout) => {
diff --git a/lib/gui/app/models/flash-state.js b/lib/gui/app/models/flash-state.js
index 59cad9f7..9c28ce1e 100644
--- a/lib/gui/app/models/flash-state.js
+++ b/lib/gui/app/models/flash-state.js
@@ -138,8 +138,6 @@ exports.setProgressState = (state) => {
     })
   })
 
-  console.log('store.dispatch', 'SET_FLASH_STATE')
-
   store.dispatch({
     type: store.Actions.SET_FLASH_STATE,
     data
diff --git a/lib/gui/app/models/store.js b/lib/gui/app/models/store.js
index 96fbd8d2..0492b455 100644
--- a/lib/gui/app/models/store.js
+++ b/lib/gui/app/models/store.js
@@ -508,3 +508,20 @@ module.exports = _.merge(redux.createStore(storeReducer, DEFAULT_STATE), {
   Actions: ACTIONS,
   Defaults: DEFAULT_STATE
 })
+
+module.exports.observe = (onChange) => {
+  let currentState
+
+  function changeHandler() {
+    const nextState = module.exports.getState()
+    if (!_.isEqual(nextState, currentState)) {
+      currentState = nextState
+      onChange(currentState)
+    }
+  }
+
+  const unsubscribe = module.exports.subscribe(changeHandler)
+  changeHandler()
+  return unsubscribe
+}
+
diff --git a/lib/gui/app/modules/image-writer.js b/lib/gui/app/modules/image-writer.js
index fcc82d31..22dd826a 100644
--- a/lib/gui/app/modules/image-writer.js
+++ b/lib/gui/app/modules/image-writer.js
@@ -313,7 +313,6 @@ exports.flash = (image, drives) => {
   analytics.logEvent('Flash', analyticsData)
 
   return exports.performWrite(image, drives, (state) => {
-    console.log('performWriate.onProgress', state)
     flashState.setProgressState(state)
   }).then(flashState.unsetFlashingFlag).then(() => {
     if (flashState.wasLastFlashCancelled()) {
diff --git a/lib/gui/app/pages/main/controllers/flash.js b/lib/gui/app/pages/main/controllers/flash.js
index 1feea4c5..86474ab6 100644
--- a/lib/gui/app/pages/main/controllers/flash.js
+++ b/lib/gui/app/pages/main/controllers/flash.js
@@ -140,10 +140,7 @@ module.exports = function (
 
     // Trigger Angular digests along with store updates, as the flash state
     // updates. Without this there is essentially no progress to watch.
-    const unsubscribe = store.subscribe(() => {
-      debug('store.onChange')
-      $timeout()
-    })
+    const unsubscribe = store.observe($timeout)
 
     const iconPath = '../../../assets/icon.png'
 

From e0ebdc904586aae5afff28ca9d2de71b26db25b2 Mon Sep 17 00:00:00 2001
From: Jonas Hermsmeier <jhermsmeier@gmail.com>
Date: Fri, 25 May 2018 20:35:00 +0200
Subject: [PATCH 6/8] fix(test): Fix lint errors & tests

---
 lib/gui/app/models/local-settings.js         | 21 ++++++++++++---
 lib/gui/app/models/settings.js               | 15 ++++++++---
 lib/gui/app/models/store.js                  | 20 +++++++++++---
 lib/gui/app/pages/main/controllers/flash.js  |  1 -
 tests/gui/models/settings.spec.js            | 28 +-------------------
 tests/shared/models/available-drives.spec.js |  4 +--
 tests/shared/models/selection-state.spec.js  |  4 +--
 7 files changed, 50 insertions(+), 43 deletions(-)

diff --git a/lib/gui/app/models/local-settings.js b/lib/gui/app/models/local-settings.js
index b7096c34..ae9eceaa 100644
--- a/lib/gui/app/models/local-settings.js
+++ b/lib/gui/app/models/local-settings.js
@@ -21,6 +21,13 @@ const Bluebird = require('bluebird')
 const fs = require('fs')
 const path = require('path')
 
+/**
+ * @summary Number of spaces to indent JSON output with
+ * @type {Number}
+ * @constant
+ */
+const JSON_INDENT = 2
+
 /**
  * @summary Userdata directory path
  * @description
@@ -86,6 +93,7 @@ const readConfigFile = (filename) => {
  * @private
  *
  * @param {String} filename - file path
+ * @param {Object} data - data
  * @fulfil {Object} data - data
  * @returns {Promise}
  *
@@ -97,7 +105,7 @@ const readConfigFile = (filename) => {
  */
 const writeConfigFile = (filename, data) => {
   return new Bluebird((resolve, reject) => {
-    const contents = JSON.stringify(data, null, 2)
+    const contents = JSON.stringify(data, null, JSON_INDENT)
     fs.writeFile(filename, contents, (error) => {
       if (error) {
         reject(error)
@@ -161,8 +169,13 @@ exports.writeAll = (settings) => {
  * });
  */
 exports.clear = () => {
-  // TODO: Unlink config file
-  return Bluebird.try(() => {
-    // settingsStorage.clearAll()
+  return new Bluebird((resolve, reject) => {
+    fs.unlink(HOME_CONFIG_PATH, (error) => {
+      if (error) {
+        reject(error)
+      } else {
+        resolve()
+      }
+    })
   })
 }
diff --git a/lib/gui/app/models/settings.js b/lib/gui/app/models/settings.js
index 0ad76ecb..3364d69c 100644
--- a/lib/gui/app/models/settings.js
+++ b/lib/gui/app/models/settings.js
@@ -66,6 +66,7 @@ let settings = _.cloneDeep(DEFAULT_SETTINGS)
  */
 exports.reset = () => {
   debug('reset')
+
   // TODO: Remove default settings from config file (?)
   settings = _.cloneDeep(DEFAULT_SETTINGS)
   return localSettings.writeAll(settings)
@@ -76,7 +77,7 @@ exports.reset = () => {
  * @function
  * @public
  *
- * @param {Object} settings - settings
+ * @param {Object} value - value
  * @returns {Promise}
  *
  * @example
@@ -103,9 +104,9 @@ exports.assign = (value) => {
   const newSettings = _.assign({}, settings, value)
 
   return localSettings.writeAll(newSettings)
-    .then((localSettings) => {
+    .then((updatedSettings) => {
       // NOTE: Only update in memory settings when successfully written
-      settings = localSettings
+      settings = updatedSettings
     })
 }
 
@@ -156,9 +157,16 @@ exports.set = (key, value) => {
     }))
   }
 
+  const previousValue = settings[key]
+
   settings[key] = value
 
   return localSettings.writeAll(settings)
+    .catch((error) => {
+      // Revert to previous value if persisting settings failed
+      settings[key] = previousValue
+      throw error
+    })
 }
 
 /**
@@ -173,7 +181,6 @@ exports.set = (key, value) => {
  * const value = settings.get('unmountOnSuccess');
  */
 exports.get = (key) => {
-  // debug('get', key)
   return _.cloneDeep(_.get(settings, [ key ]))
 }
 
diff --git a/lib/gui/app/models/store.js b/lib/gui/app/models/store.js
index 0492b455..e3334093 100644
--- a/lib/gui/app/models/store.js
+++ b/lib/gui/app/models/store.js
@@ -509,10 +509,25 @@ module.exports = _.merge(redux.createStore(storeReducer, DEFAULT_STATE), {
   Defaults: DEFAULT_STATE
 })
 
+/**
+ * @summary Observe the store for changes
+ * @param {Function} onChange - change handler
+ * @returns {Function} unsubscribe
+ * @example
+ * store.observe((newState) => {
+ *   // ...
+ * })
+ */
 module.exports.observe = (onChange) => {
-  let currentState
+  let currentState = null
 
-  function changeHandler() {
+  /**
+   * @summary Internal change detection handler
+   * @private
+   * @example
+   * store.subscribe(changeHandler)
+   */
+  const changeHandler = () => {
     const nextState = module.exports.getState()
     if (!_.isEqual(nextState, currentState)) {
       currentState = nextState
@@ -524,4 +539,3 @@ module.exports.observe = (onChange) => {
   changeHandler()
   return unsubscribe
 }
-
diff --git a/lib/gui/app/pages/main/controllers/flash.js b/lib/gui/app/pages/main/controllers/flash.js
index 86474ab6..4188f019 100644
--- a/lib/gui/app/pages/main/controllers/flash.js
+++ b/lib/gui/app/pages/main/controllers/flash.js
@@ -28,7 +28,6 @@ const path = require('path')
 const store = require('../../../models/store')
 const constraints = require('../../../../../shared/drive-constraints')
 const availableDrives = require('../../../models/available-drives')
-const debug = require('debug')('etcher:controller:flash')
 
 module.exports = function (
   $q,
diff --git a/tests/gui/models/settings.spec.js b/tests/gui/models/settings.spec.js
index 7f10da11..bbc672bd 100644
--- a/tests/gui/models/settings.spec.js
+++ b/tests/gui/models/settings.spec.js
@@ -176,20 +176,10 @@ describe('Browser: settings', function () {
       })
     })
 
-    it('should throw if setting an object', function (done) {
-      settings.set('foo', {
-        setting: 1
-      }).asCallback((error) => {
-        m.chai.expect(error).to.be.an.instanceof(Error)
-        m.chai.expect(error.message).to.equal('Invalid setting value: [object Object] for foo')
-        done()
-      })
-    })
-
     it('should throw if setting an array', function (done) {
       settings.assign([ 1, 2, 3 ]).asCallback((error) => {
         m.chai.expect(error).to.be.an.instanceof(Error)
-        m.chai.expect(error.message).to.equal('Invalid setting value: 1,2,3 for foo')
+        m.chai.expect(error.message).to.equal('Settings must be an object')
         done()
       })
     })
@@ -212,22 +202,6 @@ describe('Browser: settings', function () {
       })
     })
 
-    it('should not store invalid settings to the local machine', function () {
-      return localSettings.readAll().then((data) => {
-        m.chai.expect(data.foo).to.be.undefined
-
-        return new Bluebird((resolve) => {
-          settings.set('foo', [ 1, 2, 3 ]).asCallback((error) => {
-            m.chai.expect(error).to.be.an.instanceof(Error)
-            m.chai.expect(error.message).to.equal('Invalid setting value: 1,2,3 for foo')
-            return resolve()
-          })
-        })
-      }).then(localSettings.readAll).then((data) => {
-        m.chai.expect(data.foo).to.be.undefined
-      })
-    })
-
     it('should not change the application state if storing to the local machine results in an error', function (done) {
       settings.set('foo', 'bar').then(() => {
         m.chai.expect(settings.get('foo')).to.equal('bar')
diff --git a/tests/shared/models/available-drives.spec.js b/tests/shared/models/available-drives.spec.js
index 82447c8e..28a8a194 100644
--- a/tests/shared/models/available-drives.spec.js
+++ b/tests/shared/models/available-drives.spec.js
@@ -18,8 +18,8 @@
 
 const m = require('mochainon')
 const path = require('path')
-const availableDrives = require('../../../lib/gui/app/available-drives')
-const selectionState = require('../../../lib/gui/app/selection-state')
+const availableDrives = require('../../../lib/gui/app/models/available-drives')
+const selectionState = require('../../../lib/gui/app/models/selection-state')
 const constraints = require('../../../lib/shared/drive-constraints')
 
 describe('Model: availableDrives', function () {
diff --git a/tests/shared/models/selection-state.spec.js b/tests/shared/models/selection-state.spec.js
index 49531883..cf276589 100644
--- a/tests/shared/models/selection-state.spec.js
+++ b/tests/shared/models/selection-state.spec.js
@@ -19,8 +19,8 @@
 const m = require('mochainon')
 const _ = require('lodash')
 const path = require('path')
-const availableDrives = require('../../../lib/gui/app/available-drives')
-const selectionState = require('../../../lib/gui/app/selection-state')
+const availableDrives = require('../../../lib/gui/app/models/available-drives')
+const selectionState = require('../../../lib/gui/app/models/selection-state')
 
 describe('Model: selectionState', function () {
   describe('given a clean state', function () {

From 3d47f494a8ade53195814a0c05b73460b846a0b0 Mon Sep 17 00:00:00 2001
From: Jonas Hermsmeier <jhermsmeier@gmail.com>
Date: Mon, 28 May 2018 16:35:09 +0200
Subject: [PATCH 7/8] fix(app): Fix config path on Windows, typos

---
 lib/gui/app/models/local-settings.js | 25 ++++++++-----------------
 1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/lib/gui/app/models/local-settings.js b/lib/gui/app/models/local-settings.js
index ae9eceaa..3405b5fc 100644
--- a/lib/gui/app/models/local-settings.js
+++ b/lib/gui/app/models/local-settings.js
@@ -41,24 +41,15 @@ const JSON_INDENT = 2
  */
 const USER_DATA_DIR = electron.remote.app.getPath('userData')
 
-/**
- * @summary Local settings filename
- * @constant
- * @type {String}
- */
-const RCFILE = '.etcher.json'
-
 /**
  * @summary Configuration file path
  * @type {String}
  * @constant
  */
-const HOME_CONFIG_PATH = process.platform === 'win32'
-  ? path.join(USER_DATA_DIR, RCFILE)
-  : path.join(USER_DATA_DIR, 'config.json')
+const CONFIG_PATH = path.join(USER_DATA_DIR, 'config.json')
 
 /**
- * @summary Read a local .etcherrc file
+ * @summary Read a local config.json file
  * @function
  * @private
  *
@@ -67,7 +58,7 @@ const HOME_CONFIG_PATH = process.platform === 'win32'
  * @returns {Promise}
  *
  * @example
- * readConfigFile('.etcherrc').then((settings) => {
+ * readConfigFile('config.json').then((settings) => {
  *   console.log(settings)
  * })
  */
@@ -88,7 +79,7 @@ const readConfigFile = (filename) => {
 }
 
 /**
- * @summary Read a local .etcherrc file
+ * @summary Write to the local configuration file
  * @function
  * @private
  *
@@ -98,7 +89,7 @@ const readConfigFile = (filename) => {
  * @returns {Promise}
  *
  * @example
- * writeConfigFile('.etcherrc', { something: 'good' })
+ * writeConfigFile('config.json', { something: 'good' })
  *   .then(() => {
  *     console.log('data written')
  *   })
@@ -130,7 +121,7 @@ const writeConfigFile = (filename, data) => {
  * });
  */
 exports.readAll = () => {
-  return readConfigFile(HOME_CONFIG_PATH)
+  return readConfigFile(CONFIG_PATH)
 }
 
 /**
@@ -150,7 +141,7 @@ exports.readAll = () => {
  * });
  */
 exports.writeAll = (settings) => {
-  return writeConfigFile(HOME_CONFIG_PATH, settings)
+  return writeConfigFile(CONFIG_PATH, settings)
 }
 
 /**
@@ -170,7 +161,7 @@ exports.writeAll = (settings) => {
  */
 exports.clear = () => {
   return new Bluebird((resolve, reject) => {
-    fs.unlink(HOME_CONFIG_PATH, (error) => {
+    fs.unlink(CONFIG_PATH, (error) => {
       if (error) {
         reject(error)
       } else {

From ad6be11bbca1bbe3f17e207bd94e9cd17624e6b5 Mon Sep 17 00:00:00 2001
From: Jonas Hermsmeier <jhermsmeier@gmail.com>
Date: Tue, 29 May 2018 17:36:51 +0200
Subject: [PATCH 8/8] refactor(store): Return unsubscribe directly

---
 lib/gui/app/models/store.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/gui/app/models/store.js b/lib/gui/app/models/store.js
index e3334093..262a632a 100644
--- a/lib/gui/app/models/store.js
+++ b/lib/gui/app/models/store.js
@@ -535,7 +535,7 @@ module.exports.observe = (onChange) => {
     }
   }
 
-  const unsubscribe = module.exports.subscribe(changeHandler)
   changeHandler()
-  return unsubscribe
+
+  return module.exports.subscribe(changeHandler)
 }