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

Support OpenAPI 3.0 discriminator.mapping #2003

Open
silverwind opened this issue Jun 7, 2022 · 8 comments · May be fixed by #2263
Open

Support OpenAPI 3.0 discriminator.mapping #2003

silverwind opened this issue Jun 7, 2022 · 8 comments · May be fixed by #2263

Comments

@silverwind
Copy link

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.

@epoberezkin
Copy link
Member

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
Copy link
Member

epoberezkin commented Jun 8, 2022

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.

@thetumper
Copy link

thetumper commented Oct 25, 2022

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
Copy link

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?

@heath-freenome
Copy link

@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
Copy link

@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!

@epoberezkin
Copy link
Member

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

@heath-freenome
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

4 participants