diff --git a/core/server/lib/image/manipulator.js b/core/server/lib/image/manipulator.js index 5a3549af64..2da4b93fd4 100644 --- a/core/server/lib/image/manipulator.js +++ b/core/server/lib/image/manipulator.js @@ -1,5 +1,6 @@ const Promise = require('bluebird'); const common = require('../common'); +const fs = require('fs-extra'); /** * @NOTE: Sharp cannot operate on the same image path, that's why we have to use in & out paths. @@ -8,15 +9,10 @@ const common = require('../common'); * https://github.com/lovell/sharp/issues/1360. */ const process = (options = {}) => { - let sharp, img; + let sharp, img, originalData, originalSize; try { sharp = require('sharp'); - - // @NOTE: workaround for Windows as libvips keeps a reference to the input file - // which makes it impossible to fs.unlink() it on cleanup stage - sharp.cache(false); - img = sharp(options.in); } catch (err) { return Promise.reject(new common.errors.InternalServerError({ message: 'Sharp wasn\'t installed', @@ -25,8 +21,21 @@ const process = (options = {}) => { })); } - return img.metadata() + // @NOTE: workaround for Windows as libvips keeps a reference to the input file + // which makes it impossible to fs.unlink() it on cleanup stage + sharp.cache(false); + + return fs.readFile(options.in) + .then((data) => { + originalData = data; + + // @NOTE: have to use constructor with Buffer for sharp to be able to expose size property + img = sharp(data); + }) + .then(() => img.metadata()) .then((metadata) => { + originalSize = metadata.size; + if (metadata.width > options.width) { img.resize(options.width); } @@ -34,9 +43,14 @@ const process = (options = {}) => { // CASE: if you call `rotate` it will automatically remove the orientation (and all other meta data) and rotates // based on the orientation. It does not rotate if no orientation is set. img.rotate(); + return img.toBuffer({resolveWithObject: true}); }) - .then(() => { - return img.toFile(options.out); + .then(({data, info}) => { + if (info.size > originalSize) { + return fs.writeFile(options.out, originalData); + } else { + return fs.writeFile(options.out, data); + } }) .catch((err) => { throw new common.errors.InternalServerError({ diff --git a/core/test/unit/lib/image/manipulator_spec.js b/core/test/unit/lib/image/manipulator_spec.js index c9fe6be047..939d77983b 100644 --- a/core/test/unit/lib/image/manipulator_spec.js +++ b/core/test/unit/lib/image/manipulator_spec.js @@ -1,5 +1,6 @@ const should = require('should'); const sinon = require('sinon'); +const fs = require('fs-extra'); const common = require('../../../../server/lib/common'); const manipulator = require('../../../../server/lib/image/manipulator'); const testUtils = require('../../../utils'); @@ -15,11 +16,14 @@ describe('lib/image: manipulator', function () { let sharp, sharpInstance; beforeEach(function () { + sandbox.stub(fs, 'readFile').resolves('original'); + sandbox.stub(fs, 'writeFile').resolves(); + sharpInstance = { metadata: sandbox.stub(), resize: sandbox.stub(), rotate: sandbox.stub(), - toFile: sandbox.stub() + toBuffer: sandbox.stub(), }; sharp = sandbox.stub().callsFake(() => { @@ -32,35 +36,82 @@ describe('lib/image: manipulator', function () { it('resize image', function () { sharpInstance.metadata.resolves({width: 2000, height: 2000}); - sharpInstance.toFile.resolves(); + sharpInstance.toBuffer.resolves({ + data: 'manipulated', + info: { + size: 42 + } + }); return manipulator.process({width: 1000}) .then(() => { sharp.cache.calledOnce.should.be.true(); sharpInstance.metadata.calledOnce.should.be.true(); - sharpInstance.toFile.calledOnce.should.be.true(); sharpInstance.resize.calledOnce.should.be.true(); sharpInstance.rotate.calledOnce.should.be.true(); + + fs.writeFile.calledOnce.should.be.true(); + fs.writeFile.calledWith('manipulated'); }); }); it('skip resizing if image is too small', function () { sharpInstance.metadata.resolves({width: 50, height: 50}); - sharpInstance.toFile.resolves(); + sharpInstance.toBuffer.resolves({ + data: 'manipulated', + info: { + size: 42 + } + }); return manipulator.process({width: 1000}) .then(() => { sharp.cache.calledOnce.should.be.true(); sharpInstance.metadata.calledOnce.should.be.true(); - sharpInstance.toFile.calledOnce.should.be.true(); sharpInstance.resize.calledOnce.should.be.false(); sharpInstance.rotate.calledOnce.should.be.true(); + + fs.writeFile.calledOnce.should.be.true(); + fs.writeFile.calledWith('manipulated'); + }); + }); + + it('uses original image as an output when the size (bytes) is bigger after manipulation', function () { + sharpInstance.metadata.resolves({ + width: 2000, + size: 10 + }); + + sharpInstance.toBuffer.resolves({ + data: 'manipulated', + info: { + size: 42 + } + }); + + return manipulator.process({width: 1000}) + .then(() => { + sharp.cache.calledOnce.should.be.true(); + sharpInstance.metadata.calledOnce.should.be.true(); + sharpInstance.resize.calledOnce.should.be.true(); + sharpInstance.rotate.calledOnce.should.be.true(); + sharpInstance.toBuffer.calledOnce.should.be.true(); + + fs.writeFile.calledOnce.should.be.true(); + fs.writeFile.calledWith('original'); }); }); it('sharp throws error during processing', function () { sharpInstance.metadata.resolves({width: 500, height: 500}); - sharpInstance.toFile.rejects(new Error('whoops')); + sharpInstance.toBuffer.resolves({ + data: 'manipulated', + info: { + size: 42 + } + }); + + fs.writeFile.rejects(new Error('whoops')); return manipulator.process({width: 2000}) .then(() => { @@ -71,9 +122,10 @@ describe('lib/image: manipulator', function () { err.code.should.eql('IMAGE_PROCESSING'); sharp.cache.calledOnce.should.be.true; sharpInstance.metadata.calledOnce.should.be.true(); - sharpInstance.toFile.calledOnce.should.be.true(); sharpInstance.resize.calledOnce.should.be.false(); sharpInstance.rotate.calledOnce.should.be.true(); + + fs.writeFile.calledOnce.should.be.true(); }); }); }); diff --git a/package.json b/package.json index 91e5a7c068..7032405976 100644 --- a/package.json +++ b/package.json @@ -101,7 +101,7 @@ "xml": "1.0.1" }, "optionalDependencies": { - "sharp": "0.20.7", + "sharp": "0.21.0", "sqlite3": "4.0.3" }, "devDependencies": { diff --git a/yarn.lock b/yarn.lock index 3cbe2789bd..5fd9aee260 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4166,7 +4166,7 @@ mysql@2.16.0, mysql@^2.11.1: safe-buffer "5.1.2" sqlstring "2.3.1" -nan@^2.10.0, nan@^2.6.2: +nan@^2.10.0, nan@^2.11.1, nan@^2.6.2: version "2.11.1" resolved "https://registry.yarnpkg.com/nan/-/nan-2.11.1.tgz#90e22bccb8ca57ea4cd37cc83d3819b52eea6766" @@ -4190,6 +4190,11 @@ nanomatch@^1.2.9: snapdragon "^0.8.1" to-regex "^3.0.1" +napi-build-utils@^1.0.1: + version "1.0.1" + resolved "https://registry.yarnpkg.com/napi-build-utils/-/napi-build-utils-1.0.1.tgz#1381a0f92c39d66bf19852e7873432fc2123e508" + integrity sha512-boQj1WFgQH3v4clhu3mTNfP+vOBxorDlE8EKiMjUlLG3C4qAESnn9AxIOkFgTR2c9LtzNjPrjS60cT27ZKBhaA== + natural-compare@^1.4.0: version "1.4.0" resolved "https://registry.yarnpkg.com/natural-compare/-/natural-compare-1.4.0.tgz#4abebfeed7541f2c27acfb29bdbbd15c8d5ba4f7" @@ -4976,21 +4981,23 @@ prebuild-install@^2.3.0: tunnel-agent "^0.6.0" which-pm-runs "^1.0.0" -prebuild-install@^4.0.0: - version "4.0.0" - resolved "https://registry.yarnpkg.com/prebuild-install/-/prebuild-install-4.0.0.tgz#206ce8106ce5efa4b6cf062fc8a0a7d93c17f3a8" +prebuild-install@^5.2.0: + version "5.2.1" + resolved "https://registry.yarnpkg.com/prebuild-install/-/prebuild-install-5.2.1.tgz#87ba8cf17c65360a75eefeb3519e87973bf9791d" + integrity sha512-9DAccsInWHB48TBQi2eJkLPE049JuAI6FjIH0oIrij4bpDVEbX6JvlWRAcAAlUqBHhjgq0jNqA3m3bBXWm9v6w== dependencies: detect-libc "^1.0.3" expand-template "^1.0.2" github-from-package "0.0.0" minimist "^1.2.0" mkdirp "^0.5.1" + napi-build-utils "^1.0.1" node-abi "^2.2.0" noop-logger "^0.1.1" npmlog "^4.0.1" os-homedir "^1.0.1" pump "^2.0.1" - rc "^1.1.6" + rc "^1.2.7" simple-get "^2.7.0" tar-fs "^1.13.0" tunnel-agent "^0.6.0" @@ -5538,18 +5545,19 @@ setprototypeof@1.1.0: version "1.1.0" resolved "https://registry.yarnpkg.com/setprototypeof/-/setprototypeof-1.1.0.tgz#d0bd85536887b6fe7c0d818cb962d9d91c54e656" -sharp@0.20.7: - version "0.20.7" - resolved "https://registry.yarnpkg.com/sharp/-/sharp-0.20.7.tgz#d6e1abbe91453e2200090043f91a5cd345ee95cf" +sharp@0.21.0: + version "0.21.0" + resolved "https://registry.yarnpkg.com/sharp/-/sharp-0.21.0.tgz#e3cf2e4cb9382caf78efb3d45252381730e899c4" + integrity sha512-qr6yMl0ju8EGMtjIj5U1Ojj8sKuZ99/DQaNKWmoFHxqg3692AFSrEiPI/yr0O05OWtGD8LuCw8WSGmnZcNrZaA== dependencies: color "^3.0.0" detect-libc "^1.0.3" fs-copy-file-sync "^1.1.1" - nan "^2.10.0" + nan "^2.11.1" npmlog "^4.1.2" - prebuild-install "^4.0.0" + prebuild-install "^5.2.0" semver "^5.5.1" - simple-get "^2.8.1" + simple-get "^3.0.3" tar "^4.4.6" tunnel-agent "^0.6.0" @@ -5623,7 +5631,7 @@ simple-dom@0.3.2: version "0.3.2" resolved "https://registry.yarnpkg.com/simple-dom/-/simple-dom-0.3.2.tgz#0663d10f1556f1500551d518f56e3aba0871371d" -simple-get@^2.7.0, simple-get@^2.8.1: +simple-get@^2.7.0: version "2.8.1" resolved "https://registry.yarnpkg.com/simple-get/-/simple-get-2.8.1.tgz#0e22e91d4575d87620620bc91308d57a77f44b5d" dependencies: @@ -5631,6 +5639,15 @@ simple-get@^2.7.0, simple-get@^2.8.1: once "^1.3.1" simple-concat "^1.0.0" +simple-get@^3.0.3: + version "3.0.3" + resolved "https://registry.yarnpkg.com/simple-get/-/simple-get-3.0.3.tgz#924528ac3f9d7718ce5e9ec1b1a69c0be4d62efa" + integrity sha512-Wvre/Jq5vgoz31Z9stYWPLn0PqRqmBDpFSdypAnHu5AvRVCYPRYGnvryNLiXu8GOBNDH82J2FRHUGMjjHUpXFw== + dependencies: + decompress-response "^3.3.0" + once "^1.3.1" + simple-concat "^1.0.0" + simple-html-tokenizer@0.5.7: version "0.5.7" resolved "https://registry.yarnpkg.com/simple-html-tokenizer/-/simple-html-tokenizer-0.5.7.tgz#8eca336ecfbe2b3c6166cbb31b2682088de79f40"