Skip to content

Commit

Permalink
feat: request and reply reference type encapsulated
Browse files Browse the repository at this point in the history
  • Loading branch information
Eomm committed May 12, 2024
1 parent 8817f52 commit 47d294a
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 38 deletions.
2 changes: 0 additions & 2 deletions docs/Reference/Warnings.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
- [FSTWRN002](#FSTWRN002)
- [Fastify Deprecation Codes](#fastify-deprecation-codes)
- [FSTDEP005](#FSTDEP005)
- [FSTDEP006](#FSTDEP006)
- [FSTDEP007](#FSTDEP007)
- [FSTDEP008](#FSTDEP008)
- [FSTDEP009](#FSTDEP009)
Expand Down Expand Up @@ -73,7 +72,6 @@ Deprecation codes are further supported by the Node.js CLI options:
| Code | Description | How to solve | Discussion |
| ---- | ----------- | ------------ | ---------- |
| <a id="FSTDEP005">FSTDEP005</a> | You are accessing the deprecated `request.connection` property. | Use `request.socket`. | [#2594](https://github.com/fastify/fastify/pull/2594) |
| <a id="FSTDEP006">FSTDEP006</a> | You are decorating Request/Reply with a reference type. This reference is shared amongst all requests. | Do not use Arrays/Objects as values when decorating Request/Reply. | [#2688](https://github.com/fastify/fastify/pull/2688) |
| <a id="FSTDEP007">FSTDEP007</a> | You are trying to set a HEAD route using `exposeHeadRoute` route flag when a sibling route is already set. | Remove `exposeHeadRoutes` or explicitly set `exposeHeadRoutes` to `false` | [#2700](https://github.com/fastify/fastify/pull/2700) |
| <a id="FSTDEP008">FSTDEP008</a> | You are using route constraints via the route `{version: "..."}` option. | Use `{constraints: {version: "..."}}` option. | [#2682](https://github.com/fastify/fastify/pull/2682) |
| <a id="FSTDEP009">FSTDEP009</a> | You are using a custom route versioning strategy via the server `{versioning: "..."}` option. | Use `{constraints: {version: "..."}}` option. | [#2682](https://github.com/fastify/fastify/pull/2682) |
Expand Down
26 changes: 18 additions & 8 deletions lib/decorate.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
'use strict'

const deepClone = require('rfdc')({ circles: true, proto: false })

/* eslint no-prototype-builtins: 0 */

const {
Expand All @@ -16,8 +18,6 @@ const {
FST_ERR_DEC_DEPENDENCY_INVALID_TYPE
} = require('./errors')

const { FSTDEP006 } = require('./warnings')

function decorate (instance, name, fn, dependencies) {
if (Object.prototype.hasOwnProperty.call(instance, name)) {
throw new FST_ERR_DEC_ALREADY_PRESENT(name)
Expand Down Expand Up @@ -56,9 +56,19 @@ function decorateConstructor (konstructor, name, fn, dependencies) {
}
}

function checkReferenceType (name, fn) {
function encapsulateReferenceType (name, fn) {
if (typeof fn === 'object' && fn && !(typeof fn.getter === 'function' || typeof fn.setter === 'function')) {
FSTDEP006(name)
return encapsulatedDecorator(fn)
}

return fn
}

function encapsulatedDecorator (originalValue) {
return {
getter: function getter () {
return deepClone(originalValue)
}
}
}

Expand Down Expand Up @@ -112,15 +122,15 @@ function checkDependencies (instance, name, deps) {

function decorateReply (name, fn, dependencies) {
assertNotStarted(this, name)
checkReferenceType(name, fn)
decorateConstructor(this[kReply], name, fn, dependencies)
const decorator = encapsulateReferenceType(name, fn)
decorateConstructor(this[kReply], name, decorator, dependencies)
return this
}

function decorateRequest (name, fn, dependencies) {
assertNotStarted(this, name)
checkReferenceType(name, fn)
decorateConstructor(this[kRequest], name, fn, dependencies)
const decorator = encapsulateReferenceType(name, fn)
decorateConstructor(this[kRequest], name, decorator, dependencies)
return this
}

Expand Down
7 changes: 0 additions & 7 deletions lib/warnings.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ const { createDeprecation, createWarning } = require('process-warning')
/**
* Deprecation codes:
* - FSTDEP005
* - FSTDEP006
* - FSTDEP007
* - FSTDEP008
* - FSTDEP009
Expand All @@ -26,11 +25,6 @@ const FSTDEP005 = createDeprecation({
message: 'You are accessing the deprecated "request.connection" property. Use "request.socket" instead.'
})

const FSTDEP006 = createDeprecation({
code: 'FSTDEP006',
message: 'You are decorating Request/Reply with a reference type. This reference is shared amongst all requests. Use onRequest hook instead. Property: %s'
})

const FSTDEP007 = createDeprecation({
code: 'FSTDEP007',
message: 'You are trying to set a HEAD route using "exposeHeadRoute" route flag when a sibling route is already set. See documentation for more info.'
Expand Down Expand Up @@ -107,7 +101,6 @@ const FSTSEC001 = createWarning({

module.exports = {
FSTDEP005,
FSTDEP006,
FSTDEP007,
FSTDEP008,
FSTDEP009,
Expand Down
70 changes: 49 additions & 21 deletions test/decorator.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -789,36 +789,64 @@ test('decorate* should throw if called after ready', async t => {
await fastify.close()
})

test('decorate* should emit warning if an array is passed', t => {
t.plan(1)
test('decorate* should encapsulate the value if an array is passed', async t => {
t.plan(6)

function onWarning (name) {
t.equal(name, 'test_array')
}
const fastify = Fastify()
fastify.decorateRequest('test_array', [])
fastify.decorateReply('test_array', [])

const decorate = proxyquire('../lib/decorate', {
'./warnings': {
FSTDEP006: onWarning
}
fastify.get('/', async function (req, reply) {
t.same(req.test_array, [])
t.same(reply.test_array, [])

req.test_array.push(1)
reply.test_array.push(2)
})
const fastify = proxyquire('..', { './lib/decorate.js': decorate })()
fastify.decorateRequest('test_array', [])

await fastify.inject('/')
await fastify.inject('/')
await fastify.inject('/')
})

test('decorate* should emit warning if object type is passed', t => {
t.plan(1)
test('decorate* should encapsulate the value if an array is passed (deep clone)', async t => {
t.plan(6)

function onWarning (name) {
t.equal(name, 'test_object')
}
const fastify = Fastify()
fastify.decorateRequest('test_array', [{ a: { b: { c: 1 } } }])
fastify.decorateReply('test_array', [{ a: { b: { c: 1 } } }])

const decorate = proxyquire('../lib/decorate', {
'./warnings': {
FSTDEP006: onWarning
}
fastify.get('/', async function (req, reply) {
t.same(req.test_array, [{ a: { b: { c: 1 } } }])
t.same(reply.test_array, [{ a: { b: { c: 1 } } }])

req.test_array[0].a.b.c = 99
reply.test_array[0].a.b.c = 99
})
const fastify = proxyquire('..', { './lib/decorate.js': decorate })()

await fastify.inject('/')
await fastify.inject('/')
await fastify.inject('/')
})

test('decorate* should encapsulate the value if object type is passed', { only: true }, async t => {
t.plan(6)

const fastify = Fastify()
fastify.decorateRequest('test_object', { foo: 'bar' })
fastify.decorateReply('test_object', { foo: 'bar' })

fastify.get('/', async function (req, reply) {
t.same(req.test_object, { foo: 'bar' })
t.same(reply.test_object, { foo: 'bar' })

req.test_object.user = 'user'
reply.test_array.user = 'user'
})

await fastify.inject('/')
await fastify.inject('/')
await fastify.inject('/')
})

test('decorate* should not emit warning if object with getter/setter is passed', t => {
Expand Down

0 comments on commit 47d294a

Please sign in to comment.