[Java] Fix object defaults with enum properties generating invalid fluent builder expressions#24299
[Java] Fix object defaults with enum properties generating invalid fluent builder expressions#24299nh-ive wants to merge 4 commits into
Conversation
| defaultPropertyExpression = String.join( | ||
| ".", | ||
| cp.datatypeWithEnum, | ||
| sanitizeName(camelize(key)) + "Enum", |
There was a problem hiding this comment.
This "Enum" looks out-of-place. There are some odd corner-cases where a "Enum"-suffix is added, but that is not the default naming convention AFAIK.
Might stem from not using the title-property, or perhaps because the schema name in #24298 is in lowercase?
There was a problem hiding this comment.
Might be taken from https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java#L1728 or similar. But if that is the case then it should of course be preferable to change it to a named variable (or that the method is used to some extent).
There was a problem hiding this comment.
Updated to use the new toEnumName(String) helper instead of hardcoding the enum suffix
There was a problem hiding this comment.
Great @nh-ive , thanks!
@Mattias-Sehlstedt I thought that suffix was added for edge-cases, like when using reserved keywords? 🤔 But maybe I am mistaken...
There was a problem hiding this comment.
Just from a user perspective, it has always generated with the Enum suffix for me, but I haven't been using it for that long, so I might be missing some historical context
There was a problem hiding this comment.
To my knowledge the Enum suffix comes from when you have inline enum definitions. So when you have
properties:
status:
type: "string"
description: "Order Status"
enum:
- "placed"
- "approved"
- "delivered"rather than
properties:
status:
ref: '#/components/schemas/OrderStatus'I.e., the Codegen will create a name for the enum structure for you.
I personally always prefer to have the enums be defined as refs, so that there are never duplicates, and their reuse is extremely clear.
There was a problem hiding this comment.
Ah, that explains it! I always use $ref, so I didn't consider that - thanks for looking into it!
Okay, should a test-cases be added for both these scenarios, to ensure this is named correctly?
- Inline:
private OutputFormat format = new OutputFormat().order(OutputFormat.OrderEnum.SIMILARITY).limit(10); - Ref:
private OutputFormat format = new OutputFormat().order(OutputFormat.Order.SIMILARITY).limit(10);
Fixes #24298
Added an enum case for the if statement in the method
toObjectDefaultValueof the classAbstractJavaCodegenso the Java fluent builder expression generates correctly when using object defaults on an object with enum properties.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.
Java:
@bbdouglas (2017/07) @sreeshas (2017/08) @jfiala (2017/08) @lukoyanov (2017/09) @cbornet (2017/09) @jeff9finger (2018/01) @karismann (2019/03) @Zomzog (2019/04) @lwlee2608 (2019/10) @martin-mfg (2023/08)
Summary by cubic
Fixes invalid Java fluent builder expressions for object defaults with enum properties by handling enums in
AbstractJavaCodegen#toObjectDefaultValueand resolving enum class names viatoEnumName. Removes a leftover debug print; generated models now reference the correct enum constants and compile.Written for commit 4b87c8c. Summary will update on new commits.