Created by: spacether
This PR improves how we define and handle discriminators. We have added a feature flag legacyDiscriminatorBehavior When it is set to false we:
- define the CodegenDiscriminator on all models that have a discriminator, including the ones that allOf inherit it from an ancestor
- fill in the discriminator MappedModels map with data from data from 6 sources (see them described below)
- validate the discriminator presence: validate that oneOf or anyOf models contain the required discriminator. If they do not contain it helpful errors are thrown.
I am doing this for the the below reasons:
Not all codegen models use extend to make composed models. On generators that don't do that, they could lack the inherited discriminator. Python experimental is an example where we don't use class inheritance to allOf inherit parent classes/models. In that generator, discriminator is defined in each codegenModel, so we should not rely on java's composed schema inheritance solution to set a granparent's discriminator on a child model and a grandchild model that allOf inherit their ancestors.
We also need to do this if we want the values of the CodegenDiscriminator MappedModels to differ from grandparent to parent to child. When we include the below 1-6 sources of MappedModels, the MappedModels map will differ depending upon which model you are looking at (grandparent/parent/child)
My solution is to have the code set the discriminator on each model that contains a discriminator. Prior to this PR models that inherit a discriminator do not contain a discriminator. Then on that CodegenDiscriminator add the following to MappedModels:
- any mappings from the current or inherited schema Discriminator Mapping
- any
x-discriminator-value
mappings in:- child models that allOf inherit from the the current schema
- oneOf models in the current schema
- anyOf models in the current schema
- any child models that allOf inherit from the the current schema
- any descendent models that allOf inherit from the above child models
- oneOf models in the current schema
- anyOf models in the current schema
Why Feature Flag This?
- Some generators (javascript, probably others) implement discriminators differently and do not work if we set the flag to True. Cordoning this off behind the feature flag allows us to fix them later.
- This is a really big change
- I want to support legacy users who bring their old mustache files but use the latest generator code
- Even though I could get all tests to pass, that does not mean that all models are being tested
- When the flag is true, Java models Cat and Dog now have discriminators with empty maps. I do not know if that is okay.
Background Context:
When using oneOf or anyOf models with a discriminator in a parent composed schema, the discriminator map does not include the child oneOf models. This is because our existing java code was only covering the use case where there are descedant schemas which allOf inherit from the current schemas. It was not aware of and handling cases where the current schema is composed and there are oneOf any anyOf schemas that should be added to the discriminator MappedModels.
Below is an example spec where the oneOf models should be in FruitRequiredDesc's discriminator map:
FruitType:
properties:
fruitType:
type: string
required:
- fruitType
FruitRequiredDesc:
oneOf:
- $ref: '#/components/schemas/AppleRequiredDesc'
- $ref: '#/components/schemas/BananaRequiredDesc'
discriminator:
propertyName: fruitType
AppleRequiredDesc:
type: object
required:
- seeds
properties:
seeds:
type: integer
allOf:
- $ref: '#/components/schemas/FruitType'
BananaRequiredDesc:
type: object
required:
- length
properties:
length:
type: integer
allOf:
- $ref: '#/components/schemas/FruitType'
Test Verification
- Many tests have been added, see the PR
Related Issues
If merged, this will close out https://github.com/OpenAPITools/openapi-generator/issues/4904
-
Read the contribution guidelines. -
If contributing template-only or documentation-only changes which will change sample output, build the project before. -
Run the shell script(s) under ./bin/
(or Windows batch scripts under.\bin\windows
) to update Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit, and these must match the expectations made by your contribution. You only need to run./bin/{LANG}-petstore.sh
,./bin/openapi3/{LANG}-petstore.sh
if updating the code or mustache templates for a language ({LANG}
) (e.g. php, ruby, python, etc). -
File the PR against the correct branch: master
,4.3.x
,5.0.x
. Default:master
. -
Copy the technical committee to review the pull request if your PR is targeting a particular programming language.
Core Team: @wing328 (2015/07) @jimschubert (2016/05) @cbornet (2016/05) @ackintosh (2018/02) @jmini (2018/04) @etherealjoy (2019/06)