ajv icon indicating copy to clipboard operation
ajv copied to clipboard

Support OpenAPI 3.0 `discriminator.mapping`

Open silverwind opened this issue 2 years ago • 10 comments

What version of Ajv you are you using?

8.11.0

What problem do you want to solve?

I'd like to validate JSON schema extracted from OpenAPI 3.0 that includes discriminator.mapping. Currently having it will make AJV throw a unavoidable discriminator: mapping is not supported error. Example code that throws:

import Ajv from "ajv";

const schema = {
  type: "object",
  properties: {
    type: {
      type: "string"
    },
    value: {
      oneOf: [
        {type: "string"},
        {type: "number"},
      ],
      discriminator: {
        propertyName: "type",
        mapping: {
          string: {type: "string"},
          number: {type: "number"},
        },
      },
    },
  },
};

const ajv = new Ajv({discriminator: true});
const validate = ajv.compile(schema); // Error: discriminator: mapping is not supported
console.info(validate({type: "string", value: "foo"}));

What do you think is the correct solution to problem?

Either implement discriminator.mapping as described here or alternatively gracefully ignore the presence of it so user code can implement this feature itself.

Will you be able to implement it?

I could probably make it gracefully ignore the property.

silverwind avatar Jun 07 '22 14:06 silverwind

Mapping in OpenAPI is defined as Map[string, string], so you are not supposed to have schemas in it, only $refs. As such, it is redundant, as the validation result does not depend on it - it is determined by the keys in schemas in oneOf.

The general design approach is to prohibit schema elements that are ignored or can lead to contradictory results.

We can make mapping allowed in discriminator by adding discriminator to strict mode, so it will require either strictDiscriminator: false/"log" or strict: {discriminator: false/"log"} in options to ignore/log it, rather than fail.

See https://ajv.js.org/strict-mode.html

epoberezkin avatar Jun 08 '22 09:06 epoberezkin

Also, your schema is incorrect, discriminator has to be used on the object level, where you have tag property in, so it is supposed to be:

const schema = {
  type: "object",
  oneOf: [
    {
      properties: {
        type: {const: "string"},
        value: {type: "string"}
      },
      properties: {
        type: {const: "number"},
        value: {type: "number"}
      }
    }
  ]
  properties: {
    type: {type: "string"},
  },
  required: ["type", "value"]
  discriminator: {
    propertyName: "type",
    // mapping: {
    //  string: {type: "string"},
    //  number: {type: "number"},
    // },
  },
};

You can also note that the presence of discriminator never affects the validation result, it only results in shorter validation process, but it doesn't change the validity of the data - so you have to check that if removing discriminator changes the meaning of your schema, then it is not a correct schema.

Unlike JSON Schema, JTD defines discriminator closer to what you expect it to be.

epoberezkin avatar Jun 08 '22 09:06 epoberezkin

We can make mapping allowed in discriminator by adding discriminator to strict mode, so it will require either strictDiscriminator: false/"log" or strict: {discriminator: false/"log"} in options to ignore/log it, rather than fail.

See https://ajv.js.org/strict-mode.html

These options are not working for me. I'm still seeing "discriminator: mapping is not supported" when doing ajv.compile. I guess what I also don't understand is that my schema compiled with v6.12.6, but is broken now with the latest (migrating to v8).

thetumper avatar Oct 25 '22 19:10 thetumper

I guess what I also don't understand is that my schema compiled with v6.12.6, but is broken now with the latest (migrating to v8).

Any thoughts on why this is?

thetumper avatar Jan 10 '23 20:01 thetumper

@thetumper I'm guessing they haven't actually added the strictDiscriminator option to AJV yet. @epoberezkin Can you consider doing this sooner than later please? It would be nice to not have these errors

heath-freenome avatar Mar 20 '23 18:03 heath-freenome

@epoberezkin Let me be more clear. I'm a maintainer on react-jsonschema-form and we are trying to add support for discriminator into our code. Unfortunately, this issue will break most of the schemas that I personally am interested in using because they all have mappings that I'd rather not have to remove. So I would LOVE to see this new feature added so that I can turn it on. Thanks!

heath-freenome avatar Mar 23 '23 16:03 heath-freenome

Correct, it's not added. PR is welcome!

epoberezkin avatar Mar 24 '23 09:03 epoberezkin

@epoberezkin I wish I had the bandwidth to help on this. How soon might you be able to provide one?

heath-freenome avatar Mar 30 '23 16:03 heath-freenome