-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
[JAVA] Prioritize mapped discriminators over generated (relates to issue #12777) #15284
[JAVA] Prioritize mapped discriminators over generated (relates to issue #12777) #15284
Conversation
@wing328 Could you have a look at this change? (the failed check is inherited from the issue in master with the python-nextgen client and is not related) |
Helpful fix |
@@ -128,7 +128,13 @@ public int compareTo(MappedModel other) { | |||
} else if (other.getMappingName() == null) { | |||
return -1; | |||
} | |||
return getMappingName().compareTo(other.getMappingName()); | |||
|
|||
final boolean mappedEqualsModelName = getMappingName().equals(getModelName()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you please add a line or two explaining the change/code block?
does it aim to maintain the same order in the spec?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added the comment and rebased the branch with current master @wing328
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rebased branch to deal with removed python samples
6d5ac19
to
c251fb7
Compare
c251fb7
to
e7e2914
Compare
Thank you! This is a really helpful fix for us! |
Please merge it before 7.0.1 release, we really like this feature. :) We've just faced with this issue... |
Can someone please share a spec to reproduce the issue? I tested with the one in #12777 (comment) but the output remains the same with this patch/PR. |
In the original issue different people actually refer to different issues. This PR solves the most generic of those. Namely that objects are serialized using the proper value as per the key in the discriminator spec. I don't have time to test it right now, but I believe this spec shows the problem
The resulting interface "ParentChild.java" should look like this with this patch:
whereas without it the ordering will be different, resulting in jackson picking the wrong values during serialization. |
FYI @OpenAPITools/generator-core-team |
…sue OpenAPITools#12777) (OpenAPITools#15284) * prioritize mapped discriminators over generated * update samples with new ordering * explain reason behind discriminator prioritization * add new samples * prioritize explicit mapping over any generated mappings * update examples to reflect new logic * update tests to reflect explicit mappings
This PR introduces a slightly smarter ordering of the discriminators which prioritizes actual non-generated mappings as defined in the original spec above others as suggested here #12777 (comment).
The result is that the correct value is selected to populate the discriminator field during serialization based on the actual discriminator mapping is available in the schema.
PR checklist
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*
.For Windows users, please run the script in Git BASH.
master
(6.3.0) (minor release - breaking changes with fallbacks),7.0.x
(breaking changes without fallbacks)