From 1bfcee06e25db9a87bdab28f8ce338c52cab01f5 Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Thu, 26 Jan 2017 12:01:53 -0400 Subject: [PATCH] refactor(image-stream): stream original/final sizes (#1050) The `image-stream` module currently returns a readable stream, a transform stream, a "size", and an optional "estimatedUncompressedSize". Based on this information, its hard to say what "size" represents. Some format handlers return the compressed size and provide a decompression transform stream while others return the decompressed stream directly and put the decompressed size in "size". As a way to simplify this, every format handler now returns a "size" object with the following properties: - `original`: The original compressed size - `final.estimated`: Whether the final size is an estimation or not - `final.value`: The final uncompressed size As a bonus, we extract the file size retrieval logic to `imageStream.getFromFilePath()`, which is the onlt part where the concept of a file should be referred to. Signed-off-by: Juan Cruz Viotti --- lib/cli/writer.js | 5 ++- lib/gui/os/dialog/services/dialog.js | 2 +- lib/image-stream/archive.js | 10 ++++- lib/image-stream/handlers.js | 66 +++++++++++++++++++++------- lib/image-stream/index.js | 12 +++-- npm-shrinkwrap.json | 5 +++ package.json | 1 + tests/image-stream/bz2.spec.js | 8 +++- tests/image-stream/gz.spec.js | 13 ++++-- tests/image-stream/img.spec.js | 8 +++- tests/image-stream/tester.js | 6 +-- tests/image-stream/xz.spec.js | 11 ++++- tests/image-stream/zip.spec.js | 8 +++- 13 files changed, 122 insertions(+), 33 deletions(-) diff --git a/lib/cli/writer.js b/lib/cli/writer.js index cfc13c83..1539c102 100644 --- a/lib/cli/writer.js +++ b/lib/cli/writer.js @@ -71,7 +71,10 @@ exports.writeImage = (imagePath, drive, options, onProgress) => { fd: driveFileDescriptor, device: drive.raw, size: drive.size - }, image, { + }, { + stream: image.stream, + size: image.size.original + }, { check: options.validateWriteOnSuccess, transform: image.transform, bmap: image.bmap, diff --git a/lib/gui/os/dialog/services/dialog.js b/lib/gui/os/dialog/services/dialog.js index 663b3bbe..f0c6bf16 100644 --- a/lib/gui/os/dialog/services/dialog.js +++ b/lib/gui/os/dialog/services/dialog.js @@ -74,7 +74,7 @@ module.exports = function($q, SupportedFormatsModel) { imageStream.getImageMetadata(imagePath).then((metadata) => { metadata.path = imagePath; - metadata.size = metadata.estimatedUncompressedSize || metadata.size; + metadata.size = metadata.size.final.value; return resolve(metadata); }).catch(reject); }); diff --git a/lib/image-stream/archive.js b/lib/image-stream/archive.js index 1f48a320..0e18fe20 100644 --- a/lib/image-stream/archive.js +++ b/lib/image-stream/archive.js @@ -190,8 +190,16 @@ exports.extractImage = (archive, hooks) => { }) }).then((results) => { results.metadata.stream = results.imageStream; - results.metadata.size = imageEntry.size; results.metadata.transform = new PassThroughStream(); + + results.metadata.size = { + original: imageEntry.size, + final: { + estimation: false, + value: imageEntry.size + } + }; + return results.metadata; }); }); diff --git a/lib/image-stream/handlers.js b/lib/image-stream/handlers.js index e64e0149..3ee3e7bd 100644 --- a/lib/image-stream/handlers.js +++ b/lib/image-stream/handlers.js @@ -40,13 +40,22 @@ module.exports = { * @memberof handlers * * @param {String} file - file path + * @param {Object} options - options + * @param {Number} [options.size] - file size + * * @fulfil {Object} - image metadata * @returns {Promise} */ - 'application/x-bzip2': (file) => { + 'application/x-bzip2': (file, options) => { return Bluebird.props({ stream: fs.createReadStream(file), - size: fs.statAsync(file).get('size'), + size: { + original: options.size, + final: { + estimation: true, + value: options.size + } + }, transform: Bluebird.resolve(unbzip2Stream()) }); }, @@ -58,15 +67,25 @@ module.exports = { * @memberof handlers * * @param {String} file - file path + * @param {Object} options - options + * @param {Number} [options.size] - file size + * * @fulfil {Object} - image metadata * @returns {Promise} */ - 'application/gzip': (file) => { - return Bluebird.props({ - stream: fs.createReadStream(file), - size: fs.statAsync(file).get('size'), - estimatedUncompressedSize: gzipUncompressedSize.fromFileAsync(file), - transform: Bluebird.resolve(zlib.createGunzip()) + 'application/gzip': (file, options) => { + return gzipUncompressedSize.fromFileAsync(file).then((uncompressedSize) => { + return Bluebird.props({ + stream: fs.createReadStream(file), + size: { + original: options.size, + final: { + estimation: true, + value: uncompressedSize + } + }, + transform: Bluebird.resolve(zlib.createGunzip()) + }); }); }, @@ -77,20 +96,28 @@ module.exports = { * @memberof handlers * * @param {String} file - file path + * @param {Object} options - options + * @param {Number} [options.size] - file size + * * @fulfil {Object} - image metadata * @returns {Promise} */ - 'application/x-xz': (file) => { + 'application/x-xz': (file, options) => { return fs.openAsync(file, 'r').then((fileDescriptor) => { return lzma.parseFileIndexFDAsync(fileDescriptor).tap(() => { return fs.closeAsync(fileDescriptor); }); }).then((metadata) => { return { - stream: fs.createReadStream(file) - .pipe(lzma.createDecompressor()), - size: metadata.uncompressedSize, - transform: new PassThroughStream() + stream: fs.createReadStream(file), + size: { + original: options.size, + final: { + estimation: false, + value: metadata.uncompressedSize + } + }, + transform: lzma.createDecompressor() }; }); }, @@ -116,13 +143,22 @@ module.exports = { * @memberof handlers * * @param {String} file - file path + * @param {Object} options - options + * @param {Number} [options.size] - file size + * * @fulfil {Object} - image metadata * @returns {Promise} */ - 'application/octet-stream': (file) => { + 'application/octet-stream': (file, options) => { return Bluebird.props({ stream: fs.createReadStream(file), - size: fs.statAsync(file).get('size'), + size: { + original: options.size, + final: { + estimation: false, + value: options.size + } + }, transform: Bluebird.resolve(new PassThroughStream()) }); } diff --git a/lib/image-stream/index.js b/lib/image-stream/index.js index faa60a93..da881cfa 100644 --- a/lib/image-stream/index.js +++ b/lib/image-stream/index.js @@ -18,6 +18,8 @@ const _ = require('lodash'); const Bluebird = require('bluebird'); +const fs = Bluebird.promisifyAll(require('fs')); +const isStream = require('isstream'); const utils = require('./utils'); const handlers = require('./handlers'); const supportedFileTypes = require('./supported'); @@ -61,11 +63,15 @@ exports.getFromFilePath = (file) => { return Bluebird.try(() => { const type = utils.getArchiveMimeType(file); - if (!handlers[type]) { + if (!_.has(handlers, type)) { throw new Error('Invalid image'); } - return handlers[type](file); + return fs.statAsync(file).then((fileStats) => { + return _.invoke(handlers, type, file, { + size: fileStats.size + }); + }); }).then((image) => { return _.omitBy(image, _.isUndefined); }); @@ -101,7 +107,7 @@ exports.getFromFilePath = (file) => { */ exports.getImageMetadata = (file) => { return exports.getFromFilePath(file).then((image) => { - return _.omitBy(image, _.isObject); + return _.omitBy(image, isStream); }); }; diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index 7de6e48a..ef6ac9e8 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -512,6 +512,11 @@ "from": "isarray@0.0.1", "resolved": "https://registry.npmjs.org/isarray/-/isarray-0.0.1.tgz" }, + "isstream": { + "version": "0.1.2", + "from": "isstream@>=0.1.1 <0.2.0", + "resolved": "https://registry.npmjs.org/isstream/-/isstream-0.1.2.tgz" + }, "isexe": { "version": "1.1.2", "from": "isexe@>=1.1.1 <2.0.0", diff --git a/package.json b/package.json index 48935fda..034bd711 100644 --- a/package.json +++ b/package.json @@ -80,6 +80,7 @@ "gzip-uncompressed-size": "^1.0.0", "immutable": "^3.8.1", "is-elevated": "^1.0.0", + "isstream": "^0.1.2", "lodash": "^4.5.1", "lzma-native": "^1.5.2", "node-ipc": "^8.9.2", diff --git a/tests/image-stream/bz2.spec.js b/tests/image-stream/bz2.spec.js index 34da9f7d..6c7af238 100644 --- a/tests/image-stream/bz2.spec.js +++ b/tests/image-stream/bz2.spec.js @@ -47,7 +47,13 @@ describe('ImageStream: BZ2', function() { imageStream.getImageMetadata(image).then((metadata) => { m.chai.expect(metadata).to.deep.equal({ - size: expectedSize + size: { + original: expectedSize, + final: { + estimation: true, + value: expectedSize + } + } }); done(); }); diff --git a/tests/image-stream/gz.spec.js b/tests/image-stream/gz.spec.js index 46907f5a..08cf10c1 100644 --- a/tests/image-stream/gz.spec.js +++ b/tests/image-stream/gz.spec.js @@ -43,13 +43,18 @@ describe('ImageStream: GZ', function() { it('should return the correct metadata', function(done) { const image = path.join(GZ_PATH, 'raspberrypi.img.gz'); - const expectedSize = fs.statSync(path.join(IMAGES_PATH, 'raspberrypi.img')).size; - const size = fs.statSync(path.join(GZ_PATH, 'raspberrypi.img.gz')).size; + const uncompressedSize = fs.statSync(path.join(IMAGES_PATH, 'raspberrypi.img')).size; + const compressedSize = fs.statSync(path.join(GZ_PATH, 'raspberrypi.img.gz')).size; imageStream.getImageMetadata(image).then((metadata) => { m.chai.expect(metadata).to.deep.equal({ - estimatedUncompressedSize: expectedSize, - size: size + size: { + original: compressedSize, + final: { + estimation: true, + value: uncompressedSize + } + } }); done(); }); diff --git a/tests/image-stream/img.spec.js b/tests/image-stream/img.spec.js index 666788d1..16e16552 100644 --- a/tests/image-stream/img.spec.js +++ b/tests/image-stream/img.spec.js @@ -46,7 +46,13 @@ describe('ImageStream: IMG', function() { imageStream.getImageMetadata(image).then((metadata) => { m.chai.expect(metadata).to.deep.equal({ - size: expectedSize + size: { + original: expectedSize, + final: { + estimation: false, + value: expectedSize + } + } }); done(); }); diff --git a/tests/image-stream/tester.js b/tests/image-stream/tester.js index 16d04732..07ee063c 100644 --- a/tests/image-stream/tester.js +++ b/tests/image-stream/tester.js @@ -60,10 +60,10 @@ exports.extractFromFilePath = function(file, image) { imageStream.getFromFilePath(file).then(function(results) { if (!_.some([ - results.size === fs.statSync(file).size, - results.size === fs.statSync(image).size + results.size.original === fs.statSync(file).size, + results.size.original === fs.statSync(image).size ])) { - throw new Error('Invalid size: ' + results.size); + throw new Error('Invalid size: ' + results.size.original); } const stream = results.stream diff --git a/tests/image-stream/xz.spec.js b/tests/image-stream/xz.spec.js index 28dda9f1..cffb11ab 100644 --- a/tests/image-stream/xz.spec.js +++ b/tests/image-stream/xz.spec.js @@ -43,11 +43,18 @@ describe('ImageStream: XZ', function() { it('should return the correct metadata', function(done) { const image = path.join(XZ_PATH, 'raspberrypi.img.xz'); - const expectedSize = fs.statSync(path.join(IMAGES_PATH, 'raspberrypi.img')).size; + const compressedSize = fs.statSync(image).size; + const uncompressedSize = fs.statSync(path.join(IMAGES_PATH, 'raspberrypi.img')).size; imageStream.getImageMetadata(image).then((metadata) => { m.chai.expect(metadata).to.deep.equal({ - size: expectedSize + size: { + original: compressedSize, + final: { + estimation: false, + value: uncompressedSize + } + } }); done(); }); diff --git a/tests/image-stream/zip.spec.js b/tests/image-stream/zip.spec.js index b380aab7..81be1e07 100644 --- a/tests/image-stream/zip.spec.js +++ b/tests/image-stream/zip.spec.js @@ -71,7 +71,13 @@ describe('ImageStream: ZIP', function() { imageStream.getImageMetadata(image).then((metadata) => { m.chai.expect(metadata).to.deep.equal({ - size: expectedSize + size: { + original: expectedSize, + final: { + estimation: false, + value: expectedSize + } + } }); done(); });