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

Proper use of polymorphism / inheritance in Open API 3 #2116

Closed
PierrePrimot opened this issue Jan 23, 2020 · 10 comments
Closed

Proper use of polymorphism / inheritance in Open API 3 #2116

PierrePrimot opened this issue Jan 23, 2020 · 10 comments
Assignees
Labels
clarification requests to clarify, but not change, part of the spec discriminator

Comments

@PierrePrimot
Copy link

I just want to make a correct specification with basic inheritance anywhere in the schemas, be able to show it properly in the swagger editor and generate some code with it, but I can't. I am clearly missing something in Open API 3.

In the specification document (3.0.2), an example uses oneOf, and optionally the Discriminator block :

    MyResponseType:  
     oneOf:
      - $ref: '#/components/schemas/Cat'
      - $ref: '#/components/schemas/Dog'
      - $ref: '#/components/schemas/Lizard'  
     discriminator:
        propertyName: petType

In this scenario, the only constraint that Cat, Dog and Lizard must satisfy is the mandatory field petType, and a predetermined value.
Then, one can read :

To avoid redundancy, the discriminator MAY be added to a parent schema
definition, and all schemas comprising the parent schema in an allOf
construct may be used as an alternate schema.

I find it nice since it offers a proper use of the natural inheritance of the 3 entities :

components:
  schemas:
    Pet:
      type: object
      required:
      - petType
      properties:
        petType:
          type: string
      discriminator:
        propertyName: petType
        mapping:
          dog: Dog
    Cat:
      allOf:
      - $ref: '#/components/schemas/Pet'
      - type: object
        # all other properties specific to a `Cat`
        properties:
          name:
            type: string
etc

By the way, the spec is a bit ambiguous with

The discriminator object is legal only when using one of the composite
keywords oneOf, anyOf, allOf

Pet does not contains any of these keywords !

However, this second example does not show the consequences on the MyResponseType block.

  1. Does it stay the same ? If so, what should be generated, say, in Java ? A MyResponseType and a Pet classes ? What should the MyResponseType class look like ?
  2. Should MyResponseType be removed from the file, and Pet used instead ? If so, the swagger editor is not capable of tracing the subtypes anymore...
  3. Then, should we add the oneOf to the Pet schema, so that the swagger editor can display the subtypes ? I've tried, it works, and validators say it's Open API 3 compliant, but warn that a cycle gets in the way. It seems to me that this would be formally wrong : an http body with a Cat (petType Cat) with all the Cat and Dog properties would validate because of the oneOf Cat/Dog inherited from Pet. Am I right to interprete it this way ?

It may be correlated with [#403]

@philsturgeon
Copy link
Contributor

Could you take a swing at a PR to visualize what you think is correct? Then we can shape that PR into a useful wording improvement.

@PierrePrimot
Copy link
Author

PierrePrimot commented Feb 11, 2020

@philsturgeon I am a bit confused by your suggestion, as I still don't know what is both correct towards Open Api3 and functional in the swagger editor and generators. It's seems to me that this very simple situation is a dead-end. Let met put it another way :

Let's consider the general case where MyResponseType is used as the type of a field X in another schema.

  1. if MyResponseType is used without Pet, the description of the json is correct, but generators are lost when generating the field X that could contain one of the three possibilities.
  2. if MyResponseType and Pet both exist, common Java generators are lost too (they should generate a field X of type Pet, but in reality, it is not what happens)
  3. if Pet is used all alone, then the explicit knowledge of its subclasses is lost, therefore the swagger editor will only show the field X with the type Pet
  4. if this knowledge is added in Pet (oneOf...), then the constraints are wrong towards the desired json (and it does not make any sense that a Cat inherits "one of" the other animals properties)

I would eliminate 1 (impossible to generate accurately), 3 (useful information lost for editors) and 4 (incorrect constraints).

So... maybe the correct approach is the second one, as it is the neatest description possible. Maybe it's only a concern for generators. They should implement a bit of logic : get rid of MyResponseType and choose Pet as the type of the field X.

@philsturgeon
Copy link
Contributor

What I mean is that from the words being used I am not sure I'm following, but a PR could make the discussion more complete in my head.

That said, with a discussion about potentially deprecating discriminator, I wonder if if the examples and discussion in here can take care of your use case? #2143

@bmmule
Copy link

bmmule commented Nov 29, 2021

Consider in the case of "allOf" , i don't want to use one of the properties from my reference object , how should I achieve that?

example :
recipientAddress:
allOf:
- $ref: '#/components/schemas/Addresses'
Addresses:
properties:
addressCategory:
type: string
maxLength: 10
description: Type of Address
enum:
- PERMANENT
- COMMUNICATON
example: PERMANENT
addressLine1:
type: string
maxLength: 40
description: First line of the Address
example: host tower

here i dont want property "addressCategory" in my object recipientAddress.

@karenetheridge
Copy link
Member

i don't want to use one of the properties from my reference object

allOf:
  - $ref: '#/components/schemas/Addresses'
  - not:
    required:
    - addressCategory

@hirleydayan
Copy link

hirleydayan commented Nov 30, 2021

i don't want to use one of the properties from my reference object

allOf:
  - $ref: '#/components/schemas/Addresses'
  - not:
    required:
    - addressCategory

Hello! Is it possible to have a structure like the following, or any other structure that could allow us to have "required" or "not required" structures per $ref within anyOf? Thanks!

anyOf:
  - $ref: '#/components/schemas/Addresses_1'
    not:
      required:
        - addressCategory_1
  - $ref: '#/components/schemas/Addresses_2'
    not:
      required:
        - addressCategory_2

@msciortino
Copy link

Any news here?

@jemiller0
Copy link

oneOf confuses me. Lets say the above was a property in an object rather than the return value from a web API call. How would a code generator know what base class to use for the type of the property? It would have to check the base classes for each of the listed subclasses and find the most top-level one? Personally, I don't think oneOf makes any sense for this. Why not, rather than enumerating the subclasses, just list the base class name and the discriminator field?

@handrews
Copy link
Member

@jemiller0 because all of this is trying to leverage JSON Schema, which was not built with code generation in mind at all. It's a constraint system, and things like oneOf and anyOf make a lot of sense as dynamic constraints. But not as a static type generation system.

OAS 3.1 allows using JSON Schema extension vocabularies, and the hope was for improved code generation support through that mechanism. So far that has not happened, but some major tooling only recently caught up to OAS 3.1 so despite being several years since its release, it's still early in terms of people working with it. We'll see what develops - if nothing does, OAS 4 (Moonwalk) might have to take a different approach (IMHO).

@handrews
Copy link
Member

This issue kind-of turned into a grab-bag of questions, most of which aren't addressable by spec changes so I am going to close this. Here are answers to the questions that don't already seem to be answered here:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification requests to clarify, but not change, part of the spec discriminator
Projects
None yet
Development

No branches or pull requests

8 participants