refactor(enum-utils): add utility methods for simplified enum schema creation#24353
Merged
Merged
Conversation
Member
it's ok as this change should have not impacted to the output (e.g. auto-generated SDKs, etc) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Enum handling has over the years evolved with how they are constructed and interpreted. From just being a String value all the time, to adding better support for numbers, and later that they can be decorated better with both
descriptionanddeprecated. They also support "defaults" for the cases where one might want to use an enum to express a limited value space, but to also capture that it might not be entirely static (i.e., it might happen that it is extended/shrunk, which is a breaking change).It is now so complex that I would argue that they are also a suitable candidate for its own
Utilsclass. This is inspired by my comment on a recent upgrade to the enum handling.If this is considered helpful, then I can also investigate moving further enum logic that resides within the now quite large
OpenAPINormalizerto the util class. Once all functionality has been moved, then it would be suitable to investigate creating aCodegenEnumclass (as mentioned in the PR thread too) that more formally express and carry the different enum settings that the project supports (similar to what exists for theDiscriminatornow for example).This is a minor breaking change to the
OpenAPINormalizersince its methodcreateSimplifiedEnumSchemawill change slightly with
Map<Object, String> enumValuesbecomingMap<Object, EnumUtils.EnumExtensions> enumExtensions.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.
Summary by cubic
Introduces
EnumUtilsto centralize enum schema creation and refactorsOpenAPINormalizerto use it. AddsCodegenConstants.X_ENUM_DEPRECATED; behavior stays the same.Refactors
EnumUtils.createSimplifiedEnumSchema(...)andEnumExtensionsto handle descriptions and deprecations.OpenAPINormalizer#createSimplifiedEnumSchemanow delegates toEnumUtils.CodegenConstants.X_ENUM_DEPRECATEDfor thex-enum-deprecatedextension.Migration
OpenAPINormalizer:createSimplifiedEnumSchema(Schema, Map<Object, String>, Map<Object, Boolean>, String, String)createSimplifiedEnumSchema(Schema, Map<Object, EnumUtils.EnumExtensions>, String, String)Map<Object, EnumUtils.EnumExtensions>and build entries withnew EnumUtils.EnumExtensions(description, deprecated).Written for commit 0e52d13. Summary will update on new commits.