🐛 Fixed image optimisation for input image being smaller than optimized one

closes #10144

- When the input image is well optimized and has smaller byte size than the processed one it's still being used
- Bumped sharp version to have access to `size` property

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(() => {
+        .then(({data, info}) => {
-            return img.toFile(options.out);
+            if (info.size > originalSize) {
+                return fs.writeFile(options.out, originalData);
+            } else {
+                return fs.writeFile(options.out, data);
+            }
         })
         .catch((err) => {
             throw new common.errors.InternalServerError({

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();
                 });
         });
     });

package.json

@@ -101,7 +101,7 @@
     "xml": "1.0.1"
   },
   "optionalDependencies": {
-    "sharp": "0.20.7",
+    "sharp": "0.21.0",
     "sqlite3": "4.0.3"
   },
   "devDependencies": {

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:
+prebuild-install@^5.2.0:
-  version "4.0.0"
+  version "5.2.1"
-  resolved "https://registry.yarnpkg.com/prebuild-install/-/prebuild-install-4.0.0.tgz#206ce8106ce5efa4b6cf062fc8a0a7d93c17f3a8"
+  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:
+sharp@0.21.0:
-  version "0.20.7"
+  version "0.21.0"
-  resolved "https://registry.yarnpkg.com/sharp/-/sharp-0.20.7.tgz#d6e1abbe91453e2200090043f91a5cd345ee95cf"
+  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"