Fix issue 18889,15274 - configOptions to not remove enum prefix and oneOf subtypes annotations example - #18998
Fix issue 18889,15274 - configOptions to not remove enum prefix and oneOf subtypes annotations example#18998rodrigoma3 wants to merge 6 commits into
Conversation
| removeEnumValuePrefixOpts.put("false", | ||
| "No changes to the enum's are made."); | ||
| removeEnumValuePrefixOpts.put("true", | ||
| "With this option disabled, each enum will have the common prefix between them removed. This is the default option."); |
There was a problem hiding this comment.
Not: disabled -> enabled? Since this is when it's true?
There was a problem hiding this comment.
I did not understand your question. This option was implemented by someone else and that's how it ended up, I just wrote what happens with each option.
There was a problem hiding this comment.
I think it's with the option enabled, each enum will have the common prefix between them removed.
There was a problem hiding this comment.
Exactly. It was done like this. But it's been like this since Feb 8, 2020. PR link merged #5166
| = "false"; | ||
| public static final String PREPEND_FORM_OR_BODY_PARAMETERS_VALUE = "true"; | ||
| public static final String ENUM_UNKNOWN_DEFAULT_CASE_VALUE = "false"; | ||
| public static final String REMOVE_ENUM_VALUE_PREFIX_VALUE = "true"; |
There was a problem hiding this comment.
Why are you adding this here if you're referencing the default codegen key?
My understanding is that these are for generator specific keys (which you aren't using)
There was a problem hiding this comment.
It was breaking the tests saying that this new property needed to be added. I just followed the ENUM_UNKNOWN_DEFAULT_CASE_VALUE example to solve the test.
There was a problem hiding this comment.
Did this need to be defined here to just added to the builder below using .put(CodegenConstants.REMOVE_ENUM_VALUE_PREFIX, REMOVE_ENUM_VALUE_PREFIX_VALUE)?
This definition (BashClientCodegen.REMOVE_ENUM_VALUE_PREFIX_VALUE) isn't being used or referenced anywhere
Which tests was failing?
| .put(CodegenConstants.LEGACY_DISCRIMINATOR_BEHAVIOR, "true") | ||
| .put(CodegenConstants.DISALLOW_ADDITIONAL_PROPERTIES_IF_NOT_PRESENT, "true") | ||
| .put(CodegenConstants.ENUM_UNKNOWN_DEFAULT_CASE, ENUM_UNKNOWN_DEFAULT_CASE_VALUE) | ||
| .put(CodegenConstants.REMOVE_ENUM_VALUE_PREFIX, REMOVE_ENUM_VALUE_PREFIX_VALUE) |
There was a problem hiding this comment.
Did none of the samples change for any of these generators when you added the option?
Do they properly support this option?
There was a problem hiding this comment.
This new option was implemented in DefaultCodegen and has been in use for a long time.
My intention here is not to develop a new feature but to make its configuration available via configOptions so that devs can find this option more easily.
I just received test errors and resolved them.
welshm
left a comment
There was a problem hiding this comment.
Looks good to me - although ideally we don't need to replicate each option in each sub-generator and can just refer to the global option
| = "false"; | ||
| public static final String PREPEND_FORM_OR_BODY_PARAMETERS_VALUE = "true"; | ||
| public static final String ENUM_UNKNOWN_DEFAULT_CASE_VALUE = "false"; | ||
| public static final String REMOVE_ENUM_VALUE_PREFIX_VALUE = "true"; |
There was a problem hiding this comment.
Did this need to be defined here to just added to the builder below using .put(CodegenConstants.REMOVE_ENUM_VALUE_PREFIX, REMOVE_ENUM_VALUE_PREFIX_VALUE)?
This definition (BashClientCodegen.REMOVE_ENUM_VALUE_PREFIX_VALUE) isn't being used or referenced anywhere
Which tests was failing?
I agree. However, the project is very big and I'm still getting to know it. For now, I will continue as is being done as I believe it is safer. In the future maybe I will be a little more daring. |

An option was implemented to not remove the common prefix between enums, but it was not available in the documentation and does not work by adding it to configOptions.
I just added it to cliOptions so that it appears in the documentation and works through configOptions in the pom.
merged: [feature] Add option to disable stripping of common prefix enum #5166
fix [BUG][JAVA] Enum not generated correctly for contents having common prefix #18889
I created a test to show how using oneOf with mapping works without duplicating class mappings.
fix [BUG][Spring] oneOf keyword generates wrong subtype annotations. #15274
example
PR checklist
Commit all changed files.
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*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
master(upcoming 7.6.0 minor release - breaking changes with fallbacks),8.0.x(breaking changes without fallbacks)@cachescrubber (2022/02) @welshm (2022/02) @MelleD (2022/02) @atextor (2022/02) @manedev79 (2022/02) @javisst (2022/02) @borsch (2022/02) @banlevente (2022/02) @Zomzog (2022/09) @wing328