diff --git a/lib/gui/app.js b/lib/gui/app.js index 67623c66..16f0b4c8 100644 --- a/lib/gui/app.js +++ b/lib/gui/app.js @@ -146,7 +146,7 @@ app.controller('AppController', function( this.selection = SelectionStateModel; this.drives = DrivesModel; this.writer = ImageWriterService; - this.settings = SettingsModel.data; + this.settings = SettingsModel; this.tooltipModal = TooltipModalService; this.handleError = (error) => { diff --git a/lib/gui/components/update-notifier/controllers/update-notifier.js b/lib/gui/components/update-notifier/controllers/update-notifier.js index 29f0d256..468cc1ae 100644 --- a/lib/gui/components/update-notifier/controllers/update-notifier.js +++ b/lib/gui/components/update-notifier/controllers/update-notifier.js @@ -23,14 +23,14 @@ module.exports = function($uibModalInstance, SettingsModel) { // If the controller is instantiated, means the modal was shown. // Compare that to `UpdateNotifierService.notify()`, which could // have been called, but the modal could have failed to be shown. - SettingsModel.data.lastUpdateNotify = Date.now(); + SettingsModel.set('lastUpdateNotify', Date.now()); /** - * @summary Settings data + * @summary Settings model * @type Object * @public */ - this.settings = SettingsModel.data; + this.settings = SettingsModel; /** * @summary Close the modal diff --git a/lib/gui/components/update-notifier/services/update-notifier.js b/lib/gui/components/update-notifier/services/update-notifier.js index efa4acbf..1ed87a42 100644 --- a/lib/gui/components/update-notifier/services/update-notifier.js +++ b/lib/gui/components/update-notifier/services/update-notifier.js @@ -54,14 +54,14 @@ module.exports = function($uibModal, UPDATE_NOTIFIER_SLEEP_TIME, ManifestBindSer * } */ this.shouldCheckForUpdates = () => { - const lastUpdateNotify = SettingsModel.data.lastUpdateNotify; + const lastUpdateNotify = SettingsModel.get('lastUpdateNotify'); - if (!SettingsModel.data.sleepUpdateCheck || !lastUpdateNotify) { + if (!SettingsModel.get('sleepUpdateCheck') || !lastUpdateNotify) { return true; } if (lastUpdateNotify - Date.now() > UPDATE_NOTIFIER_SLEEP_TIME) { - SettingsModel.data.sleepUpdateCheck = false; + SettingsModel.set('sleepUpdateCheck', false); return true; } diff --git a/lib/gui/components/update-notifier/templates/update-notifier-modal.tpl.html b/lib/gui/components/update-notifier/templates/update-notifier-modal.tpl.html index c89eb4f1..683f713c 100644 --- a/lib/gui/components/update-notifier/templates/update-notifier-modal.tpl.html +++ b/lib/gui/components/update-notifier/templates/update-notifier-modal.tpl.html @@ -13,7 +13,7 @@
diff --git a/lib/gui/models/settings.js b/lib/gui/models/settings.js index f698af91..c4c9be02 100644 --- a/lib/gui/models/settings.js +++ b/lib/gui/models/settings.js @@ -21,25 +21,62 @@ */ const angular = require('angular'); -require('ngstorage'); +const Store = require('./store'); const MODULE_NAME = 'Etcher.Models.Settings'; -const SettingsModel = angular.module(MODULE_NAME, [ - 'ngStorage' -]); +const SettingsModel = angular.module(MODULE_NAME, []); -SettingsModel.service('SettingsModel', function($localStorage) { +SettingsModel.service('SettingsModel', function() { /** - * @summary Settings data - * @type Object + * @summary Set a setting value + * @function * @public + * + * @param {String} key - setting key + * @param {*} value - setting value + * + * @example + * SettingsModel.set('unmountOnSuccess', true); */ - this.data = $localStorage.$default({ - errorReporting: true, - unmountOnSuccess: true, - validateWriteOnSuccess: true, - sleepUpdateCheck: false - }); + this.set = (key, value) => { + Store.dispatch({ + type: Store.Actions.SET_SETTING, + data: { + key, + value + } + }); + }; + + /** + * @summary Get a setting value + * @function + * @public + * + * @param {String} key - setting key + * @returns {*} setting value + * + * @example + * const value = SettingsModel.get('unmountOnSuccess'); + */ + this.get = (key) => { + return this.getAll()[key]; + }; + + /** + * @summary Get all setting values + * @function + * @public + * + * @returns {Object} all setting values + * + * @example + * const allSettings = SettingsModel.getAll(); + * console.log(allSettings.unmountOnSuccess); + */ + this.getAll = () => { + return Store.getState().get('settings').toJS(); + }; }); diff --git a/lib/gui/models/store.js b/lib/gui/models/store.js index e9a1bf0e..5855fd5b 100644 --- a/lib/gui/models/store.js +++ b/lib/gui/models/store.js @@ -19,6 +19,7 @@ const Immutable = require('immutable'); const _ = require('lodash'); const redux = require('redux'); +const persistState = require('redux-localstorage'); /** * @summary Application default state @@ -34,9 +35,24 @@ const DEFAULT_STATE = Immutable.fromJS({ flashState: { percentage: 0, speed: 0 + }, + settings: { + errorReporting: true, + unmountOnSuccess: true, + validateWriteOnSuccess: true, + sleepUpdateCheck: false, + lastUpdateNotify: null } }); +/** + * @summary State path to be persisted + * @type String + * @constant + * @private + */ +const PERSISTED_PATH = 'settings'; + /** * @summary Application supported action messages * @type Object @@ -51,7 +67,8 @@ const ACTIONS = _.fromPairs(_.map([ 'SELECT_DRIVE', 'SELECT_IMAGE', 'REMOVE_DRIVE', - 'REMOVE_IMAGE' + 'REMOVE_IMAGE', + 'SET_SETTING' ], (message) => { return [ message, message ]; })); @@ -263,6 +280,29 @@ const storeReducer = (state, action) => { return state.deleteIn([ 'selection', 'image' ]); } + case ACTIONS.SET_SETTING: { + const key = action.data.key; + const value = action.data.value; + + if (!key) { + throw new Error('Missing setting key'); + } + + if (!_.isString(key)) { + throw new Error(`Invalid setting key: ${key}`); + } + + if (!DEFAULT_STATE.get('settings').has(key)) { + throw new Error(`Unsupported setting: ${key}`); + } + + if (_.isObject(value)) { + throw new Error(`Invalid setting value: ${value}`); + } + + return state.setIn([ 'settings', key ], value); + } + default: { return state; } @@ -270,6 +310,56 @@ const storeReducer = (state, action) => { } }; -module.exports = _.merge(redux.createStore(storeReducer), { - Actions: ACTIONS +module.exports = _.merge(redux.createStore( + storeReducer, + DEFAULT_STATE, + redux.compose(persistState(PERSISTED_PATH, { + + // The following options are set for the sole + // purpose of dealing correctly with ImmutableJS + // collections. + // See: https://github.com/elgerlambert/redux-localstorage#immutable-data + + slicer: (key) => { + return (state) => { + return state.get(key); + }; + }, + + serialize: (collection) => { + return JSON.stringify(collection.toJS()); + }, + + deserialize: (data) => { + return Immutable.fromJS(JSON.parse(data)); + }, + + merge: (state, subset) => { + + // In the first run, there will be no information + // to deserialize. In this case, we avoid merging, + // otherwise we will be basically erasing the property + // we aim the keep serialising the in future. + if (!subset) { + return; + } + + // Blindly setting the state to the deserialised subset + // means that a user could manually edit `localStorage` + // and extend the application state settings with + // unsupported properties, since it can bypass validation. + // + // The alternative, which would be dispatching each + // deserialised settins through the appropriate action + // is not very elegant, nor performant, so we decide + // to intentionally ignore this little flaw since + // adding extra properties makes no damage at all. + return state.set(PERSISTED_PATH, subset); + + } + + })) +), { + Actions: ACTIONS, + Defaults: DEFAULT_STATE }); diff --git a/lib/gui/modules/analytics.js b/lib/gui/modules/analytics.js index b2878078..085a7ddf 100644 --- a/lib/gui/modules/analytics.js +++ b/lib/gui/modules/analytics.js @@ -78,7 +78,7 @@ analytics.config(($provide) => { return (exception, cause) => { const SettingsModel = $injector.get('SettingsModel'); - if (SettingsModel.data.errorReporting) { + if (SettingsModel.get('errorReporting')) { $window.trackJs.track(exception); } @@ -96,7 +96,7 @@ analytics.config(($provide) => { const SettingsModel = $injector.get('SettingsModel'); - if (SettingsModel.data.errorReporting) { + if (SettingsModel.get('errorReporting')) { $window.trackJs.console.debug(message); } @@ -144,7 +144,7 @@ analytics.service('AnalyticsService', function($log, $mixpanel, SettingsModel) { */ this.logEvent = (message, data) => { - if (SettingsModel.data.errorReporting) { + if (SettingsModel.get('errorReporting')) { // Clone data before passing it to `mixpanel.track` // since this function mutates the object adding diff --git a/lib/gui/modules/image-writer.js b/lib/gui/modules/image-writer.js index 992c4b5b..c4657d3d 100644 --- a/lib/gui/modules/image-writer.js +++ b/lib/gui/modules/image-writer.js @@ -197,7 +197,10 @@ imageWriter.service('ImageWriterService', function($q, $timeout, SettingsModel) */ this.performWrite = (image, drive, onProgress) => { return $q((resolve, reject) => { - const child = childWriter.write(image, drive, SettingsModel.data); + const child = childWriter.write(image, drive, { + validateWriteOnSuccess: SettingsModel.get('validateWriteOnSuccess'), + unmountOnSuccess: SettingsModel.get('unmountOnSuccess') + }); child.on('error', reject); child.on('done', resolve); child.on('progress', onProgress); diff --git a/lib/gui/pages/finish/controllers/finish.js b/lib/gui/pages/finish/controllers/finish.js index 43500c06..24094ee9 100644 --- a/lib/gui/pages/finish/controllers/finish.js +++ b/lib/gui/pages/finish/controllers/finish.js @@ -19,11 +19,11 @@ module.exports = function($state, ImageWriterService, SelectionStateModel, AnalyticsService, SettingsModel) { /** - * @summary Settings data + * @summary Settings model * @type Object * @public */ - this.settings = SettingsModel.data; + this.settings = SettingsModel; /** * @summary Source checksum diff --git a/lib/gui/pages/finish/templates/success.tpl.html b/lib/gui/pages/finish/templates/success.tpl.html index b10ce993..c78dc05c 100644 --- a/lib/gui/pages/finish/templates/success.tpl.html +++ b/lib/gui/pages/finish/templates/success.tpl.html @@ -2,7 +2,7 @@

Flash Complete!

-

Safely ejected and ready for use

+

Safely ejected and ready for use

diff --git a/lib/gui/pages/settings/controllers/settings.js b/lib/gui/pages/settings/controllers/settings.js index 2a66af75..b96b9b05 100644 --- a/lib/gui/pages/settings/controllers/settings.js +++ b/lib/gui/pages/settings/controllers/settings.js @@ -19,10 +19,17 @@ module.exports = function(SettingsModel) { /** - * @summary Settings data + * @summary Current settings value * @type Object * @public */ - this.storage = SettingsModel.data; + this.currentData = SettingsModel.getAll(); + + /** + * @summary Settings model + * @type Object + * @public + */ + this.model = SettingsModel; }; diff --git a/lib/gui/pages/settings/templates/settings.tpl.html b/lib/gui/pages/settings/templates/settings.tpl.html index b0469e42..79f0d71d 100644 --- a/lib/gui/pages/settings/templates/settings.tpl.html +++ b/lib/gui/pages/settings/templates/settings.tpl.html @@ -3,21 +3,30 @@
diff --git a/lib/gui/partials/main.html b/lib/gui/partials/main.html index ad7db37d..12544267 100644 --- a/lib/gui/partials/main.html +++ b/lib/gui/partials/main.html @@ -98,10 +98,10 @@
- + Your removable drive did not pass validation check.
Please insert another one and
- + Oops, seems something went wrong. Click to retry
diff --git a/package.json b/package.json index 5bf28e40..d63ec932 100644 --- a/package.json +++ b/package.json @@ -72,8 +72,8 @@ "immutable": "^3.8.1", "is-elevated": "^1.0.0", "lodash": "^4.5.1", - "ngstorage": "^0.3.10", "redux": "^3.5.2", + "redux-localstorage": "^0.4.1", "resin-cli-errors": "^1.2.0", "resin-cli-form": "^1.4.1", "resin-cli-visuals": "^1.2.8", diff --git a/tests/gui/components/update-notifier.spec.js b/tests/gui/components/update-notifier.spec.js index 8d6fe3ae..9050404d 100644 --- a/tests/gui/components/update-notifier.spec.js +++ b/tests/gui/components/update-notifier.spec.js @@ -28,7 +28,7 @@ describe('Browser: UpdateNotifier', function() { describe('given the `sleepUpdateCheck` is disabled', function() { beforeEach(function() { - SettingsModel.data.sleepUpdateCheck = false; + SettingsModel.set('sleepUpdateCheck', false); }); it('should return true', function() { @@ -41,13 +41,13 @@ describe('Browser: UpdateNotifier', function() { describe('given the `sleepUpdateCheck` is enabled', function() { beforeEach(function() { - SettingsModel.data.sleepUpdateCheck = true; + SettingsModel.set('sleepUpdateCheck', true); }); describe('given the `lastUpdateNotify` was never updated', function() { beforeEach(function() { - SettingsModel.data.lastUpdateNotify = undefined; + SettingsModel.set('lastUpdateNotify', undefined); }); it('should return true', function() { @@ -60,7 +60,7 @@ describe('Browser: UpdateNotifier', function() { describe('given the `lastUpdateNotify` was very recently updated', function() { beforeEach(function() { - SettingsModel.data.lastUpdateNotify = Date.now() + 1000; + SettingsModel.set('lastUpdateNotify', Date.now() + 1000); }); it('should return false', function() { @@ -73,7 +73,7 @@ describe('Browser: UpdateNotifier', function() { describe('given the `lastUpdateNotify` was updated long ago', function() { beforeEach(function() { - SettingsModel.data.lastUpdateNotify = Date.now() + UPDATE_NOTIFIER_SLEEP_TIME + 1000; + SettingsModel.set('lastUpdateNotify', Date.now() + UPDATE_NOTIFIER_SLEEP_TIME + 1000); }); it('should return true', function() { @@ -82,9 +82,9 @@ describe('Browser: UpdateNotifier', function() { }); it('should unset the `sleepUpdateCheck` setting', function() { - m.chai.expect(SettingsModel.data.sleepUpdateCheck).to.be.true; + m.chai.expect(SettingsModel.get('sleepUpdateCheck')).to.be.true; UpdateNotifierService.shouldCheckForUpdates(); - m.chai.expect(SettingsModel.data.sleepUpdateCheck).to.be.false; + m.chai.expect(SettingsModel.get('sleepUpdateCheck')).to.be.false; }); }); diff --git a/tests/gui/models/settings.spec.js b/tests/gui/models/settings.spec.js new file mode 100644 index 00000000..c85476b7 --- /dev/null +++ b/tests/gui/models/settings.spec.js @@ -0,0 +1,101 @@ +'use strict'; + +const m = require('mochainon'); +const _ = require('lodash'); +const angular = require('angular'); +require('angular-mocks'); +const Store = require('../../../lib/gui/models/store'); + +describe('Browser: SettingsModel', function() { + + beforeEach(angular.mock.module( + require('../../../lib/gui/models/settings') + )); + + describe('SettingsModel', function() { + + const SUPPORTED_KEYS = _.keys(Store.Defaults.get('settings').toJS()); + let SettingsModel; + + beforeEach(angular.mock.inject(function(_SettingsModel_) { + SettingsModel = _SettingsModel_; + })); + + beforeEach(function() { + this.settings = SettingsModel.getAll(); + }); + + afterEach(function() { + _.each(SUPPORTED_KEYS, (supportedKey) => { + SettingsModel.set(supportedKey, this.settings[supportedKey]); + }); + }); + + it('should be able to set and read values', function() { + const keyUnderTest = _.first(SUPPORTED_KEYS); + const originalValue = SettingsModel.get(keyUnderTest); + + SettingsModel.set(keyUnderTest, !originalValue); + m.chai.expect(SettingsModel.get(keyUnderTest)).to.equal(!originalValue); + SettingsModel.set(keyUnderTest, originalValue); + m.chai.expect(SettingsModel.get(keyUnderTest)).to.equal(originalValue); + }); + + describe('.set()', function() { + + it('should throw if the key is not supported', function() { + m.chai.expect(function() { + SettingsModel.set('foobar', true); + }).to.throw('Unsupported setting: foobar'); + }); + + it('should throw if no key', function() { + m.chai.expect(function() { + SettingsModel.set(null, true); + }).to.throw('Missing setting key'); + }); + + it('should throw if key is not a string', function() { + m.chai.expect(function() { + SettingsModel.set(1234, true); + }).to.throw('Invalid setting key: 1234'); + }); + + it('should throw if setting an object', function() { + const keyUnderTest = _.first(SUPPORTED_KEYS); + m.chai.expect(function() { + SettingsModel.set(keyUnderTest, { + x: 1 + }); + }).to.throw('Invalid setting value: [object Object]'); + }); + + it('should throw if setting an array', function() { + const keyUnderTest = _.first(SUPPORTED_KEYS); + m.chai.expect(function() { + SettingsModel.set(keyUnderTest, [ 1, 2, 3 ]); + }).to.throw('Invalid setting value: 1,2,3'); + }); + + it('should set the key to undefined if no value', function() { + const keyUnderTest = _.first(SUPPORTED_KEYS); + SettingsModel.set(keyUnderTest); + m.chai.expect(SettingsModel.get(keyUnderTest)).to.be.undefined; + }); + + }); + + describe('.getAll()', function() { + + it('should be able to read all values', function() { + const allValues = SettingsModel.getAll(); + + _.each(SUPPORTED_KEYS, function(supportedKey) { + m.chai.expect(allValues[supportedKey]).to.equal(SettingsModel.get(supportedKey)); + }); + }); + + }); + + }); +});