Delay the decision about whether to use sharp or gm to when filterInf…

…o.create() is called. At that point the source Content-Type is more likely to be available.
papandreou · Mar 10, 2016 · 8ae402a · 8ae402a
1 parent 4ca1985
commit 8ae402a
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 34 deletions.
diff --git a/lib/getFilterInfosAndTargetContentTypeFromQueryString.js b/lib/getFilterInfosAndTargetContentTypeFromQueryString.js
@@ -86,15 +86,26 @@ module.exports = function getFilterInfosAndTargetContentTypeFromQueryString(quer
 
     function flushOperations() {
         if (operations.length > 0) {
-            if (currentEngineName === 'sharp') {
-                var sharpOperationsForThisInstance = [].concat(operations);
-                operationNames.push('sharp');
-                filterInfos.push({
-                    operationName: 'sharp',
-                    usedQueryStringFragments: operations.map(function (operation) {
-                        return operation.usedQueryStringFragment;
-                    }),
-                    create: function () {
+            var engineName = currentEngineName;
+            operationNames.push('sharpOrGm');
+            filterInfos.push({
+                operationName: 'sharpOrGm',
+                operations: operations,
+                usedQueryStringFragments: operations.map(function (operation) {
+                    return operation.usedQueryStringFragment;
+                }),
+                create: function () {
+                    if (this.operations[0] && this.operations[0].sourceContentType === 'image/gif' && !this.operations.some(function (operation) {
+                        return operation.name === 'png' || operation.name === 'webp' || operation.name === 'jpeg';
+                    })) {
+                        engineName = 'gm';
+                    }
+                    var operations = this.operations;
+                    this.operationName = engineName;
+                    if (engineName === 'sharp') {
+                        var sharpOperationsForThisInstance = [].concat(operations);
+                        operationNames.push('sharp');
+
                         if (options.maxInputPixels) {
                             sharpOperationsForThisInstance.unshift({name: 'limitInputPixels', args: [options.maxInputPixels]});
                         }
@@ -107,17 +118,10 @@ module.exports = function getFilterInfosAndTargetContentTypeFromQueryString(quer
                             }
                             return sharpInstance[operation.name].apply(sharpInstance, args);
                         }, sharp());
-                    }
-                });
-            } else if (currentEngineName === 'gm') {
-                var gmOperationsForThisInstance = [].concat(operations);
-                operationNames.push('gm');
-                filterInfos.push({
-                    operationName: 'gm',
-                    usedQueryStringFragments: operations.map(function (operation) {
-                        return operation.usedQueryStringFragment;
-                    }),
-                    create: function () {
+                    } else if (engineName === 'gm') {
+                        var gmOperationsForThisInstance = [].concat(operations);
+                        operationNames.push('gm');
+
                         // For some reason the gm module doesn't expose itself as a readable/writable stream,
                         // so we need to wrap it into one:
 
@@ -213,24 +217,18 @@ module.exports = function getFilterInfosAndTargetContentTypeFromQueryString(quer
                             readStream.emit('end');
                         };
                         return readWriteStream;
+                    } else {
+                        throw new Error('Internal error');
                     }
-                });
-            } else {
-                throw new Error('Internal error');
-            }
+                }
+            });
             operations = [];
         }
         currentEngineName = undefined;
     }
 
     var keyValuePairs = queryString.split('&');
 
-    if (sourceMetadata.contentType === 'image/gif' && !keyValuePairs.some(function (keyValuePair) {
-        return keyValuePair === 'png' || keyValuePair === 'webp' || keyValuePair === 'jpeg';
-    })) {
-        currentEngineName = 'gm';
-    }
-
     keyValuePairs.forEach(function (keyValuePair) {
         var matchKeyValuePair = keyValuePair.match(/^([^=]+)(?:=(.*))?/);
         if (matchKeyValuePair) {

diff --git a/lib/processImage.js b/lib/processImage.js
@@ -99,10 +99,6 @@ module.exports = function (options) {
                     if (targetContentType) {
                         res.setHeader('Content-Type', targetContentType);
                     }
-                    if (options.debug) {
-                        // Only used by the test suite to assert that the right engine is used to process gifs:
-                        res.setHeader('X-Express-Processimage', filterInfosAndTargetFormat.operationNames.join(','));
-                    }
                     res.removeHeader('Content-Length');
                     var oldETag = res.getHeader('ETag'),
                         newETag;
@@ -121,6 +117,12 @@ module.exports = function (options) {
                     } catch (e) {
                         return sendErrorResponse(new httpErrors.BadRequest(e));
                     }
+                    if (options.debug) {
+                        // Only used by the test suite to assert that the right engine is used to process gifs:
+                        res.setHeader('X-Express-Processimage', filterInfosAndTargetFormat.filterInfos.map(function (filterInfo) {
+                            return filterInfo.operationName;
+                        }).join(','));
+                    }
                     for (var i = 0 ; i < filters.length ; i += 1) {
                         if (i < filters.length - 1) {
                             filters[i].pipe(filters[i + 1]);