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

Are discriminators inherited with allOf? #2165

Closed
spacether opened this issue Feb 28, 2020 · 3 comments
Closed

Are discriminators inherited with allOf? #2165

spacether opened this issue Feb 28, 2020 · 3 comments

Comments

@spacether
Copy link

spacether commented Feb 28, 2020

Are discriminator inheritable?

Given the following SpecA:

components:
  schemas:
    Pet:
      type: object
      required:
        - petType
      properties:
        petType:
          type: string
      discriminator:
        propertyName: petType
    Reptile:
      allOf:
        - $ref: '#/components/schemas/Pet'
    Lizard:
      allOf:
        - $ref: '#/components/schemas/Reptile'
        - type: object
          properties:
            lovesRocks:
              type: boolean

Do Reptile and Lizard contain the discriminator defined in Pet?

If discriminators are inheritable, then what is the correct interpretation of receiving payloads of type Reptile from the below specs?
SpecB (discriminator missing from Snake + Lizard)

components:
  schemas:
    Pet:
      type: object
      required:
        - petType
      properties:
        petType:
          type: string
      discriminator:
        propertyName: petType
    Reptile:
      allOf:
        - $ref: '#/components/schemas/Pet'
      oneOf:
        - $ref: '#/components/schemas/Lizard'
        - $ref: '#/components/schemas/Snake'
    Lizard:
      type: object
      properties:
        lovesRocks:
          type: boolean
    Snake:
      type: object
      properties:
        hasLegs:
          type: boolean

SpecC: (discriminator present in Snake + Lizard)

components:
  schemas:
    Pet:
      type: object
      required:
        - petType
      properties:
        petType:
          type: string
      discriminator:
        propertyName: petType
    Reptile:
      allOf:
        - $ref: '#/components/schemas/Pet'
      oneOf:
        - $ref: '#/components/schemas/Lizard'
        - $ref: '#/components/schemas/Snake'
    Lizard:
      allOf:
        - $ref: '#/components/schemas/Pet'
        - type: object
          properties:
            lovesRocks:
              type: boolean
    Snake:
      allOf:
        - $ref: '#/components/schemas/Pet'
        - type: object
          properties:
            hasLegs:
              type: boolean
@tedepstein
Copy link
Contributor

@spacether ,

To answer your question directly, allOf applies all of the included subschemas to the containing schema. In your SpecB example, every keyword from Pet -- including the definition of petType, the requirement to include the petType property, and the use of petType as the discriminator -- becomes part of Reptile.

The discriminator takes effect when a schema that includes the discriminator is directly or indirectly used in a request or response.

If you have a request body specified as $ref: "#/components/schemas/Pet", an OpenAPI-compliant schema validator should see the petType property value, resolve that value to the corresponding schema, and validate the request body against that schema. For example, if a request came in with petType: Lizard, the validator will validate against the declared schema of the request, which is Pet. Then, according to the discriminator, it should validate against the tagged schema, in this case Lizard.

Looking at this scenario in isolation, strictly speaking, Lizard does not have to include allOf: Pet because the validator is already validating against Pet as well as Lizard. And Lizard doesn't need allOf: Reptile either, unless Reptile includes some additional features not shown in your examples.

Likewise, if the request body specified $ref: "#/components/schemas/Reptile, the same logic would apply. Reptile includes allOf the declared features of Pet, including the discriminator, so a request with type: Lizard has to be valid against both Reptile and Lizard. (Also against Pet, since Reptile includes allOf: Pet.)

However, there are a few reasons why you might want Lizard (and Snake) to include allOf: Reptile:

  • If you have a request or response that refers directly to Lizard, it will only be validated against that type unless you include the allOf.
  • A code generator, example generator, or another tool would not recognize Lizard as a subtype of Reptile, so it wouldn't set up the inheritance hierarchy the way you'd expect, and it wouldn't include any features of Reptile.
  • If Lizard has subtypes of its own, and you have a request or response referencing Lizard, you would need the allOf to inherit the discriminator declared in Pet.

