[BUG] [JAVA] validateJsonElement fails for required nullable fields#22912
[BUG] [JAVA] validateJsonElement fails for required nullable fields#22912wing328 merged 10 commits intoOpenAPITools:masterfrom
Conversation
There was a problem hiding this comment.
3 issues found across 18 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/pojo.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/pojo.mustache:389">
P2: Required nullable model arrays can still throw on JSON null: the new condition allows JsonNull to pass, but getAsJsonArray is still called unconditionally, which throws IllegalStateException on JsonNull.</violation>
</file>
<file name="samples/client/petstore/java/okhttp-gson/src/main/java/org/openapitools/client/model/RequiredNullableBody.java">
<violation number="1" location="samples/client/petstore/java/okhttp-gson/src/main/java/org/openapitools/client/model/RequiredNullableBody.java:172">
P2: Enum validation does not reject unknown values; invalid `custom_enum` strings pass validation and deserialize to null, defeating required enum validation.</violation>
</file>
<file name="samples/client/petstore/java/okhttp-gson/src/main/java/org/openapitools/client/model/NullableEnum.java">
<violation number="1" location="samples/client/petstore/java/okhttp-gson/src/main/java/org/openapitools/client/model/NullableEnum.java:73">
P2: validateJsonElement ignores the NullableEnum.fromValue result; invalid enum strings return null and are accepted without error, so enum validation is ineffective for this type.</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
- 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.
modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/pojo.mustache
Outdated
Show resolved
Hide resolved
|
|
||
| public static void validateJsonElement(JsonElement jsonElement) throws IOException { | ||
| String value = jsonElement.getAsString(); | ||
| CustomEnumEnum.fromValue(value); |
There was a problem hiding this comment.
P2: Enum validation does not reject unknown values; invalid custom_enum strings pass validation and deserialize to null, defeating required enum validation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/java/okhttp-gson/src/main/java/org/openapitools/client/model/RequiredNullableBody.java, line 172:
<comment>Enum validation does not reject unknown values; invalid `custom_enum` strings pass validation and deserialize to null, defeating required enum validation.</comment>
<file context>
@@ -0,0 +1,766 @@
+
+ public static void validateJsonElement(JsonElement jsonElement) throws IOException {
+ String value = jsonElement.getAsString();
+ CustomEnumEnum.fromValue(value);
+ }
+ }
</file context>
There was a problem hiding this comment.
The validation of enums and the related template modelInnerEnum.mustache were not changed in this PR. So this was already an issue before this PR and this PR doesn't aim to fix the validation of enums
There was a problem hiding this comment.
@cubic-dev-ai can this comment be considered as solved?
There was a problem hiding this comment.
@marceljk Looking at the conversation history and the code, I can see that you've correctly identified that enum validation was not changed in this PR and was a pre-existing issue.
The comment I raised was about the validateJsonElement method for CustomEnumEnum (line 172), which calls fromValue() but doesn't throw an exception when an invalid enum value is encountered—it just returns null. This means invalid enum strings pass validation silently.
However, since:
- This enum validation behavior existed before your PR
- Your PR specifically targets the nullable + required field validation issue ([BUG] [JAVA] validateJsonElement failed when a field is decorated with both nullable and required #18516)
- You've explicitly stated this PR doesn't aim to fix enum validation
- The template
modelInnerEnum.mustachewasn't modified in this PR
Yes, this comment can be considered resolved/not applicable to this PR's scope. The enum validation issue is a separate concern that should be tracked and fixed independently if desired. Your PR successfully addresses the original bug about required nullable fields failing validation, which is what matters here.
If you want to track the enum validation issue separately, it would need changes to the modelInnerEnum.mustache template to make validateJsonElement throw an exception when fromValue() returns null for invalid values.
|
|
||
| public static void validateJsonElement(JsonElement jsonElement) throws IOException { | ||
| String value = jsonElement.getAsString(); | ||
| NullableEnum.fromValue(value); |
There was a problem hiding this comment.
P2: validateJsonElement ignores the NullableEnum.fromValue result; invalid enum strings return null and are accepted without error, so enum validation is ineffective for this type.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/java/okhttp-gson/src/main/java/org/openapitools/client/model/NullableEnum.java, line 73:
<comment>validateJsonElement ignores the NullableEnum.fromValue result; invalid enum strings return null and are accepted without error, so enum validation is ineffective for this type.</comment>
<file context>
@@ -0,0 +1,76 @@
+
+ public static void validateJsonElement(JsonElement jsonElement) throws IOException {
+ String value = jsonElement.getAsString();
+ NullableEnum.fromValue(value);
+ }
+}
</file context>
There was a problem hiding this comment.
The validation of enums and the related template modelInnerEnum.mustache were not changed in this PR. So this was already an issue before this PR and this PR doesn't aim to fix the validation of enums
|
@marceljk thanks for creating this PR can you please have a look at the feedback/review by cubic-dev-ai? (not that we must address these in this PR...) |
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/pojo.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/pojo.mustache:388">
P2: Required non-nullable arrays of models can now be set to JSON null without error because validation is skipped for JsonNull, allowing invalid payloads to pass.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/pojo.mustache
Outdated
Show resolved
Hide resolved
|
@wing328 I had a look to the feedback of cubic-dev-ai and solved or replied to the comments. |
|
can you please review the build failure? |
|
solved the failing builds |
|
thanks for your contribution. let's give it a try |
This PR supersedes #18518.
fixes the issues with required nullable fields #18516
Credits to @fanqiewanzi for the original pull request
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 @sreeshas @jfiala @lukoyanov @cbornet @jeff9finger @karismann @Zomzog @lwlee2608 @martin-mfg
Summary by cubic
Fixes #18516 by updating the Java okhttp-gson generator to accept null for fields that are both required and nullable, preventing false validation errors in validateJsonElement.
Written for commit 87d9898. Summary will update on new commits.