From a68265ee3a7c11d24f6e0937baa5f6830f7db251 Mon Sep 17 00:00:00 2001 From: Jeff Whiting Date: Wed, 4 Jan 2017 12:25:47 -0700 Subject: [PATCH 1/6] Callback implementation and tests for write and end. Callbacks will be called on `write` and `end` when supported in nodejs (requires nodejs >= 0.12.x). Credit goes to @jpodwys for the majority of the implementation. --- HISTORY.md | 3 ++ index.js | 14 ++++----- package.json | 2 +- test/compression.js | 70 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 81 insertions(+), 8 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 9df34d1c..0e75d487 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,6 +1,9 @@ unreleased ========== +1.7.0 / 2017-01-04 +================== + * Callbacks available in `write` and `end` when supported in nodejs (requires nodejs >= 0.12.x) * deps: bytes@2.4.0 * deps: compressible@~2.0.9 - Fix regex fallback to not override `compressible: false` in db diff --git a/index.js b/index.js index c0e801e7..87d3b6cf 100644 --- a/index.js +++ b/index.js @@ -74,7 +74,7 @@ function compression (options) { // proxy - res.write = function write (chunk, encoding) { + res.write = function write (chunk, encoding, cb) { if (ended) { return false } @@ -84,11 +84,11 @@ function compression (options) { } return stream - ? stream.write(new Buffer(chunk, encoding)) - : _write.call(this, chunk, encoding) + ? stream.write(new Buffer(chunk, encoding), cb) + : _write.call(this, chunk, encoding, cb) } - res.end = function end (chunk, encoding) { + res.end = function end (chunk, encoding, cb) { if (ended) { return false } @@ -103,7 +103,7 @@ function compression (options) { } if (!stream) { - return _end.call(this, chunk, encoding) + return _end.call(this, chunk, encoding, cb) } // mark ended @@ -111,8 +111,8 @@ function compression (options) { // write Buffer for Node.js 0.8 return chunk - ? stream.end(new Buffer(chunk, encoding)) - : stream.end() + ? stream.end(new Buffer(chunk, encoding), cb) + : stream.end(null, null, cb) } res.on = function on (type, listener) { diff --git a/package.json b/package.json index 58fbc26e..8e60eb9b 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "compression", "description": "Node.js compression middleware", - "version": "1.6.2", + "version": "1.7.0", "contributors": [ "Douglas Christopher Wilson ", "Jonathan Ong (http://jongleberry.com)" diff --git a/test/compression.js b/test/compression.js index 81c00149..a2f3ee34 100644 --- a/test/compression.js +++ b/test/compression.js @@ -676,6 +676,76 @@ describe('compression()', function () { .end() }) }) + + describe('when callbacks are used', function () { + it('should call the passed callbacks in the order passed when compressing', function (done) { + var hasCallbacks = false + var callbackOutput = [] + var server = createServer(null, function (req, res) { + hasCallbacks = (http.OutgoingMessage.prototype.write.length === 3 && http.OutgoingMessage.prototype.end.length === 3) + res.setHeader('Content-Type', 'text/plain') + res.write('Hello', null, function () { + callbackOutput.push(0) + }) + res.write(' World', null, function () { + callbackOutput.push(1) + }) + res.end(null, null, function () { + callbackOutput.push(2) + }) + }) + + request(server) + .get('/') + .set('Accept-Encoding', 'gzip') + .expect('Content-Encoding', 'gzip') + .end(function (err) { + if (err) { + throw new Error(err) + } + if (hasCallbacks) { + assert.equal(callbackOutput.length, 3) + assert.deepEqual(callbackOutput, [0, 1, 2]) + } + done() + }) + }) + + it('should call the passed callbacks in the order passed when not compressing', function (done) { + var hasCallbacks = false + var callbackOutput = [] + var server = createServer(null, function (req, res) { + hasCallbacks = (http.OutgoingMessage.prototype.write.length === 3 && http.OutgoingMessage.prototype.end.length === 3) + res.setHeader('Cache-Control', 'no-transform') + res.setHeader('Content-Type', 'text/plain') + res.write('hello,', null, function () { + callbackOutput.push(0) + }) + res.write(' world', null, function () { + callbackOutput.push(1) + }) + res.end(null, null, function () { + callbackOutput.push(2) + }) + }) + + request(server) + .get('/') + .set('Accept-Encoding', 'gzip') + .expect('Cache-Control', 'no-transform') + .expect(shouldNotHaveHeader('Content-Encoding')) + .end(function (err) { + if (err) { + throw new Error(err) + } + if (hasCallbacks) { + assert.equal(callbackOutput.length, 3) + assert.deepEqual(callbackOutput, [0, 1, 2]) + } + done() + }) + }) + }) }) function createServer (opts, fn) { From a7c76153dfbdc51d98dca0e2a3480cb9a96a3a6f Mon Sep 17 00:00:00 2001 From: Jeff Whiting Date: Wed, 4 Jan 2017 18:07:47 -0700 Subject: [PATCH 2/6] Callbacks are only used if nodejs supports callbacks. Updated tests to assert that no callbacks are made for nodejs < 0.12.x --- index.js | 12 +++++++----- test/compression.js | 5 +++++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/index.js b/index.js index 87d3b6cf..ec533f78 100644 --- a/index.js +++ b/index.js @@ -21,6 +21,7 @@ var debug = require('debug')('compression') var onHeaders = require('on-headers') var vary = require('vary') var zlib = require('zlib') +var http = require('http') /** * Module exports. @@ -35,6 +36,7 @@ module.exports.filter = shouldCompress */ var cacheControlNoTransformRegExp = /(?:^|,)\s*?no-transform\s*?(?:,|$)/ +var nodeHasCallbacks = (http.OutgoingMessage.prototype.write.length === 3 && http.OutgoingMessage.prototype.end.length === 3) /** * Compress response data with gzip / deflate. @@ -84,8 +86,8 @@ function compression (options) { } return stream - ? stream.write(new Buffer(chunk, encoding), cb) - : _write.call(this, chunk, encoding, cb) + ? stream.write(new Buffer(chunk, encoding), nodeHasCallbacks ? cb : undefined) + : _write.call(this, chunk, encoding, nodeHasCallbacks ? cb : undefined) } res.end = function end (chunk, encoding, cb) { @@ -103,7 +105,7 @@ function compression (options) { } if (!stream) { - return _end.call(this, chunk, encoding, cb) + return _end.call(this, chunk, encoding, nodeHasCallbacks ? cb : undefined) } // mark ended @@ -111,8 +113,8 @@ function compression (options) { // write Buffer for Node.js 0.8 return chunk - ? stream.end(new Buffer(chunk, encoding), cb) - : stream.end(null, null, cb) + ? stream.end(new Buffer(chunk, encoding), nodeHasCallbacks ? cb : undefined) + : stream.end(null, null, nodeHasCallbacks ? cb : undefined) } res.on = function on (type, listener) { diff --git a/test/compression.js b/test/compression.js index a2f3ee34..163133a4 100644 --- a/test/compression.js +++ b/test/compression.js @@ -706,6 +706,9 @@ describe('compression()', function () { if (hasCallbacks) { assert.equal(callbackOutput.length, 3) assert.deepEqual(callbackOutput, [0, 1, 2]) + } else { + //nodejs 0.10 zlib has callbacks but OutgoingMessage doesn't, assert that no callbacks were made + assert.equal(callbackOutput.length, 0) } done() }) @@ -741,6 +744,8 @@ describe('compression()', function () { if (hasCallbacks) { assert.equal(callbackOutput.length, 3) assert.deepEqual(callbackOutput, [0, 1, 2]) + } else { + assert.equal(callbackOutput.length, 0) } done() }) From ce0f02fab58623d3c009329a3d97c10a46cc0a34 Mon Sep 17 00:00:00 2001 From: Jeff Whiting Date: Wed, 4 Jan 2017 18:12:37 -0700 Subject: [PATCH 3/6] =?UTF-8?q?Fixing=20lint=20error,=20missing=20space=20?= =?UTF-8?q?after=20=E2=80=9C//=E2=80=9C?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/compression.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/compression.js b/test/compression.js index 163133a4..12bf439f 100644 --- a/test/compression.js +++ b/test/compression.js @@ -707,7 +707,7 @@ describe('compression()', function () { assert.equal(callbackOutput.length, 3) assert.deepEqual(callbackOutput, [0, 1, 2]) } else { - //nodejs 0.10 zlib has callbacks but OutgoingMessage doesn't, assert that no callbacks were made + // nodejs 0.10 zlib has callbacks but OutgoingMessage doesn't, assert that no callbacks were made assert.equal(callbackOutput.length, 0) } done() From 9d55cfd14912dccbb6b9be1ec440b61d53414d42 Mon Sep 17 00:00:00 2001 From: Jeff Whiting Date: Thu, 5 Jan 2017 16:20:19 -0700 Subject: [PATCH 4/6] Removing the version bump. --- HISTORY.md | 2 -- package.json | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 0e75d487..d7501094 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,8 +1,6 @@ unreleased ========== -1.7.0 / 2017-01-04 -================== * Callbacks available in `write` and `end` when supported in nodejs (requires nodejs >= 0.12.x) * deps: bytes@2.4.0 * deps: compressible@~2.0.9 diff --git a/package.json b/package.json index 8e60eb9b..58fbc26e 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "compression", "description": "Node.js compression middleware", - "version": "1.7.0", + "version": "1.6.2", "contributors": [ "Douglas Christopher Wilson ", "Jonathan Ong (http://jongleberry.com)" From 4a8e87fdaa97357586278280e5703280f40b5fd0 Mon Sep 17 00:00:00 2001 From: Jeff Whiting Date: Thu, 5 Jan 2017 17:37:27 -0700 Subject: [PATCH 5/6] Removing the feature detection checks. This PR now assumes that callbacks are always supported. --- index.js | 12 +++++------- test/compression.js | 7 ++----- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/index.js b/index.js index ec533f78..87d3b6cf 100644 --- a/index.js +++ b/index.js @@ -21,7 +21,6 @@ var debug = require('debug')('compression') var onHeaders = require('on-headers') var vary = require('vary') var zlib = require('zlib') -var http = require('http') /** * Module exports. @@ -36,7 +35,6 @@ module.exports.filter = shouldCompress */ var cacheControlNoTransformRegExp = /(?:^|,)\s*?no-transform\s*?(?:,|$)/ -var nodeHasCallbacks = (http.OutgoingMessage.prototype.write.length === 3 && http.OutgoingMessage.prototype.end.length === 3) /** * Compress response data with gzip / deflate. @@ -86,8 +84,8 @@ function compression (options) { } return stream - ? stream.write(new Buffer(chunk, encoding), nodeHasCallbacks ? cb : undefined) - : _write.call(this, chunk, encoding, nodeHasCallbacks ? cb : undefined) + ? stream.write(new Buffer(chunk, encoding), cb) + : _write.call(this, chunk, encoding, cb) } res.end = function end (chunk, encoding, cb) { @@ -105,7 +103,7 @@ function compression (options) { } if (!stream) { - return _end.call(this, chunk, encoding, nodeHasCallbacks ? cb : undefined) + return _end.call(this, chunk, encoding, cb) } // mark ended @@ -113,8 +111,8 @@ function compression (options) { // write Buffer for Node.js 0.8 return chunk - ? stream.end(new Buffer(chunk, encoding), nodeHasCallbacks ? cb : undefined) - : stream.end(null, null, nodeHasCallbacks ? cb : undefined) + ? stream.end(new Buffer(chunk, encoding), cb) + : stream.end(null, null, cb) } res.on = function on (type, listener) { diff --git a/test/compression.js b/test/compression.js index 12bf439f..e7d8eeef 100644 --- a/test/compression.js +++ b/test/compression.js @@ -682,6 +682,7 @@ describe('compression()', function () { var hasCallbacks = false var callbackOutput = [] var server = createServer(null, function (req, res) { + // hasCallback check can be removed once this module only supports node >= 0.12 and .travis.yml is updated to test on node >= 0.12 hasCallbacks = (http.OutgoingMessage.prototype.write.length === 3 && http.OutgoingMessage.prototype.end.length === 3) res.setHeader('Content-Type', 'text/plain') res.write('Hello', null, function () { @@ -706,9 +707,6 @@ describe('compression()', function () { if (hasCallbacks) { assert.equal(callbackOutput.length, 3) assert.deepEqual(callbackOutput, [0, 1, 2]) - } else { - // nodejs 0.10 zlib has callbacks but OutgoingMessage doesn't, assert that no callbacks were made - assert.equal(callbackOutput.length, 0) } done() }) @@ -718,6 +716,7 @@ describe('compression()', function () { var hasCallbacks = false var callbackOutput = [] var server = createServer(null, function (req, res) { + // hasCallback check can be removed once this module only supports node >= 0.12 and .travis.yml is updated to test on node >= 0.12 hasCallbacks = (http.OutgoingMessage.prototype.write.length === 3 && http.OutgoingMessage.prototype.end.length === 3) res.setHeader('Cache-Control', 'no-transform') res.setHeader('Content-Type', 'text/plain') @@ -744,8 +743,6 @@ describe('compression()', function () { if (hasCallbacks) { assert.equal(callbackOutput.length, 3) assert.deepEqual(callbackOutput, [0, 1, 2]) - } else { - assert.equal(callbackOutput.length, 0) } done() }) From 2fdf90f5c294dc2ede7fbfe7ebb88352767d2c72 Mon Sep 17 00:00:00 2001 From: Jeff Whiting Date: Thu, 5 Jan 2017 17:49:19 -0700 Subject: [PATCH 6/6] Removing conditional checks in unit tests and removing unsupported versions of node from .travis.yml. --- .travis.yml | 2 -- test/compression.js | 18 ++++-------------- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/.travis.yml b/.travis.yml index 107d1187..7fd9a70e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,5 @@ language: node_js node_js: - - "0.8" - - "0.10" - "0.12" - "1.8" - "2.5" diff --git a/test/compression.js b/test/compression.js index e7d8eeef..181de7ef 100644 --- a/test/compression.js +++ b/test/compression.js @@ -679,11 +679,8 @@ describe('compression()', function () { describe('when callbacks are used', function () { it('should call the passed callbacks in the order passed when compressing', function (done) { - var hasCallbacks = false var callbackOutput = [] var server = createServer(null, function (req, res) { - // hasCallback check can be removed once this module only supports node >= 0.12 and .travis.yml is updated to test on node >= 0.12 - hasCallbacks = (http.OutgoingMessage.prototype.write.length === 3 && http.OutgoingMessage.prototype.end.length === 3) res.setHeader('Content-Type', 'text/plain') res.write('Hello', null, function () { callbackOutput.push(0) @@ -704,20 +701,15 @@ describe('compression()', function () { if (err) { throw new Error(err) } - if (hasCallbacks) { - assert.equal(callbackOutput.length, 3) - assert.deepEqual(callbackOutput, [0, 1, 2]) - } + assert.equal(callbackOutput.length, 3) + assert.deepEqual(callbackOutput, [0, 1, 2]) done() }) }) it('should call the passed callbacks in the order passed when not compressing', function (done) { - var hasCallbacks = false var callbackOutput = [] var server = createServer(null, function (req, res) { - // hasCallback check can be removed once this module only supports node >= 0.12 and .travis.yml is updated to test on node >= 0.12 - hasCallbacks = (http.OutgoingMessage.prototype.write.length === 3 && http.OutgoingMessage.prototype.end.length === 3) res.setHeader('Cache-Control', 'no-transform') res.setHeader('Content-Type', 'text/plain') res.write('hello,', null, function () { @@ -740,10 +732,8 @@ describe('compression()', function () { if (err) { throw new Error(err) } - if (hasCallbacks) { - assert.equal(callbackOutput.length, 3) - assert.deepEqual(callbackOutput, [0, 1, 2]) - } + assert.equal(callbackOutput.length, 3) + assert.deepEqual(callbackOutput, [0, 1, 2]) done() }) })