Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: multiple route same schema #2108

Merged
merged 2 commits into from
Feb 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
39 changes: 22 additions & 17 deletions lib/schemas.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict'

const fastClone = require('rfdc')({ circles: false, proto: true })
const { kSchemaVisited } = require('./symbols')
const kFluentSchema = Symbol.for('fluent-schema-object')

const {
Expand Down Expand Up @@ -43,9 +44,17 @@ Schemas.prototype.resolve = function (id) {
}

Schemas.prototype.resolveRefs = function (routeSchemas, dontClearId, refResolver) {
// check if our schema has both querystring and query
if (routeSchemas.query && routeSchemas.querystring) {
throw new FST_ERR_SCH_DUPLICATE('querystring')
if (routeSchemas[kSchemaVisited]) {
return routeSchemas
}

// alias query to querystring schema
if (routeSchemas.query) {
// check if our schema has both querystring and query
if (routeSchemas.querystring) {
throw new FST_ERR_SCH_DUPLICATE('querystring')
}
routeSchemas.querystring = routeSchemas.query
}

// let's check if our schemas have a custom prototype
Expand All @@ -58,22 +67,17 @@ Schemas.prototype.resolveRefs = function (routeSchemas, dontClearId, refResolver
// See issue https://github.com/fastify/fastify/issues/1767
const cachedSchema = Object.assign({}, routeSchemas)

// alias query to querystring schema
if (routeSchemas.query) {
cachedSchema.querystring = routeSchemas.query
}

try {
// this will work only for standard json schemas
// other compilers such as Joi will fail
this.traverse(cachedSchema, refResolver)
this.traverse(routeSchemas, refResolver)

// when a plugin uses the 'skip-override' and call addSchema
// the same JSON will be pass throug all the avvio tree. In this case
// it is not possible clean the id. The id will be cleared
// in the startup phase by the call of validation.js. Details PR #1496
if (dontClearId !== true) {
this.cleanId(cachedSchema)
this.cleanId(routeSchemas)
}
} catch (err) {
// if we have failed because `resolve` has thrown
Expand All @@ -85,19 +89,20 @@ Schemas.prototype.resolveRefs = function (routeSchemas, dontClearId, refResolver
return cachedSchema
}

if (cachedSchema.headers) {
cachedSchema.headers = this.getSchemaAnyway(cachedSchema.headers)
if (routeSchemas.headers) {
routeSchemas.headers = this.getSchemaAnyway(routeSchemas.headers)
}

if (cachedSchema.querystring) {
cachedSchema.querystring = this.getSchemaAnyway(cachedSchema.querystring)
if (routeSchemas.querystring) {
routeSchemas.querystring = this.getSchemaAnyway(routeSchemas.querystring)
}

if (cachedSchema.params) {
cachedSchema.params = this.getSchemaAnyway(cachedSchema.params)
if (routeSchemas.params) {
routeSchemas.params = this.getSchemaAnyway(routeSchemas.params)
}

return cachedSchema
routeSchemas[kSchemaVisited] = true
return routeSchemas
}

Schemas.prototype.traverse = function (schema, refResolver) {
Expand Down
3 changes: 2 additions & 1 deletion lib/symbols.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ const keys = {
kOptions: Symbol('fastify.options'),
kGlobalHooks: Symbol('fastify.globalHooks'),
kDisableRequestLogging: Symbol('fastify.disableRequestLogging'),
kPluginNameChain: Symbol('fastify.pluginNameChain')
kPluginNameChain: Symbol('fastify.pluginNameChain'),
kSchemaVisited: Symbol('fastify.schemaVisited')
}

module.exports = keys
33 changes: 33 additions & 0 deletions test/hooks.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2783,3 +2783,36 @@ if (semver.gt(process.versions.node, '8.0.0')) {
t.pass('Skip because Node version < 8')
t.end()
}

test('onRoute should have compiled schemas - replace-way', t => {
t.plan(2)
const fastify = Fastify()

let routeConfigLink
fastify.addHook('onRoute', function (route) {
routeConfigLink = route
})

fastify.addSchema({
$id: 'mySchema',
type: 'object',
properties: {
host: { type: 'number' }
}
})

fastify.get('/', {
schema: { query: 'mySchema#' }
}, (req, reply) => { reply.send('hello') })

fastify.ready((err) => {
t.error(err)

t.deepEqual(routeConfigLink.schema.query, {
type: 'object',
properties: {
host: { type: 'number' }
}
})
})
})
8 changes: 3 additions & 5 deletions test/route.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -320,13 +320,11 @@ test('multiple routes with one schema', t => {
}
})

fastify.listen(0, error => {
fastify.ready(error => {
t.error(error)
t.strictSame(schema, {
query: {
id: { type: 'number' }
}
query: { id: { type: 'number' } },
querystring: { type: 'object', properties: { id: { type: 'number' } } }
})
fastify.server.unref()
})
})