From 564b8a79b10b4626ddb260a8eb3cb29bac5928d4 Mon Sep 17 00:00:00 2001 From: LS Hung <61226932+lokshunhung@users.noreply.github.com> Date: Fri, 29 Oct 2021 14:36:57 +0800 Subject: [PATCH 1/5] feat: req.destroySession promise api --- README.md | 3 ++- lib/fastifySession.js | 9 +++++++++ test/session.test.js | 17 +++++++++++++++++ types/types.d.ts | 1 + types/types.test-d.ts | 1 + 5 files changed, 30 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 693fe67..abfadd3 100644 --- a/README.md +++ b/README.md @@ -98,7 +98,8 @@ Allows to access or modify the session data. #### request.destroySession(callback) -Allows to destroy the session in the store +Allows to destroy the session in the store. +If callback is not passed, request.destroySession returns a promise that will reject if an error has occured. #### Session#touch() diff --git a/lib/fastifySession.js b/lib/fastifySession.js index dcd7613..c6d659a 100644 --- a/lib/fastifySession.js +++ b/lib/fastifySession.js @@ -140,6 +140,15 @@ function newSession (secret, request, cookieOpts, idGenerator, done) { function destroySession (done) { const request = this + if (!done) { + return new Promise((resolve, reject) => { + request.sessionStore.destroy(request.session.sessionId, (err) => { + request.session = null + if (err) reject(err) + else resolve() + }) + }) + } request.sessionStore.destroy(request.session.sessionId, (err) => { request.session = null done(err) diff --git a/test/session.test.js b/test/session.test.js index eea8d66..cb4d2e6 100644 --- a/test/session.test.js +++ b/test/session.test.js @@ -33,6 +33,23 @@ test('should destroy the session', async (t) => { t.is(response.statusCode, 200) }) +test('should destroy the session with promise api', async (t) => { + t.plan(2) + const port = await testServer(async (request, reply) => { + try { + await request.destroySession() + t.is(request.session, null) + reply.send(200) + } catch (err) { + t.fail(err) + } + }, DEFAULT_OPTIONS) + + const { response } = await request(`http://localhost:${port}`) + + t.is(response.statusCode, 200) +}) + test('should add session.encryptedSessionId object to request', async (t) => { t.plan(2) const port = await testServer((request, reply) => { diff --git a/types/types.d.ts b/types/types.d.ts index f540b81..2bee928 100644 --- a/types/types.d.ts +++ b/types/types.d.ts @@ -10,6 +10,7 @@ declare module "fastify" { /** A session store. */ sessionStore: Readonly; /** Allows to destroy the session in the store. */ + destroySession(): Promise; destroySession(callback: (err?: Error) => void): void; } interface Session { diff --git a/types/types.test-d.ts b/types/types.test-d.ts index 33a964e..07afd0c 100644 --- a/types/types.test-d.ts +++ b/types/types.test-d.ts @@ -56,6 +56,7 @@ app.route({ url: "/", preHandler(req, _rep, next) { req.destroySession(next); + req.destroySession().then(() => {}) }, async handler(request, reply) { expectType(request); From d51752e700e5a07e751b0067e68059c98d215169 Mon Sep 17 00:00:00 2001 From: LS Hung <61226932+lokshunhung@users.noreply.github.com> Date: Tue, 2 Nov 2021 10:46:03 +0800 Subject: [PATCH 2/5] refactor: move destroy-session function --- lib/destroy-session.js | 18 ++++++++++++++++++ lib/fastifySession.js | 18 +----------------- 2 files changed, 19 insertions(+), 17 deletions(-) create mode 100644 lib/destroy-session.js diff --git a/lib/destroy-session.js b/lib/destroy-session.js new file mode 100644 index 0000000..c7149cc --- /dev/null +++ b/lib/destroy-session.js @@ -0,0 +1,18 @@ +'use strict' + +module.exports = function destroySession (done) { + const request = this + if (!done) { + return new Promise((resolve, reject) => { + request.sessionStore.destroy(request.session.sessionId, (err) => { + request.session = null + if (err) reject(err) + else resolve() + }) + }) + } + request.sessionStore.destroy(request.session.sessionId, (err) => { + request.session = null + done(err) + }) +} diff --git a/lib/fastifySession.js b/lib/fastifySession.js index c6d659a..b2f3707 100644 --- a/lib/fastifySession.js +++ b/lib/fastifySession.js @@ -2,6 +2,7 @@ const uid = require('uid-safe').sync const fastifyPlugin = require('fastify-plugin') +const destroySession = require('./destroy-session') const Store = require('./store') const Session = require('./session') const metadata = require('./metadata') @@ -138,23 +139,6 @@ function newSession (secret, request, cookieOpts, idGenerator, done) { done() } -function destroySession (done) { - const request = this - if (!done) { - return new Promise((resolve, reject) => { - request.sessionStore.destroy(request.session.sessionId, (err) => { - request.session = null - if (err) reject(err) - else resolve() - }) - }) - } - request.sessionStore.destroy(request.session.sessionId, (err) => { - request.session = null - done(err) - }) -} - function checkOptions (options) { if (!options.secret) { return new Error('the secret option is required!') From 43314acec5ac3262ade8b7dccc8b629528627519 Mon Sep 17 00:00:00 2001 From: LS Hung <61226932+lokshunhung@users.noreply.github.com> Date: Tue, 2 Nov 2021 10:46:24 +0800 Subject: [PATCH 3/5] test: destroy-session error cases --- test/destroy-session.test.js | 88 ++++++++++++++++++++++++++++++++++++ test/session.test.js | 32 ------------- 2 files changed, 88 insertions(+), 32 deletions(-) create mode 100644 test/destroy-session.test.js diff --git a/test/destroy-session.test.js b/test/destroy-session.test.js new file mode 100644 index 0000000..da4b32b --- /dev/null +++ b/test/destroy-session.test.js @@ -0,0 +1,88 @@ +'use strict' + +const test = require('ava') +const { testServer, request, DEFAULT_OPTIONS } = require('./util') + +class ErrorStore { + get (sessionId, callback) { + throw new Error('Not implemented') + } + + set (sessionId, session, callback) { + throw new Error('Not implemented') + } + + destroy (sessionId, callback) { + callback(new Error('ErrorStore#destroy')) + } +} + +test('should successfully destroy the session with callback api', async (t) => { + t.plan(3) + const port = await testServer((request, reply) => { + request.destroySession((err) => { + t.falsy(err) + t.is(request.session, null) + reply.send(200) + }) + }, DEFAULT_OPTIONS) + + const { response } = await request(`http://localhost:${port}`) + + t.is(response.statusCode, 200) +}) + +test('should fail to destroy the session with callback api', async (t) => { + t.plan(3) + const options = { + secret: 'cNaoPYAwF60HZJzkcNaoPYAwF60HZJzk', + store: new ErrorStore() + } + const port = await testServer((request, reply) => { + request.destroySession((err) => { + t.true(err instanceof Error) + t.is(err.message, 'ErrorStore#destroy') + reply.send(200) + }) + }, options) + + const { response } = await request(`http://localhost:${port}`) + + t.is(response.statusCode, 200) +}) + +test('should successfully destroy the session with promise api', async (t) => { + t.plan(2) + const port = await testServer(async (request, reply) => { + try { + await request.destroySession() + t.is(request.session, null) + reply.send(200) + } catch (err) { + t.fail(err) + } + }, DEFAULT_OPTIONS) + + const { response } = await request(`http://localhost:${port}`) + + t.is(response.statusCode, 200) +}) + +test('should fail to destroy the session with promise api', async (t) => { + t.plan(2) + const options = { + secret: 'cNaoPYAwF60HZJzkcNaoPYAwF60HZJzk', + store: new ErrorStore() + } + const port = await testServer(async (request, reply) => { + await t.throwsAsync(async () => await request.destroySession(), { + instanceOf: Error, + message: 'ErrorStore#destroy' + }) + reply.send(200) + }, options) + + const { response } = await request(`http://localhost:${port}`) + + t.is(response.statusCode, 200) +}) diff --git a/test/session.test.js b/test/session.test.js index cb4d2e6..3f3bb9d 100644 --- a/test/session.test.js +++ b/test/session.test.js @@ -18,38 +18,6 @@ test('should add session object to request', async (t) => { t.is(statusCode, 200) }) -test('should destroy the session', async (t) => { - t.plan(3) - const port = await testServer((request, reply) => { - request.destroySession((err) => { - t.falsy(err) - t.is(request.session, null) - reply.send(200) - }) - }, DEFAULT_OPTIONS) - - const { response } = await request(`http://localhost:${port}`) - - t.is(response.statusCode, 200) -}) - -test('should destroy the session with promise api', async (t) => { - t.plan(2) - const port = await testServer(async (request, reply) => { - try { - await request.destroySession() - t.is(request.session, null) - reply.send(200) - } catch (err) { - t.fail(err) - } - }, DEFAULT_OPTIONS) - - const { response } = await request(`http://localhost:${port}`) - - t.is(response.statusCode, 200) -}) - test('should add session.encryptedSessionId object to request', async (t) => { t.plan(2) const port = await testServer((request, reply) => { From 45ed7e870df98cf5c942ea3a1682d38d60f6fc26 Mon Sep 17 00:00:00 2001 From: LS Hung <61226932+lokshunhung@users.noreply.github.com> Date: Fri, 29 Oct 2021 15:23:26 +0800 Subject: [PATCH 4/5] feat: sessionStore promise api --- lib/fastifySession.js | 4 +- lib/proxy-store.js | 50 ++++++++++++ test/proxy-store.test.js | 159 +++++++++++++++++++++++++++++++++++++++ types/types.d.ts | 16 ++-- types/types.test-d.ts | 2 +- 5 files changed, 224 insertions(+), 7 deletions(-) create mode 100644 lib/proxy-store.js create mode 100644 test/proxy-store.test.js diff --git a/lib/fastifySession.js b/lib/fastifySession.js index b2f3707..98cc5c6 100644 --- a/lib/fastifySession.js +++ b/lib/fastifySession.js @@ -3,6 +3,7 @@ const uid = require('uid-safe').sync const fastifyPlugin = require('fastify-plugin') const destroySession = require('./destroy-session') +const createProxyStore = require('./proxy-store') const Store = require('./store') const Session = require('./session') const metadata = require('./metadata') @@ -15,11 +16,12 @@ function session (fastify, options, next) { } options = ensureDefaults(options) + const proxyStore = createProxyStore(options.store) fastify.decorate('decryptSession', (sessionId, request, callback) => { decryptSession(sessionId, options, request, callback) }) - fastify.decorateRequest('sessionStore', { getter: () => options.store }) + fastify.decorateRequest('sessionStore', { getter: () => proxyStore }) fastify.decorateRequest('session', null) fastify.decorateRequest('destroySession', destroySession) fastify.addHook('onRequest', onRequest(options)) diff --git a/lib/proxy-store.js b/lib/proxy-store.js new file mode 100644 index 0000000..6f364cd --- /dev/null +++ b/lib/proxy-store.js @@ -0,0 +1,50 @@ +'use strict' + +module.exports = function createProxyStore (store) { + const promisifyStore = { + get (sessionId, callback) { + if (callback) { + return store.get(sessionId, callback) + } + return new Promise((resolve, reject) => { + store.get(sessionId, (err, session) => { + if (err) reject(err) + else resolve(session) + }) + }) + }, + + set (sessionId, session, callback) { + if (callback) { + return store.set(sessionId, session, callback) + } + return new Promise((resolve, reject) => { + store.set(sessionId, session, (err) => { + if (err) reject(err) + else resolve() + }) + }) + }, + + destroy (sessionId, callback) { + if (callback) { + return store.destroy(sessionId, callback) + } + return new Promise((resolve, reject) => { + store.destroy(sessionId, (err) => { + if (err) reject(err) + else resolve() + }) + }) + } + } + + return new Proxy(store, { + get (target, property) { + if (property in promisifyStore) { + return promisifyStore[property] + } + return target[property] + } + }) +} diff --git a/test/proxy-store.test.js b/test/proxy-store.test.js new file mode 100644 index 0000000..d5a453b --- /dev/null +++ b/test/proxy-store.test.js @@ -0,0 +1,159 @@ +'use strict' + +const test = require('ava') +const MemoryStore = require('../lib/store') +const createProxyStore = require('../lib/proxy-store') + +test('should work with callbacks for `get`, `set`, `destroy', async (t) => { + t.plan(6) + const store = new MemoryStore() + const proxyStore = createProxyStore(store) + await new Promise(resolve => { + proxyStore.set('one', { foo: 1 }, (err) => { + t.falsy(err) + proxyStore.get('one', (err, data) => { + t.falsy(err) + t.deepEqual(data, { foo: 1 }) + proxyStore.destroy('one', (err) => { + t.falsy(err) + proxyStore.get('one', (err, data) => { + t.falsy(err) + t.falsy(data) + resolve() + }) + }) + }) + }) + }) +}) + +test('should work with promise api for `get`, `set`, `destroy`', async (t) => { + t.plan(2) + const store = new MemoryStore() + const proxyStore = createProxyStore(store) + await proxyStore.set('one', { foo: 1 }) + const data = await proxyStore.get('one') + t.deepEqual(data, { foo: 1 }) + await proxyStore.destroy('one') + const empty = await proxyStore.get('one') + t.falsy(empty) +}) + +test('should reject promise if callback argument is not used', async (t) => { + t.plan(3) + class ErrorStore { + get (sessionId, callback) { + callback(new Error('ErrorStore#get'), null) + } + + set (sessionId, session, callback) { + callback(new Error('ErrorStore#set')) + } + + destroy (sessionId, callback) { + callback(new Error('ErrorStore#destroy')) + } + } + const store = new ErrorStore() + const proxyStore = createProxyStore(store) + try { + await proxyStore.get('id') + t.fail('expect promise rejection with no callback argument') + } catch (err) { + t.true(err instanceof Error) + } + try { + await proxyStore.set('id', { foo: 1 }) + t.fail('expect promise rejection with no callback argument') + } catch (err) { + t.true(err instanceof Error) + } + try { + await proxyStore.destroy('id') + t.fail('expect promise rejection with no callback argument') + } catch (err) { + t.true(err instanceof Error) + } +}) + +test('should not reject promise if callback is passed', async (t) => { + t.plan(4) + class ErrorStore { + get (sessionId, callback) { + callback(new Error('ErrorStore#get'), null) + } + + set (sessionId, session, callback) { + callback(new Error('ErrorStore#set')) + } + + destroy (sessionId, callback) { + callback(new Error('ErrorStore#destroy')) + } + } + const store = new ErrorStore() + const proxyStore = createProxyStore(store) + try { + await proxyStore.get('id', (err, data) => { + t.true(err instanceof Error) + t.falsy(data) + }) + } catch (err) { + t.fail('unexpected promise rejection with callback argument passed') + } + try { + await proxyStore.set('id', { foo: 1 }, (err) => { + t.true(err instanceof Error) + }) + } catch (err) { + t.fail('unexpected promise rejection with callback argument passed') + } + try { + await proxyStore.destroy('id', (err) => { + t.true(err instanceof Error) + }) + } catch (err) { + t.fail('unexpected promise rejection with callback argument passed') + } +}) + +test('should not mutate store instance by monkey-patch methods', (t) => { + t.plan(3) + class Store { + constructor () { + this.originalMethods = { get: this.get, set: this.set, destroy: this.destroy } + } + + get () {} + set () {} + destroy () {} + } + const store = new Store() + const originalMethods = store.originalMethods + createProxyStore(store) + t.is(store.get, originalMethods.get) + t.is(store.set, originalMethods.set) + t.is(store.destroy, originalMethods.destroy) +}) + +test('should allow referencing store methods other than `get`, `set`, `destroy`', (t) => { + t.plan(4) + class Store { + get () {} + set () {} + destroy () {} + foo () { + return 1 + } + + bar () { + return this.foo() + 1 + } + } + const store = new Store() + const proxyStore = createProxyStore(store) + t.is(proxyStore.foo(), 1) + t.is(proxyStore.bar(), 2) + t.is(proxyStore.foo, store.foo) + t.is(proxyStore.bar, store.bar) +}) diff --git a/types/types.d.ts b/types/types.d.ts index 2bee928..939a9a9 100644 --- a/types/types.d.ts +++ b/types/types.d.ts @@ -8,7 +8,7 @@ declare module "fastify" { /** Allows to access or modify the session data. */ session: Fastify.Session; /** A session store. */ - sessionStore: Readonly; + sessionStore: Readonly; /** Allows to destroy the session in the store. */ destroySession(): Promise; destroySession(callback: (err?: Error) => void): void; @@ -26,10 +26,16 @@ declare module "fastify" { declare namespace FastifySessionPlugin { interface SessionStore { set(sessionId: string, session: Fastify.Session, callback: (err?: Error) => void): void; - get( - sessionId: string, - callback: (err?: Error, session?: Fastify.Session) => void - ): void; + get(sessionId: string, callback: (err?: Error, session?: Fastify.Session) => void): void; + destroy(sessionId: string, callback: (err?: Error) => void): void; + } + + interface ProxySessionStore { + set(sessionId: string, session: Fastify.Session): Promise; + set(sessionId: string, session: Fastify.Session, callback: (err?: Error) => void): void; + get(sessionId: string): Promise; + get(sessionId: string, callback: (err?: Error, session?: Fastify.Session) => void): void; + destroy(sessionId: string): Promise; destroy(sessionId: string, callback: (err?: Error) => void): void; } diff --git a/types/types.test-d.ts b/types/types.test-d.ts index 07afd0c..3147790 100644 --- a/types/types.test-d.ts +++ b/types/types.test-d.ts @@ -61,7 +61,7 @@ app.route({ async handler(request, reply) { expectType(request); expectType(reply); - expectType>(request.sessionStore); + expectType>(request.sessionStore); expectError((request.sessionStore = null)); expectError(request.session.doesNotExist()); expectType<{ id: number } | undefined>(request.session.user); From 6b1cdab5559f3436005acddb0225af6ed529137c Mon Sep 17 00:00:00 2001 From: LS Hung <61226932+lokshunhung@users.noreply.github.com> Date: Tue, 2 Nov 2021 12:34:15 +0800 Subject: [PATCH 5/5] docs: add promise apis docs --- README.md | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/README.md b/README.md index abfadd3..0d761dd 100644 --- a/README.md +++ b/README.md @@ -96,11 +96,41 @@ Function used to generate new session IDs. Defaults to [`uid(24)`](https://githu Allows to access or modify the session data. +#### request.sessionStore + +Access the store instance from `options.store`. It proxies the `get`, `set`, `destroy` method on the store instance, such that it returns a promise when the callback argument is not passed. Other methods and properties on the store instance can be accessed transparently. + +```ts +// Using callbacks +request.sessionStore.get(sessionId, (err, session) => {}) +request.sessionStore.set(sessionId, session, (err) => {}) +request.sessionStore.destroy(sessionId, (err) => {}) +// Using promises +request.sessionStore.get(sessionId).then((session) => {}).catch((err) => {}) +request.sessionStore.set(sessionId, session).catch((err) => {}) +request.sessionStore.destroy(sessionId).catch((err) => {}) +// Using async/await +const session = await request.sessionStore.get(sessionId) +await request.sessionStore.set(sessionId, session) +await request.sessionStore.destroy(sessionId) +// Some store implementations has extra methods, they can be used normally +request.sessionStore.touch(sessionId, session, (err) => {}) +``` + #### request.destroySession(callback) Allows to destroy the session in the store. If callback is not passed, request.destroySession returns a promise that will reject if an error has occured. +```ts +// Using callbacks +request.destroySession((err) => {}) +// Using promises +request.destroySession.catch((err) => {}) +// Using async/await +await request.destroySession() +``` + #### Session#touch() Updates the `expires` property of the session.