[JAVA] [PERF] improve java enum fromValue generation#22995
Draft
davidricodias wants to merge 8 commits intoOpenAPITools:masterfrom
Draft
[JAVA] [PERF] improve java enum fromValue generation#22995davidricodias wants to merge 8 commits intoOpenAPITools:masterfrom
davidricodias wants to merge 8 commits intoOpenAPITools:masterfrom
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
9 issues found across 190 files
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/openapi-generator/src/main/resources/JavaJaxRS/modelEnum.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/JavaJaxRS/modelEnum.mustache:58">
P2: `useEnumCaseInsensitive` now calls `toLowerCase` on any enum value type, which breaks compilation for non-String enums when the flag is enabled.</violation>
<violation number="2" location="modules/openapi-generator/src/main/resources/JavaJaxRS/modelEnum.mustache:58">
P2: Case-insensitive lookup lowercases the input but BY_VALUE is populated with original-cased keys, so valid enum values with uppercase characters will not resolve when useEnumCaseInsensitive is enabled.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/Java/libraries/native/modelEnum.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/Java/libraries/native/modelEnum.mustache:52">
P2: Case-insensitive enum lookup lowercases the input but the BY_VALUE map is populated with original casing, so mixed-case enum values will not be found when useEnumCaseInsensitive is enabled.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/modelEnum.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/modelEnum.mustache:41">
P2: Case-insensitive lookup lowercases the input but the BY_VALUE map is populated with unnormalized keys, so enums with uppercase values will not be found.</violation>
<violation number="2" location="modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/modelEnum.mustache:59">
P2: useEnumCaseInsensitive applies to all enums, but this template always calls value.toLowerCase(...) without checking isString, causing compilation errors for non-String enum value types.</violation>
</file>
<file name="perf/jmh/src/main/java/org/openapitools/perf/EnumLookupJmhBenchmark.java">
<violation number="1" location="perf/jmh/src/main/java/org/openapitools/perf/EnumLookupJmhBenchmark.java:2">
P2: The new benchmark file is marked as “MUST be deleted before merging,” but it is being added in this PR. Either remove the file before merge or update the comment to reflect that it is intended to stay, to avoid misleading maintainers.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/Java/modelEnum.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/Java/modelEnum.mustache:52">
P2: Case-insensitive lookup lowercases the input but the BY_VALUE map is populated with unnormalized keys, so enums with uppercase/mixed-case values will no longer resolve.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/java-helidon/server/libraries/mp/modelEnum.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/java-helidon/server/libraries/mp/modelEnum.mustache:42">
P2: Case-insensitive lookup lowercases the input, but BY_VALUE is populated with original values, so mixed/upper-case enum values won't be found when useEnumCaseInsensitive is enabled.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/java-micronaut/common/model/modelEnum.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/java-micronaut/common/model/modelEnum.mustache:40">
P2: Case-insensitive deserialization lowercases the input but the BY_VALUE map is populated with original casing, so mixed-case enum values won't resolve.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
| } | ||
| {{#isNullable}}return null;{{/isNullable}}{{^isNullable}}throw new IllegalArgumentException("Unexpected value '" + value + "'");{{/isNullable}} | ||
| return Optional.ofNullable({{#useEnumCaseInsensitive}}value == null ? null : BY_VALUE.get(value.toLowerCase(Locale.ROOT)){{/useEnumCaseInsensitive}}{{^useEnumCaseInsensitive}}BY_VALUE.get(value){{/useEnumCaseInsensitive}}).{{#isNullable}}orElse(null){{/isNullable}}{{^isNullable}}{{#enumUnknownDefaultCase}}{{#allowableValues}}{{#enumVars}}{{#-last}}orElse({{{name}}}){{/-last}}{{/enumVars}}{{/allowableValues}}{{/enumUnknownDefaultCase}}{{^enumUnknownDefaultCase}}orElseThrow(() -> new IllegalArgumentException("Unexpected value '" + value + "'")){{/enumUnknownDefaultCase}}{{/isNullable}}; |
Contributor
There was a problem hiding this comment.
P2: useEnumCaseInsensitive now calls toLowerCase on any enum value type, which breaks compilation for non-String enums when the flag is enabled.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/JavaJaxRS/modelEnum.mustache, line 58:
<comment>`useEnumCaseInsensitive` now calls `toLowerCase` on any enum value type, which breaks compilation for non-String enums when the flag is enabled.</comment>
<file context>
@@ -43,12 +55,7 @@ import com.fasterxml.jackson.annotation.JsonValue;
- }
- }
- {{#isNullable}}return null;{{/isNullable}}{{^isNullable}}throw new IllegalArgumentException("Unexpected value '" + value + "'");{{/isNullable}}
+ return Optional.ofNullable({{#useEnumCaseInsensitive}}value == null ? null : BY_VALUE.get(value.toLowerCase(Locale.ROOT)){{/useEnumCaseInsensitive}}{{^useEnumCaseInsensitive}}BY_VALUE.get(value){{/useEnumCaseInsensitive}}).{{#isNullable}}orElse(null){{/isNullable}}{{^isNullable}}{{#enumUnknownDefaultCase}}{{#allowableValues}}{{#enumVars}}{{#-last}}orElse({{{name}}}){{/-last}}{{/enumVars}}{{/allowableValues}}{{/enumUnknownDefaultCase}}{{^enumUnknownDefaultCase}}orElseThrow(() -> new IllegalArgumentException("Unexpected value '" + value + "'")){{/enumUnknownDefaultCase}}{{/isNullable}};
}
{{/jackson}}
</file context>
Suggested change
| return Optional.ofNullable({{#useEnumCaseInsensitive}}value == null ? null : BY_VALUE.get(value.toLowerCase(Locale.ROOT)){{/useEnumCaseInsensitive}}{{^useEnumCaseInsensitive}}BY_VALUE.get(value){{/useEnumCaseInsensitive}}).{{#isNullable}}orElse(null){{/isNullable}}{{^isNullable}}{{#enumUnknownDefaultCase}}{{#allowableValues}}{{#enumVars}}{{#-last}}orElse({{{name}}}){{/-last}}{{/enumVars}}{{/allowableValues}}{{/enumUnknownDefaultCase}}{{^enumUnknownDefaultCase}}orElseThrow(() -> new IllegalArgumentException("Unexpected value '" + value + "'")){{/enumUnknownDefaultCase}}{{/isNullable}}; | |
| return Optional.ofNullable({{#useEnumCaseInsensitive}}{{#isString}}value == null ? null : BY_VALUE.get(value.toLowerCase(Locale.ROOT)){{/isString}}{{^isString}}BY_VALUE.get(value){{/isString}}{{/useEnumCaseInsensitive}}{{^useEnumCaseInsensitive}}BY_VALUE.get(value){{/useEnumCaseInsensitive}}).{{#isNullable}}orElse(null){{/isNullable}}{{^isNullable}}{{#enumUnknownDefaultCase}}{{#allowableValues}}{{#enumVars}}{{#-last}}orElse({{{name}}}){{/-last}}{{/enumVars}}{{/allowableValues}}{{/enumUnknownDefaultCase}}{{^enumUnknownDefaultCase}}orElseThrow(() -> new IllegalArgumentException("Unexpected value '" + value + "'")){{/enumUnknownDefaultCase}}{{/isNullable}}; |
| @@ -2,6 +2,10 @@ | |||
| import com.fasterxml.jackson.annotation.JsonCreator; | |||
Contributor
There was a problem hiding this comment.
P2: Case-insensitive lookup lowercases the input but BY_VALUE is populated with original-cased keys, so valid enum values with uppercase characters will not resolve when useEnumCaseInsensitive is enabled.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/JavaJaxRS/modelEnum.mustache, line 58:
<comment>Case-insensitive lookup lowercases the input but BY_VALUE is populated with original-cased keys, so valid enum values with uppercase characters will not resolve when useEnumCaseInsensitive is enabled.</comment>
<file context>
@@ -43,12 +55,7 @@ import com.fasterxml.jackson.annotation.JsonValue;
- }
- }
- {{#isNullable}}return null;{{/isNullable}}{{^isNullable}}throw new IllegalArgumentException("Unexpected value '" + value + "'");{{/isNullable}}
+ return Optional.ofNullable({{#useEnumCaseInsensitive}}value == null ? null : BY_VALUE.get(value.toLowerCase(Locale.ROOT)){{/useEnumCaseInsensitive}}{{^useEnumCaseInsensitive}}BY_VALUE.get(value){{/useEnumCaseInsensitive}}).{{#isNullable}}orElse(null){{/isNullable}}{{^isNullable}}{{#enumUnknownDefaultCase}}{{#allowableValues}}{{#enumVars}}{{#-last}}orElse({{{name}}}){{/-last}}{{/enumVars}}{{/allowableValues}}{{/enumUnknownDefaultCase}}{{^enumUnknownDefaultCase}}orElseThrow(() -> new IllegalArgumentException("Unexpected value '" + value + "'")){{/enumUnknownDefaultCase}}{{/isNullable}};
}
{{/jackson}}
</file context>
|
|
||
| static { | ||
| for ({{{datatypeWithEnum}}}{{^datatypeWithEnum}}{{{classname}}}{{/datatypeWithEnum}} e: values()) { | ||
| BY_VALUE.put(e.value, e); |
Contributor
There was a problem hiding this comment.
P2: Case-insensitive enum lookup lowercases the input but the BY_VALUE map is populated with original casing, so mixed-case enum values will not be found when useEnumCaseInsensitive is enabled.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/Java/libraries/native/modelEnum.mustache, line 52:
<comment>Case-insensitive enum lookup lowercases the input but the BY_VALUE map is populated with original casing, so mixed-case enum values will not be found when useEnumCaseInsensitive is enabled.</comment>
<file context>
@@ -39,8 +43,16 @@ import java.net.URI;
+ static {
+ for ({{{datatypeWithEnum}}}{{^datatypeWithEnum}}{{{classname}}}{{/datatypeWithEnum}} e: values()) {
+ BY_VALUE.put(e.value, e);
+ }
+ }
</file context>
Suggested change
| BY_VALUE.put(e.value, e); | |
| BY_VALUE.put({{#useEnumCaseInsensitive}}e.value.toLowerCase(Locale.ROOT){{/useEnumCaseInsensitive}}{{^useEnumCaseInsensitive}}e.value{{/useEnumCaseInsensitive}}, e); |
|
|
||
| static { | ||
| for ({{{datatypeWithEnum}}}{{^datatypeWithEnum}}{{{classname}}}{{/datatypeWithEnum}} e: values()) { | ||
| BY_VALUE.put(e.value, e); |
Contributor
There was a problem hiding this comment.
P2: Case-insensitive lookup lowercases the input but the BY_VALUE map is populated with unnormalized keys, so enums with uppercase values will not be found.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/modelEnum.mustache, line 41:
<comment>Case-insensitive lookup lowercases the input but the BY_VALUE map is populated with unnormalized keys, so enums with uppercase values will not be found.</comment>
<file context>
@@ -28,8 +32,16 @@ import com.google.gson.stream.JsonWriter;
+ static {
+ for ({{{datatypeWithEnum}}}{{^datatypeWithEnum}}{{{classname}}}{{/datatypeWithEnum}} e: values()) {
+ BY_VALUE.put(e.value, e);
+ }
+ }
</file context>
| } | ||
| } | ||
| {{#isNullable}}return null;{{/isNullable}}{{^isNullable}}{{#enumUnknownDefaultCase}}{{#allowableValues}}{{#enumVars}}{{#-last}}return {{{name}}};{{/-last}}{{/enumVars}}{{/allowableValues}}{{/enumUnknownDefaultCase}}{{^enumUnknownDefaultCase}}throw new IllegalArgumentException("Unexpected value '" + value + "'");{{/enumUnknownDefaultCase}}{{/isNullable}} | ||
| return Optional.ofNullable({{#useEnumCaseInsensitive}}value == null ? null : BY_VALUE.get(value.toLowerCase(Locale.ROOT)){{/useEnumCaseInsensitive}}{{^useEnumCaseInsensitive}}BY_VALUE.get(value){{/useEnumCaseInsensitive}}).{{#isNullable}}orElse(null){{/isNullable}}{{^isNullable}}{{#enumUnknownDefaultCase}}{{#allowableValues}}{{#enumVars}}{{#-last}}orElse({{{name}}}){{/-last}}{{/enumVars}}{{/allowableValues}}{{/enumUnknownDefaultCase}}{{^enumUnknownDefaultCase}}orElseThrow(() -> new IllegalArgumentException("Unexpected value '" + value + "'")){{/enumUnknownDefaultCase}}{{/isNullable}}; |
Contributor
There was a problem hiding this comment.
P2: useEnumCaseInsensitive applies to all enums, but this template always calls value.toLowerCase(...) without checking isString, causing compilation errors for non-String enum value types.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/modelEnum.mustache, line 59:
<comment>useEnumCaseInsensitive applies to all enums, but this template always calls value.toLowerCase(...) without checking isString, causing compilation errors for non-String enum value types.</comment>
<file context>
@@ -44,12 +56,7 @@ import com.google.gson.stream.JsonWriter;
- }
- }
- {{#isNullable}}return null;{{/isNullable}}{{^isNullable}}{{#enumUnknownDefaultCase}}{{#allowableValues}}{{#enumVars}}{{#-last}}return {{{name}}};{{/-last}}{{/enumVars}}{{/allowableValues}}{{/enumUnknownDefaultCase}}{{^enumUnknownDefaultCase}}throw new IllegalArgumentException("Unexpected value '" + value + "'");{{/enumUnknownDefaultCase}}{{/isNullable}}
+ return Optional.ofNullable({{#useEnumCaseInsensitive}}value == null ? null : BY_VALUE.get(value.toLowerCase(Locale.ROOT)){{/useEnumCaseInsensitive}}{{^useEnumCaseInsensitive}}BY_VALUE.get(value){{/useEnumCaseInsensitive}}).{{#isNullable}}orElse(null){{/isNullable}}{{^isNullable}}{{#enumUnknownDefaultCase}}{{#allowableValues}}{{#enumVars}}{{#-last}}orElse({{{name}}}){{/-last}}{{/enumVars}}{{/allowableValues}}{{/enumUnknownDefaultCase}}{{^enumUnknownDefaultCase}}orElseThrow(() -> new IllegalArgumentException("Unexpected value '" + value + "'")){{/enumUnknownDefaultCase}}{{/isNullable}};
}
</file context>
Suggested change
| return Optional.ofNullable({{#useEnumCaseInsensitive}}value == null ? null : BY_VALUE.get(value.toLowerCase(Locale.ROOT)){{/useEnumCaseInsensitive}}{{^useEnumCaseInsensitive}}BY_VALUE.get(value){{/useEnumCaseInsensitive}}).{{#isNullable}}orElse(null){{/isNullable}}{{^isNullable}}{{#enumUnknownDefaultCase}}{{#allowableValues}}{{#enumVars}}{{#-last}}orElse({{{name}}}){{/-last}}{{/enumVars}}{{/allowableValues}}{{/enumUnknownDefaultCase}}{{^enumUnknownDefaultCase}}orElseThrow(() -> new IllegalArgumentException("Unexpected value '" + value + "'")){{/enumUnknownDefaultCase}}{{/isNullable}}; | |
| return Optional.ofNullable({{#useEnumCaseInsensitive}}{{#isString}}value == null ? null : BY_VALUE.get(value.toLowerCase(Locale.ROOT)){{/isString}}{{^isString}}BY_VALUE.get(value){{/isString}}{{/useEnumCaseInsensitive}}{{^useEnumCaseInsensitive}}BY_VALUE.get(value){{/useEnumCaseInsensitive}}).{{#isNullable}}orElse(null){{/isNullable}}{{^isNullable}}{{#enumUnknownDefaultCase}}{{#allowableValues}}{{#enumVars}}{{#-last}}orElse({{{name}}}){{/-last}}{{/enumVars}}{{/allowableValues}}{{/enumUnknownDefaultCase}}{{^enumUnknownDefaultCase}}orElseThrow(() -> new IllegalArgumentException("Unexpected value '" + value + "'")){{/enumUnknownDefaultCase}}{{/isNullable}}; |
| @@ -0,0 +1,235 @@ | |||
| /** | |||
| * This is a performance benchmark that MUST be deleted before merging, as it is not intended to be part of the codebase. | |||
Contributor
There was a problem hiding this comment.
P2: The new benchmark file is marked as “MUST be deleted before merging,” but it is being added in this PR. Either remove the file before merge or update the comment to reflect that it is intended to stay, to avoid misleading maintainers.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At perf/jmh/src/main/java/org/openapitools/perf/EnumLookupJmhBenchmark.java, line 2:
<comment>The new benchmark file is marked as “MUST be deleted before merging,” but it is being added in this PR. Either remove the file before merge or update the comment to reflect that it is intended to stay, to avoid misleading maintainers.</comment>
<file context>
@@ -0,0 +1,235 @@
+/**
+ * This is a performance benchmark that MUST be deleted before merging, as it is not intended to be part of the codebase.
+ * It is only meant to be used for local testing and experimentation while the PR is on the fly.
+ * The benchmark compares the performance of different approaches to look up enum values by their string representation, including:
</file context>
|
|
||
| static { | ||
| for ({{{datatypeWithEnum}}}{{^datatypeWithEnum}}{{{classname}}}{{/datatypeWithEnum}} e: values()) { | ||
| BY_VALUE.put(e.value, e); |
Contributor
There was a problem hiding this comment.
P2: Case-insensitive lookup lowercases the input but the BY_VALUE map is populated with unnormalized keys, so enums with uppercase/mixed-case values will no longer resolve.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/Java/modelEnum.mustache, line 52:
<comment>Case-insensitive lookup lowercases the input but the BY_VALUE map is populated with unnormalized keys, so enums with uppercase/mixed-case values will no longer resolve.</comment>
<file context>
@@ -39,8 +43,16 @@ import java.net.URI;
+ static {
+ for ({{{datatypeWithEnum}}}{{^datatypeWithEnum}}{{{classname}}}{{/datatypeWithEnum}} e: values()) {
+ BY_VALUE.put(e.value, e);
+ }
+ }
</file context>
Suggested change
| BY_VALUE.put(e.value, e); | |
| BY_VALUE.put({{#useEnumCaseInsensitive}}e.value.toLowerCase(Locale.ROOT){{/useEnumCaseInsensitive}}{{^useEnumCaseInsensitive}}e.value{{/useEnumCaseInsensitive}}, e); |
|
|
||
| static { | ||
| for ({{{datatypeWithEnum}}}{{^datatypeWithEnum}}{{classname}}{{/datatypeWithEnum}} e: values()) { | ||
| BY_VALUE.put(e.value, e); |
Contributor
There was a problem hiding this comment.
P2: Case-insensitive lookup lowercases the input, but BY_VALUE is populated with original values, so mixed/upper-case enum values won't be found when useEnumCaseInsensitive is enabled.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/java-helidon/server/libraries/mp/modelEnum.mustache, line 42:
<comment>Case-insensitive lookup lowercases the input, but BY_VALUE is populated with original values, so mixed/upper-case enum values won't be found when useEnumCaseInsensitive is enabled.</comment>
<file context>
@@ -28,8 +33,16 @@
+ static {
+ for ({{{datatypeWithEnum}}}{{^datatypeWithEnum}}{{classname}}{{/datatypeWithEnum}} e: values()) {
+ BY_VALUE.put(e.value, e);
+ }
+ }
</file context>
Contributor
There was a problem hiding this comment.
Yep, it does not respect the case. This only works for exact equals.
Contributor
There was a problem hiding this comment.
Oh, actually, I don't think it will be possible, at least not with some workaround. What if EXAMPLE_1("Example") and EXAMPLE_2("example") exists, there will be a key conflict when populating the Map after making the value lowercase...
|
|
||
| static { | ||
| for ({{#datatypeWithEnum}}{{{.}}}{{/datatypeWithEnum}}{{^datatypeWithEnum}}{{{classname}}}{{/datatypeWithEnum}} e: values()) { | ||
| BY_VALUE.put(e.value, e); |
Contributor
There was a problem hiding this comment.
P2: Case-insensitive deserialization lowercases the input but the BY_VALUE map is populated with original casing, so mixed-case enum values won't resolve.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/java-micronaut/common/model/modelEnum.mustache, line 40:
<comment>Case-insensitive deserialization lowercases the input but the BY_VALUE map is populated with original casing, so mixed-case enum values won't resolve.</comment>
<file context>
@@ -27,8 +31,16 @@ import com.fasterxml.jackson.annotation.JsonValue;
+ static {
+ for ({{#datatypeWithEnum}}{{{.}}}{{/datatypeWithEnum}}{{^datatypeWithEnum}}{{{classname}}}{{/datatypeWithEnum}} e: values()) {
+ BY_VALUE.put(e.value, e);
+ }
+ }
</file context>
davidricodias
changed the title
Chrimle
reviewed
Feb 17, 2026
| } | ||
| } | ||
| {{#isNullable}}return null;{{/isNullable}}{{^isNullable}}throw new IllegalArgumentException("Unexpected value '" + value + "'");{{/isNullable}} | ||
| return Optional.ofNullable({{#useEnumCaseInsensitive}}value == null ? null : BY_VALUE.get(value.toLowerCase(Locale.ROOT)){{/useEnumCaseInsensitive}}{{^useEnumCaseInsensitive}}BY_VALUE.get(value){{/useEnumCaseInsensitive}}).{{#isNullable}}orElse(null){{/isNullable}}{{^isNullable}}{{#enumUnknownDefaultCase}}{{#allowableValues}}{{#enumVars}}{{#-last}}orElse({{{name}}}){{/-last}}{{/enumVars}}{{/allowableValues}}{{/enumUnknownDefaultCase}}{{^enumUnknownDefaultCase}}orElseThrow(() -> new IllegalArgumentException("Unexpected value '" + value + "'")){{/enumUnknownDefaultCase}}{{/isNullable}}; |
Contributor
There was a problem hiding this comment.
Why value == null ? null : BY_VALUE.get(value.toLowerCase(Locale.ROOT)), when it's already in an Optional-stream?
Contributor
There was a problem hiding this comment.
What happens if the value is non-String, and the flag useEnumCaseInsensitive is true?
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
2 participants
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.
Description
This PR changes the way the java modelEnum.mustache templates generate the enums. The change is to introduce a HashMap that its filled during the static phase of the enum initialization, and is kept filled through the lifetime of the object.
The files that need revision are the modelEnum.mustache files.
This is my first PR on this project, so I kindly ask you some patience if I am missing any steps on your PR process :)
Thread safety
The initialization phase is done at
staticinit time. No need for barriers.The lookup is not hot. No need for barriers for the
get.Test
Unit
Two client codegen test have been updated to now be explicit about the use of
toLowerCase(Locale.ROOT).Perf
I have introduced in this PR a JMH benchmark for you to checkout the distinct ways i approached this perf. The benchmark class is at org.openapitools.perf.EnumLookupJmhBenchmark. This benchmark suite must be deleted before merge, as is there to provide a basis for discussion.
The results are
on the case sensitive case, a
x4speedup. On the case insensitive case a x2 speedup.The benchmark can be run with
./mvnw -f perf/jmh/pom.xml -q clean package && java -jar perf/jmh/target/benchmarks.jar EnumLookupJmhBenchmarkCloses #22995
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(upcoming7.x.0minor release - breaking changes with fallbacks),8.0.x(breaking changes without fallbacks)"fixes #123"present in the PR description)@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)