From 9f59c64f1d89a6139d796d4460ccda9b8a346929 Mon Sep 17 00:00:00 2001 From: Sean McMurray Date: Fri, 23 Oct 2015 12:52:29 -0700 Subject: [PATCH 1/4] Convert from generator to promise. --- index.js | 61 +++++++++++++++++++++++++++++----------------------- package.json | 2 +- 2 files changed, 35 insertions(+), 28 deletions(-) diff --git a/index.js b/index.js index cfb94ce..628691a 100644 --- a/index.js +++ b/index.js @@ -1,4 +1,3 @@ - var extname = require('path').extname var calculate = require('etag') var fs = require('mz/fs') @@ -9,35 +8,43 @@ var notfound = { ENOTDIR: true, } -module.exports = function* sendfile(path) { - var stats = yield fs.stat(path).catch(onstaterror) - if (!stats) return null - if (!stats.isFile()) return stats - - this.response.status = 200 - this.response.lastModified = stats.mtime - this.response.length = stats.size - this.response.type = extname(path) - if (!this.response.etag) this.response.etag = calculate(stats, { - weak: true +module.exports = function sendfile(ctx, path) { + return new Promise(function(resolve, reject){ + fs.stat(path, function(err, stats){ + if (err) return reject(err); + return resolve(stats); + }); }) + .catch(onstaterror) + .then(function(stats){ + if (!stats) return null + if (!stats.isFile()) return stats + + ctx.response.status = 200 + ctx.response.lastModified = stats.mtime + ctx.response.length = stats.size + ctx.response.type = extname(path) + if (!ctx.response.etag) ctx.response.etag = calculate(stats, { + weak: true + }) - // fresh based solely on last-modified - var fresh = this.request.fresh - switch (this.request.method) { - case 'HEAD': - this.response.status = fresh ? 304 : 200 - break - case 'GET': - if (fresh) { - this.response.status = 304 - } else { - this.body = fs.createReadStream(path) - } - break - } + // fresh based solely on last-modified + var fresh = ctx.request.fresh + switch (ctx.request.method) { + case 'HEAD': + ctx.response.status = fresh ? 304 : 200 + break + case 'GET': + if (fresh) { + ctx.response.status = 304 + } else { + ctx.body = fs.createReadStream(path) + } + break + } - return stats + return stats + }); } function onstaterror(err) { diff --git a/package.json b/package.json index cc20871..a1c0954 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "koa-sendfile", "description": "basic file-sending utility for koa", - "version": "1.1.1", + "version": "1.2.0", "author": { "name": "Jonathan Ong", "email": "me@jongleberry.com", From 688bc595af4bc4004773119fe3f5e32c6fdf9af1 Mon Sep 17 00:00:00 2001 From: Sean McMurray Date: Fri, 23 Oct 2015 14:45:57 -0700 Subject: [PATCH 2/4] Updated README and tests --- README.md | 14 +++++++++----- test/index.js | 46 +++++++++++++++++++++++++--------------------- 2 files changed, 34 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index 4246baf..df14684 100644 --- a/README.md +++ b/README.md @@ -24,18 +24,22 @@ It does not: ## API -### var stats = yield* sendfile.call(this, filename) +### sendfile(context, filename) -You must pass `this` as the context. `filename` is the filename of the file. +You must pass the koa context. `filename` is the filename of the file. -`stats` is the `fs.stat()` result of the filename. If `stats` exists, that doesn't mean that a response is set - the filename could be a directory. Instead, check `if (this.status)`. +sendfile returns a promise that resolves to the `fs.stat()` result of the filename. If sendfile() resolves, that doesn't mean that a response is set - the filename could be a directory. Instead, check `if (context.status)`. ```js var sendfile = require('koa-sendfile') app.use(function* (next) { - var stats = yield* sendfile.call(this, '/Users/jong/.bash_profile') - if (!this.status) this.throw(404) + var context = this; + sendfile(this, '/Users/jong/.bash_profile') + .then(function(){ + if (!context.status) context.throw(404) + }) + .catch(context.throw.bind(context)) }) ``` diff --git a/test/index.js b/test/index.js index 3d596b2..6f5b184 100644 --- a/test/index.js +++ b/test/index.js @@ -11,8 +11,8 @@ describe('when path is a directory', function () { it('should 404', function (done) { var app = koa() app.use(function* (next) { - var stats = yield* sendfile.call(this, __dirname) - assert.ok(stats) + sendfile(this, __dirname) + .then(assert.ok) }) request(app.listen()) @@ -25,8 +25,8 @@ describe('when path does not exist', function () { it('should 404', function (done) { var app = koa() app.use(function* (next) { - var stats = yield* sendfile.call(this, __dirname + '/kljasdlkfjaklsdf') - assert.ok(!stats) + sendfile(this, __dirname + '/kljasdlkfjaklsdf') + .then(function(stats){ assert.ok(!stats) }) }) request(app.listen()) @@ -42,11 +42,13 @@ describe('when path exists', function () { var stats, tag tag = fs.stat(process.cwd() + '/test/fixture.txt') app.use(function* (next) { - stats = yield* sendfile.call(this, process.cwd() + '/test/fixture.txt') - assert.ok(stats) - tag = calculate(stats, { - weak: true - }) + sendfile(this, process.cwd() + '/test/fixture.txt') + .then(function(s){ + assert.ok(stats=s) + tag = calculate(stats, { + weak: true + }) + }); }) request(app.listen()) @@ -73,8 +75,8 @@ describe('when path exists', function () { }) app.use(function* (next) { - stats = yield* sendfile.call(this, process.cwd() + '/test/fixture.txt') - assert.ok(stats) + sendfile(this, process.cwd() + '/test/fixture.txt') + .then(assert.ok) }) request(app.listen()) @@ -89,8 +91,8 @@ describe('when path exists', function () { var stats fs.stat(process.cwd() + '/test/fixture.txt').then(function(stat) { app.use(function* (next) { - stats = yield* sendfile.call(this, process.cwd() + '/test/fixture.txt') - assert.ok(stats) + sendfile(this, process.cwd() + '/test/fixture.txt') + .then(assert.ok) }) request(app.listen()) @@ -104,10 +106,12 @@ describe('when path exists', function () { var app = koa() var stream app.use(function* (next) { - var stats = yield* sendfile.call(this, process.cwd() + '/test/fixture.txt') - assert.ok(stats) - stream = this.body - this.socket.emit('error', new Error('boom')) + sendfile(this, process.cwd() + '/test/fixture.txt') + .then(function(stats){ + assert.ok(stats) + stream = this.body + this.socket.emit('error', new Error('boom')) + }) }) request(app.listen()) @@ -124,8 +128,8 @@ describe('when path exists', function () { it('should 200', function (done) { var app = koa() app.use(function* (next) { - var stats = yield* sendfile.call(this, process.cwd() + '/test/fixture.txt') - assert.ok(stats) + sendfile(this, process.cwd() + '/test/fixture.txt') + then(assert.ok) }) request(app.listen()) @@ -139,8 +143,8 @@ describe('when path exists', function () { var app = koa() fs.stat(process.cwd() + '/test/fixture.txt').then(function(stat) { app.use(function* (next) { - stats = yield* sendfile.call(this, process.cwd() + '/test/fixture.txt') - assert.ok(stats) + sendfile(this, process.cwd() + '/test/fixture.txt') + .then(assert.ok) }) request(app.listen()) From b52ca91296e286dff53cbe413cd220a879fc9c0e Mon Sep 17 00:00:00 2001 From: Sean McMurray Date: Fri, 23 Oct 2015 17:43:19 -0700 Subject: [PATCH 3/4] Working unit tests. Removed redundant promise wrapper. --- index.js | 10 ++-------- test/index.js | 19 ++++++++++--------- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/index.js b/index.js index 628691a..e1a5350 100644 --- a/index.js +++ b/index.js @@ -9,13 +9,7 @@ var notfound = { } module.exports = function sendfile(ctx, path) { - return new Promise(function(resolve, reject){ - fs.stat(path, function(err, stats){ - if (err) return reject(err); - return resolve(stats); - }); - }) - .catch(onstaterror) + return fs.stat(path) .then(function(stats){ if (!stats) return null if (!stats.isFile()) return stats @@ -44,7 +38,7 @@ module.exports = function sendfile(ctx, path) { } return stats - }); + }, onstaterror); } function onstaterror(err) { diff --git a/test/index.js b/test/index.js index 6f5b184..f4d1b41 100644 --- a/test/index.js +++ b/test/index.js @@ -42,7 +42,7 @@ describe('when path exists', function () { var stats, tag tag = fs.stat(process.cwd() + '/test/fixture.txt') app.use(function* (next) { - sendfile(this, process.cwd() + '/test/fixture.txt') + yield sendfile(this, process.cwd() + '/test/fixture.txt') .then(function(s){ assert.ok(stats=s) tag = calculate(stats, { @@ -75,7 +75,7 @@ describe('when path exists', function () { }) app.use(function* (next) { - sendfile(this, process.cwd() + '/test/fixture.txt') + yield sendfile(this, process.cwd() + '/test/fixture.txt') .then(assert.ok) }) @@ -91,7 +91,7 @@ describe('when path exists', function () { var stats fs.stat(process.cwd() + '/test/fixture.txt').then(function(stat) { app.use(function* (next) { - sendfile(this, process.cwd() + '/test/fixture.txt') + yield sendfile(this, process.cwd() + '/test/fixture.txt') .then(assert.ok) }) @@ -106,11 +106,12 @@ describe('when path exists', function () { var app = koa() var stream app.use(function* (next) { - sendfile(this, process.cwd() + '/test/fixture.txt') + var context = this; + yield sendfile(this, process.cwd() + '/test/fixture.txt') .then(function(stats){ assert.ok(stats) - stream = this.body - this.socket.emit('error', new Error('boom')) + stream = context.body + context.socket.emit('error', new Error('boom')) }) }) @@ -128,8 +129,8 @@ describe('when path exists', function () { it('should 200', function (done) { var app = koa() app.use(function* (next) { - sendfile(this, process.cwd() + '/test/fixture.txt') - then(assert.ok) + yield sendfile(this, process.cwd() + '/test/fixture.txt') + .then(assert.ok) }) request(app.listen()) @@ -143,7 +144,7 @@ describe('when path exists', function () { var app = koa() fs.stat(process.cwd() + '/test/fixture.txt').then(function(stat) { app.use(function* (next) { - sendfile(this, process.cwd() + '/test/fixture.txt') + yield sendfile(this, process.cwd() + '/test/fixture.txt') .then(assert.ok) }) From 2a44e9f32cff81bbdcaeef885d69ada1b764da1b Mon Sep 17 00:00:00 2001 From: Sean McMurray Date: Sat, 24 Oct 2015 08:45:25 -0700 Subject: [PATCH 4/4] Revert version number. Yield instead of thenning in unit tests. --- README.md | 8 ++------ package.json | 2 +- test/index.js | 43 ++++++++++++++++++------------------------- 3 files changed, 21 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index df14684..5530c70 100644 --- a/README.md +++ b/README.md @@ -34,12 +34,8 @@ sendfile returns a promise that resolves to the `fs.stat()` result of the filena var sendfile = require('koa-sendfile') app.use(function* (next) { - var context = this; - sendfile(this, '/Users/jong/.bash_profile') - .then(function(){ - if (!context.status) context.throw(404) - }) - .catch(context.throw.bind(context)) + var stats = yield sendfile(this, '/Users/jong/.bash_profile') + if (!this.status) this.throw(404) }) ``` diff --git a/package.json b/package.json index a1c0954..cc20871 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "koa-sendfile", "description": "basic file-sending utility for koa", - "version": "1.2.0", + "version": "1.1.1", "author": { "name": "Jonathan Ong", "email": "me@jongleberry.com", diff --git a/test/index.js b/test/index.js index f4d1b41..25583c7 100644 --- a/test/index.js +++ b/test/index.js @@ -11,8 +11,8 @@ describe('when path is a directory', function () { it('should 404', function (done) { var app = koa() app.use(function* (next) { - sendfile(this, __dirname) - .then(assert.ok) + var stats = yield sendfile(this, __dirname) + assert.ok(stats) }) request(app.listen()) @@ -25,8 +25,8 @@ describe('when path does not exist', function () { it('should 404', function (done) { var app = koa() app.use(function* (next) { - sendfile(this, __dirname + '/kljasdlkfjaklsdf') - .then(function(stats){ assert.ok(!stats) }) + var stats = yield sendfile(this, __dirname + '/kljasdlkfjaklsdf') + assert.ok(!stats) }) request(app.listen()) @@ -42,13 +42,11 @@ describe('when path exists', function () { var stats, tag tag = fs.stat(process.cwd() + '/test/fixture.txt') app.use(function* (next) { - yield sendfile(this, process.cwd() + '/test/fixture.txt') - .then(function(s){ - assert.ok(stats=s) - tag = calculate(stats, { - weak: true - }) - }); + stats = yield sendfile(this, process.cwd() + '/test/fixture.txt') + assert.ok(stats) + tag = calculate(stats, { + weak: true + }) }) request(app.listen()) @@ -68,15 +66,14 @@ describe('when path exists', function () { it('should support weak etag 304', function (done) { var app = koa() - var stats, tag fs.stat(process.cwd() + '/test/fixture.txt').then(function(stat) { - tag = calculate(stat, { + var tag = calculate(stat, { weak: true }) app.use(function* (next) { - yield sendfile(this, process.cwd() + '/test/fixture.txt') - .then(assert.ok) + var stats = yield sendfile(this, process.cwd() + '/test/fixture.txt') + assert.ok(stats) }) request(app.listen()) @@ -88,11 +85,10 @@ describe('when path exists', function () { it('should support last-modified 304', function (done) { var app = koa() - var stats fs.stat(process.cwd() + '/test/fixture.txt').then(function(stat) { app.use(function* (next) { - yield sendfile(this, process.cwd() + '/test/fixture.txt') - .then(assert.ok) + var stats = yield sendfile(this, process.cwd() + '/test/fixture.txt') + assert.ok(stats) }) request(app.listen()) @@ -106,13 +102,10 @@ describe('when path exists', function () { var app = koa() var stream app.use(function* (next) { - var context = this; - yield sendfile(this, process.cwd() + '/test/fixture.txt') - .then(function(stats){ - assert.ok(stats) - stream = context.body - context.socket.emit('error', new Error('boom')) - }) + var stats = yield sendfile(this, process.cwd() + '/test/fixture.txt') + assert.ok(stats) + stream = this.body + this.socket.emit('error', new Error('boom')) }) request(app.listen())