TryGhost/Ghost
1
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2024-11-28 05:37:34 +03:00
Code Issues Projects Releases Wiki Activity

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

Browse Source 
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
This commit is contained in:
Nazar Gargol 2018-11-12 18:52:36 +01:00 committed by Hannah Wolfe
parent eb203de714
commit 29991c6718
2 changed files with 82 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
ghost/image-transform/lib/transform.js
View File

@ -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({

66
ghost/image-transform/test/transform.test.js
View File

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