More lenient JSON handling #452
Conversation
* Accept JSON arrays as List/array * Accept JSON map as Map * Handle boxing when cehcking types * Adjust exception handling
📝 WalkthroughWalkthroughThe pull request introduces comprehensive modifications to the JSON serialisation and parsing components in the Joda Beans library. The changes primarily focus on enhancing error handling mechanisms, refining method signatures, and improving exception management across multiple classes in the Changes
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/main/java/org/joda/beans/ser/json/AbstractJsonReader.java (3)
87-96: Increased specificity of thrown exceptionsWrapping I/O problems in
UncheckedIOExceptionand class issues inIllegalArgumentExceptionhelps users diagnose errors more precisely. This approach is consistent with Java best practices, but ensure that any calling code is prepared for these unchecked exceptions.
126-126: Enriched error messageAppending
ex.getMessage()is helpful, but consider including the full stack trace when debugging. For production, this is acceptable; for more verbose diagnostics in testing, you might consider logging the stack trace.
369-369: Expanded numeric handlingThe method provides thorough numeric conversions and also leniently treats
nullasNaNfor floating types. This can be surprising in certain scenarios, so ensure it is well-documented for end users.src/main/java/org/joda/beans/ser/json/JodaBeanSimpleJsonReader.java (1)
52-53: Refined method signatureDeclaring
throws UncheckedIOExceptionandIllegalArgumentExceptionprovides clearer expectations of possible runtime issues. Ensure downstream callers are prepared for these unchecked exceptions where necessary.src/main/java/org/joda/beans/ser/json/JodaBeanJsonReader.java (1)
78-79: Matching documentationThis now mirrors the approach of the
read(String)methods. Make sure any example snippets or end-user documentation also reflect these exceptions.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/org/joda/beans/ser/json/AbstractJsonReader.java(14 hunks)src/main/java/org/joda/beans/ser/json/JodaBeanJsonReader.java(5 hunks)src/main/java/org/joda/beans/ser/json/JodaBeanSimpleJsonReader.java(3 hunks)src/test/java/org/joda/beans/ser/json/TestSerializeJsonSimple.java(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
src/test/java/org/joda/beans/ser/json/TestSerializeJsonSimple.java (2)
Pattern **/*.java: - Review code using Java 21 standards, taking into account the rules defined by src/main/checkstyle/checkstyle.xml.
- Validate that code indentation uses spaces, not tabs, with an indent of multiple of 4.
- Validate that immutable local variables are not annotated with
finalunless the variable is required for use in an inner class. - Favour use of
varkeyword for type declarations.varmay also be used when the value is a castnull. - Use a coding standard where multi-line expressions have operators and tenary separators at the end of line.
- Propose changes that only use the Java 21 API, not the API of Guava.
- The pattern matching
instanceofexpression safely handlesnull, returningfalse.
Pattern **/test/java/**/*.java: For test code, focus on:
- Correctness of test assertions
- Test coverage of edge cases
- Clear test naming and documentation
- Encourage test methods to be package-scoped where possible
- Be more lenient with code style and minor optimisations
src/main/java/org/joda/beans/ser/json/JodaBeanSimpleJsonReader.java (2)
Pattern **/*.java: - Review code using Java 21 standards, taking into account the rules defined by src/main/checkstyle/checkstyle.xml.
- Validate that code indentation uses spaces, not tabs, with an indent of multiple of 4.
- Validate that immutable local variables are not annotated with
finalunless the variable is required for use in an inner class. - Favour use of
varkeyword for type declarations.varmay also be used when the value is a castnull. - Use a coding standard where multi-line expressions have operators and tenary separators at the end of line.
- Propose changes that only use the Java 21 API, not the API of Guava.
- The pattern matching
instanceofexpression safely handlesnull, returningfalse.
Pattern **/main/java/**/*.java: - This project is mature and must provide a stable backwards-compatible public Java API.
- In the 'Walkthrough' section, you must always provide a list of up to 25 changes to the public Java API that will affect end users.
If there are no changes, you must explicitly state that there are no changes to the public Java API in this PR.
The public Java API is defined as public and protected methods on public classes, plus the filemodule-info.java.
Provide the list by deeply analysing code flow, which incudes analysing code flow through private methods and calls to Guava and Java 21.
Changes to be reported on include:- New or removed methods in the public Java API
- Changes to method return types or parameter types in the public Java API
- Changes to method behaviour in the public Java API that might affect consumers
- This project uses
System.out.printlninstead of logging - This project tends to prefer
forloops to streams for performance reasons, however either form is acceptable.
Do not make suggestions to change between streams and for loops or vice versa.
src/main/java/org/joda/beans/ser/json/JodaBeanJsonReader.java (2)
Pattern **/*.java: - Review code using Java 21 standards, taking into account the rules defined by src/main/checkstyle/checkstyle.xml.
- Validate that code indentation uses spaces, not tabs, with an indent of multiple of 4.
- Validate that immutable local variables are not annotated with
finalunless the variable is required for use in an inner class. - Favour use of
varkeyword for type declarations.varmay also be used when the value is a castnull. - Use a coding standard where multi-line expressions have operators and tenary separators at the end of line.
- Propose changes that only use the Java 21 API, not the API of Guava.
- The pattern matching
instanceofexpression safely handlesnull, returningfalse.
Pattern **/main/java/**/*.java: - This project is mature and must provide a stable backwards-compatible public Java API.
- In the 'Walkthrough' section, you must always provide a list of up to 25 changes to the public Java API that will affect end users.
If there are no changes, you must explicitly state that there are no changes to the public Java API in this PR.
The public Java API is defined as public and protected methods on public classes, plus the filemodule-info.java.
Provide the list by deeply analysing code flow, which incudes analysing code flow through private methods and calls to Guava and Java 21.
Changes to be reported on include:- New or removed methods in the public Java API
- Changes to method return types or parameter types in the public Java API
- Changes to method behaviour in the public Java API that might affect consumers
- This project uses
System.out.printlninstead of logging - This project tends to prefer
forloops to streams for performance reasons, however either form is acceptable.
Do not make suggestions to change between streams and for loops or vice versa.
src/main/java/org/joda/beans/ser/json/AbstractJsonReader.java (2)
Pattern **/*.java: - Review code using Java 21 standards, taking into account the rules defined by src/main/checkstyle/checkstyle.xml.
- Validate that code indentation uses spaces, not tabs, with an indent of multiple of 4.
- Validate that immutable local variables are not annotated with
finalunless the variable is required for use in an inner class. - Favour use of
varkeyword for type declarations.varmay also be used when the value is a castnull. - Use a coding standard where multi-line expressions have operators and tenary separators at the end of line.
- Propose changes that only use the Java 21 API, not the API of Guava.
- The pattern matching
instanceofexpression safely handlesnull, returningfalse.
Pattern **/main/java/**/*.java: - This project is mature and must provide a stable backwards-compatible public Java API.
- In the 'Walkthrough' section, you must always provide a list of up to 25 changes to the public Java API that will affect end users.
If there are no changes, you must explicitly state that there are no changes to the public Java API in this PR.
The public Java API is defined as public and protected methods on public classes, plus the filemodule-info.java.
Provide the list by deeply analysing code flow, which incudes analysing code flow through private methods and calls to Guava and Java 21.
Changes to be reported on include:- New or removed methods in the public Java API
- Changes to method return types or parameter types in the public Java API
- Changes to method behaviour in the public Java API that might affect consumers
- This project uses
System.out.printlninstead of logging - This project tends to prefer
forloops to streams for performance reasons, however either form is acceptable.
Do not make suggestions to change between streams and for loops or vice versa.
🔇 Additional comments (28)
src/test/java/org/joda/beans/ser/json/TestSerializeJsonSimple.java (1)
209-209: Maintain consistency with the revised approach to exception handling
The test now checks for IllegalArgumentException rather than ClassCastException. This aligns well with the updated JSON logic and is a more appropriate exception type for an incorrect root type argument.
src/main/java/org/joda/beans/ser/json/AbstractJsonReader.java (19)
23-25: Imports look appropriate
These imports align correctly with the classes referenced below and adhere to Java 21 usage.
32-32: Additional import
The import for ResolvedType is correctly introduced, matching its usage later in the code.
36-36: Additional import
The import for SerIteratorFactory is necessary for the newly expanded logic of parsing unknown arrays and objects.
84-85: Method documentation updated
The new exception declarations in the Javadoc are consistent with the refined error handling strategy.
Line range hint 100-122: Enhanced parseBean method
This segment refines the bean creation logic, using var for local variables in line with Java 21. No obvious issues are found; the error handling is now clearer by segregating IO exceptions from general failures.
122-123: Re-throwing IOException
Re-throwing IOException preserves critical information about the underlying error condition, which is valuable for debugging.
137-137: Extended method signature
Declaring IOException and ClassNotFoundException clarifies the possible failures. The signature is consistent with the enhanced parsing logic.
193-199: Lenient array handling
Treating unknown arrays as either actual arrays or lists fosters more flexible JSON parsing, which aligns with the PR's objectives of lenient JSON handling.
202-204: Lenient map handling
Accepting unknown objects as maps widens compatibility for varied JSON object structures. This appears consistent with the leniency requirement.
207-207: Typed bean method
The logic for verifying the type as a Bean or throwing an IllegalArgumentException adds robustness to root-type validation.
227-227: Typed simple value resolution
Decoding the effective type and validating its compatibility with the declared type handles primitive conversions reliably.
231-234: Boxing adjustment
The code checks if the types match, accounting for primitive/boxed equivalences. This is neatly handled with minimal overhead.
246-246: Typed meta parsing
This leverages the IteratorFactory to handle meta definitions. The approach is clear and well structured under the new exception handling regime.
259-259: Dispatcher for iterable categories
Routing logic for distinct iterable categories is straightforward, adhering to the design changes that aim for more extensive JSON coverage.
273-273: Map handling with array or object
Providing array-based or object-based alternatives is a convenient approach for multiple JSON representations.
301-301: Table handling
Using a structured approach for rows and columns expands usage scenarios for JSON that contains tabular data.
318-318: Grid handling
The code enabling dimension-based reading fits neatly into the new lenient strategy, ensuring clarity of row/column usage.
343-343: Counted items
Allowing repeated items with a count matches JSON representing aggregated or bulk data formats.
358-358: Array building
Reading item by item and then calling iterable.build() is consistent with the rest of the framework’s design.
src/main/java/org/joda/beans/ser/json/JodaBeanSimpleJsonReader.java (3)
20-20: New import for UncheckedIOException
This import is justified by the refined error handling pattern across the library.
67-68: Refined method signature
Same reasoning applies here – clarifying exceptions fosters robust client code.
73-74: Use of var
The var usage is in line with modern Java coding guidelines, improving readability when the type is clear from context.
src/main/java/org/joda/beans/ser/json/JodaBeanJsonReader.java (5)
20-20: New import for UncheckedIOException
Consistent with the broader JSON reading architecture, ensuring uniform handling of I/O errors.
51-52: Updated documentation
The inclusion of UncheckedIOException in the Javadoc clarifies the potential error contract.
65-66: Exception declaration
Specifying UncheckedIOException and IllegalArgumentException is a beneficial step towards consistent runtime error handling throughout the library.
92-93: Refined method signature
The shift to explicitly declare these runtime exceptions provides transparency for integrators using the library.
98-99: Simplified method logic
Removing extra try-catch blocks reduces duplication and complexity, letting exceptions propagate where appropriate.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores