jeans/etcher
jeans/etcher
1
0
Fork 0
mirror of https://github.com/balena-io/etcher.git synced 2025-07-29 14:16:36 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(GUI): make settings model setter asynchronous (#1597)

Browse Source 
This is part of the process of implementing support for a configuration
file. We previously decoupled the Redux store from localStorage by
moving the logic that actually persists the data to localStorage to a
local-settings.js file, however the localStorage API is synchronous, so
it follows that at the moment, all functions that interact with are also
synchronous.

Moving to storing the settings to a file means turning all these
functions to promises, which we do in this commit, in order to not mix
the addition of the configuration file feature with the huge amount of
refactoring it requires.

See: https://github.com/resin-io/etcher/issues/1356
Change-Type: patch
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
This commit is contained in:
Juan Cruz Viotti 2017-08-01 10:27:15 -04:00 committed by GitHub
parent 205711da7e
commit fc3eeff1c9
6 changed files with 325 additions and 220 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
lib/gui/app.js
View File

@ -93,8 +93,7 @@ app.run(() => {
version: currentVersion
});
settings.load();
settings.load().then(() => {
const shouldCheckForUpdates = updateNotifier.shouldCheckForUpdates({
currentVersion,
lastSleptUpdateNotifier: settings.get('lastSleptUpdateNotifier'),
@ -110,7 +109,8 @@ app.run(() => {
updatesEnabled,
releaseType: currentReleaseType
});
return;
return Bluebird.resolve();
}
const updateSemverRange = packageJSON.updates.semverRange;
@ -123,7 +123,7 @@ app.run(() => {
includeUnstableChannel
});
s3Packages.getLatestVersion(currentReleaseType, {
return s3Packages.getLatestVersion(currentReleaseType, {
range: updateSemverRange,
includeUnstableChannel
}).then((latestVersion) => {
@ -153,6 +153,7 @@ app.run(() => {
return updateNotifier.notify(latestVersion, {
allowSleepUpdateCheck: currentReleaseType === release.RELEASE_TYPE.PRODUCTION
});
});
}).catch(exceptionReporter.report);
});

10
lib/gui/components/update-notifier.js
View File

@ -124,16 +124,20 @@ exports.notify = (version, options = {}) => {
sleepUpdateCheck: checkboxChecked || false
});
});
}).then((results) => {
}).tap((results) => {
// Only update the last slept update timestamp if the
// user ticked the "Remind me again in ..." checkbox,
// but didn't agree.
if (results.sleepUpdateCheck && !results.agreed) {
settings.set('lastSleptUpdateNotifier', Date.now());
settings.set('lastSleptUpdateNotifierVersion', packageJSON.version);
return Bluebird.all([
settings.set('lastSleptUpdateNotifier', Date.now()),
settings.set('lastSleptUpdateNotifierVersion', packageJSON.version)
]);
}
return Bluebird.resolve();
}).then((results) => {
analytics.logEvent('Close update modal', {
sleepUpdateCheck: results.sleepUpdateCheck,
notifyVersion: version,

24
lib/gui/models/local-settings.js
View File

@ -16,6 +16,8 @@
'use strict';
const Bluebird = require('bluebird');
/**
* @summary Local storage settings key
* @constant
@ -28,13 +30,18 @@ const LOCAL_STORAGE_SETTINGS_KEY = 'etcher-settings';
* @function
* @public
*
* @returns {Object} local settings
* @fulfil {Object} - local settings
* @returns {Promise}
*
* @example
* const settings = localSettings.readAll();
* localSettings.readAll().then((settings) => {
* console.log(settings);
* });
*/
exports.readAll = () => {
return Bluebird.try(() => {
return JSON.parse(localStorage.getItem(LOCAL_STORAGE_SETTINGS_KEY)) || {};
});
};
/**
@ -43,15 +50,20 @@ exports.readAll = () => {
* @public
*
* @param {Object} settings - settings
* @returns {Promise}
*
* @example
* localSettings.writeAll({
* foo: 'bar'
* }).then(() => {
* console.log('Done!');
* });
*/
exports.writeAll = (settings) => {
const INDENTATION_SPACES = 2;
return Bluebird.try(() => {
localStorage.setItem(LOCAL_STORAGE_SETTINGS_KEY, JSON.stringify(settings, null, INDENTATION_SPACES));
});
};
/**
@ -62,9 +74,15 @@ exports.writeAll = (settings) => {
* @description
* Exported for testing purposes
*
* @returns {Promise}
*
* @example
* localSettings.clear();
* localSettings.clear().then(() => {
* console.log('Done!');
* });
*/
exports.clear = () => {
return Bluebird.try(() => {
localStorage.removeItem(LOCAL_STORAGE_SETTINGS_KEY);
});
};

60
lib/gui/models/settings.js
View File

@ -21,6 +21,7 @@
*/
const _ = require('lodash');
const Bluebird = require('bluebird');
const localSettings = require('./local-settings');
const store = require('../../shared/store');
const errors = require('../../shared/errors');
@ -39,30 +40,35 @@ const errors = require('../../shared/errors');
* write issues by rolling back to the previous settings if so.
*
* @param {Object} settings - settings
* @returns {Promise}
*
* @example
* setSettingsObject({ foo: 'bar' });
* 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
});
const result = _.attempt(localSettings.writeAll, settings);
}).then(() => {
// Revert the application state if writing the data
// to the local machine was not successful
if (_.isError(result)) {
return localSettings.writeAll(settings).catch((error) => {
store.dispatch({
type: store.Actions.SET_SETTINGS,
data: currentSettings
});
throw result;
}
throw error;
});
});
};
/**
@ -77,10 +83,16 @@ const DEFAULT_SETTINGS = store.Defaults.get('settings').toJS();
* @function
* @public
*
* @returns {Promise}
*
* @example
* settings.reset();
* settings.reset().then(() => {
* console.log('Done!');
* });
*/
exports.reset = _.partial(setSettingsObject, DEFAULT_SETTINGS);
exports.reset = () => {
return setSettingsObject(DEFAULT_SETTINGS);
};
/**
* @summary Extend the current settings
@ -88,20 +100,23 @@ exports.reset = _.partial(setSettingsObject, DEFAULT_SETTINGS);
* @public
*
* @param {Object} settings - settings
* @returns {Promise}
*
* @example
* settings.assign({
* foo: 'bar'
* }).then(() => {
* console.log('Done!');
* });
*/
exports.assign = (settings) => {
if (_.isNil(settings)) {
throw errors.createError({
return Bluebird.reject(errors.createError({
title: 'Missing settings'
});
}));
}
setSettingsObject(_.assign(exports.getAll(), settings));
return setSettingsObject(_.assign(exports.getAll(), settings));
};
/**
@ -109,11 +124,15 @@ exports.assign = (settings) => {
* @function
* @public
*
* @returns {Promise}
*
* @example
* settings.load();
* settings.load().then(() => {
* console.log('Done!');
* });
*/
exports.load = () => {
exports.assign(localSettings.readAll());
return localSettings.readAll().then(exports.assign);
};
/**
@ -123,24 +142,27 @@ exports.load = () => {
*
* @param {String} key - setting key
* @param {*} value - setting value
* @returns {Promise}
*
* @example
* settings.set('unmountOnSuccess', true);
* settings.set('unmountOnSuccess', true).then(() => {
* console.log('Done!');
* });
*/
exports.set = (key, value) => {
if (_.isNil(key)) {
throw errors.createError({
return Bluebird.reject(errors.createError({
title: 'Missing setting key'
});
}));
}
if (!_.isString(key)) {
throw errors.createError({
return Bluebird.reject(errors.createError({
title: `Invalid setting key: ${key}`
});
}));
}
exports.assign({
return exports.assign({
[key]: value
});
};

205
tests/gui/models/settings.spec.js
View File

@ -2,6 +2,7 @@
const m = require('mochainon');
const _ = require('lodash');
const Bluebird = require('bluebird');
const store = require('../../../lib/shared/store');
const settings = require('../../../lib/gui/models/settings');
const localSettings = require('../../../lib/gui/models/local-settings');
@ -9,45 +10,52 @@ const localSettings = require('../../../lib/gui/models/local-settings');
describe('Browser: settings', function() {
beforeEach(function() {
settings.reset();
return settings.reset();
});
const DEFAULT_SETTINGS = store.Defaults.get('settings').toJS();
it('should be able to set and read values', function() {
m.chai.expect(settings.get('foo')).to.be.undefined;
settings.set('foo', true);
return settings.set('foo', true).then(() => {
m.chai.expect(settings.get('foo')).to.be.true;
settings.set('foo', false);
return settings.set('foo', false);
}).then(() => {
m.chai.expect(settings.get('foo')).to.be.false;
});
});
describe('.reset()', function() {
it('should reset the settings to their default values', function() {
m.chai.expect(settings.getAll()).to.deep.equal(DEFAULT_SETTINGS);
settings.set('foo', 1234);
return settings.set('foo', 1234).then(() => {
m.chai.expect(settings.getAll()).to.not.deep.equal(DEFAULT_SETTINGS);
settings.reset();
return settings.reset();
}).then(() => {
m.chai.expect(settings.getAll()).to.deep.equal(DEFAULT_SETTINGS);
});
});
it('should reset the local settings to their default values', function() {
settings.set('foo', 1234);
m.chai.expect(localSettings.readAll()).to.not.deep.equal(DEFAULT_SETTINGS);
settings.reset();
m.chai.expect(localSettings.readAll()).to.deep.equal(DEFAULT_SETTINGS);
return settings.set('foo', 1234).then(localSettings.readAll).then((data) => {
m.chai.expect(data).to.not.deep.equal(DEFAULT_SETTINGS);
return settings.reset();
}).then(localSettings.readAll).then((data) => {
m.chai.expect(data).to.deep.equal(DEFAULT_SETTINGS);
});
});
describe('given the local settings are cleared', function() {
beforeEach(function() {
localSettings.clear();
return localSettings.clear();
});
it('should set the local settings to their default values', function() {
settings.reset();
m.chai.expect(localSettings.readAll()).to.deep.equal(DEFAULT_SETTINGS);
return settings.reset().then(localSettings.readAll).then((data) => {
m.chai.expect(data).to.deep.equal(DEFAULT_SETTINGS);
});
});
});
@ -56,73 +64,87 @@ describe('Browser: settings', function() {
describe('.assign()', function() {
it('should throw if no settings', function() {
m.chai.expect(function() {
settings.assign();
}).to.throw('Missing setting');
it('should throw if no settings', function(done) {
settings.assign().asCallback((error) => {
m.chai.expect(error).to.be.an.instanceof(Error);
m.chai.expect(error.message).to.equal('Missing settings');
done();
});
});
it('should throw if setting an array', function() {
m.chai.expect(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();
});
}).to.throw('Invalid setting value: 1,2,3 for bar');
});
it('should not override all settings', function() {
settings.assign({
return settings.assign({
foo: 'bar',
bar: 'baz'
});
}).then(() => {
m.chai.expect(settings.getAll()).to.deep.equal(_.assign({}, DEFAULT_SETTINGS, {
foo: 'bar',
bar: 'baz'
}));
});
});
it('should not store invalid settings to the local machine', function() {
m.chai.expect(localSettings.readAll().foo).to.be.undefined;
return localSettings.readAll().then((data) => {
m.chai.expect(data.foo).to.be.undefined;
m.chai.expect(() => {
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;
});
}).to.throw('Invalid setting value: 1,2,3');
m.chai.expect(localSettings.readAll().foo).to.be.undefined;
});
it('should store the settings to the local machine', function() {
m.chai.expect(localSettings.readAll().foo).to.be.undefined;
m.chai.expect(localSettings.readAll().bar).to.be.undefined;
return localSettings.readAll().then((data) => {
m.chai.expect(data.foo).to.be.undefined;
m.chai.expect(data.bar).to.be.undefined;
settings.assign({
return settings.assign({
foo: 'bar',
bar: 'baz'
});
m.chai.expect(localSettings.readAll().foo).to.equal('bar');
m.chai.expect(localSettings.readAll().bar).to.equal('baz');
}).then(localSettings.readAll).then((data) => {
m.chai.expect(data.foo).to.equal('bar');
m.chai.expect(data.bar).to.equal('baz');
});
});
it('should not change the application state if storing to the local machine results in an error', function() {
settings.set('foo', 'bar');
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');
const localSettingsWriteAllStub = m.sinon.stub(localSettings, 'writeAll');
localSettingsWriteAllStub.throws(new Error('localSettings error'));
localSettingsWriteAllStub.returns(Bluebird.reject(new Error('localSettings error')));
m.chai.expect(() => {
settings.assign({
foo: 'baz'
});
}).to.throw('localSettings error');
}).asCallback((error) => {
m.chai.expect(error).to.be.an.instanceof(Error);
m.chai.expect(error.message).to.equal('localSettings error');
localSettingsWriteAllStub.restore();
m.chai.expect(settings.get('foo')).to.equal('bar');
done();
});
}).catch(done);
});
});
@ -135,18 +157,21 @@ describe('Browser: settings', function() {
};
m.chai.expect(settings.getAll()).to.deep.equal(DEFAULT_SETTINGS);
localSettings.writeAll(object);
return localSettings.writeAll(object).then(() => {
m.chai.expect(settings.getAll()).to.deep.equal(DEFAULT_SETTINGS);
settings.load();
return settings.load();
}).then(() => {
m.chai.expect(settings.getAll()).to.deep.equal(_.assign({}, DEFAULT_SETTINGS, object));
});
});
it('should keep the application state intact if there are no local settings', function() {
m.chai.expect(settings.getAll()).to.deep.equal(DEFAULT_SETTINGS);
localSettings.clear();
settings.load();
return localSettings.clear().then(settings.load).then(() => {
m.chai.expect(settings.getAll()).to.deep.equal(DEFAULT_SETTINGS);
});
});
});
@ -154,72 +179,94 @@ describe('Browser: settings', function() {
it('should set an unknown key', function() {
m.chai.expect(settings.get('foobar')).to.be.undefined;
settings.set('foobar', true);
return settings.set('foobar', true).then(() => {
m.chai.expect(settings.get('foobar')).to.be.true;
});
it('should throw if no key', function() {
m.chai.expect(function() {
settings.set(null, true);
}).to.throw('Missing setting key');
});
it('should throw if key is not a string', function() {
m.chai.expect(function() {
settings.set(1234, true);
}).to.throw('Invalid setting key: 1234');
it('should reject if no key', function(done) {
settings.set(null, true).asCallback((error) => {
m.chai.expect(error).to.be.an.instanceof(Error);
m.chai.expect(error.message).to.equal('Missing setting key');
done();
});
});
it('should throw if setting an object', function() {
m.chai.expect(function() {
it('should throw if key is not a string', function(done) {
settings.set(1234, true).asCallback((error) => {
m.chai.expect(error).to.be.an.instanceof(Error);
m.chai.expect(error.message).to.equal('Invalid setting key: 1234');
done();
});
});
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();
});
}).to.throw('Invalid setting value: [object Object] for foo');
});
it('should throw if setting an array', function() {
m.chai.expect(function() {
settings.set('foo', [ 1, 2, 3 ]);
}).to.throw('Invalid setting value: 1,2,3 for foo');
it('should throw if setting an array', function(done) {
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');
done();
});
});
it('should set the key to undefined if no value', function() {
settings.set('foo', 'bar');
return settings.set('foo', 'bar').then(() => {
m.chai.expect(settings.get('foo')).to.equal('bar');
settings.set('foo');
return settings.set('foo');
}).then(() => {
m.chai.expect(settings.get('foo')).to.be.undefined;
});
});
it('should store the setting to the local machine', function() {
m.chai.expect(localSettings.readAll().foo).to.be.undefined;
settings.set('foo', 'bar');
m.chai.expect(localSettings.readAll().foo).to.equal('bar');
return localSettings.readAll().then((data) => {
m.chai.expect(data.foo).to.be.undefined;
return settings.set('foo', 'bar');
}).then(localSettings.readAll).then((data) => {
m.chai.expect(data.foo).to.equal('bar');
});
});
it('should not store invalid settings to the local machine', function() {
m.chai.expect(localSettings.readAll().foo).to.be.undefined;
return localSettings.readAll().then((data) => {
m.chai.expect(data.foo).to.be.undefined;
m.chai.expect(() => {
settings.set('foo', [ 1, 2, 3 ]);
}).to.throw('Invalid setting value: 1,2,3');
m.chai.expect(localSettings.readAll().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() {
settings.set('foo', 'bar');
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');
const localSettingsWriteAllStub = m.sinon.stub(localSettings, 'writeAll');
localSettingsWriteAllStub.throws(new Error('localSettings error'));
m.chai.expect(() => {
settings.set('foo', 'baz');
}).to.throw('localSettings error');
localSettingsWriteAllStub.returns(Bluebird.reject(new Error('localSettings error')));
settings.set('foo', 'baz').asCallback((error) => {
m.chai.expect(error).to.be.an.instanceof(Error);
m.chai.expect(error.message).to.equal('localSettings error');
localSettingsWriteAllStub.restore();
m.chai.expect(settings.get('foo')).to.equal('bar');
done();
});
}).catch(done);
});
});

39
tests/gui/pages/main.spec.js
View File

@ -241,9 +241,10 @@ describe('Browser: MainPage', function() {
speed: 100000000000000
});
settings.set('unmountOnSuccess', true);
return settings.set('unmountOnSuccess', true).then(() => {
m.chai.expect(controller.getProgressButtonLabel()).to.equal('0%');
});
});
it('should handle percentage == 0, type = write, unmountOnSuccess', function() {
const controller = $controller('FlashController', {
@ -257,9 +258,10 @@ describe('Browser: MainPage', function() {
speed: 0
});
settings.set('unmountOnSuccess', true);
return settings.set('unmountOnSuccess', true).then(() => {
m.chai.expect(controller.getProgressButtonLabel()).to.equal('Starting...');
});
});
it('should handle percentage == 0, type = write, !unmountOnSuccess', function() {
const controller = $controller('FlashController', {
@ -273,9 +275,10 @@ describe('Browser: MainPage', function() {
speed: 0
});
settings.set('unmountOnSuccess', false);
return settings.set('unmountOnSuccess', false).then(() => {
m.chai.expect(controller.getProgressButtonLabel()).to.equal('Starting...');
});
});
it('should handle percentage == 0, type = check, unmountOnSuccess', function() {
const controller = $controller('FlashController', {
@ -289,9 +292,10 @@ describe('Browser: MainPage', function() {
speed: 0
});
settings.set('unmountOnSuccess', true);
return settings.set('unmountOnSuccess', true).then(() => {
m.chai.expect(controller.getProgressButtonLabel()).to.equal('Starting...');
});
});
it('should handle percentage == 0, type = check, !unmountOnSuccess', function() {
const controller = $controller('FlashController', {
@ -305,9 +309,10 @@ describe('Browser: MainPage', function() {
speed: 0
});
settings.set('unmountOnSuccess', false);
return settings.set('unmountOnSuccess', false).then(() => {
m.chai.expect(controller.getProgressButtonLabel()).to.equal('Starting...');
});
});
it('should handle percentage == 50, type = write, unmountOnSuccess', function() {
const controller = $controller('FlashController', {
@ -321,9 +326,10 @@ describe('Browser: MainPage', function() {
speed: 1000
});
settings.set('unmountOnSuccess', true);
return settings.set('unmountOnSuccess', true).then(() => {
m.chai.expect(controller.getProgressButtonLabel()).to.equal('50%');
});
});
it('should handle percentage == 50, type = write, !unmountOnSuccess', function() {
const controller = $controller('FlashController', {
@ -337,9 +343,10 @@ describe('Browser: MainPage', function() {
speed: 1000
});
settings.set('unmountOnSuccess', false);
return settings.set('unmountOnSuccess', false).then(() => {
m.chai.expect(controller.getProgressButtonLabel()).to.equal('50%');
});
});
it('should handle percentage == 50, type = check, unmountOnSuccess', function() {
const controller = $controller('FlashController', {
@ -353,9 +360,10 @@ describe('Browser: MainPage', function() {
speed: 1000
});
settings.set('unmountOnSuccess', true);
return settings.set('unmountOnSuccess', true).then(() => {
m.chai.expect(controller.getProgressButtonLabel()).to.equal('50% Validating...');
});
});
it('should handle percentage == 50, type = check, !unmountOnSuccess', function() {
const controller = $controller('FlashController', {
@ -369,9 +377,10 @@ describe('Browser: MainPage', function() {
speed: 1000
});
settings.set('unmountOnSuccess', false);
return settings.set('unmountOnSuccess', false).then(() => {
m.chai.expect(controller.getProgressButtonLabel()).to.equal('50% Validating...');
});
});
it('should handle percentage == 100, type = write, unmountOnSuccess', function() {
const controller = $controller('FlashController', {
@ -385,9 +394,10 @@ describe('Browser: MainPage', function() {
speed: 1000
});
settings.set('unmountOnSuccess', true);
return settings.set('unmountOnSuccess', true).then(() => {
m.chai.expect(controller.getProgressButtonLabel()).to.equal('Finishing...');
});
});
it('should handle percentage == 100, type = write, !unmountOnSuccess', function() {
const controller = $controller('FlashController', {
@ -401,9 +411,10 @@ describe('Browser: MainPage', function() {
speed: 1000
});
settings.set('unmountOnSuccess', false);
return settings.set('unmountOnSuccess', false).then(() => {
m.chai.expect(controller.getProgressButtonLabel()).to.equal('Finishing...');
});
});
it('should handle percentage == 100, type = check, unmountOnSuccess', function() {
const controller = $controller('FlashController', {
@ -417,9 +428,10 @@ describe('Browser: MainPage', function() {
speed: 1000
});
settings.set('unmountOnSuccess', true);
return settings.set('unmountOnSuccess', true).then(() => {
m.chai.expect(controller.getProgressButtonLabel()).to.equal('Unmounting...');
});
});
it('should handle percentage == 100, type = check, !unmountOnSuccess', function() {
const controller = $controller('FlashController', {
@ -433,9 +445,10 @@ describe('Browser: MainPage', function() {
speed: 1000
});
settings.set('unmountOnSuccess', false);
return settings.set('unmountOnSuccess', false).then(() => {
m.chai.expect(controller.getProgressButtonLabel()).to.equal('Finishing...');
});
});
});