Improvements and bug fixes related to resizing with format conversion (#103)

kdybicz · web-flow · commit f9eed2f1f5b6 · 2017-02-20T20:43:07.000+01:00
* simplifying the format conversion approach

* allowing to set quality level for the JpegOptim optimizer

* fixing issue with incorrect file extension after format conversion

* adding new test cases

* adding error handler for child processes to help investigate issues with binaries

* asking Travis to install missing dependency

* switching test helper methods to async style
diff --git a/.travis.yml b/.travis.yml
@@ -1,5 +1,9 @@
 language: node_js
 node_js:
   - 4
+addons:
+  apt:
+    packages:
+    - libjpeg62
 after_success:
   - './node_modules/.bin/nyc report --reporter=text-lcov | ./node_modules/.bin/coveralls'
diff --git a/lib/ImageData.js b/lib/ImageData.js
@@ -4,6 +4,7 @@ const path = require("path");
 const imageType = require("image-type");
 
 class ImageType {
+
     /**
      * Gets real image type from ImageData
      *
@@ -158,8 +159,8 @@ class ImageData {
     combineWithDirectory(directory, filePrefix, fileSuffix) {
         const prefix = filePrefix || "";
         const suffix = fileSuffix || "";
-        const fileName = this.baseName.substr(0, this.baseName.lastIndexOf('.'));
-        const extension = this.baseName.substr(this.baseName.lastIndexOf('.'));
+        const fileName = path.parse(this.baseName).name;
+        const extension = "." + this.type.ext;
         if ( directory != null ) {
             // ./X , ../X , . , ..
             if ( directory.match(/^\.\.?\//) || directory.match(/^\.\.?$/) ) {
diff --git a/lib/ImageReducer.js b/lib/ImageReducer.js
@@ -2,11 +2,11 @@
 
 const ImageData      = require("./ImageData");
 const Mozjpeg        = require("./optimizer/Mozjpeg");
+const JpegOptim      = require("./optimizer/JpegOptim");
 const Pngquant       = require("./optimizer/Pngquant");
 const Gifsicle       = require("./optimizer/Gifsicle");
 const ReadableStream = require("./ReadableImageStream");
 const StreamChain    = require("./StreamChain");
-// const JpegOptim    = require("./optimizer/JpegOptim");
 
 class ImageReducer {
 
@@ -59,37 +59,23 @@ class ImageReducer {
         console.log("Reducing to: " + (this.option.directory || "in-place"));
 
         const streams = [];
-
-        let outputType;
-
-        if ( type == "gif" ) {
-            outputType = type;
-        } else if ( "quality" in this.option ) {
-            outputType = "jpg"; // force JPEG when quality given
-        } else if ( "format" in this.option ) {
-            outputType = this.option.format;
-        } else {
-            outputType = type;
-        }
-
-        switch ( outputType ) {
+        switch ( type ) {
             case "png":
                 streams.push(new Pngquant());
                 break;
             case "jpg":
             case "jpeg":
-                streams.push(new Mozjpeg(this.option.quality));
-                // switch JPEG optimizer
-                // if ( this.option.jpegOptimizer === "jpegoptim" ) { // using jpegoptim
-                //     streams.push(new JpegOptim());
-                // } else {                                           // using mozjpeg
-                // }
+                if ( this.option.jpegOptimizer === "jpegoptim" ) { // using jpegoptim
+                    streams.push( new JpegOptim( this.option.quality ) );
+                } else {                                           // using mozjpeg
+                    streams.push( new Mozjpeg( this.option.quality ) );
+                }
                 break;
             case "gif":
                 streams.push(new Gifsicle());
                 break;
             default:
-                throw new Error("Unexcepted output type: " + outputType);
+                throw new Error("Unexpected output type: " + type);
         }
 
         return streams;
diff --git a/lib/ImageResizer.js b/lib/ImageResizer.js
@@ -41,9 +41,6 @@ class ImageResizer {
             if ( "background" in this.options ) {
               img = img.background(this.options.background).flatten();
             }
-            if ( "quality" in this.options ) {
-              img = img.quality(this.options.quality);
-            }
             if ( "crop" in this.options ) {
                 var cropArgs = this.options.crop.match(cropSpec);
                 const cropWidth = cropArgs[1];
@@ -53,45 +50,25 @@ class ImageResizer {
                 const cropPercent = cropArgs[5];
                 img = img.crop(cropWidth, cropHeight, cropX, cropY, cropPercent === "%");
             }
+            if( "format" in this.options ) {
+                img = img.setFormat(this.options.format);
+            }
 
-            img.toBuffer(this.detectFormat(image), (err, buffer) => {
+            img.toBuffer((err, buffer) => {
                 if (err) {
-                    return reject(err);
+                    reject(err);
+                } else {
+                    resolve(new ImageData(
+                        image.fileName,
+                        image.bucketName,
+                        buffer,
+                        image.headers,
+                        acl
+                    ));
                 }
-                resolve(new ImageData(
-                    image.fileName,
-                    image.bucketName,
-                    buffer,
-                    image.headers,
-                    acl
-                ));
             });
         });
     }
-
-    /**
-     * Format detection
-     *
-     * @protected
-     * @param ImageData image
-     * @return string
-     * @throws Error
-     */
-    detectFormat(image) {
-        // Does options exists?
-        if ( "format" in this.options ) {
-            return this.options.format.toUpperCase();
-        }
-
-        // Detect from MimeType
-        switch (image.type.mime) {
-          case "image/gif":  return "GIF";
-          case "image/jpeg": return "JPG";
-          case "image/png":  return "PNG";
-          default:
-            throw new Error("Unexpected MimeType: " + image.type.mime);
-        }
-    }
 }
 
 module.exports = ImageResizer;
diff --git a/lib/optimizer/JpegOptim.js b/lib/optimizer/JpegOptim.js
@@ -8,12 +8,19 @@ class Jpegoptim extends Optimizer {
      *
      * @constructor
      * @extends Optimizer
+     * @param Number|undefined quality
      */
-    constructor() {
+    constructor(quality) {
         super();
 
         this.command = this.findBin("jpegoptim");
         this.args    = ["--stdin", "-s", "--all-progressive", "--stdout"];
+
+        // determine quality if supplied
+        if ( quality ) {
+            this.args.unshift(quality);
+            this.args.unshift("-m");
+        }
     }
 }
 
diff --git a/lib/optimizer/Optimizer.js b/lib/optimizer/Optimizer.js
@@ -21,7 +21,11 @@ class Optimizer {
      * @return ChildProcess
      */
     spawnProcess() {
-        return spawn(this.command, this.args);
+        const process = spawn(this.command, this.args);
+        process.on('err', (err) => {
+            console.log(`Child process ended with error code ${err}`);
+        });
+        return process
     }
 
     /**
diff --git a/test/e2e-jpeg-jpegoptim.js b/test/e2e-jpeg-jpegoptim.js
@@ -15,7 +15,7 @@ const setting        = JSON.parse(fs.readFileSync(sourceFile));
 let processor;
 let images;
 
-test.before(() => {
+test.before(async t => {
     sinon.stub(S3, "getObject", () => {
         return fsP.readFile(`${__dirname}/fixture/fixture.jpg`).then(data => {
             return new ImageData(
@@ -25,23 +25,23 @@ test.before(() => {
             );
         });
     });
-    images = [];
     sinon.stub(S3, "putObject", (image) => {
         images.push(image);
-        return new Promise((resolve) => resolve(image));
+        return Promise.resolve(image);
     });
 });
 
-test.after(() => {
+test.after(async t => {
     S3.getObject.restore();
     S3.putObject.restore();
 });
 
-test.beforeEach(() => {
+test.beforeEach(async t => {
     processor = new ImageProcessor(setting.Records[0].s3, {
         done: () => {},
         fail: () => {}
     });
+    images = [];
 });
 
 test("Reduce JPEG with no configuration", async t => {
@@ -80,3 +80,18 @@ test("Reduce JPEG with bucket/directory configuration", async t => {
     t.true(image.data.length > 0);
     t.true(image.data.length < fixture.length);
 });
+
+
+test("Reduce JPEG with quality", async t => {
+    await processor.run(new Config({
+        "reduce": {
+            "quality": 90,
+            "jpegOptimizer": "jpegoptim"
+        }
+    }));
+    t.is(images.length, 1);
+    const image = images.shift();
+    const fixture = await fsP.readFile(`${__dirname}/fixture/fixture.jpg`);
+    t.true(image.data.length > 0);
+    t.true(image.data.length < fixture.length);
+});
diff --git a/test/e2e-jpeg.js b/test/e2e-jpeg.js
@@ -15,7 +15,7 @@ const setting        = JSON.parse(fs.readFileSync(sourceFile));
 let processor;
 let images;
 
-test.before(() => {
+test.before(async t => {
     sinon.stub(S3, "getObject", () => {
         return fsP.readFile(`${__dirname}/fixture/fixture.jpg`).then(data => {
             return new ImageData(
@@ -25,23 +25,23 @@ test.before(() => {
             );
         });
     });
-    images = [];
     sinon.stub(S3, "putObject", (image) => {
         images.push(image);
         return Promise.resolve(image);
     });
 });
 
-test.after(() => {
+test.after(async t => {
     S3.getObject.restore();
     S3.putObject.restore();
 });
 
-test.beforeEach(() => {
+test.beforeEach(async t => {
     processor = new ImageProcessor(setting.Records[0].s3, {
         done: () => {},
         fail: () => {}
     });
+    images = [];
 });
 
 test("Reduce JPEG with no configuration", async t => {
@@ -78,3 +78,44 @@ test("Reduce JPEG with bucket/directory configuration", async t => {
     t.true(image.data.length > 0);
     t.true(image.data.length < fixture.length);
 });
+
+test("Resize JPEG with quality", async t => {
+    await processor.run(new Config({
+        "resizes": [
+            {
+                "size": 100,
+                "quality": 90
+            }
+        ]
+    }));
+    t.is(images.length, 1);
+    const image = images.shift();
+    const fixture = await fsP.readFile(`${__dirname}/fixture/fixture.jpg`);
+    t.is(image.fileName, "HappyFace.jpg");
+    t.true(image.data.length > 0);
+    t.true(image.data.length < fixture.length);
+});
+
+test("Resize JPEG with format", async t => {
+    await processor.run(new Config({
+        "resizes": [
+            {
+                "size": 100,
+                "format": "png"
+            },
+            {
+                "size": 100,
+                "format": "gif"
+            }
+        ]
+    }));
+    t.is(images.length, 2);
+
+    const pngImage = images.shift();
+    t.is(pngImage.fileName, "HappyFace.png");
+    t.true(pngImage.data.length > 0);
+
+    const gifImage = images.shift();
+    t.is(gifImage.fileName, "HappyFace.gif");
+    t.true(gifImage.data.length > 0);
+});
diff --git a/test/e2e-png.js b/test/e2e-png.js
@@ -15,7 +15,7 @@ const setting        = JSON.parse(fs.readFileSync(sourceFile));
 let processor;
 let images;
 
-test.before(() => {
+test.before(async t => {
     sinon.stub(S3, "getObject", () => {
         return fsP.readFile(`${__dirname}/fixture/fixture.png`).then(data => {
             return new ImageData(
@@ -25,23 +25,23 @@ test.before(() => {
             );
         });
     });
-    images = [];
     sinon.stub(S3, "putObject", (image) => {
         images.push(image);
-        return new Promise((resolve) => resolve(image));
+        return Promise.resolve(image);
     });
 });
 
-test.after(() => {
+test.after(async t => {
     S3.getObject.restore();
     S3.putObject.restore();
 });
 
-test.beforeEach(() => {
+test.beforeEach(async t => {
     processor = new ImageProcessor(setting.Records[0].s3, {
         done: () => {},
         fail: () => {}
     });
+    images = [];
 });
 
 test("Reduce PNG with no configuration", async t => {

Original file line number	Diff line number	Diff line change
`@@ -8,12 +8,19 @@ class Jpegoptim extends Optimizer {`
`8`	`8`	`*`
`9`	`9`	`* @constructor`
`10`	`10`	`* @extends Optimizer`
	`11`	`+ * @param Number\|undefined quality`
`11`	`12`	`*/`
`12`		`- constructor() {`
	`13`	`+ constructor(quality) {`
`13`	`14`	`super();`
`14`	`15`
`15`	`16`	`this.command = this.findBin("jpegoptim");`
`16`	`17`	`this.args = ["--stdin", "-s", "--all-progressive", "--stdout"];`
	`18`	`+`
	`19`	`+ // determine quality if supplied`
	`20`	`+ if ( quality ) {`
	`21`	`+ this.args.unshift(quality);`
	`22`	`+ this.args.unshift("-m");`
	`23`	`+ }`
`17`	`24`	`}`
`18`	`25`	`}`
`19`	`26`