-
-
Notifications
You must be signed in to change notification settings - Fork 210
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
feat: support local reference in schema, support definitions keyword #676
base: master
Are you sure you want to change the base?
Changes from all commits
9c63e1b
5b026bc
04cd48a
2c4f510
e2d1a8b
76722f6
70fed8f
15f60aa
c6bc3bb
e291219
a1971e0
7b40e40
e16d173
5e68803
784fdbc
1dd9db9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -352,3 +352,155 @@ test('renders $ref schema with additional keywords', async (t) => { | |
t.match(res.statusCode, 400) | ||
t.match(openapiObject.paths['/url1'].get.parameters[0].schema, cookie) | ||
}) | ||
|
||
test('support absolute refs in schema', async (t) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These due tests:
Are already supported by json-schema-resolver Eomm/json-schema-resolver#8 The So the questions are:
So, right now, I think this fix does not solve the core problem There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed, this tests were passing in the json-schema-resolver lib, and lib itself is perfectly fine. However, the core problem is in the difference between json-schema standard, and swagger/openapi one. First, the 'definitions' keyword. It is supported by json-schema standard, but not supported in openapi/swagger. However, it can be provided by user in schema. Issues related to that problem: #639, #524, #612. Here is my comment on why this occurs - #639 (comment). To fix that, Second one, the support of local $refs in openapi(and possibly swagger). This is explained in #676 (comment) by Ivan. In short, local $ref like To fix that, we use
Additional tests that were introduced in test/util.js can give a more clear picture on this I think. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the detailed information, I need to catch up on this 👍🏽 |
||
const fastify = Fastify() | ||
await fastify.register(fastifySwagger, { openapi: {} }) | ||
fastify.register(async (instance) => { | ||
instance.addSchema( | ||
{ | ||
$id: 'ObjectA', | ||
type: 'object', | ||
properties: { | ||
example: { | ||
type: 'string' | ||
} | ||
} | ||
} | ||
) | ||
instance.addSchema( | ||
{ | ||
$id: 'ObjectC', | ||
type: 'object', | ||
properties: { | ||
referencedObjA: { | ||
$ref: 'ObjectA#' | ||
}, | ||
referencedObjC: { | ||
$ref: 'ObjectC#/properties/ObjectD' | ||
}, | ||
ObjectD: { | ||
type: 'object', | ||
properties: { | ||
d: { | ||
type: 'string' | ||
} | ||
} | ||
} | ||
} | ||
} | ||
) | ||
instance.post('/third/:sample', { | ||
schema: { | ||
body: { | ||
$ref: 'ObjectC#' | ||
}, | ||
params: { | ||
$ref: 'ObjectC#' | ||
}, | ||
response: { 200: { $ref: 'ObjectC#' } } | ||
} | ||
}, async () => ({ result: true })) | ||
}) | ||
|
||
await fastify.ready() | ||
|
||
const openapiObject = fastify.swagger() | ||
t.equal(typeof openapiObject, 'object') | ||
|
||
// if validation is passed = success | ||
await Swagger.validate(openapiObject) | ||
}) | ||
|
||
test('support relative refs in schema', async (t) => { | ||
const fastify = Fastify() | ||
await fastify.register(fastifySwagger, { openapi: {} }) | ||
fastify.register(async (instance) => { | ||
instance.addSchema({ | ||
$id: 'ObjectA', | ||
type: 'object', | ||
properties: { | ||
sample: { | ||
type: 'object', | ||
properties: { | ||
a: { type: 'string' }, | ||
b: { type: 'object', properties: { d: { type: 'string' } } } | ||
} | ||
}, | ||
someValue: { type: 'string' }, | ||
relativeExample: { | ||
$ref: '#/properties/sample' | ||
} | ||
} | ||
}) | ||
|
||
instance.post('/first/:sample', { | ||
schema: { | ||
body: { | ||
$ref: 'ObjectA#/properties/relativeExample' | ||
}, | ||
params: { | ||
$ref: 'ObjectA#/properties/relativeExample' | ||
}, | ||
response: { 200: { $ref: 'ObjectA#/properties/relativeExample' } } | ||
} | ||
}, async () => ({ result: true })) | ||
}) | ||
|
||
await fastify.ready() | ||
|
||
const openapiObject = fastify.swagger() | ||
t.equal(typeof openapiObject, 'object') | ||
|
||
// if validation is passed = success | ||
await Swagger.validate(openapiObject) | ||
}) | ||
|
||
test('support definitions keyword in schema', async (t) => { | ||
const fastify = Fastify() | ||
await fastify.register(fastifySwagger, { openapi: {} }) | ||
|
||
fastify.register(async (instance) => { | ||
instance.addSchema({ | ||
$id: 'ObjectA', | ||
type: 'object', | ||
definitions: { | ||
sample: { | ||
type: 'object', | ||
properties: { | ||
a: { type: 'string' }, | ||
b: { type: 'object', properties: { d: { type: 'string' } } } | ||
} | ||
}, | ||
someValue: { type: 'string' }, | ||
relativeExample: { | ||
$ref: '#/definitions/sample' | ||
} | ||
} | ||
}) | ||
|
||
instance.post('/first/:sample', { | ||
schema: { | ||
body: { | ||
$ref: 'ObjectA#/definitions/relativeExample' | ||
}, | ||
params: { | ||
$ref: 'ObjectA#/definitions/relativeExample' | ||
}, | ||
response: { 200: { $ref: 'ObjectA#/definitions/relativeExample' } } | ||
} | ||
}, async () => ({ result: true })) | ||
}) | ||
|
||
await fastify.ready() | ||
|
||
const openapiObject = fastify.swagger() | ||
t.equal(typeof openapiObject, 'object') | ||
|
||
// definitions are transformed to properties | ||
// previous properties obj take precedence over definitions obj | ||
t.equal(openapiObject.paths['/first/{sample}'].post.requestBody.content['application/json'].schema.$ref, '#/components/schemas/def-0/properties/relativeExample') | ||
t.equal(openapiObject.paths['/first/{sample}'].post.responses['200'].content['application/json'].schema.$ref, '#/components/schemas/def-0/properties/relativeExample') | ||
|
||
await Swagger.validate(openapiObject) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you move schema from the
definitions
to theproperties
? You shouldn't create new properties.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a workaround because 'definitions' keyword itself is not allowed(and will trigger error on openapi schema validation), but we still need to support it(#676 (comment))
The only other way I can think of is somehow allowing additional keywords, adding something like 'x-definitions'(or even the 'definitions' keyword itself) and moving data there, but I don't see a straightforward way to implement it.