Main schemas isolation

[ivan-tymoshenko]

Prerequisites

• [X] I have written a descriptive issue title
• [X] I have searched existing issues to ensure the issue has not already been raised

Issue

I'm working on some optimization for FJS and trying to resolve the endpoints' schemas isolation. I was wondering how it was resolved here, but it turns out all endpoints' schemas are in the same context.

The problem is that you can define a nested schema in a global namespace.

t.test('global refs in main schemas', (t) => {
  t.plan(2)
  const factory = AjvCompiler()
  const compiler = factory({}, fastifyAjvOptionsDefault)
  const validatorFunc1 = compiler({
    schema: {
      definitions: {
        globalIdSchema: {
          $id: 'globalId',
          type: 'integer',
        }
      },
      $ref: 'globalId'
    }
  })
  const validatorFunc2 = compiler({
    schema: {
      definitions: {
        globalIdSchema: {
          $id: 'globalId',
          type: 'string',
        }
      },
      $ref: 'globalId'
    }
  })
  t.equal(validatorFunc1(3), true)
  t.equal(validatorFunc2(3), false)
})

[jsumners]

I think those two schemas are just wrong. You've written two schemas that self-referencially create schemas with conflicting identifiers. Every schema in the container should have a unique identifier. If schemas have conflicting identifiers then the schema author(s) have done it wrong.

See https://json-schema.org/draft/2020-12/json-schema-core.html#name-the-id-keyword

[ivan-tymoshenko]

So users can't create two endpoints with the same nested schemas? So users can't create two endpoints with the same nested schemas? I thought that all context schemas that you add with fastify.addSchema should be unique (and have unique nested schemas), but I thought that endpoint schemas should be isolated from each other.

@jsumners So you think that users should not create routes like that?

fastify.post('/a', {
  handler () { },
  schema: {
    type: 'object',
    properties: {
      a: { $id: 'globalId', type: 'string' },
      b: { $ref: 'globalId' },
    }
  }
})

fastify.post('/b', {
  handler () { },
  schema: {
    type: 'object',
    properties: {
      a: { $id: 'globalId', type: 'integer' },
      b: { $ref: 'globalId' },
    }
  }
})

[jsumners]

You are specifying anonymous schemas in that example. They do not have identifiers. Your $ref in the b definitions are referring a property within the same schema, but they are also declared incorrectly. They should really be { $ref: '#/properties/a' }. See https://json-schema.org/understanding-json-schema/structuring.html#defs

[ivan-tymoshenko]

You can reference the property either with json pointer or schema $id. I wrote valid JSON schema draft 7 syntaxes. Here is an example: https://json-schema.org/understanding-json-schema/structuring.html#bundling

+ Ajv works in this way.

[jsumners]

I wrote valid JSON schema draft 7 syntaxes.

I didn't say the schemas are invalid, only that the values used for the $ref keywords are invalid. Your link says the same thing I wrote previously.

[ivan-tymoshenko]

Ok, I wrote valid values in $ref. Why do you think that it's invalid?

That's a quote from the link you sent me. "Although we can identify any subschema using JSON Pointers or NAMED ANCHORS, the $defs keyword gives us a standardized place to keep subschemas intended for reuse in the current schema document."

[jsumners]

Look at the example you linked. All $ref values should be fragment identifiers or relative to the base URI of the schema.

[ivan-tymoshenko]

1. In Draft 4-7, an $id in a subschema did not indicate an embedded schema. Instead it was simply a base URI change in a single schema document.
2. I can give you an example with relative to the base URL.

   
   fastify.post('/a', {
   handler () { },
   schema: {
   $id: 'hello',
   type: 'object',
   properties: {
     a: { $id: 'hello/globalId', type: 'string' },
     b: { $ref: 'hello/globalId' },
   }
   }
   })

fastify.post('/b', { handler () { }, schema: { $id: 'hello', type: 'object', properties: { a: { $id: 'hello/globalId', type: 'integer' }, b: { $ref: 'hello/globalId' }, } } })

[ivan-tymoshenko]

https://datatracker.ietf.org/doc/html/draft-handrews-json-schema-01#section-8.2.4

[jsumners]

I'm sorry, I really don't understand what you are trying to prove. As I originally stated, schema identifiers are URIs by spec (https://json-schema.org/draft/2020-12/json-schema-core.html#name-the-id-keyword). URIs, by spec (https://www.rfc-editor.org/rfc/rfc3986#section-1.1), are unique globally:

URIs have a global scope and are interpreted consistently regardless of context, though the result of that interpretation may be in relation to the end-user's context.

Therefore, schemas in a schema container, e.g. an AJV instance, can either be anonymous or identified by identifiers specified by their $id property and are globally unique in the container.

[ivan-tymoshenko]

I'm talking about isolation. I'm asking maybe two route schemas shouldn't be in the same container. If we have set up like that.

fastify.addSchema(/* A */)
fastify.addSchema(/* B */)

fastify.post('/foo', {
  handler () { },
  schema: { /* C */ }
})

fastify.post('/bar', {
  handler () { },
  schema: { /* D */ }
})

Maybe we should have 2 containers: { 'A', 'B', 'C' } and { 'A', 'B', 'D' }

[ivan-tymoshenko]

If you and someone else will confirm that it's a user's responsibility to manage conflicts between routes' schemas, that they are in the same namespace, and that they can be referenced to each other, I would be ok with it.

[Eomm]

I think your test is a reasonable use case but creating an ajv container for each route is overkilling.

We could set the addUsedSchema: false as default and it solves this use case and it will be not necessary to create more containers.

Unluckily, because of the coerceTypes: 'array' setting, your example will always fail because the number is coerced. See the example at the bottom.

const Ajv = require('ajv')
const ajv = new Ajv({
  addUsedSchema: false,
  coerceTypes: 'array', // uncomment
  useDefaults: true,
  removeAdditional: true,
  addUsedSchema: false,
  allErrors: false
})

let validate

validate = ajv.compile({

  definitions: {
    globalIdSchema: {
      $id: 'globalId',
      type: 'integer'
    }
  },
  $ref: 'globalId'

})
test(3)

validate = ajv.compile({
  definitions: {
    globalIdSchema: {
      $id: 'globalId',
      type: 'string'
    }
  },
  $ref: 'globalId'
})
test(3)

function test (data) {
  const valid = validate(data)
  if (valid) console.log('Valid!')
  else console.log('Invalid: ' + ajv.errorsText(validate.errors))
}

[ivan-tymoshenko]

@Eomm It looks like addUsedSchema already resolves that. And the error was caused by type coercion.