Also, there are a few problems with Reptile and its subtypes:

  • The oneOf says that the instance has to match exactly one of the included subschemas. But Lizard and Snake are almost indistinguishable to a schema validator unless you do something to exclude one or the other. There are a couple of common ways to do this:

    • You can use additionalProperties: false in Lizard and Snake so that properties not explicitly defined in those schemas are not allowed, and make at least one identifying property required. You could declare additionalProperties false in both Lizard and Snake, make lovesRocks required in Lizard, and make hasLegs required in Snake. So a request with lovesRocks will pass validation in Lizard, but fail validation in Snake, which satisfies the oneOf condition in Reptile.

    • You can require a specific value on the discriminator property. In Lizard, petType can specify enum: {Lizard} so it only matches a request with petType: Lizard. Likewise with Snake.

    • Another approach would be for Reptile to replace the oneOf with its own enum on petType. Reptile can limit petType to one of the known subtypes by including enum: {Lizard, Snake} on the petType property.

  • To be clear, the oneOf or enum constraint on Reptile isn't required for discriminator to work. If you're using an OpenAPI-compliant validator, it should extend validation to the specified schema as described above. The only purpose of the oneOf or enum in Reptile is to make sure that a request or response specified as Reptile will only pass validation if it is one of the known Reptile subtypes.

  • Your SpecC example has Reptile, Lizard, and Snake all inheriting allOf: Pet. This is fine, but it doesn't allow you to introduce additional features into Reptile and inherit them into the two subtypes. If you did replace allOf: Pet with allOf: Reptile in the subtype schemas, you'd have a circular dependency with the oneOf in Reptile. There are different ways around this, but maybe the most straightforward solution would be to replace the oneOf with an enum or remove it altogether, as suggested above.

There's some relevant discussion in #2141 that might also be useful.

@spacether
Copy link
Author

spacether commented Feb 29, 2020

Thank you for that very thorough explanation. From it my understanding is that yes, all schemas that allOf contain a schema which includes a discriminator then also include that discriminator. That discriminator includes both propertyName and mapping.
This leads to these next questions.

SpecD (mapping defined in discriminator):

components:
  schemas:
    Pet:
      type: object
      required:
        - petType
      properties:
        petType:
          type: string
      discriminator:
        propertyName: petType
        mapping:
          Reptile: '#/components/schemas/Reptile'
          Snake: '#/components/schemas/Snake'
          Lizard: '#/components/schemas/Lizard'
    Reptile:
      allOf:
        - $ref: '#/components/schemas/Pet'
    Lizard:
      allOf:
        - $ref: '#/components/schemas/Reptile'
        - type: object
          properties:
            lovesRocks:
              type: boolean
    Snake:
        - $ref: '#/components/schemas/Reptile'
        - type: object
          properties:
            hasLegs:
              type: boolean

Question about discriminator mapping

For specD Reptile, Lizard and Snake all contain the discriminator defined in Pet, yes?
That means that they also includes that mapping definition, yes?
So can an endpoint that receives a Lizard be sent a. Snake payload?
{petType: 'Snake', 'hasLegs': True}
My guess is that no it can't because it would fail Lizard validation.
But what if the payload was:
{petType: 'Snake', 'hasLegs': True, 'lovesRocks': True}
Then we could receive a Snake in a Lizard endpoint because the payload validates against Lizard and Snake, yes?

Which leads to my next question about additionalProperties.
Reading the spec I see that additionalProperties defaults to True (OpenAPI spec version >= 3.0.2)
Does that mean that if additionalProperties is absent from the model then all extra parameters sent on that model are accepted?

Question about AdditionalProperties

SpecB (discriminator missing from Snake + Lizard)

components:
  schemas:
    Pet:
      type: object
      required:
        - petType
      properties:
        petType:
          type: string
      discriminator:
        propertyName: petType
    Reptile:
      allOf:
        - $ref: '#/components/schemas/Pet'
      oneOf:
        - $ref: '#/components/schemas/Lizard'
        - $ref: '#/components/schemas/Snake'
    Lizard:
      type: object
      properties:
        lovesRocks:
          type: boolean
    Snake:
      type: object
      properties:
        hasLegs:
          type: boolean

So looking at SpecB above my interpretation is this:
To an endpoint that accepts Reptile, I send this payload:
{petType: 'Snake', 'hasLegs': True}
We deserialize in Reptile and see that all properties are accounted for and pass validation.
Because the discriminator is present, we pass in those inputs to the Snake model.
That model accepts its parameter hasLegs and petType because petType is an additionalProperty and we default to additionalProperties True.
Finally we have something like:
snake = Snake(petType: 'Snake', hasLegs: True)
snake.petType == 'Snake'
snake.hasLegs == True
Is that a correct interpretation?

What would we do if the spec version was 3.0.1? Then additionalProperties is not True by default. In that case wouldn't deserializing into a final Snake instance fail because the property PetType is not part of the Snake schema? In that case would the returned instance be of type Reptile?

@handrews
Copy link
Member

I think the only remaining question here is about additionalProperties defaulting to true, which it has always done in every version of JSON Schema and OAS. So there's no special case for OAS <=3.0.1.

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

No branches or pull requests

3 participants