[Java][jersey3] Add error entity deserialization to ApiException#23542
[Java][jersey3] Add error entity deserialization to ApiException#23542Chhida wants to merge 15 commits intoOpenAPITools:masterfrom
Conversation
- Add errorEntity field and getErrorEntity() method to ApiException - Add deserializeErrorEntity method to ApiClient - Pass errorTypes map from API methods to invokeAPI - Enables automatic deserialization of error response bodies - Fixes OpenAPITools#4777
- Add JavaJersey3ErrorEntityTest with 8 test cases - Tests verify template changes for errorEntity feature - 642 Java tests passed - no regressions - Fixes OpenAPITools#4777
The test was using String(byte[]) without specifying charset, which is flagged by forbiddenapis as using the default charset.
|
Thanks for the PR but your commit (as shown in the Commits tab) is not linked to your Github account, which means this PR won't count as your contribution in https://github.com/OpenAPITools/openapi-generator/graphs/contributors. Let me know if you need help fixing it. |
|
please follow step 3 to update the samples as well so that CI can verify the change |
- Regenerate samples to verify templates work correctly - ApiException now contains errorEntity and getErrorEntity() - API methods include localVarErrorTypes for error deserialization - Fixes OpenAPITools#4777
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 9 files (changes from recent commits).
Prompt for AI agents (unresolved 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="samples/client/petstore/java/jersey3/src/main/java/org/openapitools/client/ApiClient.java">
<violation number="1" location="samples/client/petstore/java/jersey3/src/main/java/org/openapitools/client/ApiClient.java:1366">
P2: Error deserialization fallback returns a synthetic String on failure, producing non-null `errorEntity` values instead of `null` and changing error-entity semantics.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Chhida
force-pushed
the
feature/jersey3-error-deserialization
branch
from
f411d96 to
3ec7cc4
Compare
When deserializeErrorEntity fails, return null instead of a synthetic String message to maintain correct errorEntity semantics (null on failure). Fixes P2 issue from cubic-dev-ai review.
Hi @wing328, Thanks for the feedback! I’ve updated the samples as requested (step 3), and all commits are now correctly linked to my GitHub account. Everything should be ready for review. Thanks! |
Hi @wing328, I've verified that the KotlinReservedWordsTest failure is a pre-existing flaky test - it passes locally (101 tests, 0 failures). This is unrelated to the jersey3 errorEntity changes. Could you please confirm this and proceed with the review? |
|
cc @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) |
|
is it correct to say that you've been using this fix in your production environment and it has been working fine for your use cases? |
- Add JavaJersey3ErrorEntityFunctionalTest - Verifies generated templates include errorEntity field and methods - Verifies deserializeErrorEntity returns null on failure (P2 fix) - Related to issue OpenAPITools#4777 and PR OpenAPITools#23542
- Correct the JERSEY3_TEMPLATE_DIR path - All 4 functional tests now pass
Hi @wing328, Yes, the fix has been verified and is working correctly:
The functionality has been verified through tests and confirmed by our team. We're waiting for approval of the PR so we can deploy it in our production environment. |
| } | ||
| } | ||
|
|
||
| private Object deserializeErrorEntity(Map<String, GenericType> errorTypes, Response response) { |
There was a problem hiding this comment.
please add a docstring explaining what this function does
| @@ -0,0 +1,125 @@ | |||
| /* | |||
| * Copyright 2018 OpenAPI-Generator Contributors (https://openapi-generator.tech) | |||
| * Copyright 2018 SmartBear Software | |||
| @@ -0,0 +1,151 @@ | |||
| /* | |||
| * Copyright 2018 OpenAPI-Generator Contributors (https://openapi-generator.tech) | |||
| * Copyright 2018 SmartBear Software | |||
- Add docstring to deserializeErrorEntity method - Remove SmartBear Software copyright from test files
There was a problem hiding this comment.
3 issues found across 68 files (changes from recent commits).
Prompt for AI agents (unresolved 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="samples/client/petstore/java/jersey3/src/main/java/org/openapitools/client/ApiException.java">
<violation number="1" location="samples/client/petstore/java/jersey3/src/main/java/org/openapitools/client/ApiException.java:22">
P2: Generated source includes a per-run timestamp in `@Generated` despite `hideGenerationTimestamp: true`, causing non-deterministic regeneration diffs.</violation>
</file>
<file name="samples/client/petstore/java/jersey3-oneOf/src/main/java/org/openapitools/client/JavaTimeFormatter.java">
<violation number="1" location="samples/client/petstore/java/jersey3-oneOf/src/main/java/org/openapitools/client/JavaTimeFormatter.java:23">
P2: Generated sample code embeds a run-specific `@Generated` date despite timestamp-hiding config, causing non-deterministic output and diff churn.</violation>
</file>
<file name="samples/client/petstore/java/jersey3-oneOf/src/main/java/org/openapitools/client/RFC3339JavaTimeModule.java">
<violation number="1" location="samples/client/petstore/java/jersey3-oneOf/src/main/java/org/openapitools/client/RFC3339JavaTimeModule.java:22">
P2: Generated code now embeds a run-specific timestamp in `@Generated`, causing non-deterministic regeneration and unnecessary diff churn.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Regenerate jersey3 and jersey3-oneOf samples with hideGenerationTimestamp=true to fix P2 non-deterministic timestamp issues in @generated annotations.
Add jakarta.validation-api and commons-lang3 dependencies to fix compilation errors in Quadrilateral, SimpleQuadrilateral, and ComplexQuadrilateral models.
|
You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment |
@cubic-dev-ai review |
@Chhida I have started the AI code review. It will take a few minutes to complete. |
- Add errorEntity field and getErrorEntity() method to ApiException - Add deserializeErrorEntity() method to ApiClient for error deserialization - Update API methods to pass errorTypes map for automatic error handling - Add unit tests for errorEntity feature - Regenerate jersey3 and jersey3-oneOf samples - Fix sample pom.xml to include required dependencies (validation, commons-lang3, http-signature)
Chhida
force-pushed
the
feature/jersey3-error-deserialization
branch
from
e986801 to
5bc693d
Compare
@cubic-dev-ai review |
@Chhida I have started the AI code review. It will take a few minutes to complete. |
- Add errorEntity field and getErrorEntity() method to ApiException - Add deserializeErrorEntity() method to ApiClient for error deserialization - Update API methods to pass errorTypes map for automatic error handling - Add unit tests for errorEntity feature
- Add errorEntity field and getErrorEntity() method to ApiException - Add deserializeErrorEntity() method to ApiClient for error deserialization - Update API methods to pass errorTypes map for automatic error handling - Add unit tests for errorEntity feature - Regenerate jersey3 and jersey3-oneOf samples - Fix sample pom.xml to include required dependencies (validation, commons-lang3, http-signature)
@cubic-dev-ai review |
@Chhida I have started the AI code review. It will take a few minutes to complete. |
@cubic-dev-ai review |
@Chhida I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
2 issues found across 107 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. cubic prioritises the most important files to review.
Prompt for AI agents (unresolved 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/Java/libraries/jersey3/api.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/Java/libraries/jersey3/api.mustache:200">
P2: Error-type map population skips the first response unconditionally, which can omit a valid error/default schema and break ApiException error deserialization.</violation>
</file>
<file name="samples/client/petstore/java/jersey3/src/main/java/org/openapitools/client/ApiException.java">
<violation number="1" location="samples/client/petstore/java/jersey3/src/main/java/org/openapitools/client/ApiException.java:29">
P2: Adding a non-transient `Object` field to a Serializable exception can break Java serialization when error models are not Serializable.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| {{/hasAuthMethods}} | ||
| final Map<String, GenericType> localVarErrorTypes = new HashMap<String, GenericType>(); | ||
| {{#responses}} | ||
| {{^-first}} |
There was a problem hiding this comment.
P2: Error-type map population skips the first response unconditionally, which can omit a valid error/default schema and break ApiException error deserialization.
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/jersey3/api.mustache, line 200:
<comment>Error-type map population skips the first response unconditionally, which can omit a valid error/default schema and break ApiException error deserialization.</comment>
<file context>
@@ -195,12 +195,20 @@ public class {{classname}} {
{{/hasAuthMethods}}
+ final Map<String, GenericType> localVarErrorTypes = new HashMap<String, GenericType>();
+ {{#responses}}
+ {{^-first}}
+ {{#dataType}}
+ localVarErrorTypes.put("{{code}}", new GenericType<{{{dataType}}}>() {});
</file context>
| private int code = 0; | ||
| private Map<String, List<String>> responseHeaders = null; | ||
| private String responseBody = null; | ||
| private Object errorEntity = null; |
There was a problem hiding this comment.
P2: Adding a non-transient Object field to a Serializable exception can break Java serialization when error models are not Serializable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/java/jersey3/src/main/java/org/openapitools/client/ApiException.java, line 29:
<comment>Adding a non-transient `Object` field to a Serializable exception can break Java serialization when error models are not Serializable.</comment>
<file context>
@@ -26,6 +26,7 @@ public class ApiException extends Exception {
private int code = 0;
private Map<String, List<String>> responseHeaders = null;
private String responseBody = null;
+ private Object errorEntity = null;
public ApiException() {}
</file context>
| private Object errorEntity = null; | |
| private transient Object errorEntity = null; |
2 participants
Pull Request Description
Title
[Java][jersey3] Add error entity deserialization to ApiExceptionDescription
Add support for deserializing error response bodies into the model types defined in the OpenAPI specification for the jersey3 library.
Problem
When an OpenAPI spec defines error response schemas (e.g.,
ErreurApiWrapperfor 400 errors), these models are generated but not used by the generated client code. TheApiExceptiononly contains the raw response body as a string, requiring manual deserialization.Solution
errorEntityfield toApiExceptionto store the deserialized error objectgetErrorEntity()method to retrieve the deserialized error modelChanges
apiException.mustache: AdderrorEntityfield, constructor, andgetErrorEntity()methodapi.mustache: BuildlocalVarErrorTypesmap from API responsesApiClient.mustache: AdddeserializeErrorEntitymethod and updateinvokeAPIsignatureUsage
Testing
Related Issues
Checklist
Summary by cubic
Adds automatic error model deserialization to
jersey3clients soApiExceptioncarries a typed error entity while keeping the raw response body. Regeneratesjersey3andjersey3-oneOfsamples with deterministic output and adds tests and missing dependencies.New Features
ApiException: addederrorEntityandgetErrorEntity().ApiClient: addeddeserializeErrorEntity; buffers the entity and attaches it toApiException.invokeAPInow accepts anerrorTypesmap.GenericTypemap for error responses ("0"for default). AddedJavaJersey3ErrorEntityTestandJavaJersey3ErrorEntityFunctionalTest.Bug Fixes
hideGenerationTimestamp=true, fix functional test template path, specify UTF-8 for forbidden API checks, add Javadoc todeserializeErrorEntity, remove outdated headers. Add missing sample deps:jakarta.validation-api(provided),commons-lang3,org.tomitribe:tomitribe-http-signatures.Written for commit 5dedcc8. Summary will update on new commits.