diff --git a/index.js b/index.js index 42639913..84b766dd 100644 --- a/index.js +++ b/index.js @@ -370,16 +370,27 @@ function session(options) { function reload(callback) { debug('reloading %s', this.id) - _reload.call(this, function () { - wrapmethods(req.session) - callback.apply(this, arguments) - }) + if (callback) { + _reload.call(this, function () { + wrapmethods(req.session) + callback.apply(this, arguments) + }) + return + } + + return _reload.call(this) + .then(function() { + return wrapmethods(req.session) + }) + .catch(function(err) { + return err + }) } function save() { debug('saving %s', this.id); savedHash = hash(this); - _save.apply(this, arguments); + return _save.apply(this, arguments); } Object.defineProperty(sess, 'reload', { diff --git a/package.json b/package.json index d473f546..15169c97 100644 --- a/package.json +++ b/package.json @@ -21,6 +21,7 @@ }, "devDependencies": { "after": "0.8.2", + "bluebird": "3.5.3", "cookie-parser": "1.4.3", "eslint": "3.19.0", "eslint-plugin-markdown": "1.0.0", diff --git a/session/session.js b/session/session.js index fee7608c..4763ea9f 100644 --- a/session/session.js +++ b/session/session.js @@ -63,15 +63,30 @@ defineMethod(Session.prototype, 'resetMaxAge', function resetMaxAge() { /** * Save the session data with optional callback `fn(err)`. * - * @param {Function} fn - * @return {Session} for chaining + * @param {Function} [fn] + * @return {Promise} * @api public */ -defineMethod(Session.prototype, 'save', function save(fn) { - this.req.sessionStore.set(this.id, this, fn || function(){}); - return this; -}); +defineMethod(Session.prototype, 'save', function save (fn) { + if (fn) { + this.req.sessionStore.set(this.id, this, fn) + return + } + + if (!fn && !global.Promise) { + this.req.sessionStore.set(this.id, this, function(){}) + return + } + + var sess = this + return new Promise(function (resolve, reject) { + sess.req.sessionStore.set(sess.id, sess, function (err) { + if (err) reject(err) + resolve() + }) + }) +}) /** * Re-loads the session data _without_ altering @@ -80,23 +95,38 @@ defineMethod(Session.prototype, 'save', function save(fn) { * `req.session` property will be a new `Session` object, * although representing the same session. * - * @param {Function} fn - * @return {Session} for chaining + * @param {Function} [fn] + * @return {Promise} * @api public */ -defineMethod(Session.prototype, 'reload', function reload(fn) { +defineMethod(Session.prototype, 'reload', function reload (fn) { var req = this.req var store = this.req.sessionStore + if (fn) { + store.get(this.id, function (err, sess) { + if (err) return fn(err) + if (!sess) return fn(new Error('failed to load session')) + store.createSession(req, sess) + fn() + }) + return + } - store.get(this.id, function(err, sess){ - if (err) return fn(err); - if (!sess) return fn(new Error('failed to load session')); - store.createSession(req, sess); - fn(); - }); - return this; -}); + if (!fn && !global.Promise) { + throw new Error('must use callback without promises') + } + + var parent = this + return new Promise(function (resolve, reject) { + store.get(parent.id, function (err, sess) { + if (err) reject(err) + if (!sess) reject(new Error('failed to load session')) + store.createSession(req, sess) + resolve() + }) + }) +}) /** * Destroy `this` session. @@ -107,10 +137,24 @@ defineMethod(Session.prototype, 'reload', function reload(fn) { */ defineMethod(Session.prototype, 'destroy', function destroy(fn) { - delete this.req.session; - this.req.sessionStore.destroy(this.id, fn); - return this; -}); + delete this.req.session + if (fn) { + this.req.sessionStore.destroy(this.id, fn) + return + } + + if (!fn && !global.Promise) { + throw new Error('must use callback without promises') + } + + var parent = this + return new Promise(function (resolve, reject) { + parent.req.sessionStore.destroy(parent.id, function(err) { + if (err) reject(err) + resolve() + }) + }) +}) /** * Regenerate this request's session. @@ -121,9 +165,23 @@ defineMethod(Session.prototype, 'destroy', function destroy(fn) { */ defineMethod(Session.prototype, 'regenerate', function regenerate(fn) { - this.req.sessionStore.regenerate(this.req, fn); - return this; -}); + if (fn) { + this.req.sessionStore.regenerate(this.req, fn) + return + } + + if (!fn && !global.Promise) { + throw new Error('must use callback without promises') + } + + var sess = this + return new Promise(function (resolve, reject) { + sess.req.sessionStore.regenerate(sess.req, function(err) { + if (err) reject(err) + resolve() + }) + }) +}) /** * Helper function for creating a method on a prototype. diff --git a/test/session.js b/test/session.js index 6862ccc4..58e41f48 100644 --- a/test/session.js +++ b/test/session.js @@ -12,6 +12,11 @@ var util = require('util') var Cookie = require('../session/cookie') +var Promise = global.Promise || require('bluebird') + +// Add Promise to mocha's global list +global.Promise = global.Promise + var min = 60 * 1000; describe('session()', function(){ @@ -1540,7 +1545,7 @@ describe('session()', function(){ }) }) - describe('.destroy()', function(){ + describe('.destroy()', function () { it('should destroy the previous session', function(done){ var server = createServer(null, function (req, res) { req.session.destroy(function (err) { @@ -1554,9 +1559,70 @@ describe('session()', function(){ .expect(shouldNotHaveHeader('Set-Cookie')) .expect(200, 'undefined', done) }) + + describe('with global Promise', function () { + beforeEach(function () { + global.Promise = Promise + }) + + afterEach(function () { + global.Promise = undefined + }) + + it('should return Promise without callback', function (done) { + var server = createServer(null, function (req, res) { + req.session.destroy() + .then(function () { + res.end() + }) + .catch(function (err) { + res.statusCode = 500 + res.end(err.message) + }) + }) + + request(server) + .get('/') + .expect(200, done) + }) + + it('should not return Promise with callback', function (done) { + var server = createServer(null, function (req, res) { + var ret = req.session.destroy(function (err) { + res.statusCode = (!err && ret === undefined) ? 200 : 500 + res.end() + }) + }) + + request(server) + .get('/') + .expect(200, done) + }) + }) + + describe('without global Promise', function () { + beforeEach(function () { + global.Promise = undefined + }) + + afterEach(function () { + global.Promise = Promise + }) + + it('should require callback', function (done) { + var server = createServer(null, function (req, res) { + req.session.destroy() + res.end() + }) + + request(server) + .get('/') + .expect(500, 'must use callback without promises', done) + }) + }) }) - describe('.regenerate()', function(){ + describe('.regenerate()', function () { it('should destroy/replace the previous session', function(done){ var server = createServer(null, function (req, res) { var id = req.session.id @@ -1579,6 +1645,83 @@ describe('session()', function(){ .expect(200, 'false', done) }); }) + + describe('with global Promise', function () { + beforeEach(function () { + global.Promise = Promise + }) + + afterEach(function () { + global.Promise = undefined + }) + + it('should return Promise without callback', function (done) { + var server = createServer(null, function (req, res) { + var id = req.session.id + req.session.regenerate() + .then(function() { + res.end(String(req.session.id === id)) + }) + .catch(function () { + res.statusCode = 500 + }) + }) + + request(server) + .get('/') + .expect(200, 'false', done) + }) + + it('should not return Promise with callback', function(done){ + var server = createServer(null, function (req, res) { + var id = req.session.id + var ret = req.session.regenerate(function (err) { + res.statusCode = (!err && ret === undefined) ? 200 : 500 + res.end(String(req.session.id === id)) + }) + }) + + request(server) + .get('/') + .expect(200, 'false', done) + }) + }) + + describe('without global Promise', function () { + beforeEach(function () { + global.Promise = undefined + }) + + afterEach(function () { + global.Promise = Promise + }) + + it('should error without callback', function (done) { + var server = createServer(null, function (req, res) { + req.session.regenerate() + res.end() + }) + + request(server) + .get('/') + .expect(500, 'must use callback without promises', done) + }) + + it('should not return Promise with callback', function(done){ + var server = createServer(null, function (req, res) { + var id = req.session.id + var ret = req.session.regenerate(function (err) { + res.statusCode = (!err && ret === undefined) ? 200 : 500 + res.end(String(req.session.id === id)) + }) + }) + + request(server) + .get('/') + .expect(shouldSetCookie('connect.sid')) + .expect(200, 'false', done) + }) + }) }) describe('.reload()', function () { @@ -1622,6 +1765,90 @@ describe('session()', function(){ }) }) + it('should return Promise without callback', function (done) { + var server = createServer(null, function (req, res) { + if (req.url === '/') { + req.session.active = true + res.end('session created') + return + } + + req.session.url = req.url + + if (req.url === '/bar') { + res.end('saw ' + req.session.url) + return + } + + request(server) + .get('/bar') + .set('Cookie', val) + .expect(200, 'saw /bar', function (err, resp) { + if (err) return done(err) + req.session.reload() + .then(function () { + res.end('saw ' + req.session.url) + }) + .catch(function (err) { + if (err) return done(err) + }) + }) + }) + var val + + request(server) + .get('/') + .expect(200, 'session created', function (err, res) { + if (err) return done(err) + val = cookie(res) + request(server) + .get('/foo') + .set('Cookie', val) + .expect(200, 'saw /bar', done) + }) + }) + + it('should not return promise with callback', function (done) { + var server = createServer(null, function (req, res) { + if (req.url === '/') { + req.session.active = true + res.end('session created') + return + } + + req.session.url = req.url + + if (req.url === '/bar') { + res.end('saw ' + req.session.url) + return + } + + request(server) + .get('/bar') + .set('Cookie', val) + .expect(200, 'saw /bar', function (err, resp) { + if (err) return done(err) + var ret = req.session.reload(function (err) { + if (err) return done(err) + res.statusCode = (ret === undefined) ? 200 : 500 + res.end('saw ' + req.session.url) + }) + }) + }) + var val + + request(server) + .get('/') + .expect(200, 'session created', function (err, res) { + if (err) return done(err) + val = cookie(res) + request(server) + .get('/foo') + .set('Cookie', val) + .expect(200, 'saw /bar', done) + }) + }) + it('should error is session missing', function (done) { var store = new session.MemoryStore() var server = createServer({ store: store }, function (req, res) { @@ -1650,6 +1877,92 @@ describe('session()', function(){ .expect(500, 'failed to load session', done) }) }) + + describe('with global Promise', function () { + beforeEach(function () { + global.Promise = Promise + }) + + afterEach(function () { + global.Promise = undefined + }) + + it('should return Promise without callback', function (done) { + var server = createServer(null, function (req, res) { + if (req.url === '/') { + req.session.active = true + res.end('session created') + return + } + + req.session.reload() + .then(function () { + res.end() + }) + .catch(function (err) { + res.statusCode = 500 + res.end(err.message) + }) + }) + + request(server) + .get('/') + .expect(200, 'session created', function (err, res) { + if (err) return done(err) + request(server) + .get('/foo') + .set('Cookie', cookie(res)) + .expect(200, done) + }) + }) + + it('should not return promise with callback', function (done) { + var server = createServer(null, function (req, res) { + if (req.url === '/') { + req.session.active = true + res.end('session created') + return + } + + req.session.reload(function (err) { + if (!err) return res.end() + res.statusCode = 500 + res.end(err.message) + }) + }) + + request(server) + .get('/') + .expect(200, 'session created', function (err, res) { + if (err) return done(err) + request(server) + .get('/foo') + .set('Cookie', cookie(res)) + .expect(200, done) + }) + }) + }) + + describe('without global Promise', function () { + beforeEach(function () { + global.Promise = undefined + }) + + afterEach(function () { + global.Promise = Promise + }) + + it('should require callback', function (done) { + var server = createServer(null, function (req, res) { + req.session.reload() + res.end() + }) + + request(server) + .get('/') + .expect(500, 'must use callback without promises', done) + }) + }) }) describe('.save()', function () { @@ -1671,6 +1984,43 @@ describe('session()', function(){ .expect(200, 'stored', done) }) + it('should return Promise without callback', function (done) { + var store = new session.MemoryStore() + var server = createServer({ store: store }, function (req, res) { + req.session.hit = true + req.session.save() + .then(function () { + store.get(req.session.id, function (err, sess) { + if (err) return res.end(err.message) + res.end(sess ? 'stored' : 'empty') + }) + }) + .catch(function (err) { + if (err) return res.end(err.message) + }) + }) + + request(server) + .get('/') + .expect(200, 'stored', done) + }) + + it('should not return Promise with callback', function (done) { + var store = new session.MemoryStore() + var server = createServer({ store: store }, function (req, res) { + req.session.hit = true + var ret = req.session.save(function (err) { + if (err) return res.end(err.message) + res.statusCode = (ret === undefined) ? 200 : 500 + res.end() + }) + }) + + request(server) + .get('/') + .expect(200, done) + }) + it('should prevent end-of-request save', function (done) { var store = new session.MemoryStore() var server = createServer({ store: store }, function (req, res) { @@ -1718,6 +2068,76 @@ describe('session()', function(){ .expect(200, 'saved', done) }) }) + + describe('with global Promise', function () { + beforeEach(function () { + global.Promise = Promise + }) + + afterEach(function () { + global.Promise = undefined + }) + + it('should return Promise without callback', function (done) { + var store = new session.MemoryStore() + var server = createServer({ store: store }, function (req, res) { + req.session.hit = true + req.session.save() + .then(function () { + store.get(req.session.id, function (err, sess) { + if (err) return res.end(err.message) + res.end(sess ? 'stored' : 'empty') + }) + }) + .catch(function (err) { + res.statusCode = 500 + res.end(err.message) + }) + }) + + request(server) + .get('/') + .expect(200, 'stored', done) + }) + + it('should not return Promise with callback', function (done) { + var store = new session.MemoryStore() + var server = createServer({ store: store }, function (req, res) { + req.session.hit = true + var ret = req.session.save(function (err) { + res.statusCode = (!err && ret === undefined) ? 200 : 500 + res.end() + }) + }) + + request(server) + .get('/') + .expect(200, done) + }) + }) + + describe('without global Promise', function () { + beforeEach(function () { + global.Promise = undefined + }) + + afterEach(function () { + global.Promise = Promise + }) + + it('should work without callback', function (done) { + var store = new session.MemoryStore() + var server = createServer({ store: store }, function (req, res) { + req.session.hit = true + req.session.save() + res.end() + }) + + request(server) + .get('/') + .expect(200, done) + }) + }) }) describe('.touch()', function () { @@ -2160,7 +2580,12 @@ function createRequestListener(opts, fn) { return } - respond(req, res) + try { + respond(req, res) + } catch (e) { + res.statusCode = 500 + res.end(e.message) + } }) } }