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

Cleanup Jackson type info mess fixes (#9441) #11691

Merged
merged 2 commits into from
Mar 7, 2022

Conversation

rpost
Copy link
Contributor

@rpost rpost commented Feb 22, 2022

@borsch
Copy link
Member

borsch commented Feb 23, 2022

LGTM

@cachescrubber
Copy link
Contributor

I have no time yet to dig into this - but just based on the title i guess this will affect

#11650

please keep that in Mind.

@rpost
Copy link
Contributor Author

rpost commented Feb 23, 2022

I have no time yet to dig into this - but just based on the title i guess this will affect

#11650

please keep that in Mind.

do you want me to somehow respond to this?

@welshm
Copy link
Contributor

welshm commented Feb 23, 2022

I have no time yet to dig into this - but just based on the title i guess this will affect
#11650
please keep that in Mind.

do you want me to somehow respond to this?

Looks like that PR will change how oneOf is handled by using interfaces instead of class inheritence.

Given this change only affects annotations, there will be a (small) collision on needing to rectify the annotations on the interfaces. I don't see any major issues with this change going in first given it's isolated to annotations

@MelleD
Copy link
Contributor

MelleD commented Feb 23, 2022

Looks good

@cachescrubber
Copy link
Contributor

You changed a sample configuration which is already removed in master, bin/configs/spring-mvc.yaml. Could you please update your branch?

@cachescrubber
Copy link
Contributor

do you want me to somehow respond to this?

I just wanted to express that there iss another pending PR heavily relying on the JsonTypeInfo to work properly and I want to make sure this does not affect the implementation there.

@wing328 wing328 changed the base branch from 6.0.x to master February 25, 2022 17:43
@wing328
Copy link
Member

wing328 commented Feb 25, 2022

I've changed the target branch of this PR to master (as it will be released as 6.0.0).

Please resolve the merge conflicts and let me know if you need help on that.

@wing328 wing328 added Enhancement: Code Cleanup General refactoring, removal of deprecated things, commenting, etc. Server: Java Server: Spring labels Feb 25, 2022
@wing328 wing328 added this to the 6.0.0 milestone Feb 25, 2022
@rpost
Copy link
Contributor Author

rpost commented Feb 28, 2022

Please resolve the merge conflicts and let me know if you need help on that.

Rebased and squashed.

@wing328 wing328 merged commit c22997b into OpenAPITools:master Mar 7, 2022
@jbeullen
Copy link

jbeullen commented Feb 28, 2024

This change introduces a change in the OpenAPI specification.
The following snippet is valid:

"discriminator": {
  "propertyName": "species",
  "mapping": {
    "Dog": "#/components/schemas/Dog",
    "Cat": "#/components/schemas/Cat",
    "Tiger": "#/components/schemas/Cat"
   }
}

The OpenAPI specification allows for n species to be mapped to 1 component type.
This MR has now put the following limitation on the OpenAPI specification, forcing the relation to 1 to 1.
This will break all existing contracts that use n to 1 mapping.

This is described in following BUG.

This project contains a unit test that reproduces the issue.

Can this be reverted/fixed asap?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement: Code Cleanup General refactoring, removal of deprecated things, commenting, etc. Server: Java Server: Spring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants