Skip to content

Commit

Permalink
Don't create the filter instances before checking whether a 304 can b…
Browse files Browse the repository at this point in the history 
…e returned.

Warning: This renames lib/getFilter{ => Info}sAndTargetContentTypeFromQueryString.js and changes its API, which some other projects are relying on.
  • Loading branch information
@papandreou
papandreou committed Sep 11, 2014
1 parent 4bf9e1a commit 2429db1
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 63 deletions.
122 changes: 63 additions & 59 deletions ...ersAndTargetContentTypeFromQueryString.js → ...fosAndTargetContentTypeFromQueryString.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ Object.keys(gm.prototype).forEach(function (propertyName) {
}
});

module.exports = function getFiltersAndTargetContentTypeFromQueryString(queryString, rootPath, sourceFilePath) {
var filters = [],
module.exports = function getFilterInfosAndTargetContentTypeFromQueryString(queryString, rootPath, sourceFilePath) {
var filterInfos = [],
gmOperations = [],
operationNames = [],
usedQueryStringFragments = [],
Expand All @@ -30,64 +30,65 @@ module.exports = function getFiltersAndTargetContentTypeFromQueryString(queryStr
function flushGmOperations() {
if (gmOperations.length > 0) {
var gmOperationsForThisInstance = [].concat(gmOperations);
// For some reason the gm module doesn't expose itself as a readable/writable stream,
// so we need to wrap it into one:
operationNames.push('gm');
filterInfos.push({
operationName: 'gm',
usedQueryStringFragments: gmOperations.map(function (gmOperation) {
return gmOperation.usedQueryStringFragment;
}),
create: function () {
// For some reason the gm module doesn't expose itself as a readable/writable stream,
// so we need to wrap it into one:

var readStream = new Stream();
readStream.readable = true;
var readStream = new Stream();
readStream.readable = true;

var readWriteStream = new Stream();
readWriteStream.readable = readWriteStream.writable = true;
var spawned = false;
var readWriteStream = new Stream();
readWriteStream.readable = readWriteStream.writable = true;
var spawned = false;

readWriteStream.write = function (chunk) {
if (!spawned) {
spawned = true;
var gmInstance = gm(readStream);
gmOperationsForThisInstance.forEach(function (gmOperation) {
gmInstance = gmInstance[gmOperation.name].apply(gmInstance, gmOperation.args);
});
var seenData = false,
hasEnded = false;
gmInstance.stream(function (err, stdout, stderr) {
if (err) {
hasEnded = true;
return readWriteStream.emit('error', err);
}
stdout.on('data', function (chunk) {
seenData = true;
readWriteStream.emit('data', chunk);
}).on('end', function () {
if (!hasEnded) {
if (seenData) {
readWriteStream.emit('end');
} else {
readWriteStream.emit('error', new Error('The gm stream ended without emitting any data'));
readWriteStream.write = function (chunk) {
if (!spawned) {
spawned = true;
var gmInstance = gm(readStream);
gmOperationsForThisInstance.forEach(function (gmOperation) {
gmInstance = gmInstance[gmOperation.name].apply(gmInstance, gmOperation.args);
});
var seenData = false,
hasEnded = false;
gmInstance.stream(function (err, stdout, stderr) {
if (err) {
hasEnded = true;
return readWriteStream.emit('error', err);
}
hasEnded = true;
}
});
});
}
readStream.emit('data', chunk);
};
readWriteStream.end = function (chunk) {
if (chunk) {
readWriteStream.write(chunk);
stdout.on('data', function (chunk) {
seenData = true;
readWriteStream.emit('data', chunk);
}).on('end', function () {
if (!hasEnded) {
if (seenData) {
readWriteStream.emit('end');
} else {
readWriteStream.emit('error', new Error('The gm stream ended without emitting any data'));
}
hasEnded = true;
}
});
});
}
readStream.emit('data', chunk);
};
readWriteStream.end = function (chunk) {
if (chunk) {
readWriteStream.write(chunk);
}
readStream.emit('end');
};
return readWriteStream;
}
readStream.emit('end');
};

readWriteStream.usedQueryStringFragments = [];

gmOperations.forEach(function (gmOperation) {
readWriteStream.usedQueryStringFragments.push(gmOperation.usedQueryStringFragment);
});
operationNames.push('gm');
readWriteStream.operationName = 'gm';
filters.push(readWriteStream);
gmOperations = [];
}
gmOperations = [];
}

queryString.split('&').forEach(function (keyValuePair) {
Expand Down Expand Up @@ -124,13 +125,16 @@ module.exports = function getFiltersAndTargetContentTypeFromQueryString(queryStr
if (FilterConstructor) {
operationNames.push(operationNameLowerCase);
flushGmOperations();
if (operationNameLowerCase === 'svgfilter') {
if (operationNameLowerCase === 'svgfilter' && rootPath && sourceFilePath) {
operationArgs.push('--root', 'file://' + rootPath, '--url', 'file://' + sourceFilePath);
}
var filter = new FilterConstructor(operationArgs);
filter.operationName = operationNameLowerCase;
filter.usedQueryStringFragments = [keyValuePair];
filters.push(filter);
filterInfos.push({
create: function () {
return new FilterConstructor(operationArgs);
},
operationName: operationNameLowerCase,
usedQueryStringFragments: [keyValuePair]
})
usedQueryStringFragments.push(keyValuePair);
if (operationNameLowerCase === 'inkscape') {
targetContentType = 'image/' + filter.outputFormat;
Expand All @@ -146,7 +150,7 @@ module.exports = function getFiltersAndTargetContentTypeFromQueryString(queryStr
return {
targetContentType: targetContentType,
operationNames: operationNames,
filters: filters,
filterInfos: filterInfos,
usedQueryStringFragments: usedQueryStringFragments,
leftOverQueryStringFragments: leftOverQueryStringFragments
};
Expand Down
11 changes: 7 additions & 4 deletions lib/processImage.js
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
var Path = require('path'),
getFiltersAndTargetContentTypeFromQueryString = require('./getFiltersAndTargetContentTypeFromQueryString');
getFilterInfosAndTargetContentTypeFromQueryString = require('./getFilterInfosAndTargetContentTypeFromQueryString');

require('express-hijackresponse');

Expand Down Expand Up @@ -46,10 +46,9 @@ module.exports = function (options) {
}

if (contentType && contentType.indexOf('image/') === 0) {
var filtersAndTargetFormat = getFiltersAndTargetContentTypeFromQueryString(queryString, options.root, Path.resolve(options.root, req.url.substr(1))),
filters = filtersAndTargetFormat.filters,
var filtersAndTargetFormat = getFilterInfosAndTargetContentTypeFromQueryString(queryString, options.root, options.root && Path.resolve(options.root, req.url.substr(1))),
targetContentType = filtersAndTargetFormat.targetContentType;
if (filtersAndTargetFormat.filters.length === 0) {
if (filtersAndTargetFormat.filterInfos.length === 0) {
return res.unhijack(true);
}
if (targetContentType) {
Expand All @@ -67,6 +66,10 @@ module.exports = function (options) {
}
}

var filters = filtersAndTargetFormat.filterInfos.map(function (filterInfo) {
return filterInfo.create();
});

for (var i = 0 ; i < filters.length ; i += 1) {
if (i < filters.length - 1) {
filters[i].pipe(filters[i + 1]);
Expand Down

0 comments on commit 2429db1

Please sign in to comment.