Skip to content

Commit

Permalink
Skip encapsulation of Request and Reply when not needed (#3479)
Browse files Browse the repository at this point in the history
* Skip encapsulation of Request and Reply when not needed

* Update test/decorator.test.js

Co-authored-by: Tomas Della Vedova <delvedor@users.noreply.github.com>

Co-authored-by: Tomas Della Vedova <delvedor@users.noreply.github.com>
  • Loading branch information
mcollina and delvedor committed Nov 24, 2021
1 parent fd8503a commit 94aab54
Show file tree
Hide file tree
Showing 11 changed files with 54 additions and 25 deletions.
3 changes: 0 additions & 3 deletions fastify.js
Original file line number Diff line number Diff line change
Expand Up @@ -296,9 +296,6 @@ function fastify (options) {
initialConfig
}

fastify[kReply].prototype.server = fastify
fastify[kRequest].prototype.server = fastify

Object.defineProperties(fastify, {
pluginName: {
get () {
Expand Down
4 changes: 3 additions & 1 deletion lib/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const { kFourOhFourContext, kReplySerializerDefault } = require('./symbols.js')

// Objects that holds the context of every request
// Every route holds an instance of this object.
function Context (schema, handler, Reply, Request, contentTypeParser, config, errorHandler, bodyLimit, logLevel, logSerializers, attachValidation, replySerializer, schemaErrorFormatter) {
function Context (schema, handler, Reply, Request, contentTypeParser, config, errorHandler, bodyLimit, logLevel, logSerializers, attachValidation, replySerializer, schemaErrorFormatter, server) {
this.schema = schema
this.handler = handler
this.Reply = Reply
Expand All @@ -26,6 +26,8 @@ function Context (schema, handler, Reply, Request, contentTypeParser, config, er
this.attachValidation = attachValidation
this[kReplySerializerDefault] = replySerializer
this.schemaErrorFormatter = schemaErrorFormatter || defaultSchemaErrorFormatter
// TODO simplify the arguments, we can pass way less arguments if we pass server
this.server = server
}

function defaultSchemaErrorFormatter (errors, dataVar) {
Expand Down
5 changes: 4 additions & 1 deletion lib/decorate.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
const {
kReply,
kRequest,
kState
kState,
kHasBeenDecorated
} = require('./symbols.js')

const {
Expand Down Expand Up @@ -40,6 +41,8 @@ function decorateConstructor (konstructor, name, fn, dependencies) {
throw new FST_ERR_DEC_ALREADY_PRESENT(name)
}

konstructor[kHasBeenDecorated] = true

checkDependencies(instance, name, dependencies)

if (fn && (typeof fn.getter === 'function' || typeof fn.setter === 'function')) {
Expand Down
3 changes: 0 additions & 3 deletions lib/pluginOverride.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,7 @@ module.exports = function override (old, fn, opts) {
instance[kChildren] = []

instance[kReply] = Reply.buildReply(instance[kReply])
instance[kReply].prototype.server = instance

instance[kRequest] = Request.buildRequest(instance[kRequest])
instance[kRequest].prototype.server = instance

instance[kContentTypeParser] = ContentTypeParser.helpers.buildContentTypeParser(instance[kContentTypeParser])
instance[kHooks] = buildHooks(instance[kHooks])
Expand Down
13 changes: 8 additions & 5 deletions lib/reply.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ Object.defineProperties(Reply.prototype, {
return this.request.context
}
},
server: {
get () {
return this.request.context.server
}
},
sent: {
enumerable: true,
get () {
Expand Down Expand Up @@ -94,10 +99,6 @@ Object.defineProperties(Reply.prototype, {
set (value) {
this.code(value)
}
},
server: {
value: null,
writable: true
}
})

Expand Down Expand Up @@ -589,7 +590,9 @@ function buildReply (R) {
this[prop.key] = prop.value
}
}
_Reply.prototype = new R()
Object.setPrototypeOf(_Reply.prototype, R.prototype)
Object.setPrototypeOf(_Reply, R)
_Reply.parent = R
_Reply.props = props
return _Reply
}
Expand Down
19 changes: 14 additions & 5 deletions lib/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
const proxyAddr = require('proxy-addr')
const semver = require('semver')
const warning = require('./warnings')
const {
kHasBeenDecorated
} = require('./symbols')

function Request (id, params, req, query, log, context) {
this.id = id
Expand Down Expand Up @@ -62,8 +65,10 @@ function buildRegularRequest (R) {
this[prop.key] = prop.value
}
}
_Request.prototype = new R()
Object.setPrototypeOf(_Request.prototype, Request.prototype)
Object.setPrototypeOf(_Request, Request)
_Request.props = props
_Request.parent = R

return _Request
}
Expand All @@ -78,6 +83,9 @@ function buildRequestWithTrustProxy (R, trustProxy) {
const _Request = buildRegularRequest(R)
const proxyFn = getTrustProxyFn(trustProxy)

// This is a more optimized version of decoration
_Request[kHasBeenDecorated] = true

Object.defineProperties(_Request.prototype, {
ip: {
get () {
Expand Down Expand Up @@ -113,6 +121,11 @@ function buildRequestWithTrustProxy (R, trustProxy) {
}

Object.defineProperties(Request.prototype, {
server: {
get () {
return this.context.server
}
},
url: {
get () {
return this.raw.url
Expand Down Expand Up @@ -181,10 +194,6 @@ Object.defineProperties(Request.prototype, {
set (headers) {
this.additionalHeaders = headers
}
},
server: {
value: null,
writable: true
}
})

Expand Down
21 changes: 19 additions & 2 deletions lib/route.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ const {
kRequestPayloadStream,
kDisableRequestLogging,
kSchemaErrorFormatter,
kErrorHandler
kErrorHandler,
kHasBeenDecorated
} = require('./symbols.js')
const { buildErrorHandler } = require('./error-handler')

Expand Down Expand Up @@ -216,7 +217,15 @@ function buildRouting (options) {
this[kReply],
this[kRequest],
this[kContentTypeParser],
config
config,
opts.errorHandler || this[kErrorHandler],
opts.bodyLimit,
opts.logLevel,
opts.logSerializers,
opts.attachValidation,
this[kReplySerializerDefault],
opts.schemaErrorFormatter || this[kSchemaErrorFormatter],
this
)

if (opts.version) {
Expand Down Expand Up @@ -259,6 +268,14 @@ function buildRouting (options) {
context[hook] = toSet.length ? toSet : null
}

// Optimization: avoid encapsulation if no decoration has been done.
while (!context.Request[kHasBeenDecorated] && context.Request.parent) {
context.Request = context.Request.parent
}
while (!context.Reply[kHasBeenDecorated] && context.Reply.parent) {
context.Reply = context.Reply.parent
}

// Must store the 404 Context in 'preReady' because it is only guaranteed to
// be available after all of the plugins and routes have been loaded.
fourOhFour.setContext(this, context)
Expand Down
3 changes: 2 additions & 1 deletion lib/symbols.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ const keys = {
kPluginNameChain: Symbol('fastify.pluginNameChain'),
// This symbol is only meant to be used for fastify tests and should not be used for any other purpose
kTestInternals: Symbol('fastify.testInternals'),
kErrorHandler: Symbol('fastify.errorHandler')
kErrorHandler: Symbol('fastify.errorHandler'),
kHasBeenDecorated: Symbol('fastify.hasBeenDecorated')
}

module.exports = keys
4 changes: 2 additions & 2 deletions test/500s.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ test('custom 500', t => {

fastify.setErrorHandler(function (err, request, reply) {
t.type(request, 'object')
t.type(request, fastify[symbols.kRequest])
t.type(request, fastify[symbols.kRequest].parent)
reply
.code(500)
.type('text/plain')
Expand Down Expand Up @@ -74,7 +74,7 @@ test('encapsulated 500', t => {

f.setErrorHandler(function (err, request, reply) {
t.type(request, 'object')
t.type(request, f[symbols.kRequest])
t.type(request, fastify[symbols.kRequest].parent)
reply
.code(500)
.type('text/plain')
Expand Down
2 changes: 1 addition & 1 deletion test/helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ module.exports.payloadMethod = function (method, t, isSetErrorHandler = false) {
if (isSetErrorHandler) {
fastify.setErrorHandler(function (err, request, reply) {
t.type(request, 'object')
t.type(request, fastify[symbols.kRequest])
t.type(request, fastify[symbols.kRequest].parent)
reply
.code(err.statusCode)
.type('application/json; charset=utf-8')
Expand Down
2 changes: 1 addition & 1 deletion test/request-error.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ test('default 400 on request error with custom error handler', t => {

fastify.setErrorHandler(function (err, request, reply) {
t.type(request, 'object')
t.type(request, fastify[kRequest])
t.type(request, fastify[kRequest].parent)
reply
.code(err.statusCode)
.type('application/json; charset=utf-8')
Expand Down

0 comments on commit 94aab54

Please sign in to comment.