[Java][jersey3] Add error entity deserialization to ApiException#23542
[Java][jersey3] Add error entity deserialization to ApiException#23542Chhida wants to merge 17 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.
| 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; |
- Remove {{^-first}} to include ALL responses in errorType map (not just from 2nd response)
- Add transient keyword to errorEntity field for serialization safety
- Update test to expect transient keyword
- Regenerate samples with fixes
|
@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.
1 issue found across 4 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-oneOf/pom.xml">
<violation number="1" location="samples/client/petstore/java/jersey3-oneOf/pom.xml:361">
P2: Duplicate `junit-version` property declaration creates ambiguous/dead configuration and increases maintenance risk.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Duplicate property removed to fix P2 issue from cubic-dev-ai review.
|
@cubic-dev-ai review |
1 similar comment
|
@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.
3 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/ApiClient.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/Java/libraries/jersey3/ApiClient.mustache:1218">
P2: Public operation-aware `invokeAPI` signature was changed without preserving the previous overload, causing source compatibility break for existing callers/overrides.</violation>
</file>
<file name="samples/client/petstore/java/jersey3-oneOf/src/main/java/org/openapitools/client/ApiException.java">
<violation number="1" location="samples/client/petstore/java/jersey3-oneOf/src/main/java/org/openapitools/client/ApiException.java:29">
P2: `errorEntity` is non-transient in a serializable exception, which can trigger `NotSerializableException` when the field holds a non-serializable model.</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: Added non-transient `Object` field in a Serializable exception can trigger `NotSerializableException` when exceptions are serialized.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| GenericType<T> returnType, | ||
| boolean isBodyNullable) | ||
| boolean isBodyNullable, | ||
| Map<String, GenericType> errorTypes |
There was a problem hiding this comment.
P2: Public operation-aware invokeAPI signature was changed without preserving the previous overload, causing source compatibility break for existing callers/overrides.
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/ApiClient.mustache, line 1218:
<comment>Public operation-aware `invokeAPI` signature was changed without preserving the previous overload, causing source compatibility break for existing callers/overrides.</comment>
<file context>
@@ -1212,7 +1214,9 @@ public class ApiClient{{#jsr310}} extends JavaTimeFormatter{{/jsr310}} {
GenericType<T> returnType,
- boolean isBodyNullable)
+ boolean isBodyNullable,
+ Map<String, GenericType> errorTypes
+ )
throws ApiException {
</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: errorEntity is non-transient in a serializable exception, which can trigger NotSerializableException when the field holds a non-serializable model.
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-oneOf/src/main/java/org/openapitools/client/ApiException.java, line 29:
<comment>`errorEntity` is non-transient in a serializable exception, which can trigger `NotSerializableException` when the field holds a non-serializable model.</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; |
| 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: Added non-transient Object field in a Serializable exception can trigger NotSerializableException when exceptions are serialized.
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>Added non-transient `Object` field in a Serializable exception can trigger `NotSerializableException` when exceptions are serialized.</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. Also marks the error entity as transient for safe serialization.New Features
ApiException: addederrorEntityandgetErrorEntity().ApiClient: addeddeserializeErrorEntity; buffers the entity and attaches it toApiException.invokeAPInow accepts anerrorTypesmap.GenericTypemap for error responses ("0"for default).Bug Fixes
errorTypesmap; makeerrorEntitytransient.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-apias provided,commons-lang3,org.tomitribe:tomitribe-http-signatures), and remove duplicatejunit-versionproperty frompom.xml.Written for commit b20c211. Summary will update on new commits.