[Java][jersey3] Add error entity deserialization to ApiException#23542
[Java][jersey3] Add error entity deserialization to ApiException#23542Chhida wants to merge 18 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.
- 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>
|
@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.
11 issues found across 98 files (changes from recent commits).
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="samples/client/petstore/java/jersey3/src/main/java/org/openapitools/client/model/Mammal.java">
<violation number="1">
P2: Missing discriminator value will cause a NullPointerException at the switch statement and prevent the existing oneOf fallback deserialization from running.</violation>
</file>
<file name="samples/client/petstore/java/jersey3/src/main/java/org/openapitools/client/model/MapTest.java">
<violation number="1">
P2: Using `EqualsBuilder.reflectionEquals` with `testRecursive=true` compares map internals instead of `Map.equals`, so two maps with identical entries but different internal layout can become unequal and produce inconsistent hash codes.</violation>
</file>
<file name="samples/client/petstore/java/jersey3/src/main/java/org/openapitools/client/model/ArrayOfArrayOfNumberOnly.java">
<violation number="1">
P2: `EqualsBuilder.reflectionEquals(..., true)` performs recursive reflection into collection internals, so equality becomes implementation-detail dependent (e.g., ArrayList capacity/modCount) instead of content-based `List.equals`. This is a behavioral regression from the prior explicit field comparison and can make logically equal models compare unequal.</violation>
</file>
<file name="samples/client/petstore/java/jersey3/src/main/java/org/openapitools/client/model/Apple.java">
<violation number="1">
P2: @Pattern uses Java regex syntax; the added JavaScript-style `/.../i` delimiters make the regex match literal slashes and a trailing `i`, so normal origin values won’t validate.</violation>
</file>
<file name="samples/client/petstore/java/jersey3/src/main/java/org/openapitools/client/model/MammalAnyof.java">
<violation number="1">
P2: additionalProperties support is non-functional: custom serialization writes only the actual instance and deserialization never populates the map, so values added via putAdditionalProperty are silently dropped.</violation>
</file>
<file name="samples/client/petstore/java/jersey3/src/main/java/org/openapitools/client/model/ArrayTest.java">
<violation number="1">
P2: Recursive reflection-based equals/hashCode changes equality semantics for collection fields (compares internal ArrayList state instead of List.equals), which can yield incorrect equality results vs the prior explicit field comparison.</violation>
</file>
<file name="samples/client/petstore/java/jersey3/src/main/java/org/openapitools/client/model/Dog.java">
<violation number="1">
P2: `equals` uses recursive reflection while `hashCode` uses non-recursive object hash codes, which can violate equals/hashCode contract for arbitrary nested `additionalProperties` values.</violation>
</file>
<file name="samples/client/petstore/java/jersey3/src/main/java/org/openapitools/client/model/Cat.java">
<violation number="1">
P2: Using recursive reflection-based equals on a model that now contains arbitrary additionalProperties can throw InaccessibleObjectException on Java 9+ (reflective access into JDK classes) and changes equality semantics away from explicit field comparison.</violation>
</file>
<file name="samples/client/petstore/java/jersey3/src/main/java/org/openapitools/client/model/MixedPropertiesAndAdditionalPropertiesClass.java">
<violation number="1">
P2: Using EqualsBuilder.reflectionEquals with recursive reflection can throw InaccessibleObjectException on Java 9+ when it reflects into JDK types (UUID/OffsetDateTime/HashMap), and it changes equality semantics versus explicit field comparisons.</violation>
</file>
<file name="samples/client/petstore/java/jersey3/src/main/java/org/openapitools/client/model/BananaReq.java">
<violation number="1">
P2: Reflection-based equals with recursive comparison can throw InaccessibleObjectException on JDK 9+ module encapsulation and is a behavioral regression from explicit field comparison. Consider reverting to explicit Objects.equals to avoid reflective access into JDK types like BigDecimal.</violation>
</file>
<file name="samples/client/petstore/java/jersey3/src/main/java/org/openapitools/client/model/OuterComposite.java">
<violation number="1">
P2: equals now uses recursive reflection (testRecursive=true), which changes equality semantics and can cause reflective access failures on JDK types like BigDecimal under Java 9+ module encapsulation. This is a regression from the previous explicit field comparisons.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
@wing328 These 11 P2 issues reported (Mammal, MapTest, ArrayOfArrayOfNumberOnly, Apple, MammalAnyof, ArrayTest, Dog, Cat, MixedPropertiesAndAdditionalPropertiesClass, BananaReq, OuterComposite) concern pre-existing models in the generated samples - are these issues related to PR #23542 (error entity deserialization) or are they pre-existing regressions in the Jersey3 templates? PR #23542 only modifies:
These reported issues appear to be related to:
Please confirm whether these issues need to be fixed in this PR or if they are out of scope (pre-existing template issues). |
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. Regenerates samples and adds tests; fixes edge cases and build issues.New Features
ApiException: added transienterrorEntityandgetErrorEntity().ApiClient: addeddeserializeErrorEntity; buffers the entity;invokeAPInow accepts anerrorTypesmap.GenericTypemap for error responses ("0"for default).Bug Fixes
errorTypesmap; markerrorEntityas transient.hideGenerationTimestamp=true; fix functional test template path and UTF-8 charset; add Javadoc todeserializeErrorEntity; regeneratejersey3samples and add missing deps (jakarta.validation-apias provided,commons-lang3,org.tomitribe:tomitribe-http-signatures); remove duplicatejunit-version; regenerate again to fix sample compilation.Written for commit 75b7418. Summary will update on new commits.