mirror of
https://github.com/TryGhost/Ghost.git
synced 2025-02-17 23:44:39 -05:00
Cleaned up image manipulator (#10282)
no-issue * Added InternalServerError to resizeImage * Added a redirect to original image if sharp is missing * Improved naming - safeMethod -> method * Updated process method to follow same sharp check pattern * Refactor safety wrapper into makeSafe function * Moved generic manipulation error to makeSafe function * Refactored unsafeProcess to use unsafeResizeImage * Removed CRAZY catch
This commit is contained in:
Fabien O'Carroll
Hannah Wolfe
parent
4078dd7959
commit
b4654b19d5
2 changed files with 34 additions and 102 deletions
|
|
@ -8,60 +8,20 @@ const fs = require('fs-extra');
|
|||
* We currently can't enable compression or having more config options, because of
|
||||
* https://github.com/lovell/sharp/issues/1360.
|
||||
*/
|
||||
const process = (options = {}) => {
|
||||
let sharp, img, originalData, originalSize;
|
||||
|
||||
try {
|
||||
sharp = require('sharp');
|
||||
} catch (err) {
|
||||
return Promise.reject(new common.errors.InternalServerError({
|
||||
message: 'Sharp wasn\'t installed',
|
||||
code: 'SHARP_INSTALLATION',
|
||||
err: err
|
||||
}));
|
||||
}
|
||||
|
||||
// @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);
|
||||
|
||||
const unsafeProcess = (options = {}) => {
|
||||
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);
|
||||
}
|
||||
|
||||
// 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(({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({
|
||||
message: 'Unable to manipulate image.',
|
||||
err: err,
|
||||
code: 'IMAGE_PROCESSING'
|
||||
return unsafeResizeImage(data, {
|
||||
width: options.width
|
||||
});
|
||||
})
|
||||
.then((data) => {
|
||||
return fs.writeFile(options.out, data);
|
||||
});
|
||||
};
|
||||
|
||||
const resizeImage = (originalBuffer, {width, height} = {}) => {
|
||||
const unsafeResizeImage = (originalBuffer, {width, height} = {}) => {
|
||||
const sharp = require('sharp');
|
||||
return sharp(originalBuffer)
|
||||
.resize(width, height, {
|
||||
|
|
@ -76,12 +36,24 @@ const resizeImage = (originalBuffer, {width, height} = {}) => {
|
|||
});
|
||||
};
|
||||
|
||||
module.exports.process = process;
|
||||
module.exports.safeResizeImage = (buffer, options) => {
|
||||
const makeSafe = fn => (...args) => {
|
||||
try {
|
||||
require('sharp');
|
||||
return resizeImage(buffer, options);
|
||||
} catch (e) {
|
||||
return Promise.resolve(buffer);
|
||||
} catch (err) {
|
||||
return Promise.reject(new common.errors.InternalServerError({
|
||||
message: 'Sharp wasn\'t installed',
|
||||
code: 'SHARP_INSTALLATION',
|
||||
err: err
|
||||
}));
|
||||
}
|
||||
return fn(...args).catch((err) => {
|
||||
throw new common.errors.InternalServerError({
|
||||
message: 'Unable to manipulate image.',
|
||||
err: err,
|
||||
code: 'IMAGE_PROCESSING'
|
||||
});
|
||||
});
|
||||
};
|
||||
|
||||
module.exports.process = makeSafe(unsafeProcess);
|
||||
module.exports.resizeImage = makeSafe(unsafeResizeImage);
|
||||
|
|
|
|||
|
|
@ -20,9 +20,8 @@ describe('lib/image: manipulator', function () {
|
|||
sandbox.stub(fs, 'writeFile').resolves();
|
||||
|
||||
sharpInstance = {
|
||||
metadata: sandbox.stub(),
|
||||
resize: sandbox.stub(),
|
||||
rotate: sandbox.stub(),
|
||||
resize: sandbox.stub().returnsThis(),
|
||||
rotate: sandbox.stub().returnsThis(),
|
||||
toBuffer: sandbox.stub(),
|
||||
};
|
||||
|
||||
|
|
@ -30,23 +29,14 @@ describe('lib/image: manipulator', function () {
|
|||
return sharpInstance;
|
||||
});
|
||||
|
||||
sharp.cache = sandbox.stub();
|
||||
testUtils.mockNotExistingModule('sharp', sharp);
|
||||
});
|
||||
|
||||
it('resize image', function () {
|
||||
sharpInstance.metadata.resolves({width: 2000, height: 2000});
|
||||
sharpInstance.toBuffer.resolves({
|
||||
data: 'manipulated',
|
||||
info: {
|
||||
size: 42
|
||||
}
|
||||
});
|
||||
sharpInstance.toBuffer.resolves('manipulated');
|
||||
|
||||
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();
|
||||
|
||||
|
|
@ -56,20 +46,14 @@ describe('lib/image: manipulator', function () {
|
|||
});
|
||||
|
||||
it('skip resizing if image is too small', function () {
|
||||
sharpInstance.metadata.resolves({width: 50, height: 50});
|
||||
sharpInstance.toBuffer.resolves({
|
||||
data: 'manipulated',
|
||||
info: {
|
||||
size: 42
|
||||
}
|
||||
});
|
||||
sharpInstance.toBuffer.resolves('manipulated');
|
||||
|
||||
return manipulator.process({width: 1000})
|
||||
.then(() => {
|
||||
sharp.cache.calledOnce.should.be.true();
|
||||
sharpInstance.metadata.calledOnce.should.be.true();
|
||||
sharpInstance.resize.calledOnce.should.be.false();
|
||||
sharpInstance.rotate.calledOnce.should.be.true();
|
||||
sharpInstance.resize.calledOnce.should.be.true();
|
||||
should.deepEqual(sharpInstance.resize.args[0][2], {
|
||||
withoutEnlargement: true
|
||||
});
|
||||
|
||||
fs.writeFile.calledOnce.should.be.true();
|
||||
fs.writeFile.calledWith('manipulated');
|
||||
|
|
@ -77,22 +61,10 @@ describe('lib/image: manipulator', function () {
|
|||
});
|
||||
|
||||
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
|
||||
}
|
||||
});
|
||||
sharpInstance.toBuffer.resolves('manipulated to a very very very very very very very large size');
|
||||
|
||||
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();
|
||||
|
|
@ -103,13 +75,7 @@ describe('lib/image: manipulator', function () {
|
|||
});
|
||||
|
||||
it('sharp throws error during processing', function () {
|
||||
sharpInstance.metadata.resolves({width: 500, height: 500});
|
||||
sharpInstance.toBuffer.resolves({
|
||||
data: 'manipulated',
|
||||
info: {
|
||||
size: 42
|
||||
}
|
||||
});
|
||||
sharpInstance.toBuffer.resolves('manipulated');
|
||||
|
||||
fs.writeFile.rejects(new Error('whoops'));
|
||||
|
||||
|
|
@ -120,12 +86,6 @@ describe('lib/image: manipulator', function () {
|
|||
.catch((err) => {
|
||||
(err instanceof common.errors.InternalServerError).should.be.true;
|
||||
err.code.should.eql('IMAGE_PROCESSING');
|
||||
sharp.cache.calledOnce.should.be.true;
|
||||
sharpInstance.metadata.calledOnce.should.be.true();
|
||||
sharpInstance.resize.calledOnce.should.be.false();
|
||||
sharpInstance.rotate.calledOnce.should.be.true();
|
||||
|
||||
fs.writeFile.calledOnce.should.be.true();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue