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

feature: Add optional to show schema id instead of def-# #4

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 4 additions & 3 deletions ref-resolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ const kConsumed = Symbol('json-schema-resolver.consumed') // when an external js

const defaultOpts = {
target: 'draft-07',
clone: false
clone: false,
showSchemaId: false
}

const targetSupported = ['draft-07'] // TODO , 'draft-08'
Expand All @@ -33,7 +34,7 @@ const targetCfg = {
// logic: https://json-schema.org/draft/2019-09/json-schema-core.html#rfc.appendix.B.1
function jsonSchemaResolver (options) {
const ee = new EventEmitter()
const { clone, target, applicationUri, externalSchemas: rootExternalSchemas } = Object.assign({}, defaultOpts, options)
const { clone, target, applicationUri, externalSchemas: rootExternalSchemas, showSchemaId } = Object.assign({}, defaultOpts, options)

const allIds = new Map()
let rolling = 0
Expand Down Expand Up @@ -137,7 +138,7 @@ function jsonSchemaResolver (options) {
const id = URI.serialize(baseUri) + rel
if (!allIds.has(id)) {
debug('Collected $id %s', id)
json[kRefToDef] = `def-${rolling++}`
json[kRefToDef] = showSchemaId ? id : `def-${rolling++}`
allIds.set(id, json)
} else {
debug('WARN duplicated id %s .. IGNORED - ', id)
Expand Down
56 changes: 56 additions & 0 deletions test/ref-resolver.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -244,3 +244,59 @@ test('absolute $ref #2', t => {
t.equal(out.properties.address.$ref, '#/definitions/def-0')
t.equal(out.properties.houses.items.$ref, '#/definitions/def-0')
})

const phoneNumberSchemaId = {
Copy link
Owner

Choose a reason for hiding this comment

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

Could you remove this const?

This is to avoid issue related to not cloned object, but always fresh ones
I would move it to an external file and load with factory

$id: 'phoneNumber',
type: 'object',
properties: {
countryCode: { type: 'string' },
number: { type: 'string' }
}
}

test('set showSchemaId to be true with relativeId', t => {
t.plan(4)

const schema = factory('relativeId-showSchemaId')
const externalSchemas = [phoneNumberSchemaId]

const resolver = RefResolver({
clone: true,
showSchemaId: true
})
const out = resolver.resolve(schema, {
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not sure this is working since the out is equals to factory('relativeId-showSchemaId')
So nothing has been resolved

The purpose of this lib (in conjunction with fastify-swagger) is to transform external $ref to local definition refs.

To do it you can:

  • create a single json out of the box with the definitions
  • use $ref to reference external schema

Since this module finds a relative schema, it will no resolve the $ref since it assumes that the definitions it is already there - as it should be.

So we need to change these test to absolute references instead and fix invalid local references

externalSchemas
})
const definitions = resolver.definitions()

t.deepEquals(definitions, {
definitions: {
[phoneNumberSchemaId.$id]: phoneNumberSchemaId
}
})
t.equal(out.properties.personal.homeNumber.$ref, '#/definitions/phoneNumber')
t.equal(out.properties.personal.mobilePhoneNumber.$ref, '#/definitions/phoneNumber')
t.equal(out.properties.work.$ref, '#/definitions/phoneNumber')
})

test('set showSchemaId to be true with absoluteId', t => {
t.plan(1)

const schema = factory('absoluteId-localRef')
const externalSchemas = [phoneNumberSchemaId]

const resolver = RefResolver({
clone: true,
showSchemaId: true
})
resolver.resolve(schema, {
externalSchemas
})
const definitions = resolver.definitions()

t.deepEquals(definitions, {
definitions: {
'http://example.com/phoneNumber': phoneNumberSchemaId
Copy link
Owner

Choose a reason for hiding this comment

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

I would add a more complex test too, described here.

  • one file with an absolute $id http://example.net/root.json with 2 $ref
    • one relative ref to a local definition for the '#/definitions/phoneNumber
    • one relative ref to an external definition for the external.json#/definitions/phoneNumber
  • one file with absolute $id http://example.net/external.json

}
})
})
18 changes: 18 additions & 0 deletions test/schemas/relativeId-showSchemaId.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
module.exports = {
$id: 'personPhoneNumbers',
Copy link
Owner

Choose a reason for hiding this comment

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

This schema is invalid due the missing local $ref
you may check here

type: 'object',
properties: {
personal: {
type: 'object',
homeNumber: {
$ref: '#/definitions/phoneNumber'
},
mobilePhoneNumber: {
$ref: '#/definitions/phoneNumber'
}
},
work: {
$ref: '#/definitions/phoneNumber'
}
}
}