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.

Sign up for GitHub

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

OpenApi : nested ref in #/components/schemas/ #472

Merged
merged 5 commits into from
Sep 26, 2021
Merged

OpenApi : nested ref in #/components/schemas/ #472

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e62fee5
#462 Allow nested to resolve for OpenAPI : failing test
briceruzand Sep 20, 2021
47114c9
#462 Allow nested to resolve for OpenAPI
briceruzand Sep 20, 2021
b7a8814
add prepareOpenapiSchemas to resolve refs
cjsewell Sep 11, 2021
c4dd960
#462 Allow nested $ref to resolve for OpenAPI
briceruzand Sep 20, 2021
2f28ad6
#462 Provide more complex test
briceruzand Sep 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions lib/spec/openapi/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

const yaml = require('js-yaml')
const { shouldRouteHide } = require('../../util/common')
const { prepareDefaultOptions, prepareOpenapiObject, prepareOpenapiMethod, normalizeUrl } = require('./utils')
const { prepareDefaultOptions, prepareOpenapiObject, prepareOpenapiMethod, prepareOpenapiSchemas, normalizeUrl } = require('./utils')

module.exports = function (opts, cache, routes, Ref, done) {
let ref
Expand All @@ -20,16 +20,20 @@ module.exports = function (opts, cache, routes, Ref, done) {
const openapiObject = prepareOpenapiObject(defOpts, done)

ref = Ref()
openapiObject.components.schemas = {
openapiObject.components.schemas = prepareOpenapiSchemas({
...openapiObject.components.schemas,
...(ref.definitions().definitions)
}
}, ref)

// 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
// definitions are added by resolve but they are replace by components.schemas
Object.values(openapiObject.components.schemas)
.forEach((_) => { delete _.$id })
.forEach((_) => {
delete _.$id
delete _.definitions
})

for (const route of routes) {
const schema = defOpts.transform
Expand Down
13 changes: 13 additions & 0 deletions lib/spec/openapi/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,9 +319,22 @@ function prepareOpenapiMethod (schema, ref, openapiObject) {
return openapiMethod
}

function prepareOpenapiSchemas (schemas, ref) {
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 describe what you need to archive here?
I'm not sure to fully understand

With some testing this is not working if I put additional schemas in the bottom test:

    instance.addSchema({ $id: 'OrderItem', type: 'object', properties: { id: { type: 'integer' } } })
+    instance.addSchema({ $id: 'ProductItem', type: 'object', properties: { id: { type: 'integer' } } })
    instance.addSchema({ $id: 'Order', type: 'object', properties: { products: { type: 'array', items: { $ref: 'OrderItem' } } } })
    instance.post('/', { schema: { body: { $ref: 'Order' }, response: { 200: { $ref: 'Order' } } } }, () => {})
+    instance.post('/other', { schema: { body: { $ref: 'ProductItem' } } }, () => {})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi Ecomm, I d'like to replace local $ref with local openapi components schema (prefixed by #/components/schemas/) in generated openapi components schema.
With that change, openapi components will be able to reference other local components.

I wrote a simple test of my use case (too simple may be) and I pick the function from cjsewell@1a816ee which seems to work well on my test.

I going to improve my test (with a more complex case, has you done). In try too investigate why the test hang when you add other schema definitions.
Do you have a better idea on that function implementation?

Thx

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have investigated on hanging test you where face to.
I seems to be cause by unfinished fastify-swagger initialization whereas fastify fastify.ready() is ok.
I have rewrote test using async/await to provide more readable tests and I never have this trouble anymore.

Hanging test behavior also exists on your test case without pull request feature.

return Object.entries(schemas)
.reduce((res, [name, schema]) => {
const _ = { ...schema }
const resolved = transformDefsToComponents(ref.resolve(_, { externalSchemas: [schemas] }))
return {
...res,
[name]: resolved
}
}, {})
}

module.exports = {
prepareDefaultOptions,
prepareOpenapiObject,
prepareOpenapiMethod,
prepareOpenapiSchemas,
normalizeUrl
}
39 changes: 36 additions & 3 deletions test/spec/openapi/refs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ const Swagger = require('swagger-parser')
const fastifySwagger = require('../../../index')
const { openapiOption } = require('../../../examples/options')

test('support $ref schema', t => {
t.plan(3)
test('support $ref schema', (t) => {
t.plan(4)
const fastify = Fastify()

fastify.register(fastifySwagger, openapiOption)
Expand All @@ -17,11 +17,44 @@ test('support $ref schema', t => {
done()
})

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

const openapiObject = fastify.swagger()
t.equal(typeof openapiObject, 'object')
t.match(Object.keys(openapiObject.components.schemas), ['def-0'])

Swagger.validate(openapiObject)
.then(function (api) {
t.pass('valid swagger object')
})
.catch(function (err) {
t.fail(err)
})
})
})

test('support nested $ref schema', (t) => {
t.plan(5)
const fastify = Fastify()

fastify.register(fastifySwagger, openapiOption)
fastify.register(function (instance, _, done) {
instance.addSchema({ $id: 'OrderItem', type: 'object', properties: { id: { type: 'integer' } } })
instance.addSchema({ $id: 'Order', type: 'object', properties: { products: { type: 'array', items: { $ref: 'OrderItem' } } } })
instance.post('/', { schema: { body: { $ref: 'Order' }, response: { 200: { $ref: 'Order' } } } }, () => {})
done()
})

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

const openapiObject = fastify.swagger()
t.equal(typeof openapiObject, 'object')
t.match(Object.keys(openapiObject.components.schemas), ['def-0', 'def-1'])

// OrderItem ref must be prefixed by '#/components/schemas/'
t.equal(openapiObject.components.schemas['def-1'].properties.products.items.$ref, '#/components/schemas/def-0')

Swagger.validate(openapiObject)
.then(function (api) {
Expand Down