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 for discriminator tag lookup in referenced schemas #2261

Open
z6n9n opened this issue May 1, 2023 · 6 comments
Open

Support for discriminator tag lookup in referenced schemas #2261

z6n9n opened this issue May 1, 2023 · 6 comments

Comments

@z6n9n
Copy link

z6n9n commented May 1, 2023

What version of Ajv you are you using?
8.12.0

What problem do you want to solve?
If the discrimantor property needs to be evaluated in a object which contains a allOf instead of direct properties, the discriminator can not be evaluated. Also the mapping param should be used if one exists.

Example of such a mapping:

    Song:
      description: A song
      type: object
      oneOf:
        - $ref: '#/components/schemas/InstrumentalSongObject'
        - $ref: '#/components/schemas/PopSongObject'
      discriminator:
        propertyName: type
        mapping:
          INSTRUMENTAL: '#/components/schemas/InstrumentalSongObject'
          POP: '#/components/schemas/PopSongObject'
    BaseSong:
      description: Base object
      type: object
      required:
        - title
      properties:
        title:
          description: The title
          type: string
    InstrumentalSong:
      description: 'Instrumental Song'
      required:
        - type
      type: object
      properties:
        type:
          type: string
          description: Song type
          enum:
            - INSTRUMENTAL
          example: INSTRUMENTAL
        band:
          description: The band
          type: string
    PopSong:
      description: 'Pop song'
      required:
        - type
      type: object
      properties:
        type:
          type: string
          description: Song type
          enum:
            - POP
          example: POP
        artist:
          description: The artist
          type: string
    InstrumentalSongObject:
      description: A instrumental song
      type: object
      allOf:
        - $ref: '#/components/schemas/BaseSong'
        - $ref: '#/components/schemas/InstrumentalSong'
    PopSongObject:
      description: A pop song
      type: object
      allOf:
        - $ref: '#/components/schemas/BaseSong'
        - $ref: '#/components/schemas/PopSong'

What do you think is the correct solution to problem?
the getMapping function in the package discriminator needs to be extended tu support allOf for evaluation of the discriminator in the props of these subobjects.

Will you be able to implement it?
yes, PR will follow

@epoberezkin
Copy link
Member

Semantics of discriminator mean that oneOf should be used, not anyOf. Why is it needed?

Discriminator in itself is no-op, but it is used as an optimisation for faster oneOf validation, requiring that the schema itself is consistent with "discriminator" semantics - that only one of the branches can be valid, and each branch requires a specific different value of discriminator tag.

@epoberezkin
Copy link
Member

I see, the issue is not named correctly - it's about looking up the name in the referenced schemas.

I don't think that it needs to be supported to be honest given the complexity. A simple workaround is to either construct the schema programmatically, so that it's included as the whole object, or to repeat the tag value on the top level.

@epoberezkin epoberezkin changed the title Support for discriminator evaluation if referenced object contains anyOf Support for discriminator tag lookup in referenced schemas May 1, 2023
@epoberezkin
Copy link
Member

but happy to look at PR, if it's simple enough it might be ok... The traversal would simply stop then at the first schema that defines the tag value.

@z6n9n
Copy link
Author

z6n9n commented May 1, 2023

The problem comes from the practices, that this is a valid schema definition for OAS. Repeating the "Base" object properties instead of using allOf would work, but results later in ugly code from the code generators based on this schema. This, because polymorphism then can not be used anymore and all objects get generated as own interface definitions. That's why such practices (combine oneOf with allOf) are preferred.

@mefellows
Copy link

Hi team,

I'd like to see this (mapping) supported also. In trivial examples, working around mapping is straightforward. When handling an OAS it's a little more complicated as the referenced schemas can't just be renamed. At the very least, it would make it difficult to present useful errors back to users as the schema would have been transformed.

Is the PR #2262 something you would consider and does it address the problem?

@cdm-2012
Copy link

cdm-2012 commented Jan 4, 2024

Hi @epoberezkin, supporting OAS mapping would be really helpful because it becomes more and more common application in the projects that I participated in. I will vote for it as well.

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

No branches or pull requests

4 participants