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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

$ref support - OAS 2.0 compliant #239

Merged
merged 16 commits into from
May 24, 2020
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,10 @@ npm run prepare

So that [swagger-ui](https://github.com/swagger-api/swagger-ui) static folder will be generated for you.

#### How work under the hood

`fastify-static` serve the `swagger-ui` static files, then it calls `/docs/json` to get the swagger file and render it.

<a name="seealso"></a>
## See also
Sometimes you already have a Swagger definition and you need to build Fastify routes from that.
Expand Down
222 changes: 142 additions & 80 deletions dynamic.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,31 @@
const fs = require('fs')
const path = require('path')
const yaml = require('js-yaml')
const Ref = require('json-schema-resolver')

module.exports = function (fastify, opts, next) {
fastify.decorate('swagger', swagger)

const routes = []
const sharedSchemasMap = new Map()
let ref

fastify.addHook('onRoute', (routeOptions) => {
routes.push(routeOptions)
})

fastify.addHook('onRegister', async (instance) => {
instance.addHook('onReady', (done) => {
Eomm marked this conversation as resolved.
Show resolved Hide resolved
const allSchemas = instance.getSchemas()
for (const schemaId of Object.keys(allSchemas)) {
if (!sharedSchemasMap.has(schemaId)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we detect if the id is being overridden by something else?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe here we can add instance prefix to schema name if it exists? In some cases I guess it could solve some schema conflicts.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem here is that onRegister is receiving for all the register so in this case:

  • register root - addSchema 1
    • child 1 registered - addSchema 2
      • subchild 2 registered - addSchema 3

For subchild 2 i will read schema 1, 2, 3
For child 1 schema 1 and 2
For root only schema 1

So it will be quite normal to have duplicate and I think it is ok since the addSchema already has a check to verify duplicate $id

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean slightly different case.

  • root - schema-1
    • child1 (prefix '/news') - item
    • child2 (prefix '/comments') - item

So if item schemas are registered in isolated scopes they will know now nothing about each other. However when you gather all them in shared schema map they will have the same id.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you leave a TODO comment in here? Or open an issue? I think we might want/be able to have a way to get all schemas defined at the current level so you do not get them all here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree and I will describe the case 👍

sharedSchemasMap.set(schemaId, allSchemas[schemaId])
}
}
done()
})
})

opts = opts || {}

opts.swagger = opts.swagger || {}
Expand Down Expand Up @@ -74,6 +89,8 @@ module.exports = function (fastify, opts, next) {
if (consumes) swaggerObject.consumes = consumes
if (produces) swaggerObject.produces = produces
if (definitions) swaggerObject.definitions = definitions
else swaggerObject.definitions = {}

if (securityDefinitions) {
swaggerObject.securityDefinitions = securityDefinitions
}
Expand All @@ -87,6 +104,22 @@ module.exports = function (fastify, opts, next) {
swaggerObject.externalDocs = externalDocs
}

if (!ref) {
const externalSchemas = Array.from(sharedSchemasMap.values())

ref = Ref({ clone: true, applicationUri: 'todo.com', externalSchemas })
swaggerObject.definitions = {
...swaggerObject.definitions,
...(ref.definitions().definitions)
}

// Swagger doesn't accept $id on /definitions schemas.
// The $ids are needed by Ref() to check the URI so we need
// to remove them at the end of the process
Object.values(swaggerObject.definitions)
.forEach(_ => { delete _.$id })
}

swaggerObject.paths = {}
for (var route of routes) {
if (route.schema && route.schema.hide) {
Expand Down Expand Up @@ -190,6 +223,40 @@ module.exports = function (fastify, opts, next) {

cache.swaggerObject = swaggerObject
return swaggerObject

function getBodyParams (parameters, body) {
const bodyResolved = ref.resolve(body)

const param = {}
param.name = 'body'
param.in = 'body'
param.schema = bodyResolved
parameters.push(param)
}

function getFormParams (parameters, form) {
const resolved = ref.resolve(form)
const add = plainJsonObjectToSwagger2('formData', resolved, swaggerObject.definitions)
add.forEach(_ => parameters.push(_))
}

function getQueryParams (parameters, query) {
const resolved = ref.resolve(query)
const add = plainJsonObjectToSwagger2('query', resolved, swaggerObject.definitions)
add.forEach(_ => parameters.push(_))
}

function getPathParams (parameters, path) {
const resolved = ref.resolve(path)
const add = plainJsonObjectToSwagger2('path', resolved, swaggerObject.definitions)
add.forEach(_ => parameters.push(_))
}

function getHeaderParams (parameters, headers) {
const resolved = ref.resolve(headers)
const add = plainJsonObjectToSwagger2('header', resolved, swaggerObject.definitions)
add.forEach(_ => parameters.push(_))
}
}

next()
Expand All @@ -205,86 +272,6 @@ function consumesFormOnly (schema) {
)
}

function getQueryParams (parameters, query) {
if (query.type && query.properties) {
// for the shorthand querystring declaration
const queryProperties = Object.keys(query.properties).reduce((acc, h) => {
const required = (query.required && query.required.indexOf(h) >= 0) || false
const newProps = Object.assign({}, query.properties[h], { required })
return Object.assign({}, acc, { [h]: newProps })
}, {})

return getQueryParams(parameters, queryProperties)
}

Object.keys(query).forEach(prop => {
const obj = query[prop]
const param = obj
param.name = prop
param.in = 'query'
parameters.push(param)
})
}

function getBodyParams (parameters, body) {
const param = {}
param.name = 'body'
param.in = 'body'
param.schema = body
parameters.push(param)
}

function getFormParams (parameters, body) {
const formParamsSchema = body.properties
if (formParamsSchema) {
Object.keys(formParamsSchema).forEach(name => {
const param = formParamsSchema[name]
delete param.$id
param.in = 'formData'
param.name = name
parameters.push(param)
})
}
}

function getPathParams (parameters, params) {
if (params.type && params.properties) {
// for the shorthand querystring declaration
return getPathParams(parameters, params.properties)
}

Object.keys(params).forEach(p => {
const param = Object.assign({}, params[p])
param.name = p
param.in = 'path'
param.required = true
parameters.push(param)
})
}

function getHeaderParams (parameters, headers) {
if (headers.type && headers.properties) {
// for the shorthand querystring declaration
const headerProperties = Object.keys(headers.properties).reduce((acc, h) => {
const required = (headers.required && headers.required.indexOf(h) >= 0) || false
const newProps = Object.assign({}, headers.properties[h], { required })
return Object.assign({}, acc, { [h]: newProps })
}, {})

return getHeaderParams(parameters, headerProperties)
}

Object.keys(headers).forEach(h =>
parameters.push({
name: h,
in: 'header',
required: headers[h].required,
description: headers[h].description,
type: headers[h].type
})
)
}

function genResponse (response) {
// if the user does not provided an out schema
if (!response) {
Expand Down Expand Up @@ -330,3 +317,78 @@ function formatParamUrl (url) {
return formatParamUrl(url.slice(0, start) + '{' + url.slice(++start, end) + '}' + url.slice(end))
}
}

// For supported keys read:
// https://swagger.io/docs/specification/2-0/describing-parameters/
function plainJsonObjectToSwagger2 (container, jsonSchema, externalSchemas) {
const obj = localRefResolve(jsonSchema, externalSchemas)
let toSwaggerProp
switch (container) {
case 'query':
toSwaggerProp = function (properyName, jsonSchemaElement) {
jsonSchemaElement.in = container
jsonSchemaElement.name = properyName
return jsonSchemaElement
}
break
case 'formData':
toSwaggerProp = function (properyName, jsonSchemaElement) {
delete jsonSchemaElement.$id
jsonSchemaElement.in = container
jsonSchemaElement.name = properyName

// https://json-schema.org/understanding-json-schema/reference/non_json_data.html#contentencoding
if (jsonSchemaElement.contentEncoding === 'binary') {
delete jsonSchemaElement.contentEncoding // Must be removed
jsonSchemaElement.type = 'file'
}

return jsonSchemaElement
}
break
case 'path':
toSwaggerProp = function (properyName, jsonSchemaElement) {
jsonSchemaElement.in = container
jsonSchemaElement.name = properyName
jsonSchemaElement.required = true
return jsonSchemaElement
}
break
case 'header':
toSwaggerProp = function (properyName, jsonSchemaElement) {
return {
in: 'header',
name: properyName,
required: jsonSchemaElement.required,
description: jsonSchemaElement.description,
type: jsonSchemaElement.type
}
}
break
}

return Object.keys(obj).reduce((acc, propKey) => {
acc.push(toSwaggerProp(propKey, obj[propKey]))
return acc
}, [])
}

function localRefResolve (jsonSchema, externalSchemas) {
if (jsonSchema.type && jsonSchema.properties) {
// for the shorthand querystring/params/headers declaration
const propertiesMap = Object.keys(jsonSchema.properties).reduce((acc, h) => {
const required = (jsonSchema.required && jsonSchema.required.indexOf(h) >= 0) || false
const newProps = Object.assign({}, jsonSchema.properties[h], { required })
return Object.assign({}, acc, { [h]: newProps })
}, {})

return propertiesMap
}

if (jsonSchema.$ref) {
// $ref is in the format: #/definitions/<resolved definition>/<optional fragment>
const localReference = jsonSchema.$ref.split('/')[2]
return localRefResolve(externalSchemas[localReference], externalSchemas)
}
return jsonSchema
}
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
"homepage": "https://github.com/fastify/fastify-swagger#readme",
"devDependencies": {
"@types/node": "^14.0.1",
"fastify": "^3.0.0-rc.1",
"fastify": "^3.0.0-rc.2",
"fs-extra": "^9.0.0",
"joi": "^14.3.1",
"joi-to-json-schema": "^5.1.0",
Expand All @@ -46,7 +46,8 @@
"@types/swagger-schema-official": "^2.0.20",
"fastify-plugin": "^2.0.0",
"fastify-static": "^3.0.0",
"js-yaml": "^3.12.1"
"js-yaml": "^3.12.1",
"json-schema-resolver": "^1.1.1-0"
},
"standard": {
"ignore": [
Expand Down
19 changes: 19 additions & 0 deletions routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,25 @@ function fastifySwagger (fastify, opts, next) {
}
})

// fastify.ready((err) => {
// if (err) {
// throw err
// }
// })
Eomm marked this conversation as resolved.
Show resolved Hide resolved
const allSchemas = fastify.getSchemas()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Eomm, do you know if in v3 this method was reworked, so it will return all schemes, even those what were registered in child scopes with fastify.register?
Cause in fastify v2 I had to go through all child scopes in order to get all of them like https://github.com/SkeLLLa/fastify-oas/blob/master/lib/openapi/index.js#L5

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope, it is encapsulated and tested here:
https://github.com/fastify/fastify/blob/master/test/schema-feature.test.js#L505

I agree that we could think a nicer solution 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think looping through children is quiet fine, unless you have schemes with same names. But in fastify-oas module there were no such issues, so it can be used.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In think the onRegister hook could let us to avoid to use the symbol, I will give it a try

Copy link
Contributor

@SkeLLLa SkeLLLa May 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more idea - is to add addSchema hook to Fastify. That will allow plugins to get schemes when they are registered. And, for example, instead of resolving them we could put schema to swagger definitions when addSchema is called and use $ref inside swagger route to that definition.

Object.keys(allSchemas).forEach(schemaId => {
fastify.log.info('Exposed $ref %s', schemaId)

fastify.route({
url: `/${schemaId}`,
method: 'GET',
schema: { hide: true },
handler: function (req, reply) {
reply.send(fastify.getSchema(schemaId))
}
})
})

fastify.route({
url: '/yaml',
method: 'GET',
Expand Down
Loading