diff --git a/HISTORY.md b/HISTORY.md index 238498ec..a8930fd0 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,6 +1,11 @@ unreleased ========== + * Support promises in `session.save()`, `session.reload()`, `session.destroy()` + and `session.regenerate()` when called without a callback + - `save()` resolves to the session it was called on, `reload()` and + `regenerate()` resolve to the new `req.session`, and `destroy()` + resolves to `undefined` * Replace `uid-safe` dependency with built-in `crypto.randomBytes` for session ID generation - Session IDs keep the same format as before (32-character base64url strings) diff --git a/README.md b/README.md index 1e30b86b..75567624 100644 --- a/README.md +++ b/README.md @@ -366,41 +366,102 @@ app.get('/', function(req, res, next) { }) ``` -#### Session.regenerate(callback) +#### Session.regenerate(callback) => Promise To regenerate the session simply invoke the method. Once complete, a new SID and `Session` instance will be initialized at `req.session` and the `callback` will be invoked. +When called without a callback, a `Promise` is returned instead, which +resolves to the newly created session. + ```js req.session.regenerate(function(err) { // will have a new session here }) + +// or promises +req.session.regenerate().then(function(session) { + // session is the new req.session +}).catch(function(err) { + // a problem... +}) + +// or async/await +(async function() { + try { + // resolves to the new req.session + await req.session.regenerate() + } catch(err) { + // a problem... + } +})() ``` -#### Session.destroy(callback) +#### Session.destroy(callback) => Promise Destroys the session and will unset the `req.session` property. Once complete, the `callback` will be invoked. +When called without a callback, a `Promise` is returned instead, which +resolves to `undefined` once the session is destroyed. + ```js req.session.destroy(function(err) { // cannot access session here }) + +// or promises +req.session.destroy().then(function() { + // cannot access session here +}).catch(function(err) { + // a problem... +}) + +// or async/await +(async function() { + try { + await req.session.destroy() + // cannot access session here + } catch(err) { + // a problem... + } +})() ``` -#### Session.reload(callback) +#### Session.reload(callback) => Promise Reloads the session data from the store and re-populates the `req.session` object. Once complete, the `callback` will be invoked. +When called without a callback, a `Promise` is returned instead, which +resolves to the reloaded session: a new `Session` object at `req.session` +representing the same session. + ```js req.session.reload(function(err) { // session updated }) + +// or promises +req.session.reload().then(function(session) { + // session is the reloaded req.session +}).catch(function(err) { + // a problem... +}) + +// or async/await +(async function() { + try { + // resolves to the reloaded req.session + await req.session.reload() + } catch(err) { + // a problem... + } +})() ``` -#### Session.save(callback) +#### Session.save(callback) => Promise Save the session back to the store, replacing the contents on the store with the contents in memory (though a store may do something else--consult the store's @@ -414,10 +475,31 @@ does not need to be called. There are some cases where it is useful to call this method, for example, redirects, long-lived requests or in WebSockets. +When called without a callback, a `Promise` is returned instead, which +resolves to the session the method was called on — even if `req.session` +has since been replaced (for example by `regenerate()` or `reload()`). + ```js req.session.save(function(err) { // session saved }) + +// or promises +req.session.save().then(function(session) { + // session saved; session is the one save() was called on +}).catch(function(err) { + // a problem... +}) + +// or async/await +(async function() { + try { + // resolves to the session save() was called on + await req.session.save() + } catch(err) { + // a problem... + } +})() ``` #### Session.touch() diff --git a/index.js b/index.js index c61dce0a..1aa4d79b 100644 --- a/index.js +++ b/index.js @@ -384,13 +384,17 @@ function session(options) { function reload(callback) { debug('reloading %s', this.id) - _reload.call(this, rewrapmethods(this, callback)) + if (typeof callback === 'function') { + return _reload.call(this, rewrapmethods(this, callback)) + } + + return _reload.call(this).finally(rewrapmethods(this, function () {})) } 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/session/session.js b/session/session.js index fee7608c..f8859cdd 100644 --- a/session/session.js +++ b/session/session.js @@ -63,14 +63,18 @@ 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 {Session|Promise} for chaining * @api public */ defineMethod(Session.prototype, 'save', function save(fn) { - this.req.sessionStore.set(this.id, this, fn || function(){}); - return this; + var self = this; + var store = this.req.sessionStore; + + return callbackOrPromise(this, fn, function (done) { + store.set(self.id, self, done); + }, self); }); /** @@ -80,49 +84,60 @@ 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 {Session|Promise} for chaining * @api public */ defineMethod(Session.prototype, 'reload', function reload(fn) { - var req = this.req - var store = this.req.sessionStore - - 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(); + var req = this.req; + var store = this.req.sessionStore; + var id = this.id; + + return callbackOrPromise(this, fn, function (done) { + store.get(id, function (err, sess) { + if (err) return done(err); + if (!sess) return done(new Error('failed to load session')); + store.createSession(req, sess); + done(); + }); }); - return this; }); /** * Destroy `this` session. * - * @param {Function} fn - * @return {Session} for chaining + * @param {Function} [fn] + * @return {Session|Promise} for chaining * @api public */ defineMethod(Session.prototype, 'destroy', function destroy(fn) { + var store = this.req.sessionStore; + var id = this.id; + delete this.req.session; - this.req.sessionStore.destroy(this.id, fn); - return this; + + return callbackOrPromise(this, fn, function (done) { + store.destroy(id, done); + }); }); /** * Regenerate this request's session. * - * @param {Function} fn - * @return {Session} for chaining + * @param {Function} [fn] + * @return {Session|Promise} for chaining * @api public */ defineMethod(Session.prototype, 'regenerate', function regenerate(fn) { - this.req.sessionStore.regenerate(this.req, fn); - return this; + var req = this.req; + var store = this.req.sessionStore; + + return callbackOrPromise(this, fn, function (done) { + store.regenerate(req, done); + }); }); /** @@ -141,3 +156,25 @@ function defineMethod(obj, name, fn) { writable: true }); }; + +/** + * Run `executor(done)` in callback or promise style: with a callback, + * return `session` for chaining; without one, return a `Promise` + * resolving to `value`, or the request's current session by default. + * + * @private + */ + +function callbackOrPromise(session, callback, executor, value) { + if (typeof callback === 'function') { + executor(callback) + return session + } + + return new Promise(function (resolve, reject) { + executor(function (err) { + if (err) return reject(err) + resolve(value !== undefined ? value : session.req.session) + }) + }) +} diff --git a/test/session.js b/test/session.js index 1e851dcd..745d9b62 100644 --- a/test/session.js +++ b/test/session.js @@ -1604,6 +1604,42 @@ describe('session()', function(){ .expect(shouldNotHaveHeader('Set-Cookie')) .expect(200, 'undefined', done) }) + + it('session with promises should resolve to undefined', function (done) { + var server = createServer(null, function (req, res) { + req.session.destroy().then(function (value) { + res.end(String(value === undefined && req.session === undefined)) + }).catch(function () { + res.statusCode = 500 + res.end() + }) + }) + + request(server) + .get('/') + .expect(shouldNotHaveHeader('Set-Cookie')) + .expect(200, 'true', done) + }) + + it('session with promises should reject on destroy error', function (done) { + var store = new session.MemoryStore() + var server = createServer({ store: store }, function (req, res) { + req.session.destroy().then(function () { + res.end('destroyed') + }).catch(function (err) { + res.statusCode = 500 + res.end(err.message) + }) + }) + + store.destroy = function destroy(sid, callback) { + callback(new Error('boom!')) + } + + request(server) + .get('/') + .expect(500, 'boom!', done) + }) }) describe('.regenerate()', function(){ @@ -1629,6 +1665,43 @@ describe('session()', function(){ .expect(200, 'false', done) }); }) + + it('session with promises should resolve to the new session', function (done) { + var server = createServer(null, function (req, res) { + var id = req.session.id + req.session.regenerate().then(function (sess) { + res.end(String(sess === req.session && sess.id !== id)) + }).catch(function () { + res.statusCode = 500 + res.end() + }) + }) + + request(server) + .get('/') + .expect(shouldSetCookie('connect.sid')) + .expect(200, 'true', done) + }) + + it('session with promises should reject on regenerate error', function (done) { + var store = new session.MemoryStore() + var server = createServer({ store: store }, function (req, res) { + req.session.regenerate().then(function () { + res.end('regenerated') + }).catch(function (err) { + res.statusCode = 500 + res.end(err.message) + }) + }) + + store.destroy = function destroy(sid, callback) { + callback(new Error('boom!')) + } + + request(server) + .get('/') + .expect(500, 'boom!', done) + }) }) describe('.reload()', function () { @@ -1672,7 +1745,7 @@ describe('session()', function(){ }) }) - it('should error is session missing', function (done) { + it('should error if session missing', function (done) { var store = new session.MemoryStore() var server = createServer({ store: store }, function (req, res) { if (req.url === '/') { @@ -1743,6 +1816,66 @@ describe('session()', function(){ .expect(200, 'ok', done) }) }) + + it('session with promises should error if session missing', function (done) { + var store = new session.MemoryStore() + var server = createServer({ store: store }, function (req, res) { + if (req.url === '/') { + req.session.active = true + res.end('session created') + return + } + + store.clear(function (err) { + if (err) return done(err) + req.session.reload().then(function() { + res.statusCode = 200 + 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(500, 'failed to load session', done) + }) + }) + + it('session with promises should resolve to the reloaded session', function (done) { + var server = createServer(null, function (req, res) { + if (req.url === '/') { + req.session.active = true + res.end('session created') + return + } + + var prev = req.session + req.session.reload().then(function (sess) { + res.end(String(sess === req.session && sess !== prev && sess.id === prev.id)) + }).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, 'true', done) + }) + }) }) describe('.save()', function () { @@ -1764,6 +1897,77 @@ describe('session()', function(){ .expect(200, 'stored', done) }) + it('session with promises should save session to store', function (done) { + var store = new session.MemoryStore() + var server = createServer({ store: store }, function (req, res) { + req.session.hit = true + req.session.save().then(function() { + // TODO: Make MemoryStore methods return promises + // so we don't have to wrap store.get around a promise + // here? + return new Promise(function(resolve, reject) { + store.get(req.session.id, function (err, sess) { + if (err) return reject(err) + resolve(sess ? 'stored' : 'empty') + }) + }) + }).then(function(message) { + res.end(message) + }).catch(function(err) { + res.end(err.message) + }) + }) + + request(server) + .get('/') + .expect(200, 'stored', done) + }) + + it('session with promises should resolve to the saved session', function (done) { + var server = createServer(null, function (req, res) { + var sess = req.session + sess.hit = true + // save first so reload can replace req.session with a new + // object, then save the old reference: the promise must + // resolve to that reference, not the current req.session + sess.save().then(function () { + return sess.reload() + }).then(function () { + return sess.save() + }).then(function (value) { + res.end(String(value === sess && value !== req.session)) + }).catch(function (err) { + res.statusCode = 500 + res.end(err.message) + }) + }) + + request(server) + .get('/') + .expect(200, 'true', done) + }) + + it('session with promises should reject on save error', function (done) { + var store = new session.MemoryStore() + var server = createServer({ store: store, saveUninitialized: false }, function (req, res) { + req.session.hit = true + req.session.save().then(function () { + res.end('saved') + }).catch(function (err) { + res.statusCode = 500 + res.end(err.message) + }) + }) + + store.set = function set(sid, sess, callback) { + callback(new Error('boom!')) + } + + request(server) + .get('/') + .expect(500, 'boom!', done) + }) + it('should prevent end-of-request save', function (done) { var store = new session.MemoryStore() var server = createServer({ store: store }, function (req, res) { @@ -1787,6 +1991,30 @@ describe('session()', function(){ }) }) + it('session with promises should prevent end-of-request save', function (done) { + var store = new session.MemoryStore() + var server = createServer({ store: store }, function (req, res) { + req.session.hit = true + req.session.save().then(function() { + res.end('saved') + }).catch(function(err) { + res.end(err.message) + }) + }) + + request(server) + .get('/') + .expect(shouldSetSessionInStore(store)) + .expect(200, 'saved', function (err, res) { + if (err) return done(err) + request(server) + .get('/') + .set('Cookie', cookie(res)) + .expect(shouldSetSessionInStore(store)) + .expect(200, 'saved', done) + }) + }) + it('should prevent end-of-request save on reloaded session', function (done) { var store = new session.MemoryStore() var server = createServer({ store: store }, function (req, res) { @@ -1812,6 +2040,36 @@ describe('session()', function(){ }) }) + it('session with promises should prevent end-of-request save on reloaded session', function (done) { + var store = new session.MemoryStore() + var server = createServer({ store: store }, function (req, res) { + req.session.hit = true + // NOTE: reload() rejects with a `failed to load session` error + // on the first request, since the session is not in the store + // yet. The previous test using callbacks ignores that error the + // same way, so ignore it here and always save for parity. + req.session.reload().catch(function () {}).then(function() { + return req.session.save() + }).then(function() { + res.end('saved') + }).catch(function(err) { + res.end(err.message) + }) + }) + + request(server) + .get('/') + .expect(shouldSetSessionInStore(store)) + .expect(200, 'saved', function (err, res) { + if (err) return done(err) + request(server) + .get('/') + .set('Cookie', cookie(res)) + .expect(shouldSetSessionInStore(store)) + .expect(200, 'saved', done) + }) + }) + describe('when saveUninitialized is false', function () { it('should prevent end-of-request save', function (done) { var store = new session.MemoryStore()