From 4abe5c87d865104266fb67161711cf539900142d Mon Sep 17 00:00:00 2001 From: Lynn Date: Wed, 7 Jun 2017 09:05:25 -0700 Subject: [PATCH] hide readResult method (#131) --- js/preload/preloadMain.js | 2 +- js/screenSnippet/ScreenSnippet.js | 138 +++++++++++++++--------------- package.json | 2 +- tests/ScreenSnippet.test.js | 10 +-- 4 files changed, 77 insertions(+), 75 deletions(-) diff --git a/js/preload/preloadMain.js b/js/preload/preloadMain.js index 90b49a5a..91092514 100644 --- a/js/preload/preloadMain.js +++ b/js/preload/preloadMain.js @@ -107,7 +107,7 @@ function createAPI() { * provides api to allow user to capture portion of screen, see api * details in screenSnipper/ScreenSnippet.js */ - ScreenSnippet: remote.require('./screenSnippet/ScreenSnippet.js'), + ScreenSnippet: remote.require('./screenSnippet/ScreenSnippet.js').ScreenSnippet, /** * Brings window forward and gives focus. diff --git a/js/screenSnippet/ScreenSnippet.js b/js/screenSnippet/ScreenSnippet.js index 64163d9c..651d910d 100644 --- a/js/screenSnippet/ScreenSnippet.js +++ b/js/screenSnippet/ScreenSnippet.js @@ -77,76 +77,80 @@ class ScreenSnippet { // processs was killed, just resolve with no data. resolve(); } else { - this._readResult(outputFileName, resolve, reject, error); + readResult.call(this, outputFileName, resolve, reject, error); } }); }); } - - // private methods below here - - _readResult(outputFileName, resolve, reject, childProcessErr) { - fs.readFile(outputFileName, (readErr, data) => { - if (readErr) { - let returnErr; - if (readErr.code === 'ENOENT') { - // no such file exists, user likely aborted - // creating snippet. also include any error when - // creating child process. - returnErr = this._createWarn('file does not exist ' + - childProcessErr); - } else { - returnErr = this._createError(readErr + ',' + - childProcessErr); - } - - reject(returnErr); - return; - } - - if (!data) { - reject(this._createWarn('no file data provided')); - return; - } - - try { - // convert binary data to base64 encoded string - let output = Buffer(data).toString('base64'); - resolve({ - type: 'image/jpg;base64', - data: output - }); - } catch (error) { - reject(this._createError(error)); - } - finally { - // remove tmp file (async) - fs.unlink(outputFileName, function(removeErr) { - // note: node complains if calling async - // func without callback. - if (removeErr) { - log.send(logLevels.ERROR, 'ScreenSnippet: error removing temp snippet file: ' + - outputFileName + ', err:' + removeErr); - } - }); - } - }); - - } - - /* eslint-disable class-methods-use-this */ - _createError(msg) { - var err = new Error(msg); - err.type = 'ERROR'; - return err; - } - - _createWarn(msg) { - var err = new Error(msg); - err.type = 'WARN'; - return err; - } - /* eslint-enable class-methods-use-this */ } -module.exports = ScreenSnippet; +// this function was moved outside of class since class is exposed to web +// client via preload API, we do NOT want web client to be able to call this +// method - then they could read any file on the disk! +function readResult(outputFileName, resolve, reject, childProcessErr) { + fs.readFile(outputFileName, (readErr, data) => { + if (readErr) { + let returnErr; + if (readErr.code === 'ENOENT') { + // no such file exists, user likely aborted + // creating snippet. also include any error when + // creating child process. + returnErr = createWarn('file does not exist ' + + childProcessErr); + } else { + returnErr = createError(readErr + ',' + + childProcessErr); + } + + reject(returnErr); + return; + } + + if (!data) { + reject(createWarn('no file data provided')); + return; + } + + try { + // convert binary data to base64 encoded string + let output = Buffer(data).toString('base64'); + resolve({ + type: 'image/jpg;base64', + data: output + }); + } catch (error) { + reject(createError(error)); + } + finally { + // remove tmp file (async) + fs.unlink(outputFileName, function(removeErr) { + // note: node complains if calling async + // func without callback. + if (removeErr) { + log.send(logLevels.ERROR, 'ScreenSnippet: error removing temp snippet file: ' + + outputFileName + ', err:' + removeErr); + } + }); + } + }); +} + +/* eslint-disable class-methods-use-this */ +function createError(msg) { + var err = new Error(msg); + err.type = 'ERROR'; + return err; +} + +function createWarn(msg) { + var err = new Error(msg); + err.type = 'WARN'; + return err; +} +/* eslint-enable class-methods-use-this */ + +module.exports = { + ScreenSnippet: ScreenSnippet, + // note: readResult only exposed for testing purposes + readResult: readResult +} diff --git a/package.json b/package.json index 549d7f9e..bbd24404 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,7 @@ "prebuild": "npm run lint && npm run test && npm run browserify-preload", "rebuild": "electron-rebuild -f", "lint": "eslint --ext .js js/", - "test": "jest --testPathPattern test", + "test": "jest --verbose --testPathPattern test", "browserify-preload": "browserify -o js/preload/_preloadMain.js -x electron --insert-global-vars=__filename,__dirname js/preload/preloadMain.js" }, "jest": { diff --git a/tests/ScreenSnippet.test.js b/tests/ScreenSnippet.test.js index 70c12f66..8bec9013 100644 --- a/tests/ScreenSnippet.test.js +++ b/tests/ScreenSnippet.test.js @@ -1,5 +1,5 @@ -const ScreenSnippet = require('../js/screenSnippet/ScreenSnippet.js'); +const { ScreenSnippet, readResult } = require('../js/screenSnippet/ScreenSnippet.js'); const path = require('path'); const fs = require('fs'); const os = require('os'); @@ -71,8 +71,7 @@ describe('Tests for ScreenSnippet', function() { it('should remove output file after completed', function(done) { createTestFile(function(testfileName) { - let s = new ScreenSnippet(); - s._readResult(testfileName, resolve); + readResult(testfileName, resolve); function resolve() { // should be long enough before file @@ -88,9 +87,8 @@ describe('Tests for ScreenSnippet', function() { }); it('should fail if output file does not exist', function(done) { - let s = new ScreenSnippet(); - let nonExistentFile = 'bogus.jpeg' - s._readResult(nonExistentFile, resolve, reject); + let nonExistentFile = 'bogus.jpeg'; + readResult(nonExistentFile, resolve, reject); function resolve() { // shouldn't get here