refactor: use an SDK orchestrator to implement drive scanning (#1707)

This is a major first step towards adopting an SDK architecture. This
commit creates an SDK adaptor with a `.scan()` function that uses
`drivelist` under the hood. Then, an SDK orchestrator is used to provide
drive scanning capabilities to the GUI.

Here's a list of some particularly interesting changes:

- The drives returned by the SDK adaptor now have a "pending" and an
  "adaptor" property. The "pending" property is a boolean flag that
  determines if the drive is ready to be used (this will come handy for
  usbboot), while the "adaptor" property simply contains the name of the
  adaptor that drive came from

- The GUI drive scanner Rx implementation was replaces with a "promise
  loop." Before, the drive scanning routine would be called every 2
  seconds (without waiting for the previous scan to complete), while
  now, the next scan happens *after* the previous scan completes. For
  this reason, I reduced the drive scanning interval timeout to match
  the timing we had before

Change-Type: patch
See: https://github.com/resin-io/etcher/pull/1686
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
This commit is contained in:
Juan Cruz Viotti 2017-08-24 17:24:48 -04:00 committed by GitHub
parent f9d7dd2977
commit 9f4712f1f8
6 changed files with 235 additions and 57 deletions

View File

@ -16,40 +16,25 @@
'use strict'
const Rx = require('rx')
const _ = require('lodash')
const EventEmitter = require('events').EventEmitter
const drivelist = require('drivelist')
const Bluebird = require('bluebird')
const settings = require('../models/settings')
const sdk = require('../../shared/sdk')
const DRIVE_SCANNER_INTERVAL_MS = 2000
const DRIVE_SCANNER_FIRST_SCAN_DELAY_MS = 0
/**
* @summary Time to wait between scans
* @type {Number}
* @constant
*/
const DRIVE_SCANNER_INTERVAL_MS = 1000
/**
* @summary Scanner event emitter singleton instance
* @type {Object}
* @constant
*/
const emitter = new EventEmitter()
/* eslint-disable lodash/prefer-lodash-method */
const availableDrives = Rx.Observable.timer(
DRIVE_SCANNER_FIRST_SCAN_DELAY_MS,
DRIVE_SCANNER_INTERVAL_MS
)
/* eslint-enable lodash/prefer-lodash-method */
.flatMap(() => {
return Rx.Observable.fromNodeCallback(drivelist.list)()
})
.map((drives) => {
if (settings.get('unsafeMode')) {
return drives
}
return _.reject(drives, {
system: true
})
})
.pausable(new Rx.Subject())
/*
* This service emits the following events:
*
@ -68,11 +53,51 @@ const availableDrives = Rx.Observable.timer(
* });
* ```
*/
availableDrives.subscribe((drives) => {
emitter.emit('drives', drives)
}, (error) => {
emitter.emit('error', error)
})
/**
* @summary Flag to control scanning status
* @type {Boolean}
*/
let scanning = false
/**
* @summary Start the scanning loop
* @function
* @private
*
* @description
* This function emits `drives` or `error` events
* using the event emitter singleton instance.
*
* @returns {Promise}
*
* @example
* scanning = true
* scan()
*/
const scan = () => {
if (!scanning) {
return Bluebird.resolve()
}
return sdk.scan({
standard: {
includeSystemDrives: settings.get('unsafeMode')
}
}).then((drives) => {
emitter.emit('drives', drives)
}).catch((error) => {
emitter.emit('error', error)
}).finally(() => {
if (!scanning) {
return Bluebird.resolve()
}
return Bluebird
.delay(DRIVE_SCANNER_INTERVAL_MS)
.then(scan)
})
}
/**
* @summary Start scanning drives
@ -83,7 +108,10 @@ availableDrives.subscribe((drives) => {
* driveScanner.start();
*/
emitter.start = () => {
availableDrives.resume()
if (!scanning) {
scanning = true
scan()
}
}
/**
@ -95,7 +123,7 @@ emitter.start = () => {
* driveScanner.stop();
*/
emitter.stop = () => {
availableDrives.pause()
scanning = false
}
module.exports = emitter

58
lib/shared/sdk/index.js Normal file
View File

@ -0,0 +1,58 @@
/*
* Copyright 2017 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.
*/
'use strict'
const Bluebird = require('bluebird')
const _ = require('lodash')
/**
* @summary The list of loaded adaptors
* @type {Object[]}
* @constant
*/
const ADAPTORS = [
require('./standard')
]
/**
* @summary Scan for drives using all registered adaptors
* @function
* @public
*
* @description
* The options object contains options for all the registered
* adaptors. For the `standard` adaptor, for example, place
* options in `options.standard`.
*
* @param {Object} options - options
* @fulfil {Object[]} - drives
* @returns {Promise}
*
* @example
* sdk.scan({
* standard: {
* includeSystemDrives: true
* }
* }).then((drives) => {
* console.log(drives)
* })
*/
exports.scan = (options) => {
return Bluebird.all(_.map(ADAPTORS, (adaptor) => {
return adaptor.scan(_.get(options, [ adaptor.name ], {}))
})).then(_.flatten)
}

View File

@ -0,0 +1,56 @@
/*
* Copyright 2017 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.
*/
'use strict'
const Bluebird = require('bluebird')
const drivelist = Bluebird.promisifyAll(require('drivelist'))
/**
* @summary The name of this adaptor
* @public
* @type {String}
* @constant
*/
exports.name = 'standard'
/**
* @summary Scan for block devices
* @function
* @public
*
* @param {Object} [options] - options
* @param {Object} [options.includeSystemDrives=false] - include system drives
* @fulfil {Object[]} - block devices
* @returns {Promise}
*
* @example
* standard.scan({
* includeSystemDrives: true
* }).each((device) => {
* console.log(device)
* })
*/
exports.scan = (options = {}) => {
// eslint-disable-next-line lodash/prefer-lodash-method
return drivelist.listAsync().filter((drive) => {
return options.includeSystemDrives || !drive.system
}).map((drive) => {
drive.pending = false
drive.adaptor = exports.name
return drive
})
}

5
npm-shrinkwrap.json generated
View File

@ -8995,11 +8995,6 @@
"from": "run-async@>=0.1.0 <0.2.0",
"resolved": "https://registry.npmjs.org/run-async/-/run-async-0.1.0.tgz"
},
"rx": {
"version": "4.1.0",
"from": "rx@4.1.0",
"resolved": "https://registry.npmjs.org/rx/-/rx-4.1.0.tgz"
},
"rx-lite": {
"version": "3.1.2",
"from": "rx-lite@>=3.1.2 <4.0.0",

View File

@ -77,7 +77,6 @@
"resin-cli-form": "1.4.1",
"resin-cli-visuals": "1.3.1",
"resin-corvus": "1.0.0-beta.28",
"rx": "4.1.0",
"semver": "5.1.1",
"sudo-prompt": "6.1.0",
"trackjs": "2.3.1",

View File

@ -18,27 +18,34 @@
const m = require('mochainon')
const os = require('os')
const Bluebird = require('bluebird')
const drivelist = require('drivelist')
const driveScanner = require('../../../lib/gui/modules/drive-scanner')
const sdk = require('../../../lib/shared/sdk')
describe('Browser: driveScanner', function () {
describe('given no available drives', function () {
beforeEach(function () {
this.drivelistStub = m.sinon.stub(drivelist, 'list')
this.drivelistStub.yields(null, [])
this.sdkScanStub = m.sinon.stub(sdk, 'scan')
this.sdkScanStub.returns(Bluebird.resolve([]))
})
afterEach(function () {
this.drivelistStub.restore()
this.sdkScanStub.restore()
})
it('should emit an empty array', function (done) {
const spy = m.sinon.spy()
driveScanner.once('drives', function (drives) {
m.chai.expect(drives).to.deep.equal([])
m.chai.expect(spy).to.not.have.been.called
driveScanner.removeListener('error', spy)
driveScanner.stop()
done()
})
driveScanner.on('error', spy)
driveScanner.start()
})
})
@ -46,17 +53,19 @@ describe('Browser: driveScanner', function () {
describe('given only system available drives', function () {
beforeEach(function () {
this.drivelistStub = m.sinon.stub(drivelist, 'list')
this.drivelistStub.yields(null, [ {
device: '/dev/sda',
description: 'WDC WD10JPVX-75J',
size: '931.5G',
mountpoints: [
{
path: '/'
}
],
system: true
} ])
this.drivelistStub.yields(null, [
{
device: '/dev/sda',
description: 'WDC WD10JPVX-75J',
size: '931.5G',
mountpoints: [
{
path: '/'
}
],
system: true
}
])
})
afterEach(function () {
@ -64,12 +73,17 @@ describe('Browser: driveScanner', function () {
})
it('should emit an empty array', function (done) {
const spy = m.sinon.spy()
driveScanner.once('drives', function (drives) {
m.chai.expect(drives).to.deep.equal([])
m.chai.expect(spy).to.not.have.been.called
driveScanner.removeListener('error', spy)
driveScanner.stop()
done()
})
driveScanner.on('error', spy)
driveScanner.start()
})
})
@ -132,6 +146,8 @@ describe('Browser: driveScanner', function () {
})
it('should emit the non removable drives', function (done) {
const spy = m.sinon.spy()
driveScanner.once('drives', function (drives) {
m.chai.expect(drives).to.deep.equal([
{
@ -144,6 +160,8 @@ describe('Browser: driveScanner', function () {
path: '/mnt/foo'
}
],
pending: false,
adaptor: 'standard',
system: false
},
{
@ -156,14 +174,19 @@ describe('Browser: driveScanner', function () {
path: '/mnt/bar'
}
],
pending: false,
adaptor: 'standard',
system: false
}
])
m.chai.expect(spy).to.not.have.been.called
driveScanner.removeListener('error', spy)
driveScanner.stop()
done()
})
driveScanner.on('error', spy)
driveScanner.start()
})
})
@ -223,6 +246,8 @@ describe('Browser: driveScanner', function () {
})
it('should emit the non removable drives', function (done) {
const spy = m.sinon.spy()
driveScanner.once('drives', function (drives) {
m.chai.expect(drives).to.deep.equal([
{
@ -231,6 +256,8 @@ describe('Browser: driveScanner', function () {
description: 'Foo',
size: '14G',
mountpoints: [],
pending: false,
adaptor: 'standard',
system: false
},
{
@ -243,14 +270,19 @@ describe('Browser: driveScanner', function () {
path: 'F:'
}
],
pending: false,
adaptor: 'standard',
system: false
}
])
m.chai.expect(spy).to.not.have.been.called
driveScanner.removeListener('error', spy)
driveScanner.stop()
done()
})
driveScanner.on('error', spy)
driveScanner.start()
})
})
@ -279,13 +311,18 @@ describe('Browser: driveScanner', function () {
})
it('should use the drive letter as the name', function (done) {
const spy = m.sinon.spy()
driveScanner.once('drives', function (drives) {
m.chai.expect(drives).to.have.length(1)
m.chai.expect(drives[0].displayName).to.equal('F:')
m.chai.expect(spy).to.not.have.been.called
driveScanner.removeListener('error', spy)
driveScanner.stop()
done()
})
driveScanner.on('error', spy)
driveScanner.start()
})
})
@ -320,13 +357,18 @@ describe('Browser: driveScanner', function () {
})
it('should join all the mountpoints in `name`', function (done) {
const spy = m.sinon.spy()
driveScanner.once('drives', function (drives) {
m.chai.expect(drives).to.have.length(1)
m.chai.expect(drives[0].displayName).to.equal('F:, G:, H:')
m.chai.expect(spy).to.not.have.been.called
driveScanner.removeListener('error', spy)
driveScanner.stop()
done()
})
driveScanner.on('error', spy)
driveScanner.start()
})
})
@ -343,7 +385,7 @@ describe('Browser: driveScanner', function () {
})
it('should emit the error', function (done) {
driveScanner.on('error', function (error) {
driveScanner.once('error', function (error) {
m.chai.expect(error).to.be.an.instanceof(Error)
m.chai.expect(error.message).to.equal('scan error')
driveScanner.stop()