Skip to content

refactor: create json schema ref resolver #507

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

Merged
merged 2 commits into from
Aug 29, 2022
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 16 additions & 11 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const { randomUUID } = require('crypto')

const validate = require('./schema-validator')
const Serializer = require('./serializer')
const RefResolver = require('./ref-resolver')
const buildAjv = require('./ajv')

let largeArraySize = 2e4
Expand Down Expand Up @@ -57,20 +58,12 @@ function resolveRef (location, ref) {
const schemaId = ref.slice(0, hashIndex) || location.schemaId
const jsonPointer = ref.slice(hashIndex) || '#'

const schemaRef = schemaId + jsonPointer
const schema = refResolver.getSchema(schemaId, jsonPointer)

let ajvSchema
try {
ajvSchema = ajvInstance.getSchema(schemaRef)
} catch (error) {
if (schema === undefined) {
throw new Error(`Cannot find reference "${ref}"`)
}

if (ajvSchema === undefined) {
throw new Error(`Cannot find reference "${ref}"`)
}

const schema = ajvSchema.schema
if (schema.$ref !== undefined) {
return resolveRef({ schema, schemaId, jsonPointer }, schema.$ref)
}
Expand All @@ -83,6 +76,7 @@ const objectReferenceSerializersMap = new Map()

let rootSchemaId = null
let ajvInstance = null
let refResolver = null
let contextFunctions = null

function build (schema, options) {
Expand All @@ -95,11 +89,13 @@ function build (schema, options) {
options = options || {}

ajvInstance = buildAjv(options.ajv)
refResolver = new RefResolver()
rootSchemaId = schema.$id || randomUUID()

isValidSchema(schema)
extendDateTimeType(schema)
ajvInstance.addSchema(schema, rootSchemaId)
refResolver.addSchema(schema, rootSchemaId)

if (options.schema) {
const externalSchemas = clone(options.schema)
Expand All @@ -114,7 +110,14 @@ function build (schema, options) {
schemaKey = key + externalSchema.$id // relative URI
}

if (ajvInstance.getSchema(schemaKey) === undefined) {
if (refResolver.getSchema(schemaKey) === undefined) {
Copy link
Member

Choose a reason for hiding this comment

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

I would try to check the ajv's readonly properties.
This does not trigger the compiling and we could avoid the deep/equal logic

(x.schemas[schemaKey] || x.refs[schemaKey]) === undefined
const Ajv = require('ajv')

const x = new Ajv()

const externalSchema = {
  $id: 'root',
  definitions: {
    subschema: {
      $id: 'subschema',
      definitions: {
        anchorSchema: {
          $id: '#anchor',
          type: 'string'
        }
      }
    }
  }
}

x.addSchema(externalSchema)

{
  const id = 'subschema'
  console.log(x.schemas[id] || x.refs[id])
}

{
  const id = 'root'
  console.log(x.schemas[id] || x.refs[id])
}

{
  const id = 'subschema#anchor'
  console.log(x.schemas[id] || x.refs[id])
}

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 only concern I have is that refs and schemas is not a part of public API and can be changed without major release.

Copy link
Member

Choose a reason for hiding this comment

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

Isn't it public?
https://github.com/ajv-validator/ajv/blob/master/lib/core.ts#L281

They would have declared as private instead

refResolver.addSchema(externalSchema, key)
}

if (
ajvInstance.refs[schemaKey] === undefined &&
ajvInstance.schemas[schemaKey] === undefined
) {
Comment on lines +117 to +120
Copy link
Member

Choose a reason for hiding this comment

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

I meant that this check should be enough to replace the new RefResolver holder as both (ajv and refresolver) are storing the same values

I will try it locally

Copy link
Member Author

@ivan-tymoshenko ivan-tymoshenko Aug 23, 2022

Choose a reason for hiding this comment

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

It won't work for nested schemas like 'subschema' and 'subschema#anchor'. I will work if we want to know if Ajv has a schema, but we can't get it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, I see. Ajv resolves references in that cases.

Another reason why I want to have a separate abstraction for this is because Ajv does two things: validates schemas at runtime and resolves references at compile time. It's not good. So in #504 I will have to create two Ajv instances (for schema validation and reference resolving), because there would be two different syntaxes.

And since we own this code it's easy to optimize it. For example in case if we want to reuse external schemas.

Copy link
Member Author

Choose a reason for hiding this comment

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

@Eomm I rebased #504 to this branch. You can see there that RefResolver/Validator have different responsibilities and different reasons to be changed.

ajvInstance.addSchema(externalSchema, schemaKey)
}
}
Expand Down Expand Up @@ -178,6 +181,7 @@ function build (schema, options) {
const stringifyFunc = contextFunc(ajvInstance, serializer)

ajvInstance = null
refResolver = null
rootSchemaId = null
contextFunctions = null
arrayItemsReferenceSerializersMap.clear()
Expand Down Expand Up @@ -493,6 +497,7 @@ function mergeAllOfSchema (location, schema, mergedSchema) {

mergedSchema.$id = `merged_${randomUUID()}`
ajvInstance.addSchema(mergedSchema)
refResolver.addSchema(mergedSchema)
location.schemaId = mergedSchema.$id
location.jsonPointer = '#'
}
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
"@fastify/deepmerge": "^1.0.0",
"ajv": "^8.10.0",
"ajv-formats": "^2.1.1",
"fast-deep-equal": "^3.1.3",
"fast-uri": "^2.1.0",
"rfdc": "^1.2.0"
},
Expand Down
82 changes: 82 additions & 0 deletions ref-resolver.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
'use strict'

const deepEqual = require('fast-deep-equal')

class RefResolver {
constructor () {
this.schemas = {}
}

addSchema (schema, schemaId) {
if (schema.$id !== undefined && schema.$id.charAt(0) !== '#') {
schemaId = schema.$id
}
this.insertSchemaBySchemaId(schema, schemaId)
this.insertSchemaSubschemas(schema, schemaId)
}

getSchema (schemaId, jsonPointer = '#') {
const schema = this.schemas[schemaId]
if (schema === undefined) {
return undefined
}
if (schema.anchors[jsonPointer] !== undefined) {
return schema.anchors[jsonPointer]
}
return getDataByJSONPointer(schema.schema, jsonPointer)
}

insertSchemaBySchemaId (schema, schemaId) {
if (
this.schemas[schemaId] !== undefined &&
!deepEqual(schema, this.schemas[schemaId].schema)
) {
throw new Error(`There is already another schema with id ${schemaId}`)
}
this.schemas[schemaId] = { schema, anchors: {} }
}

insertSchemaByAnchor (schema, schemaId, anchor) {
const { anchors } = this.schemas[schemaId]
if (
anchors[anchor] !== undefined &&
!deepEqual(schema, anchors[anchor])
) {
throw new Error(`There is already another schema with id ${schemaId}#${anchor}`)
}
anchors[anchor] = schema
}

insertSchemaSubschemas (schema, rootSchemaId) {
const schemaId = schema.$id
if (schemaId !== undefined && typeof schemaId === 'string') {
if (schemaId.charAt(0) === '#') {
this.insertSchemaByAnchor(schema, rootSchemaId, schemaId)
} else {
this.insertSchemaBySchemaId(schema, schemaId)
rootSchemaId = schemaId
}
}

for (const key in schema) {
if (typeof schema[key] === 'object' && schema[key] !== null) {
this.insertSchemaSubschemas(schema[key], rootSchemaId)
}
}
}
}

function getDataByJSONPointer (data, jsonPointer) {
const parts = jsonPointer.split('/')
let current = data
for (const part of parts) {
if (part === '' || part === '#') continue
if (typeof current !== 'object' || current === null) {
return undefined
}
current = current[part]
}
return current
}

module.exports = RefResolver
28 changes: 28 additions & 0 deletions test/ref.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1953,3 +1953,31 @@ test('should resolve absolute $refs', (t) => {

t.equal(output, JSON.stringify(object))
})

test('nested schema should overwrite anchor scope', (t) => {
t.plan(2)

const externalSchema = {
root: {
$id: 'root',
definitions: {
subschema: {
$id: 'subschema',
definitions: {
anchorSchema: {
$id: '#anchor',
type: 'string'
}
}
}
}
}
}

const data = 'test'
const stringify = build({ $ref: 'subschema#anchor' }, { schema: externalSchema })
const output = stringify(data)

t.equal(output, JSON.stringify(data))
t.throws(() => build({ $ref: 'root#anchor' }, { schema: externalSchema }))
})