From bf37ee72dffe64bcca32ec459139ab91f88e3a58 Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Thu, 12 May 2016 15:38:19 -0400 Subject: [PATCH] Prevent selection of drives that are not large enough (#408) * Make .selectImage() dialog return an object with a `path` property This allows to return more than one value for the selected image, like image size and other metadata for example. Signed-off-by: Juan Cruz Viotti * Return image size from .selectImage() dialog This property will be useful to perform some sanity checks, like ensuring the selected drive is large enough to contain the image. Signed-off-by: Juan Cruz Viotti * Store both the image path and the image size in the selection model In order to simplify accessing such properties in an encapsulated manner, `SelectionStateModel.getImage()` was replaced with the following functions: - `SelectionStateModel.getImagePath()`. - `SelectionStateModel.getImageSize()`. Signed-off-by: Juan Cruz Viotti * Increase SelectionStateModel setter validation The model not providing any kind of validation was the source of some bugs I encountered while implementing the previous commits that would not have happened otherwise. Signed-off-by: Juan Cruz Viotti * Prevent selection of drives that are not large enough - The drive selector modal was modified such that drives that are not large enough are crossed out, and the user is not able to click them. - In the case of drive auto-selection, not large enough drives won't attempt to get autoselected. This commit introduces: - The `SelectionStateModel.isDriveLargeEnough()` function. Fixes: https://github.com/resin-io/etcher/issues/344 Signed-off-by: Juan Cruz Viotti --- build/css/main.css | 19 ++ lib/gui/app.js | 16 +- .../templates/drive-selector-modal.tpl.html | 7 +- lib/gui/models/selection-state.js | 133 +++++++- lib/gui/os/dialog/services/dialog.js | 24 +- lib/gui/partials/main.html | 4 +- lib/gui/scss/components/_list-group.scss | 20 ++ lib/gui/scss/main.scss | 1 + tests/gui/models/selection-state.spec.js | 311 +++++++++++++++--- 9 files changed, 456 insertions(+), 79 deletions(-) create mode 100644 lib/gui/scss/components/_list-group.scss diff --git a/build/css/main.css b/build/css/main.css index cf217d322b..5e20218292 100644 --- a/build/css/main.css +++ b/build/css/main.css @@ -6252,6 +6252,25 @@ button.btn:focus, button.progress-button:focus { .alert-ribbon--open { top: 0; } +/* + * Copyright 2016 Resin.io + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +.list-group-item[disabled] { + text-decoration: line-through; + cursor: not-allowed; } + /* * Copyright 2016 Resin.io * diff --git a/lib/gui/app.js b/lib/gui/app.js index a63babe7b2..7d87a5e400 100644 --- a/lib/gui/app.js +++ b/lib/gui/app.js @@ -160,10 +160,14 @@ app.controller('AppController', function( // `angular.equals` is used instead of `_.isEqual` to // cope with `$$hashKey`. if (!angular.equals(self.selection.getDrive(), drive)) { - AnalyticsService.logEvent('Auto-select drive', { - device: drive.device - }); - self.selectDrive(drive); + + if (self.selection.isDriveLargeEnough(drive)) { + self.selectDrive(drive); + + AnalyticsService.logEvent('Auto-select drive', { + device: drive.device + }); + } } } @@ -181,9 +185,7 @@ app.controller('AppController', function( this.selectImage = function(image) { self.selection.setImage(image); - AnalyticsService.logEvent('Select image', { - image: image - }); + AnalyticsService.logEvent('Select image', image); }; this.openImageSelector = function() { diff --git a/lib/gui/components/drive-selector/templates/drive-selector-modal.tpl.html b/lib/gui/components/drive-selector/templates/drive-selector-modal.tpl.html index e0b499660d..311a693e89 100644 --- a/lib/gui/components/drive-selector/templates/drive-selector-modal.tpl.html +++ b/lib/gui/components/drive-selector/templates/drive-selector-modal.tpl.html @@ -6,12 +6,15 @@ diff --git a/lib/gui/models/selection-state.js b/lib/gui/models/selection-state.js index d4b439fe7f..6ef0b8d7b9 100644 --- a/lib/gui/models/selection-state.js +++ b/lib/gui/models/selection-state.js @@ -42,15 +42,88 @@ SelectionStateModel.service('SelectionStateModel', function() { * * @param {Object} drive - drive * + * @throws Will throw if drive lacks `.device`. + * @throws Will throw if `drive.device` is not a string. + * @throws Will throw if drive lacks `.name`. + * @throws Will throw if `drive.name` is not a string. + * @throws Will throw if drive lacks `.size`. + * @throws Will throw if `drive.size` is not a number. + * @throws Will throw if there is an image and the drive is not large enough. + * * @example * SelectionStateModel.setDrive({ - * device: '/dev/disk2' + * device: '/dev/disk2', + * name: 'USB drive', + * size: 999999999 * }); */ this.setDrive = function(drive) { + + if (!drive.device) { + throw new Error('Missing drive device'); + } + + if (!_.isString(drive.device)) { + throw new Error(`Invalid drive device: ${drive.device}`); + } + + if (!drive.name) { + throw new Error('Missing drive name'); + } + + if (!_.isString(drive.name)) { + throw new Error(`Invalid drive name: ${drive.name}`); + } + + if (!drive.size) { + throw new Error('Missing drive size'); + } + + if (!_.isNumber(drive.size)) { + throw new Error(`Invalid drive size: ${drive.size}`); + } + + if (!self.isDriveLargeEnough(drive)) { + throw new Error('The drive is not large enough'); + } + selection.drive = drive; }; + /** + * @summary Check if a drive is large enough for the selected image + * @function + * @public + * + * @description + * For convenience, if there is no image selected, this function + * returns true. + * + * Notice that if you select the drive before the image, the check + * will not take place and it'll be the client's responsibility + * to do so. + * + * @param {Object} drive - drive + * @returns {Boolean} whether the drive is large enough + * + * @example + * SelectionStateModel.setImage({ + * path: 'rpi.img', + * size: 100000000 + * }); + * + * if (SelectionStateModel.isDriveLargeEnough({ + * device: '/dev/disk2', + * name: 'My Drive', + * size: 123456789 + * })) { + * console.log('We can flash the image to this drive!'); + * } + */ + this.isDriveLargeEnough = function(drive) { + return (self.getImageSize() || 0) <= drive.size; + }; + /** * @summary Toggle set drive * @function @@ -76,12 +149,36 @@ SelectionStateModel.service('SelectionStateModel', function() { * @function * @public * - * @param {String} image - image + * @param {Object} image - image + * + * @throws Will throw if image lacks `.path`. + * @throws Will throw if `image.path` is not a string. + * @throws Will throw if image lacks `.size`. + * @throws Will throw if `image.size` is not a number. * * @example - * SelectionStateModel.setImage('foo.img'); + * SelectionStateModel.setImage({ + * path: 'foo.img' + * }); */ this.setImage = function(image) { + + if (!image.path) { + throw new Error('Missing image path'); + } + + if (!_.isString(image.path)) { + throw new Error(`Invalid image path: ${image.path}`); + } + + if (!image.size) { + throw new Error('Missing image size'); + } + + if (!_.isNumber(image.size)) { + throw new Error(`Invalid image size: ${image.size}`); + } + selection.image = image; }; @@ -104,17 +201,31 @@ SelectionStateModel.service('SelectionStateModel', function() { }; /** - * @summary Get image + * @summary Get image path + * @function + * @public + * + * @returns {String} image path + * + * @example + * const imagePath = SelectionStateModel.getImagePath(); + */ + this.getImagePath = function() { + return _.get(selection.image, 'path'); + }; + + /** + * @summary Get image size * @function * @public * - * @returns {String} image + * @returns {Number} image size * * @example - * const image = SelectionStateModel.getImage(); + * const imageSize = SelectionStateModel.getImageSize(); */ - this.getImage = function() { - return selection.image; + this.getImageSize = function() { + return _.get(selection.image, 'size'); }; /** @@ -146,7 +257,7 @@ SelectionStateModel.service('SelectionStateModel', function() { * } */ this.hasImage = function() { - return Boolean(self.getImage()); + return Boolean(self.getImagePath() && self.getImageSize()); }; /** @@ -157,7 +268,7 @@ SelectionStateModel.service('SelectionStateModel', function() { * @example * SelectionStateModel.removeDrive(); */ - this.removeDrive = _.partial(self.setDrive, undefined); + this.removeDrive = _.partial(_.unset, selection, 'drive'); /** * @summary Remove image @@ -167,7 +278,7 @@ SelectionStateModel.service('SelectionStateModel', function() { * @example * SelectionStateModel.removeImage(); */ - this.removeImage = _.partial(self.setImage, undefined); + this.removeImage = _.partial(_.unset, selection, 'image'); /** * @summary Clear selections diff --git a/lib/gui/os/dialog/services/dialog.js b/lib/gui/os/dialog/services/dialog.js index 2b80897620..8da032c28c 100644 --- a/lib/gui/os/dialog/services/dialog.js +++ b/lib/gui/os/dialog/services/dialog.js @@ -17,6 +17,7 @@ 'use strict'; const _ = require('lodash'); +const fs = require('fs'); const electron = require('electron'); const imageStream = require('etcher-image-stream'); @@ -30,16 +31,16 @@ module.exports = function($q) { * @description * Notice that by image, we mean *.img/*.iso/*.zip/etc files. * - * @fulfil {String} - selected image + * @fulfil {Object} - selected image * @returns {Promise}; * * @example * OSDialogService.selectImage().then(function(image) { - * console.log('The selected image is', image); + * console.log('The selected image is', image.path); * }); */ this.selectImage = function() { - return $q(function(resolve) { + return $q(function(resolve, reject) { electron.remote.dialog.showOpenDialog({ properties: [ 'openFile' @@ -55,8 +56,23 @@ module.exports = function($q) { // `_.first` is smart enough to not throw and return `undefined` // if we pass it an `undefined` value (e.g: when the selection // dialog was cancelled). - return resolve(_.first(files)); + const imagePath = _.first(files); + if (!imagePath) { + return resolve(); + } + + fs.stat(imagePath, function(error, stats) { + + if (error) { + return reject(error); + } + + return resolve({ + path: imagePath, + size: stats.size + }); + }); }); }); }; diff --git a/lib/gui/partials/main.html b/lib/gui/partials/main.html index ed35e61465..4886d61447 100644 --- a/lib/gui/partials/main.html +++ b/lib/gui/partials/main.html @@ -11,7 +11,7 @@
-
+