kenspirit / joi-to-json

Capable of converting different versions' joi object to json schema
MIT License
40 stars 18 forks source link

Add support for Joi's .link method and by extension recursive schemas #27

Closed Thomas-Smyth closed 2 years ago

Thomas-Smyth commented 2 years ago

Background - Joi Schema Recursion / Joi.link('#schema')

The following Joi schema allows a person's name and parents to be entered, and their parent's details, and their parent's details, etc.. This is done via the use of Joi's .link method that allows schema to link to other schema or themselves to allow schema to be reused for validation, including in a recursive manner.

const schema = Joi.object({
  name: Joi.string().required(),

  father: Joi.link('#person'),
  mother: Joi.link('#person'),
}).id('person');

Background - JSON Schema Recursion

The following JSON Schema and example input is a correct (or at least I hope it is) implementation of the above Joi schema. Notice the use of definitions to define schemas that can be linked and $refs to link those schemas.

{
  "$ref":  "#/definitions/person",
  "definitions": {
    "person": {
      "type": "object",
      "properties": {
        "name": { "type": "string" },
        "father": { "$ref": "#/definitions/person" },
        "mother": { "$ref": "#/definitions/person" }
      },
      "required": ["name"]
    }
  }
}
{
  "name": "Joe Bloggs",
  "father": {
    "name": "Joe Bloggs Snr.",
    "father": {
      "name": "Joe Bloggs Snr. Snr."
    }
  }
}

Current Implementation

The current implementation produces the following for the Joi schema, as you will see the current implementation does not produce the desired result.

{
  "type": "object",
  "properties": {
    "name": {
      "type": "string"
    },
    "father": {
      "type": "link"
    },
    "mother": {
      "type": "link"
    }
  },
  "required": [
    "name"
  ],
  "additionalProperties": false
}

It would be convenient to have support for this added to this library to make it possible to use it against more complex schemas.

kenspirit commented 2 years ago

Hi @Thomas-Smyth, I will take a look at this usage of joi link method later. PR is also welcome as well.

kenspirit commented 2 years ago

Version 2.3.0 supports named link now.

Thomas-Smyth commented 2 years ago

Thanks for the very quick addition!

I have, however, noticed that your implementation does not support cases where .id is used in anything but the main schema, i.e. anything that's not directly recursive? My example above unfortunately didn't demonstrate this, however, you can define IDs for subschema and not just the main schema.

const sharedSchema = Joi.string().id('SharedSchema');
const schema = Joi.object({ test: Joi.link('#SharedSchema') }).shared(sharedSchema);

In the current implementation this outputs the following JSON schema where the reference has no definition:

{
  type: 'object',
  properties: { test: { '$ref': '#/$defs/SharedSchema' } },
  additionalProperties: false
}

Furthermore, I've found an edge case where subschemas that do not form part of the same tree in the Joi schema can have duplicate IDs, as seen in the following Joi schema:

const sharedOne = Joi.string().id('#SharedOne');
const useSharedOne = Joi.link('#SharedOne').shared(SharedOne);

const sharedTwo = Joi.number().id('#SharedOne');
const useSharedTwo = Joi.link('#SharedOne').shared(SharedTwo);

const testSchema = Joi.object({
  sharedOne: useSharedOne,
  sharedTwo: useSharedTwo,
});

This shouldn't affect my usage, however, this help up opening my own PR for the above as I could not figure out how to get around it. I was thinking along the lines of defining the definitions at the appropriate level of nesting in the template and having the references reference the definition defined in one of the parent levels of nesting, however, I could not identify a way to do this with JSON schema references.

kenspirit commented 2 years ago

Hi @Thomas-Smyth, thanks for reporting this special usage. I tried to add support for normal usage with minimum support within the same schema. I will check into the shared usage of Joi. For the edge case, I wonder if they should have the same id.

Thomas-Smyth commented 2 years ago

For the edge case, I also wondered whether they should have the same id as it seems odd you can define IDs that are not unique. I think the use case for it is similar to generics in a typed language, such that you can use a schema with different subtypes.

For example, you could have a family schema, as seen below:

const family = Joi.object({
  father: Joi.link('#animal'),
  mother: Joi.link('#animal'),
  child: Joi.link('#animal'),
});

Then use this schema with different types of animals each with their own schema, as seen below:

const humanFamily = family.shared(Joi.object({ beardLength: Joi.number() }).id('Animal'));

const dogFamily = family.shared(Joi.object({ tailLength: Joi.number() }).id('Animal'));

An error is only thrown if you call .shared passing a schema with an ID that is already defined in the schema it's called on, presumably as this is when the ambiguity of having two identical IDs starts to occur.

For the first issue I mentioned, from my understanding of how it's working, you will also need to check for IDs in the shared property returned by the .describe() method, and possibly do this recursively in case the ID or sharing is done within a nested level.

kenspirit commented 2 years ago

@Thomas-Smyth , Release version 2.3.2 supports the usage of shared scenario now.

Thomas-Smyth commented 2 years ago

Thanks again for the quick patch.

I'm not noticing a change in the above example?

const SharedSchema = Joi.string().id('SharedSchema');
const Schema = Joi.object({ test: Joi.link('#SharedSchema') }).shared(SharedSchema);
{
  type: 'object',
  properties: { test: { '$ref': '#/$defs/SharedSchema' } },
  additionalProperties: false
}
kenspirit commented 2 years ago

@Thomas-Smyth Release 2.3.3 has fixed this issue. I just collected from the link schema in the previous fix. Now it's checking on all fields.

Thomas-Smyth commented 2 years ago

Although 2.3.3 still didn't work on my complex schema, I was able to move around some of the .shared calls to get it functioning. If I am able to document those cases in a simple way I'll update this issue. Otherwise, everything is working well.

Thank you very much for the quick additions!

kenspirit commented 2 years ago

@Thomas-Smyth Can you provide a minimum set of your complex schema for me to test? It was some nap time during working hour and so was trying some quick fix. Now during weekend, I could have some in-depth test and consideration.

kenspirit commented 2 years ago

Close as it's not replied for some time. Can reopen it if required.