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

Add behavior on predefined keyword (E.X. oneOf) #1113

Closed
davidnghk01 opened this issue Oct 28, 2019 · 6 comments
Closed

Add behavior on predefined keyword (E.X. oneOf) #1113

davidnghk01 opened this issue Oct 28, 2019 · 6 comments

Comments

@davidnghk01
Copy link

What version of Ajv you are you using?
6.9.2

What problem do you want to solve?
I would like implement discriminator feature on AJV by addKeyword feature

But i get stuck for AJV always report it has error under oneOf statement.
I has try call before validate

ajv.removeKeyword("oneOf")
ajv.addKeyword("oneOf", {.....})

it work for disable oneOf checking, but i would like keep the original implementation if discriminator not available, so what should i do ?

What do you think is the correct solution to problem?
Here is my attempt code.

const ajv = new Ajv(opts)
  ajv.removeKeyword("oneOf")
  ajv.addKeyword("oneOf", {
    inline: function(it, keyword, schema, parentSchema) {
      if (parentSchema.discriminator) {
        return "true" // disable the checking if discriminator available
      }
      const inlineCode = doT.compile(require("ajv/lib/dotjs/oneOf"))(
        it,
        keyword,
        schema,
        parentSchema
      )
      return inlineCode
    },
    errors: true,
    statements: true
  })

But it not working.

When i used to validate the schema

Schema

{
  $id: "fuck-off",
  oneOf: [
    {
      $ref: "#/definitions/A"
    },
    {
      $ref: "#/definitions/B"
    },
    {
      $ref: "#/definitions/C"
    }
  ],
  definitions: {
    A: {
      type: "object",
      properties: {
        objType: {
          type: "string",
          enum: ["A"]
        },
        bProp: {
          type: "string"
        }
      },
      required: ["objType", "aProp"]
    },
    B: {
      type: "object",
      properties: {
        objType: {
          type: "string",
          enum: ["B"]
        },
        bProp: {
          type: "string"
        }
      },
      required: ["objType", "bProp"]
    },
    C: {
      type: "object",
      properties: {
        objType: {
          type: "string",
          enum: ["C"]
        },
        cProp: {
          type: "string"
        }
      },
      required: ["objType", "cProp"]
    }
  }

code:

    validate({
      objType: "C"
    })

it expect should throw error because missing cProp.
But in fact i can't get all error ....

Will you be able to implement it?

@davidnghk01
Copy link
Author

Finally, i got that working code

  const orgOneOfImplementation = ajv.RULES.all.oneOf.code
  ajv.RULES.all.oneOf.code = (it, keyword, schema) => {
    if (it.schema.discriminator) {
      return undefined // disable the checking if discriminator available
    }
    return orgOneOfImplementation(it, keyword, schema)
  }

But it is possible expose official API to do that ? That look odd

@epoberezkin
Copy link
Member

I believe discriminator is no-op in the new version of swagger and should only be used to optimise oneOf behaviour (to avoid validating the branches that would fail), but it doesn't change the validation result. Discriminator doesn't really replace oneOf validation, but it shortcuts the branch selection by excluding all branches that are guaranteed to fail (so the implementation above is not correct).

You could simply add it as no-op to allow it in strictKeywords mode:

ajv.addKeyword('discriminator');

Or you could submit an enhancement PR that introduces option "discriminator" to enable discriminator keyword to optimise the validation of oneOf keyword (to only validate against one branch). Or maybe openAPI option to enable both "nullable" and "discriminator", and then "nullable" option can be deprecated and removed from the next major version.

If you go this route, I would probably like to see some extra schema validation step (possibly optional) that would guarantee that only one branch can pass for each allowed discriminator value.

And, obviously, tests both to test new behaviour and also to ensure "discriminator" is ignored without the option and all branches are still validated even if it is present.

Or it indeed can be a plug-in introducing discriminator by replacing oneOf implementation - but it feels wrong though to replace oneOf - there likely to be some edge cases...

@davidnghk01
Copy link
Author

@epoberezkin My current solution is transpiler discriminator to select keyword which introduced in
ajv-keyword

For reference , my implementation here:
createAJVInstance

@epoberezkin
Copy link
Member

Going to close this ticket, supporting discriminator seems a separate issue to the ticket title - feel free to submit / implement it though, as suggested. Converting discriminator to custom select seems wrong for various reasons.

Extending the behaviour of the standard keywords will not be supported.

@Bessonov
Copy link

Bessonov commented Nov 1, 2019

@davidnghk01 I'm not sure, that we have the same use case, but it seems to be very similar. I want to validate request body with OpenAPI-Schema. Validating object has something like state. Different states has different requirements for the same object. For example, in hidden object is everything optional, but in active not. The example with discriminator doesn't worked for me with ajv, because ajv ignores discriminator and just assume hidden state. This is fatal for validation.

The OpenApi doesn't support constants, but proposed solution from the author to add an enum seems to work:

openapi: 3.0.0
info:
  title: About MyObjects
  version: 0.0.1
paths: {}

components:
  schemas:
    MyObject:
      oneOf:
        - $ref: "#/components/schemas/MyObjectHidden"
        - $ref: "#/components/schemas/MyObjectActive"
      discriminator:
        propertyName: state
        mapping:
          HIDDEN: "#/components/schemas/MyObjectHidden"
          ACTIVE: "#/components/schemas/MyObjectActive"

    MyObjectBase:
      properties:
        phoneNumber:
          type: string

    MyObjectHidden:
      allOf:
        - $ref: "#/components/schemas/MyObjectBase"
        - type: object
          required:
            - state
          properties:
            state:
              type: string
              enum:
                - HIDDEN

    MyObjectActive:
      allOf:
        - $ref: "#/components/schemas/MyObjectBase"
        - type: object
          required:
            - state
            - phoneNumber
          properties:
            state:
              type: string
              enum:
                - ACTIVE
const valid = ajv.validate({$ref: 'openapi.yml#/components/schemas/MyObject'}, body)

The generation of typescript-fetch is broken for this example, but I hope it is fixed in 4.2.1.

@epoberezkin
Copy link
Member

see #1119 for discriminator keyword

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

No branches or pull requests

3 participants