From 2a9df9c1ec92a336d828aa012b0f6521532b9726 Mon Sep 17 00:00:00 2001 From: Igor Savin Date: Thu, 13 Aug 2020 16:51:56 +0300 Subject: [PATCH 01/17] Add test for POST request with body, using async storage in prevalidation --- test/requestContextPlugin.spec.js | 93 +++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/test/requestContextPlugin.spec.js b/test/requestContextPlugin.spec.js index 0b79eb8..c582f58 100644 --- a/test/requestContextPlugin.spec.js +++ b/test/requestContextPlugin.spec.js @@ -20,6 +20,26 @@ function initAppPost(endpoint) { return app.ready() } +function initAppPostWithPrevalidation(endpoint) { + const app = fastify({ logger: true }) + app.register(fastifyRequestContextPlugin) + + const preValidationFn = (req, reply, done) => { + const requestId = Number.parseInt(req.body.requestId) + req.requestContext.set('testKey', `testValue${requestId}`) + done() + }; + + app.route({ + url: '/', + method: ['GET', 'POST'], + preValidation: preValidationFn, + handler: endpoint, + }) + + return app.ready() +} + describe('requestContextPlugin', () => { let app afterEach(() => { @@ -171,4 +191,77 @@ describe('requestContextPlugin', () => { return promiseRequest2 }) }) + + it('correctly preserves values set in prevalidation phase within single POST request', () => { + expect.assertions(2) + + let testService + let responseCounter = 0 + return new Promise((resolveResponsePromise) => { + const promiseRequest2 = new Promise((resolveRequest2Promise) => { + const promiseRequest1 = new Promise((resolveRequest1Promise) => { + const route = (req, reply) => { + const requestId = req.requestContext.get('testKey') + + function prepareReply() { + testService.processRequest(requestId.replace('testValue', '')).then(() => { + const storedValue = req.requestContext.get('testKey') + reply.status(204).send({ + storedValue, + }) + }) + } + + // We don't want to read values until both requests wrote their values to see if there is a racing condition + if (requestId === 'testValue1') { + resolveRequest1Promise() + return promiseRequest2.then(prepareReply) + } + + if (requestId === 'testValue2') { + resolveRequest2Promise() + return promiseRequest1.then(prepareReply) + } + } + + initAppPostWithPrevalidation(route).then((_app) => { + app = _app + testService = new TestService(app) + const response1Promise = app + .inject() + .post('/') + .body({ requestId: 1 }) + .end() + .then((response) => { + expect(response.json().storedValue).toBe('testValue1') + responseCounter++ + if (responseCounter === 2) { + resolveResponsePromise() + } + }) + + const response2Promise = app + .inject() + .post('/') + .body({ requestId: 2 }) + .end() + .then((response) => { + expect(response.json().storedValue).toBe('testValue2') + responseCounter++ + if (responseCounter === 2) { + resolveResponsePromise() + } + }) + + return Promise.all([response1Promise, response2Promise]) + }) + }) + + return promiseRequest1 + }) + + return promiseRequest2 + }) + }) + }) From 4e8b091878e7102d993f166d5b3203081c037667 Mon Sep 17 00:00:00 2001 From: Igor Savin Date: Thu, 13 Aug 2020 16:53:56 +0300 Subject: [PATCH 02/17] Fix linting --- test/requestContextPlugin.spec.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/requestContextPlugin.spec.js b/test/requestContextPlugin.spec.js index c582f58..8c364e9 100644 --- a/test/requestContextPlugin.spec.js +++ b/test/requestContextPlugin.spec.js @@ -28,7 +28,7 @@ function initAppPostWithPrevalidation(endpoint) { const requestId = Number.parseInt(req.body.requestId) req.requestContext.set('testKey', `testValue${requestId}`) done() - }; + } app.route({ url: '/', @@ -263,5 +263,4 @@ describe('requestContextPlugin', () => { return promiseRequest2 }) }) - }) From 3da0d3be7f062ae21868cc11b8ed4df4a596b170 Mon Sep 17 00:00:00 2001 From: Igor Savin Date: Thu, 13 Aug 2020 17:21:31 +0300 Subject: [PATCH 03/17] Add end-to-end test --- package.json | 1 + test/requestContextPlugin.e2e.spec.js | 107 ++++++++++++++++++++++++++ 2 files changed, 108 insertions(+) create mode 100644 test/requestContextPlugin.e2e.spec.js diff --git a/package.json b/package.json index 8421e1d..9e950d1 100644 --- a/package.json +++ b/package.json @@ -33,6 +33,7 @@ "eslint-plugin-prettier": "^3.1.4", "jest": "^26.4.0", "prettier": "^2.0.5", + "superagent": "^6.0.0", "tsd": "^0.13.1", "typescript": "3.9.7" }, diff --git a/test/requestContextPlugin.e2e.spec.js b/test/requestContextPlugin.e2e.spec.js new file mode 100644 index 0000000..51f2e43 --- /dev/null +++ b/test/requestContextPlugin.e2e.spec.js @@ -0,0 +1,107 @@ +const fastify = require('fastify') +const request = require('superagent') +const { fastifyRequestContextPlugin } = require('../lib/requestContextPlugin') +const { TestService } = require('./internal/testService') + +let server +function initAppPostWithPrevalidation(endpoint) { + return new Promise((resolve, reject) => { + const app = fastify({ logger: true }) + app.register(fastifyRequestContextPlugin) + + const preValidationFn = (req, reply, done) => { + const requestId = Number.parseInt(req.body.requestId) + req.requestContext.set('testKey', `testValue${requestId}`) + done() + } + + app.route({ + url: '/', + method: ['GET', 'POST'], + preValidation: preValidationFn, + handler: endpoint, + }) + + app.listen(8085, '0.0.0.0', (err, address) => { + if (err) { + console.warn(err) + process.exit(1) + } + console.info(`Server listening at ${address}`) + resolve(app) + }) + }) +} + +describe('requestContextPlugin E2E', () => { + let app + afterEach(() => { + return server.close() + }) + + it('correctly preserves values set in prevalidation phase within single POST request', () => { + expect.assertions(2) + + let testService + let responseCounter = 0 + return new Promise((resolveResponsePromise) => { + const promiseRequest2 = new Promise((resolveRequest2Promise) => { + const promiseRequest1 = new Promise((resolveRequest1Promise) => { + const route = (req, reply) => { + const requestId = req.requestContext.get('testKey') + + function prepareReply() { + testService.processRequest(requestId.replace('testValue', '')).then(() => { + const storedValue = req.requestContext.get('testKey') + reply.status(204).send({ + storedValue, + }) + }) + } + + // We don't want to read values until both requests wrote their values to see if there is a racing condition + if (requestId === 'testValue1') { + resolveRequest1Promise() + return promiseRequest2.then(prepareReply) + } + + if (requestId === 'testValue2') { + resolveRequest2Promise() + return promiseRequest1.then(prepareReply) + } + } + + initAppPostWithPrevalidation(route).then((_app) => { + app = _app + testService = new TestService(app) + const response1Promise = request('POST', '0.0.0.0:8085') + .send({ requestId: 1 }) + .then((response) => { + expect(response.body.storedValue).toBe('testValue1') + responseCounter++ + if (responseCounter === 2) { + resolveResponsePromise() + } + }) + + const response2Promise = request('POST', '0.0.0.0:8085') + .send({ requestId: 2 }) + .then((response) => { + expect(response.body.storedValue).toBe('testValue2') + responseCounter++ + if (responseCounter === 2) { + resolveResponsePromise() + } + }) + + return Promise.all([response1Promise, response2Promise]) + }) + }) + + return promiseRequest1 + }) + + return promiseRequest2 + }) + }) +}) From 7bce481b3f11f85796b6d359cbf7bdf3a4e50698 Mon Sep 17 00:00:00 2001 From: Igor Savin Date: Thu, 13 Aug 2020 17:26:08 +0300 Subject: [PATCH 04/17] Fix linting --- test/requestContextPlugin.e2e.spec.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/requestContextPlugin.e2e.spec.js b/test/requestContextPlugin.e2e.spec.js index 51f2e43..4bccf82 100644 --- a/test/requestContextPlugin.e2e.spec.js +++ b/test/requestContextPlugin.e2e.spec.js @@ -5,7 +5,7 @@ const { TestService } = require('./internal/testService') let server function initAppPostWithPrevalidation(endpoint) { - return new Promise((resolve, reject) => { + return new Promise((resolve) => { const app = fastify({ logger: true }) app.register(fastifyRequestContextPlugin) @@ -69,6 +69,8 @@ describe('requestContextPlugin E2E', () => { resolveRequest2Promise() return promiseRequest1.then(prepareReply) } + + throw new Error(`Unexpected requestId: ${requestId}`) } initAppPostWithPrevalidation(route).then((_app) => { From 075a5263eb9048bcffcd50daa6cc0b202bfcd3b3 Mon Sep 17 00:00:00 2001 From: Igor Savin Date: Thu, 13 Aug 2020 17:55:42 +0300 Subject: [PATCH 05/17] Implement hook configuration support --- README.md | 6 ++++-- index.d.ts | 14 ++++++++++++++ index.test-d.ts | 4 ++++ lib/requestContextPlugin.js | 2 +- test/requestContextPlugin.e2e.spec.js | 14 ++++++-------- test/requestContextPlugin.spec.js | 2 +- 6 files changed, 30 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index cbee564..f3601c0 100644 --- a/README.md +++ b/README.md @@ -27,15 +27,17 @@ const { fastifyRequestContextPlugin } = require('fastify-request-context') const fastify = require('fastify'); fastify.register(fastifyRequestContextPlugin, { + hook: 'preValidation', defaultStoreValues: { user: { id: 'system' } } }); ``` -This plugin accepts option named `defaultStoreValues`. +This plugin acceptss options `hook` and `defaultStoreValues`. -`defaultStoreValues` set initial values for the store (that can be later overwritten during request execution if needed). This is an optional parameter. +* `hook` allows you to specify to which lifecycle hook should request context initialization be bound. Make sure that you are not trying to access the request context earlier in the lifecycle than you initialize it. Default value is `onRequest`. +* `defaultStoreValues` sets initial values for the store (that can be later overwritten during request execution if needed). This is an optional parameter. From there you can set a context in another hook, route, or method that is within scope. diff --git a/index.d.ts b/index.d.ts index 4039372..053cc45 100644 --- a/index.d.ts +++ b/index.d.ts @@ -7,6 +7,20 @@ export type RequestContext = { export type RequestContextOptions = { defaultStoreValues?: Record + hook?: + | 'onRequest' + | 'preParsing' + | 'preValidation' + | 'preHandler' + | 'preSerialization' + | 'onSend' + | 'onResponse' + | 'onTimeout' + | 'onError' + | 'onRoute' + | 'onRegister' + | 'onReady' + | 'onClose' } declare module 'fastify' { diff --git a/index.test-d.ts b/index.test-d.ts index 3c28ff0..56a060f 100644 --- a/index.test-d.ts +++ b/index.test-d.ts @@ -13,6 +13,10 @@ expectAssignable({}) expectAssignable({ defaultStoreValues: { a: 'dummy' }, }) +expectAssignable({ + hook: 'preValidation', + defaultStoreValues: { a: 'dummy' }, +}) expectType(app.requestContext) diff --git a/lib/requestContextPlugin.js b/lib/requestContextPlugin.js index 6a1ad07..c19275b 100644 --- a/lib/requestContextPlugin.js +++ b/lib/requestContextPlugin.js @@ -10,7 +10,7 @@ function plugin(fastify, opts, next) { fastify.decorate('requestContext', requestContext) fastify.decorateRequest('requestContext', requestContext) - fastify.addHook('onRequest', (req, res, done) => { + fastify.addHook(opts.hook || 'onRequest', (req, res, done) => { als.runWith(() => { done() }, opts.defaultStoreValues) diff --git a/test/requestContextPlugin.e2e.spec.js b/test/requestContextPlugin.e2e.spec.js index 4bccf82..0f18c68 100644 --- a/test/requestContextPlugin.e2e.spec.js +++ b/test/requestContextPlugin.e2e.spec.js @@ -3,11 +3,10 @@ const request = require('superagent') const { fastifyRequestContextPlugin } = require('../lib/requestContextPlugin') const { TestService } = require('./internal/testService') -let server function initAppPostWithPrevalidation(endpoint) { return new Promise((resolve) => { const app = fastify({ logger: true }) - app.register(fastifyRequestContextPlugin) + app.register(fastifyRequestContextPlugin, { hook: 'preValidation' }) const preValidationFn = (req, reply, done) => { const requestId = Number.parseInt(req.body.requestId) @@ -36,10 +35,11 @@ function initAppPostWithPrevalidation(endpoint) { describe('requestContextPlugin E2E', () => { let app afterEach(() => { - return server.close() + return app.close() }) it('correctly preserves values set in prevalidation phase within single POST request', () => { + jest.setTimeout(30000) expect.assertions(2) let testService @@ -47,15 +47,13 @@ describe('requestContextPlugin E2E', () => { return new Promise((resolveResponsePromise) => { const promiseRequest2 = new Promise((resolveRequest2Promise) => { const promiseRequest1 = new Promise((resolveRequest1Promise) => { - const route = (req, reply) => { + const route = (req) => { const requestId = req.requestContext.get('testKey') function prepareReply() { - testService.processRequest(requestId.replace('testValue', '')).then(() => { + return testService.processRequest(requestId.replace('testValue', '')).then(() => { const storedValue = req.requestContext.get('testKey') - reply.status(204).send({ - storedValue, - }) + return Promise.resolve({ storedValue }) }) } diff --git a/test/requestContextPlugin.spec.js b/test/requestContextPlugin.spec.js index 8c364e9..61f73c1 100644 --- a/test/requestContextPlugin.spec.js +++ b/test/requestContextPlugin.spec.js @@ -22,7 +22,7 @@ function initAppPost(endpoint) { function initAppPostWithPrevalidation(endpoint) { const app = fastify({ logger: true }) - app.register(fastifyRequestContextPlugin) + app.register(fastifyRequestContextPlugin, { hook: 'preValidation' }) const preValidationFn = (req, reply, done) => { const requestId = Number.parseInt(req.body.requestId) From d405af25d7a2e5db4bdcf4b5023ecd410a2c1d22 Mon Sep 17 00:00:00 2001 From: Igor Savin Date: Thu, 13 Aug 2020 17:56:41 +0300 Subject: [PATCH 06/17] Fix typo --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index f3601c0..9e3bc84 100644 --- a/README.md +++ b/README.md @@ -34,7 +34,7 @@ fastify.register(fastifyRequestContextPlugin, { }); ``` -This plugin acceptss options `hook` and `defaultStoreValues`. +This plugin accepts options `hook` and `defaultStoreValues`. * `hook` allows you to specify to which lifecycle hook should request context initialization be bound. Make sure that you are not trying to access the request context earlier in the lifecycle than you initialize it. Default value is `onRequest`. * `defaultStoreValues` sets initial values for the store (that can be later overwritten during request execution if needed). This is an optional parameter. From d97cb01f874be8a43db4ba5d640ff2e3cbfc1db6 Mon Sep 17 00:00:00 2001 From: Igor Savin Date: Thu, 13 Aug 2020 17:59:07 +0300 Subject: [PATCH 07/17] Remove unneeded timeout config --- test/requestContextPlugin.e2e.spec.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/requestContextPlugin.e2e.spec.js b/test/requestContextPlugin.e2e.spec.js index 0f18c68..047a93c 100644 --- a/test/requestContextPlugin.e2e.spec.js +++ b/test/requestContextPlugin.e2e.spec.js @@ -39,7 +39,6 @@ describe('requestContextPlugin E2E', () => { }) it('correctly preserves values set in prevalidation phase within single POST request', () => { - jest.setTimeout(30000) expect.assertions(2) let testService From 48c23e77714b11f9a391d1e30610af7cd19f26b7 Mon Sep 17 00:00:00 2001 From: Igor Savin Date: Thu, 13 Aug 2020 18:48:36 +0300 Subject: [PATCH 08/17] Address code review comments --- .eslintrc.json | 2 +- README.md | 2 +- index.d.ts | 30 ++++++++-------- test/requestContextPlugin.e2e.spec.js | 50 ++++++++++++--------------- 4 files changed, 40 insertions(+), 44 deletions(-) diff --git a/.eslintrc.json b/.eslintrc.json index c74eddd..a3dd428 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -12,7 +12,7 @@ "prettier" ], "parserOptions": { - "ecmaVersion": 2015, + "ecmaVersion": 2017, "sourceType": "module" }, "rules": { diff --git a/README.md b/README.md index 9e3bc84..ae7709d 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,7 @@ fastify.register(fastifyRequestContextPlugin, { This plugin accepts options `hook` and `defaultStoreValues`. -* `hook` allows you to specify to which lifecycle hook should request context initialization be bound. Make sure that you are not trying to access the request context earlier in the lifecycle than you initialize it. Default value is `onRequest`. +* `hook` allows you to specify to which lifecycle hook (or multiple hooks) should request context initialization be bound. Make sure that you are not trying to access the request context earlier in the lifecycle than you initialize it. Default value is `onRequest`. * `defaultStoreValues` sets initial values for the store (that can be later overwritten during request execution if needed). This is an optional parameter. From there you can set a context in another hook, route, or method that is within scope. diff --git a/index.d.ts b/index.d.ts index 053cc45..75dfd51 100644 --- a/index.d.ts +++ b/index.d.ts @@ -5,22 +5,24 @@ export type RequestContext = { set: (key: string, value: T) => void } +export type Hook = + | 'onRequest' + | 'preParsing' + | 'preValidation' + | 'preHandler' + | 'preSerialization' + | 'onSend' + | 'onResponse' + | 'onTimeout' + | 'onError' + | 'onRoute' + | 'onRegister' + | 'onReady' + | 'onClose' + export type RequestContextOptions = { defaultStoreValues?: Record - hook?: - | 'onRequest' - | 'preParsing' - | 'preValidation' - | 'preHandler' - | 'preSerialization' - | 'onSend' - | 'onResponse' - | 'onTimeout' - | 'onError' - | 'onRoute' - | 'onRegister' - | 'onReady' - | 'onClose' + hook?: Hook | Hook[] } declare module 'fastify' { diff --git a/test/requestContextPlugin.e2e.spec.js b/test/requestContextPlugin.e2e.spec.js index 047a93c..75cf064 100644 --- a/test/requestContextPlugin.e2e.spec.js +++ b/test/requestContextPlugin.e2e.spec.js @@ -3,33 +3,25 @@ const request = require('superagent') const { fastifyRequestContextPlugin } = require('../lib/requestContextPlugin') const { TestService } = require('./internal/testService') -function initAppPostWithPrevalidation(endpoint) { - return new Promise((resolve) => { - const app = fastify({ logger: true }) - app.register(fastifyRequestContextPlugin, { hook: 'preValidation' }) - - const preValidationFn = (req, reply, done) => { - const requestId = Number.parseInt(req.body.requestId) - req.requestContext.set('testKey', `testValue${requestId}`) - done() - } - - app.route({ - url: '/', - method: ['GET', 'POST'], - preValidation: preValidationFn, - handler: endpoint, - }) - - app.listen(8085, '0.0.0.0', (err, address) => { - if (err) { - console.warn(err) - process.exit(1) - } - console.info(`Server listening at ${address}`) - resolve(app) - }) +async function initAppPostWithPrevalidation(endpoint) { + const app = fastify({ logger: true }) + app.register(fastifyRequestContextPlugin, { hook: 'preValidation' }) + + const preValidationFn = (req, reply, done) => { + const requestId = Number.parseInt(req.body.requestId) + req.requestContext.set('testKey', `testValue${requestId}`) + done() + } + + app.route({ + url: '/', + method: ['GET', 'POST'], + preValidation: preValidationFn, + handler: endpoint, }) + + await app.listen(0) + return app } describe('requestContextPlugin E2E', () => { @@ -73,7 +65,9 @@ describe('requestContextPlugin E2E', () => { initAppPostWithPrevalidation(route).then((_app) => { app = _app testService = new TestService(app) - const response1Promise = request('POST', '0.0.0.0:8085') + const { address, port } = app.server.address() + const url = `${address}:${port}` + const response1Promise = request('POST', url) .send({ requestId: 1 }) .then((response) => { expect(response.body.storedValue).toBe('testValue1') @@ -83,7 +77,7 @@ describe('requestContextPlugin E2E', () => { } }) - const response2Promise = request('POST', '0.0.0.0:8085') + const response2Promise = request('POST', url) .send({ requestId: 2 }) .then((response) => { expect(response.body.storedValue).toBe('testValue2') From c37fe5c7e1eed5d5e2540643c8876a23dc26af8f Mon Sep 17 00:00:00 2001 From: Igor Savin Date: Thu, 13 Aug 2020 21:17:22 +0300 Subject: [PATCH 09/17] Set default to preValidation which actually works, update types, add test that covers multi-phase asyncstorage usage --- README.md | 2 +- index.d.ts | 2 - lib/requestContextPlugin.js | 2 +- test/requestContextPlugin.e2e.spec.js | 115 ++++++++++++++++++++++++++ 4 files changed, 117 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index ae7709d..2ecb1c5 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,7 @@ fastify.register(fastifyRequestContextPlugin, { This plugin accepts options `hook` and `defaultStoreValues`. -* `hook` allows you to specify to which lifecycle hook (or multiple hooks) should request context initialization be bound. Make sure that you are not trying to access the request context earlier in the lifecycle than you initialize it. Default value is `onRequest`. +* `hook` allows you to specify to which lifecycle hook (or multiple hooks) should request context initialization be bound. Make sure that you are not trying to access the request context earlier in the lifecycle than you initialize it. Default value is `preValidation`. * `defaultStoreValues` sets initial values for the store (that can be later overwritten during request execution if needed). This is an optional parameter. From there you can set a context in another hook, route, or method that is within scope. diff --git a/index.d.ts b/index.d.ts index 75dfd51..d1f3c9f 100644 --- a/index.d.ts +++ b/index.d.ts @@ -6,8 +6,6 @@ export type RequestContext = { } export type Hook = - | 'onRequest' - | 'preParsing' | 'preValidation' | 'preHandler' | 'preSerialization' diff --git a/lib/requestContextPlugin.js b/lib/requestContextPlugin.js index c19275b..cfa04bd 100644 --- a/lib/requestContextPlugin.js +++ b/lib/requestContextPlugin.js @@ -10,7 +10,7 @@ function plugin(fastify, opts, next) { fastify.decorate('requestContext', requestContext) fastify.decorateRequest('requestContext', requestContext) - fastify.addHook(opts.hook || 'onRequest', (req, res, done) => { + fastify.addHook(opts.hook || 'preValidation', (req, res, done) => { als.runWith(() => { done() }, opts.defaultStoreValues) diff --git a/test/requestContextPlugin.e2e.spec.js b/test/requestContextPlugin.e2e.spec.js index 75cf064..02dce9f 100644 --- a/test/requestContextPlugin.e2e.spec.js +++ b/test/requestContextPlugin.e2e.spec.js @@ -24,6 +24,43 @@ async function initAppPostWithPrevalidation(endpoint) { return app } +async function initAppPostWithAllPlugins(endpoint, requestHook) { + const app = fastify({ logger: true }) + app.register(fastifyRequestContextPlugin, { hook: requestHook }) + + app.addHook('onRequest', (req, reply, done) => { + req.requestContext.set('onRequest', 'dummy') + done() + }) + + app.addHook('preParsing', (req, reply, payload, done) => { + req.requestContext.set('preParsing', 'dummy') + done(null, payload) + }) + + app.addHook('preValidation', (req, reply, done) => { + const requestId = Number.parseInt(req.body.requestId) + req.requestContext.set('preValidation', requestId) + req.requestContext.set('testKey', `testValue${requestId}`) + done() + }) + + app.addHook('preHandler', (req, reply, done) => { + const requestId = Number.parseInt(req.body.requestId) + req.requestContext.set('preHandler', requestId) + done() + }) + + app.route({ + url: '/', + method: ['GET', 'POST'], + handler: endpoint, + }) + + await app.listen(0) + return app +} + describe('requestContextPlugin E2E', () => { let app afterEach(() => { @@ -97,4 +134,82 @@ describe('requestContextPlugin E2E', () => { return promiseRequest2 }) }) + + it('correctly preserves values set in multiple phases within single POST request', () => { + expect.assertions(10) + + let testService + let responseCounter = 0 + return new Promise((resolveResponsePromise) => { + const promiseRequest2 = new Promise((resolveRequest2Promise) => { + const promiseRequest1 = new Promise((resolveRequest1Promise) => { + const route = (req) => { + const onRequestValue = req.requestContext.get('onRequest') + const preParsingValue = req.requestContext.get('preParsing') + const preValidationValue = req.requestContext.get('preValidation') + const preHandlerValue = req.requestContext.get('preHandler') + + expect(onRequestValue).toBe(undefined) + expect(preParsingValue).toBe(undefined) + expect(preValidationValue).toEqual(expect.any(Number)) + expect(preHandlerValue).toEqual(expect.any(Number)) + + const requestId = `testValue${preHandlerValue}` + + function prepareReply() { + return testService.processRequest(requestId.replace('testValue', '')).then(() => { + const storedValue = req.requestContext.get('preValidation') + return Promise.resolve({ storedValue: `testValue${storedValue}` }) + }) + } + + // We don't want to read values until both requests wrote their values to see if there is a racing condition + if (requestId === 'testValue1') { + resolveRequest1Promise() + return promiseRequest2.then(prepareReply) + } + + if (requestId === 'testValue2') { + resolveRequest2Promise() + return promiseRequest1.then(prepareReply) + } + + throw new Error(`Unexpected requestId: ${requestId}`) + } + + initAppPostWithAllPlugins(route, 'preValidation').then((_app) => { + app = _app + testService = new TestService(app) + const { address, port } = app.server.address() + const url = `${address}:${port}` + const response1Promise = request('POST', url) + .send({ requestId: 1 }) + .then((response) => { + expect(response.body.storedValue).toBe('testValue1') + responseCounter++ + if (responseCounter === 2) { + resolveResponsePromise() + } + }) + + const response2Promise = request('POST', url) + .send({ requestId: 2 }) + .then((response) => { + expect(response.body.storedValue).toBe('testValue2') + responseCounter++ + if (responseCounter === 2) { + resolveResponsePromise() + } + }) + + return Promise.all([response1Promise, response2Promise]) + }) + }) + + return promiseRequest1 + }) + + return promiseRequest2 + }) + }) }) From 965f256030a43f03e2b3917bc568adf313bde9fc Mon Sep 17 00:00:00 2001 From: Igor Savin Date: Thu, 13 Aug 2020 21:45:30 +0300 Subject: [PATCH 10/17] Remove support for multiple hooks, clarify documentation --- README.md | 2 +- index.d.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 2ecb1c5..288acaa 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,7 @@ fastify.register(fastifyRequestContextPlugin, { This plugin accepts options `hook` and `defaultStoreValues`. -* `hook` allows you to specify to which lifecycle hook (or multiple hooks) should request context initialization be bound. Make sure that you are not trying to access the request context earlier in the lifecycle than you initialize it. Default value is `preValidation`. +* `hook` allows you to specify to which lifecycle hook should request context initialization be bound. Note that it should be `preValidation` or later, since context is lost after parsing the request body. Default value is `preValidation`. * `defaultStoreValues` sets initial values for the store (that can be later overwritten during request execution if needed). This is an optional parameter. From there you can set a context in another hook, route, or method that is within scope. diff --git a/index.d.ts b/index.d.ts index d1f3c9f..6f06f0e 100644 --- a/index.d.ts +++ b/index.d.ts @@ -1,4 +1,4 @@ -import Fastify, { FastifyPlugin, FastifyRequest } from 'fastify' +import { FastifyPlugin, FastifyRequest } from 'fastify' export type RequestContext = { get: (key: string) => T | undefined @@ -20,7 +20,7 @@ export type Hook = export type RequestContextOptions = { defaultStoreValues?: Record - hook?: Hook | Hook[] + hook?: Hook } declare module 'fastify' { From 5e19b08dbb0a76579c19b61a1f795e54f66a1a96 Mon Sep 17 00:00:00 2001 From: Igor Savin Date: Fri, 14 Aug 2020 11:18:13 +0300 Subject: [PATCH 11/17] Reduce duplication --- test/internal/appInitializer.js | 82 ++++++++++ test/requestContextPlugin.e2e.spec.js | 73 ++------- test/requestContextPlugin.spec.js | 225 +++++++++++--------------- 3 files changed, 189 insertions(+), 191 deletions(-) create mode 100644 test/internal/appInitializer.js diff --git a/test/internal/appInitializer.js b/test/internal/appInitializer.js new file mode 100644 index 0000000..eeaace6 --- /dev/null +++ b/test/internal/appInitializer.js @@ -0,0 +1,82 @@ +const fastify = require('fastify') +const { fastifyRequestContextPlugin } = require('../../lib/requestContextPlugin') + +function initAppGet(endpoint) { + const app = fastify({ logger: true }) + app.register(fastifyRequestContextPlugin) + + app.get('/', endpoint) + return app +} + +function initAppPost(endpoint) { + const app = fastify({ logger: true }) + app.register(fastifyRequestContextPlugin) + + app.post('/', endpoint) + + return app +} + +function initAppPostWithPrevalidation(endpoint) { + const app = fastify({ logger: true }) + app.register(fastifyRequestContextPlugin, { hook: 'preValidation' }) + + const preValidationFn = (req, reply, done) => { + const requestId = Number.parseInt(req.body.requestId) + req.requestContext.set('testKey', `testValue${requestId}`) + done() + } + + app.route({ + url: '/', + method: ['GET', 'POST'], + preValidation: preValidationFn, + handler: endpoint, + }) + + return app +} + +function initAppPostWithAllPlugins(endpoint, requestHook) { + const app = fastify({ logger: true }) + app.register(fastifyRequestContextPlugin, { hook: requestHook }) + + app.addHook('onRequest', (req, reply, done) => { + req.requestContext.set('onRequest', 'dummy') + done() + }) + + app.addHook('preParsing', (req, reply, payload, done) => { + req.requestContext.set('preParsing', 'dummy') + done(null, payload) + }) + + app.addHook('preValidation', (req, reply, done) => { + const requestId = Number.parseInt(req.body.requestId) + req.requestContext.set('preValidation', requestId) + req.requestContext.set('testKey', `testValue${requestId}`) + done() + }) + + app.addHook('preHandler', (req, reply, done) => { + const requestId = Number.parseInt(req.body.requestId) + req.requestContext.set('preHandler', requestId) + done() + }) + + app.route({ + url: '/', + method: ['GET', 'POST'], + handler: endpoint, + }) + + return app +} + +module.exports = { + initAppPostWithAllPlugins, + initAppPostWithPrevalidation, + initAppPost, + initAppGet, +} diff --git a/test/requestContextPlugin.e2e.spec.js b/test/requestContextPlugin.e2e.spec.js index 02dce9f..49e3e50 100644 --- a/test/requestContextPlugin.e2e.spec.js +++ b/test/requestContextPlugin.e2e.spec.js @@ -1,66 +1,10 @@ -const fastify = require('fastify') const request = require('superagent') -const { fastifyRequestContextPlugin } = require('../lib/requestContextPlugin') +const { + initAppPostWithPrevalidation, + initAppPostWithAllPlugins, +} = require('./internal/appInitializer') const { TestService } = require('./internal/testService') -async function initAppPostWithPrevalidation(endpoint) { - const app = fastify({ logger: true }) - app.register(fastifyRequestContextPlugin, { hook: 'preValidation' }) - - const preValidationFn = (req, reply, done) => { - const requestId = Number.parseInt(req.body.requestId) - req.requestContext.set('testKey', `testValue${requestId}`) - done() - } - - app.route({ - url: '/', - method: ['GET', 'POST'], - preValidation: preValidationFn, - handler: endpoint, - }) - - await app.listen(0) - return app -} - -async function initAppPostWithAllPlugins(endpoint, requestHook) { - const app = fastify({ logger: true }) - app.register(fastifyRequestContextPlugin, { hook: requestHook }) - - app.addHook('onRequest', (req, reply, done) => { - req.requestContext.set('onRequest', 'dummy') - done() - }) - - app.addHook('preParsing', (req, reply, payload, done) => { - req.requestContext.set('preParsing', 'dummy') - done(null, payload) - }) - - app.addHook('preValidation', (req, reply, done) => { - const requestId = Number.parseInt(req.body.requestId) - req.requestContext.set('preValidation', requestId) - req.requestContext.set('testKey', `testValue${requestId}`) - done() - }) - - app.addHook('preHandler', (req, reply, done) => { - const requestId = Number.parseInt(req.body.requestId) - req.requestContext.set('preHandler', requestId) - done() - }) - - app.route({ - url: '/', - method: ['GET', 'POST'], - handler: endpoint, - }) - - await app.listen(0) - return app -} - describe('requestContextPlugin E2E', () => { let app afterEach(() => { @@ -99,8 +43,8 @@ describe('requestContextPlugin E2E', () => { throw new Error(`Unexpected requestId: ${requestId}`) } - initAppPostWithPrevalidation(route).then((_app) => { - app = _app + app = initAppPostWithPrevalidation(route) + app.listen(0).then(() => { testService = new TestService(app) const { address, port } = app.server.address() const url = `${address}:${port}` @@ -177,8 +121,9 @@ describe('requestContextPlugin E2E', () => { throw new Error(`Unexpected requestId: ${requestId}`) } - initAppPostWithAllPlugins(route, 'preValidation').then((_app) => { - app = _app + app = initAppPostWithAllPlugins(route, 'preValidation') + + app.listen(0).then(() => { testService = new TestService(app) const { address, port } = app.server.address() const url = `${address}:${port}` diff --git a/test/requestContextPlugin.spec.js b/test/requestContextPlugin.spec.js index 61f73c1..d9da3d9 100644 --- a/test/requestContextPlugin.spec.js +++ b/test/requestContextPlugin.spec.js @@ -1,45 +1,10 @@ -const fastify = require('fastify') -const { fastifyRequestContextPlugin } = require('../lib/requestContextPlugin') +const { + initAppPost, + initAppPostWithPrevalidation, + initAppGet, +} = require('./internal/appInitializer') const { TestService } = require('./internal/testService') -function initAppGet(endpoint) { - const app = fastify({ logger: true }) - app.register(fastifyRequestContextPlugin) - - app.get('/', endpoint) - - return app.ready() -} - -function initAppPost(endpoint) { - const app = fastify({ logger: true }) - app.register(fastifyRequestContextPlugin) - - app.post('/', endpoint) - - return app.ready() -} - -function initAppPostWithPrevalidation(endpoint) { - const app = fastify({ logger: true }) - app.register(fastifyRequestContextPlugin, { hook: 'preValidation' }) - - const preValidationFn = (req, reply, done) => { - const requestId = Number.parseInt(req.body.requestId) - req.requestContext.set('testKey', `testValue${requestId}`) - done() - } - - app.route({ - url: '/', - method: ['GET', 'POST'], - preValidation: preValidationFn, - handler: endpoint, - }) - - return app.ready() -} - describe('requestContextPlugin', () => { let app afterEach(() => { @@ -79,37 +44,39 @@ describe('requestContextPlugin', () => { } } - initAppGet(route).then((_app) => { - app = _app - testService = new TestService(app) - const response1Promise = app - .inject() - .get('/') - .query({ requestId: 1 }) - .end() - .then((response) => { - expect(response.json().storedValue).toBe('testValue1') - responseCounter++ - if (responseCounter === 2) { - resolveResponsePromise() - } - }) + initAppGet(route) + .ready() + .then((_app) => { + app = _app + testService = new TestService(app) + const response1Promise = app + .inject() + .get('/') + .query({ requestId: 1 }) + .end() + .then((response) => { + expect(response.json().storedValue).toBe('testValue1') + responseCounter++ + if (responseCounter === 2) { + resolveResponsePromise() + } + }) - const response2Promise = app - .inject() - .get('/') - .query({ requestId: 2 }) - .end() - .then((response) => { - expect(response.json().storedValue).toBe('testValue2') - responseCounter++ - if (responseCounter === 2) { - resolveResponsePromise() - } - }) + const response2Promise = app + .inject() + .get('/') + .query({ requestId: 2 }) + .end() + .then((response) => { + expect(response.json().storedValue).toBe('testValue2') + responseCounter++ + if (responseCounter === 2) { + resolveResponsePromise() + } + }) - return Promise.all([response1Promise, response2Promise]) - }) + return Promise.all([response1Promise, response2Promise]) + }) }) return promiseRequest1 @@ -152,37 +119,39 @@ describe('requestContextPlugin', () => { } } - initAppPost(route).then((_app) => { - app = _app - testService = new TestService(app) - const response1Promise = app - .inject() - .post('/') - .body({ requestId: 1 }) - .end() - .then((response) => { - expect(response.json().storedValue).toBe('testValue1') - responseCounter++ - if (responseCounter === 2) { - resolveResponsePromise() - } - }) + initAppPost(route) + .ready() + .then((_app) => { + app = _app + testService = new TestService(app) + const response1Promise = app + .inject() + .post('/') + .body({ requestId: 1 }) + .end() + .then((response) => { + expect(response.json().storedValue).toBe('testValue1') + responseCounter++ + if (responseCounter === 2) { + resolveResponsePromise() + } + }) - const response2Promise = app - .inject() - .post('/') - .body({ requestId: 2 }) - .end() - .then((response) => { - expect(response.json().storedValue).toBe('testValue2') - responseCounter++ - if (responseCounter === 2) { - resolveResponsePromise() - } - }) + const response2Promise = app + .inject() + .post('/') + .body({ requestId: 2 }) + .end() + .then((response) => { + expect(response.json().storedValue).toBe('testValue2') + responseCounter++ + if (responseCounter === 2) { + resolveResponsePromise() + } + }) - return Promise.all([response1Promise, response2Promise]) - }) + return Promise.all([response1Promise, response2Promise]) + }) }) return promiseRequest1 @@ -224,37 +193,39 @@ describe('requestContextPlugin', () => { } } - initAppPostWithPrevalidation(route).then((_app) => { - app = _app - testService = new TestService(app) - const response1Promise = app - .inject() - .post('/') - .body({ requestId: 1 }) - .end() - .then((response) => { - expect(response.json().storedValue).toBe('testValue1') - responseCounter++ - if (responseCounter === 2) { - resolveResponsePromise() - } - }) + initAppPostWithPrevalidation(route) + .ready() + .then((_app) => { + app = _app + testService = new TestService(app) + const response1Promise = app + .inject() + .post('/') + .body({ requestId: 1 }) + .end() + .then((response) => { + expect(response.json().storedValue).toBe('testValue1') + responseCounter++ + if (responseCounter === 2) { + resolveResponsePromise() + } + }) - const response2Promise = app - .inject() - .post('/') - .body({ requestId: 2 }) - .end() - .then((response) => { - expect(response.json().storedValue).toBe('testValue2') - responseCounter++ - if (responseCounter === 2) { - resolveResponsePromise() - } - }) + const response2Promise = app + .inject() + .post('/') + .body({ requestId: 2 }) + .end() + .then((response) => { + expect(response.json().storedValue).toBe('testValue2') + responseCounter++ + if (responseCounter === 2) { + resolveResponsePromise() + } + }) - return Promise.all([response1Promise, response2Promise]) - }) + return Promise.all([response1Promise, response2Promise]) + }) }) return promiseRequest1 From 0be0b99f07b3accbcc75573d1258302dfbb78cdb Mon Sep 17 00:00:00 2001 From: Igor Savin Date: Fri, 14 Aug 2020 11:27:15 +0300 Subject: [PATCH 12/17] Add test case for not losing context after body parsing --- test/requestContextPlugin.e2e.spec.js | 30 +++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/test/requestContextPlugin.e2e.spec.js b/test/requestContextPlugin.e2e.spec.js index 49e3e50..661b4a2 100644 --- a/test/requestContextPlugin.e2e.spec.js +++ b/test/requestContextPlugin.e2e.spec.js @@ -157,4 +157,34 @@ describe('requestContextPlugin E2E', () => { return promiseRequest2 }) }) + + it('does not lose request context after body parsing', () => { + expect.assertions(5) + const route = (req) => { + const onRequestValue = req.requestContext.get('onRequest') + const preParsingValue = req.requestContext.get('preParsing') + const preValidationValue = req.requestContext.get('preValidation') + const preHandlerValue = req.requestContext.get('preHandler') + + expect(onRequestValue).toBe('dummy') + expect(preParsingValue).toBe('dummy') + expect(preValidationValue).toEqual(expect.any(Number)) + expect(preHandlerValue).toEqual(expect.any(Number)) + + const requestId = `testValue${preHandlerValue}` + return Promise.resolve({ storedValue: requestId }) + } + + app = initAppPostWithAllPlugins(route, 'onRequest') + + return app.listen(0).then(() => { + const { address, port } = app.server.address() + const url = `${address}:${port}` + return request('POST', url) + .send({ requestId: 1 }) + .then((response) => { + expect(response.body.storedValue).toBe('testValue1') + }) + }) + }) }) From b7384f07db1f943293cc21669dd7ba8bb77af98b Mon Sep 17 00:00:00 2001 From: Igor Savin Date: Fri, 14 Aug 2020 13:13:16 +0300 Subject: [PATCH 13/17] Monkey-patch event emitter to support all lifecycle phases --- README.md | 2 +- index.d.ts | 2 ++ lib/requestContextPlugin.js | 28 +++++++++++++++++++++++++++- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 288acaa..4ae2526 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,7 @@ fastify.register(fastifyRequestContextPlugin, { This plugin accepts options `hook` and `defaultStoreValues`. -* `hook` allows you to specify to which lifecycle hook should request context initialization be bound. Note that it should be `preValidation` or later, since context is lost after parsing the request body. Default value is `preValidation`. +* `hook` allows you to specify to which lifecycle hook should request context initialization be bound. Note that you need to initialize it on the earliest lifecycle stage that you intend to use it in, or earlier. Default value is `onRequest`. * `defaultStoreValues` sets initial values for the store (that can be later overwritten during request execution if needed). This is an optional parameter. From there you can set a context in another hook, route, or method that is within scope. diff --git a/index.d.ts b/index.d.ts index 6f06f0e..dc42b2e 100644 --- a/index.d.ts +++ b/index.d.ts @@ -6,6 +6,8 @@ export type RequestContext = { } export type Hook = + | 'onRequest' + | 'preParsing' | 'preValidation' | 'preHandler' | 'preSerialization' diff --git a/lib/requestContextPlugin.js b/lib/requestContextPlugin.js index cfa04bd..4fb51e0 100644 --- a/lib/requestContextPlugin.js +++ b/lib/requestContextPlugin.js @@ -6,12 +6,38 @@ const requestContext = { set: als.set, } +/** + * Monkey patches `.emit()` method of the given emitter, so + * that all event listeners are run in scope of the provided + * async resource. + */ +const wrapEmitter = (emitter, asyncResource) => { + const original = emitter.emit + emitter.emit = function (type, ...args) { + return asyncResource.runInAsyncScope(original, emitter, type, ...args) + } +} + +let AsyncResource +const wrapHttpEmitters = (req, res) => { + if (als.storageImplementation === 'AsyncLocalStorage') { + if (!AsyncResource) { + AsyncResource = require('async_hooks').AsyncResource + } + + const asyncResource = new AsyncResource('fastify-request-context') + wrapEmitter(req, asyncResource) + wrapEmitter(res, asyncResource) + } +} + function plugin(fastify, opts, next) { fastify.decorate('requestContext', requestContext) fastify.decorateRequest('requestContext', requestContext) - fastify.addHook(opts.hook || 'preValidation', (req, res, done) => { + fastify.addHook(opts.hook || 'onRequest', (req, res, done) => { als.runWith(() => { + wrapHttpEmitters(req.raw, res.raw) done() }, opts.defaultStoreValues) }) From d859cfa26f55763efecfb201dfb10f0ba4999dbb Mon Sep 17 00:00:00 2001 From: Igor Savin Date: Fri, 14 Aug 2020 13:18:54 +0300 Subject: [PATCH 14/17] Fix Node 10 support --- lib/requestContextPlugin.js | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/lib/requestContextPlugin.js b/lib/requestContextPlugin.js index 4fb51e0..64b2d48 100644 --- a/lib/requestContextPlugin.js +++ b/lib/requestContextPlugin.js @@ -1,5 +1,6 @@ const fp = require('fastify-plugin') const { als } = require('asynchronous-local-storage') +const { AsyncResource } = require('async_hooks') const requestContext = { get: als.get, @@ -18,17 +19,10 @@ const wrapEmitter = (emitter, asyncResource) => { } } -let AsyncResource const wrapHttpEmitters = (req, res) => { - if (als.storageImplementation === 'AsyncLocalStorage') { - if (!AsyncResource) { - AsyncResource = require('async_hooks').AsyncResource - } - - const asyncResource = new AsyncResource('fastify-request-context') - wrapEmitter(req, asyncResource) - wrapEmitter(res, asyncResource) - } + const asyncResource = new AsyncResource('fastify-request-context') + wrapEmitter(req, asyncResource) + wrapEmitter(res, asyncResource) } function plugin(fastify, opts, next) { From 4cedd50de81ecc7531a4b7532b6e88c75fd94698 Mon Sep 17 00:00:00 2001 From: Igor Savin Date: Fri, 14 Aug 2020 16:36:33 +0300 Subject: [PATCH 15/17] Implement alternate solution without monkey-patching --- lib/requestContextPlugin.js | 38 +++++++++++++++++-------------------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/lib/requestContextPlugin.js b/lib/requestContextPlugin.js index 64b2d48..fff7369 100644 --- a/lib/requestContextPlugin.js +++ b/lib/requestContextPlugin.js @@ -7,34 +7,30 @@ const requestContext = { set: als.set, } -/** - * Monkey patches `.emit()` method of the given emitter, so - * that all event listeners are run in scope of the provided - * async resource. - */ -const wrapEmitter = (emitter, asyncResource) => { - const original = emitter.emit - emitter.emit = function (type, ...args) { - return asyncResource.runInAsyncScope(original, emitter, type, ...args) - } -} - -const wrapHttpEmitters = (req, res) => { - const asyncResource = new AsyncResource('fastify-request-context') - wrapEmitter(req, asyncResource) - wrapEmitter(res, asyncResource) -} - function plugin(fastify, opts, next) { fastify.decorate('requestContext', requestContext) fastify.decorateRequest('requestContext', requestContext) + const hook = opts.hook || 'onRequest' - fastify.addHook(opts.hook || 'onRequest', (req, res, done) => { + fastify.addHook(hook, (req, res, done) => { als.runWith(() => { - wrapHttpEmitters(req.raw, res.raw) - done() + const asyncResource = new AsyncResource('fastify-request-context') + req.asyncResource = asyncResource + asyncResource.runInAsyncScope(done, req.raw) }, opts.defaultStoreValues) }) + + // Both of onRequest and preParsing are executed after the als.runWith call within the "proper" async context (AsyncResource implicitly created by ALS). + // However, preValidation, preHandler and the route handler are executed as a part of req.emit('end') call which happens + // in a different async context, as req/res may emit events in a different context. + // Related to https://github.com/nodejs/node/issues/34430 and https://github.com/nodejs/node/issues/33723 + if (hook === 'onRequest' || hook === 'preParsing') { + fastify.addHook('preValidation', (req, res, done) => { + const asyncResource = req.asyncResource + asyncResource.runInAsyncScope(done, req.raw) + }) + } + next() } From ecaea79e36a59416a00cf79c25eff7b8cb5f3f33 Mon Sep 17 00:00:00 2001 From: Igor Savin Date: Fri, 14 Aug 2020 16:43:51 +0300 Subject: [PATCH 16/17] Extend test to also cover preSerialization hook --- .eslintrc.json | 2 +- test/internal/appInitializer.js | 9 +++++++++ test/requestContextPlugin.e2e.spec.js | 4 +++- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/.eslintrc.json b/.eslintrc.json index a3dd428..d87010f 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -12,7 +12,7 @@ "prettier" ], "parserOptions": { - "ecmaVersion": 2017, + "ecmaVersion": 2018, "sourceType": "module" }, "rules": { diff --git a/test/internal/appInitializer.js b/test/internal/appInitializer.js index eeaace6..0eab6bf 100644 --- a/test/internal/appInitializer.js +++ b/test/internal/appInitializer.js @@ -65,6 +65,15 @@ function initAppPostWithAllPlugins(endpoint, requestHook) { done() }) + app.addHook('preSerialization', (req, reply, payload, done) => { + const onRequestValue = req.requestContext.get('onRequest') + const preValidationValue = req.requestContext.get('preValidation') + done(null, { + ...payload, + preSerialization1: onRequestValue, + preSerialization2: preValidationValue, + }) + }) app.route({ url: '/', method: ['GET', 'POST'], diff --git a/test/requestContextPlugin.e2e.spec.js b/test/requestContextPlugin.e2e.spec.js index 661b4a2..199fcc0 100644 --- a/test/requestContextPlugin.e2e.spec.js +++ b/test/requestContextPlugin.e2e.spec.js @@ -159,7 +159,7 @@ describe('requestContextPlugin E2E', () => { }) it('does not lose request context after body parsing', () => { - expect.assertions(5) + expect.assertions(7) const route = (req) => { const onRequestValue = req.requestContext.get('onRequest') const preParsingValue = req.requestContext.get('preParsing') @@ -184,6 +184,8 @@ describe('requestContextPlugin E2E', () => { .send({ requestId: 1 }) .then((response) => { expect(response.body.storedValue).toBe('testValue1') + expect(response.body.preSerialization1).toBe('dummy') + expect(response.body.preSerialization2).toBe(1) }) }) }) From 457f7ac246b66f2ef894279df1a468428d3b01d3 Mon Sep 17 00:00:00 2001 From: Igor Savin Date: Fri, 14 Aug 2020 17:38:18 +0300 Subject: [PATCH 17/17] Use symbol for asyncResource --- lib/requestContextPlugin.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/requestContextPlugin.js b/lib/requestContextPlugin.js index fff7369..523ef60 100644 --- a/lib/requestContextPlugin.js +++ b/lib/requestContextPlugin.js @@ -1,6 +1,7 @@ const fp = require('fastify-plugin') const { als } = require('asynchronous-local-storage') const { AsyncResource } = require('async_hooks') +const asyncResourceSymbol = Symbol('asyncResource') const requestContext = { get: als.get, @@ -10,12 +11,13 @@ const requestContext = { function plugin(fastify, opts, next) { fastify.decorate('requestContext', requestContext) fastify.decorateRequest('requestContext', requestContext) + fastify.decorateRequest(asyncResourceSymbol, null) const hook = opts.hook || 'onRequest' fastify.addHook(hook, (req, res, done) => { als.runWith(() => { const asyncResource = new AsyncResource('fastify-request-context') - req.asyncResource = asyncResource + req[asyncResourceSymbol] = asyncResource asyncResource.runInAsyncScope(done, req.raw) }, opts.defaultStoreValues) }) @@ -26,7 +28,7 @@ function plugin(fastify, opts, next) { // Related to https://github.com/nodejs/node/issues/34430 and https://github.com/nodejs/node/issues/33723 if (hook === 'onRequest' || hook === 'preParsing') { fastify.addHook('preValidation', (req, res, done) => { - const asyncResource = req.asyncResource + const asyncResource = req[asyncResourceSymbol] asyncResource.runInAsyncScope(done, req.raw) }) }