From f119b14d95adbe4e0dfd742385f96f87fe843701 Mon Sep 17 00:00:00 2001 From: Antoine Roy-Gobeil Date: Tue, 17 Sep 2019 15:59:36 -0400 Subject: [PATCH 1/7] Dockerfile: add package-lock.json before npm install, copy Orca last :racehorse: --- deployment/Dockerfile | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/deployment/Dockerfile b/deployment/Dockerfile index d39027aa..b2a2b06f 100644 --- a/deployment/Dockerfile +++ b/deployment/Dockerfile @@ -82,22 +82,6 @@ RUN apt-get update -y && \ RUN curl -L https://github.com/plotly/plotly.js/archive/master.tar.gz \ | tar -xvzf - --strip-components=3 plotly.js-master/dist/extras/mathjax -#################### -# Copy and set up Orca - -RUN wget -q -O - https://dl-ssl.google.com/linux/linux_signing_key.pub | apt-key add - && \ - sh -c 'echo "deb [arch=amd64] http://dl.google.com/linux/chrome/deb/ stable main" >> /etc/apt/sources.list.d/google-chrome.list' && \ - apt-get update -y && \ - apt-get install -y google-chrome-stable xvfb poppler-utils git && \ - rm -rf /var/lib/apt/lists/* && apt-get clean - -COPY package.json /var/www/image-exporter/ -COPY bin /var/www/image-exporter/bin -COPY src /var/www/image-exporter/src - -WORKDIR /var/www/image-exporter -RUN npm install && mkdir build - #################### # Install and configure monit COPY deployment/monitrc /etc @@ -127,6 +111,22 @@ RUN wget https://raw.githubusercontent.com/plotly/plotly.js/master/dist/plotly-g COPY deployment/ImageMagickPolicy.xml /etc/ImageMagick-6/policy.xml +#################### +# Copy and set up Orca + +RUN wget -q -O - https://dl-ssl.google.com/linux/linux_signing_key.pub | apt-key add - && \ + sh -c 'echo "deb [arch=amd64] http://dl.google.com/linux/chrome/deb/ stable main" >> /etc/apt/sources.list.d/google-chrome.list' && \ + apt-get update -y && \ + apt-get install -y google-chrome-stable xvfb poppler-utils git && \ + rm -rf /var/lib/apt/lists/* && apt-get clean + +COPY package.json /var/www/image-exporter/ +COPY package-lock.json /var/www/image-exporter/ +WORKDIR /var/www/image-exporter +RUN npm install && mkdir build +COPY bin /var/www/image-exporter/bin +COPY src /var/www/image-exporter/src + #################### # Add entrypoint script COPY deployment/entrypoint.sh / From 407fe05badc440b9af8a10ced2cb4be9a9ea2523 Mon Sep 17 00:00:00 2001 From: tarzzz Date: Mon, 23 Sep 2019 22:07:39 -0400 Subject: [PATCH 2/7] Enable additional options. * `enableLargerThanScreen`: Allows us to specify height/width larger than the max screen size. * `useContentSize`: Does not count the "top bar" in the size. Only the web-content size is taken into account. --- src/component/plotly-dash-preview/render.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/component/plotly-dash-preview/render.js b/src/component/plotly-dash-preview/render.js index 090cda62..bef71541 100644 --- a/src/component/plotly-dash-preview/render.js +++ b/src/component/plotly-dash-preview/render.js @@ -15,6 +15,8 @@ function render (info, opts, sendToMain) { const result = {} let createBrowserWindowOpts = info.browserSize ? info.browserSize : {} + createBrowserWindowOpts['enableLargerThanScreen'] = true + createBrowserWindowOpts['useContentSize'] = true createBrowserWindowOpts['show'] = opts.debug let win = remote.createBrowserWindow(createBrowserWindowOpts) From fdb0a357d32f9029447e75d0f2f7f38b8454a496 Mon Sep 17 00:00:00 2001 From: tarzzz Date: Mon, 23 Sep 2019 22:01:24 -0400 Subject: [PATCH 3/7] Use "math.ceil" for browserSize as well. --- src/component/plotly-dash-preview/parse.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/component/plotly-dash-preview/parse.js b/src/component/plotly-dash-preview/parse.js index 83a62bf2..c012e3c3 100644 --- a/src/component/plotly-dash-preview/parse.js +++ b/src/component/plotly-dash-preview/parse.js @@ -57,6 +57,10 @@ function parse (body, req, opts, sendToRenderer) { ) } + // BrowserWindow only accepts integer values: + result.browserSize['width'] = Math.ceil(result.browserSize['width']) + result.browserSize['height'] = Math.ceil(result.browserSize['height']) + sendToRenderer(null, result) } From c67d1715cc92a26731baac28cf02957d168dff62 Mon Sep 17 00:00:00 2001 From: tarzzz Date: Mon, 23 Sep 2019 22:16:26 -0400 Subject: [PATCH 4/7] update test. --- test/unit/plotly-dash-preview_test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/plotly-dash-preview_test.js b/test/unit/plotly-dash-preview_test.js index 30756d25..6078625e 100644 --- a/test/unit/plotly-dash-preview_test.js +++ b/test/unit/plotly-dash-preview_test.js @@ -51,8 +51,8 @@ tap.test('parse:', t => { // height/width are converted from microns to pixels: t.same(result.browserSize, { - height: 3.779527559055118, - width: 3.779527559055118 + height: 4, + width: 4 }) t.same(result.pdfOptions.pageSize, { height: 1000, From f5a736f1fdcfb41a17c5e2e73bb78964f07e67b9 Mon Sep 17 00:00:00 2001 From: tarzzz Date: Tue, 24 Sep 2019 17:44:16 -0400 Subject: [PATCH 5/7] Use default xvfb window size. 640x480 seems to cause issues with some dash apps, so I am reverting it to the default value (which works) until we figure out a better way to handle page sizes. --- deployment/run_server | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deployment/run_server b/deployment/run_server index 6b1657b7..23b7fdd6 100755 --- a/deployment/run_server +++ b/deployment/run_server @@ -40,7 +40,7 @@ pkill -9 Xvfb pkill -9 node pkill -9 electron -xvfb-run --auto-servernum --server-args '-screen 0 640x480x24' /var/www/image-exporter/bin/orca.js serve $REQUEST_LIMIT --safe-mode --verbose $PLOTLYJS_ARG $ORCA_IGNORECERTERRORS_ARG $@ & +xvfb-run --auto-servernum --server-args '-screen 0 1024x768x24' /var/www/image-exporter/bin/orca.js serve $REQUEST_LIMIT --safe-mode --verbose $PLOTLYJS_ARG $ORCA_IGNORECERTERRORS_ARG $@ & echo \$! > \$PIDFILE EOF From 0943a790019855649f6d9d223395c603fd94f1e0 Mon Sep 17 00:00:00 2001 From: Antoine Roy-Gobeil Date: Thu, 26 Sep 2019 15:19:55 -0400 Subject: [PATCH 6/7] dash-preview: properly parse (pdf_options.)pageSize and :lock: with test --- src/component/plotly-dash-preview/parse.js | 24 +++-- test/unit/plotly-dash-preview_test.js | 112 ++++++++++++++------- 2 files changed, 91 insertions(+), 45 deletions(-) diff --git a/src/component/plotly-dash-preview/parse.js b/src/component/plotly-dash-preview/parse.js index c012e3c3..3f26c191 100644 --- a/src/component/plotly-dash-preview/parse.js +++ b/src/component/plotly-dash-preview/parse.js @@ -36,17 +36,25 @@ function parse (body, req, opts, sendToRenderer) { result.timeOut = body.timeout result.tries = Number(result.timeOut * 1000 / cst.minInterval) - if (cst.sizeMapping[result.pdfOptions.pageSize]) { - result.browserSize = cst.sizeMapping[result.pdfOptions.pageSize] - } else if (body.pageSize && isPositiveNumeric(body.pageSize.width) && - isPositiveNumeric(body.pageSize.height)) { + var pageSize + if (result.pdfOptions.pageSize) { + pageSize = result.pdfOptions.pageSize + } else if (body.pageSize) { + pageSize = body.pageSize + } + + if (cst.sizeMapping[pageSize]) { + result.browserSize = cst.sizeMapping[pageSize] + result.pdfOptions.pageSize = pageSize + } else if (pageSize && isPositiveNumeric(pageSize.width) && + isPositiveNumeric(pageSize.height)) { result.browserSize = { - width: body.pageSize.width * cst.pixelsInMicron, - height: body.pageSize.height * cst.pixelsInMicron + width: pageSize.width * cst.pixelsInMicron, + height: pageSize.height * cst.pixelsInMicron } result.pdfOptions.pageSize = { - width: Math.ceil(body.pageSize.width), - height: Math.ceil(body.pageSize.height) + width: Math.ceil(pageSize.width), + height: Math.ceil(pageSize.height) } } else { return errorOut( diff --git a/test/unit/plotly-dash-preview_test.js b/test/unit/plotly-dash-preview_test.js index 6078625e..873642ff 100644 --- a/test/unit/plotly-dash-preview_test.js +++ b/test/unit/plotly-dash-preview_test.js @@ -2,9 +2,14 @@ const tap = require('tap') const sinon = require('sinon') const _module = require('../../src/component/plotly-dash-preview') +const constants = require('../../src/component/plotly-dash-preview/constants') const remote = require('../../src/util/remote') const { createMockWindow } = require('../common') +function clone (obj) { + return JSON.parse(JSON.stringify(obj)) +} + tap.test('parse:', t => { const fn = _module.parse @@ -30,49 +35,82 @@ tap.test('parse:', t => { t.end() }) }) - t.test('should error when pageSize is not given', t => { - fn({ + t.test('pageSize options:', t => { + const mock = { url: 'https://dash-app.com', selector: 'dummy' - }, {}, {}, (errorCode, result) => { - t.equal(errorCode, 400) - t.same(result.msg, 'pageSize must either be A3, A4, A5, Legal, Letter, ' + - 'Tabloid or an Object containing height and width in microns.') - t.end() - }) - }) - t.test('should parse properly when pageSize is given', t => { - fn({ - url: 'https://dash-app.com', - selector: 'dummy', - pageSize: { height: 1000, width: 1000 } - }, {}, {}, (errorCode, result) => { - t.equal(errorCode, null) - - // height/width are converted from microns to pixels: - t.same(result.browserSize, { - height: 4, - width: 4 + } + + t.test('should error when not given', t => { + fn({ + url: 'https://dash-app.com', + selector: 'dummy' + }, {}, {}, (errorCode, result) => { + t.equal(errorCode, 400) + t.same(result.msg, 'pageSize must either be A3, A4, A5, Legal, Letter, ' + + 'Tabloid or an Object containing height and width in microns.') + t.end() }) - t.same(result.pdfOptions.pageSize, { - height: 1000, - width: 1000 + }) + + function assertEqualSize (browserSize, pageSize) { + // Browser size is always integer pixels + var bW = browserSize.width + var bH = browserSize.height + t.ok(Number.isInteger(bW), 'browserSize.width is not an integer') + t.ok(Number.isInteger(bH), 'browserSize.height is not an integer') + var pW, pH + if (constants.sizeMapping[pageSize]) { + var equivalentPixelSize = constants.sizeMapping[pageSize] + pW = equivalentPixelSize.width + pH = equivalentPixelSize.height + } else { + pW = pageSize.width * constants.pixelsInMicron + pH = pageSize.height * constants.pixelsInMicron + } + // Round + pW = Math.ceil(pW) + pH = Math.ceil(pH) + t.equal(bW, pW, 'browser and page should have the same width') + t.equal(bH, pH, 'browser and page should have the same height') + } + + // Browser size and page size should be the same assuming a DPI of 96 + // to make sure plotly.js figures are appropriately sized right away for print + [ + [true, { height: 1000, width: 1000 }], + [true, 'Letter'], + [false, { height: 1000, width: 1000 }], + [false, 'Letter'] + ].forEach(arg => { + var toplevel = arg[0] + var pageSize = arg[1] + t.test(`should size window and page properly when ${toplevel ? '' : 'pdf_options.'}pageSize is given`, t => { + var body = clone(mock) + if (toplevel) { + body.pageSize = pageSize + } else { + body.pdf_options = { pageSize: pageSize } + } + fn(body, {}, {}, (errorCode, result) => { + t.equal(errorCode, null) + t.same(result.pdfOptions.pageSize, pageSize) + assertEqualSize(result.browserSize, result.pdfOptions.pageSize) + t.end() + }) }) - t.end() }) - }) - t.test('should parse properly when pdf_options are given', t => { - fn({ - url: 'https://dash-app.com', - selector: 'dummy', - pdf_options: { pageSize: 'Letter', marginsType: 1 } - }, {}, {}, (errorCode, result) => { - t.equal(errorCode, null) - // height/width are converted to pixels from page-type: - t.same(result.browserSize, { height: 1056, width: 816 }) - t.same(result.pdfOptions, { pageSize: 'Letter', marginsType: 1 }) - t.end() + + t.test('should passthrough pdf_options', t => { + var body = clone(mock) + body.pdf_options = { pageSize: 'Letter', marginsType: 1, crazyOptions: true } + fn(body, {}, {}, (errorCode, result) => { + t.equal(errorCode, null) + t.same(result.pdfOptions, { pageSize: 'Letter', marginsType: 1, crazyOptions: true }) + t.end() + }) }) + t.end() }) t.end() From 156a35cc5e6111a85910fa47fa6eb670db5708f8 Mon Sep 17 00:00:00 2001 From: Antoine Roy-Gobeil Date: Mon, 30 Sep 2019 14:23:09 -0400 Subject: [PATCH 7/7] dash-preview: turn mock object into a function returning a mock --- test/unit/plotly-dash-preview_test.js | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/test/unit/plotly-dash-preview_test.js b/test/unit/plotly-dash-preview_test.js index 873642ff..d2695f9a 100644 --- a/test/unit/plotly-dash-preview_test.js +++ b/test/unit/plotly-dash-preview_test.js @@ -6,10 +6,6 @@ const constants = require('../../src/component/plotly-dash-preview/constants') const remote = require('../../src/util/remote') const { createMockWindow } = require('../common') -function clone (obj) { - return JSON.parse(JSON.stringify(obj)) -} - tap.test('parse:', t => { const fn = _module.parse @@ -36,16 +32,13 @@ tap.test('parse:', t => { }) }) t.test('pageSize options:', t => { - const mock = { + const mock = () => ({ url: 'https://dash-app.com', selector: 'dummy' - } + }) t.test('should error when not given', t => { - fn({ - url: 'https://dash-app.com', - selector: 'dummy' - }, {}, {}, (errorCode, result) => { + fn(mock(), {}, {}, (errorCode, result) => { t.equal(errorCode, 400) t.same(result.msg, 'pageSize must either be A3, A4, A5, Legal, Letter, ' + 'Tabloid or an Object containing height and width in microns.') @@ -86,7 +79,7 @@ tap.test('parse:', t => { var toplevel = arg[0] var pageSize = arg[1] t.test(`should size window and page properly when ${toplevel ? '' : 'pdf_options.'}pageSize is given`, t => { - var body = clone(mock) + var body = mock() if (toplevel) { body.pageSize = pageSize } else { @@ -102,7 +95,7 @@ tap.test('parse:', t => { }) t.test('should passthrough pdf_options', t => { - var body = clone(mock) + var body = mock() body.pdf_options = { pageSize: 'Letter', marginsType: 1, crazyOptions: true } fn(body, {}, {}, (errorCode, result) => { t.equal(errorCode, null)