SRU2023 v10.1 #85
SRU2023 v10.1 #85
Conversation
…an-OffsetDateTime-OffsetTime-LocalDate Cu-2np2uh3_GH-56: JAXB upgrade plan using java.time classes
…ariable optional seconds precision
…or time information when unmarshalling into LocalDate objects
* Gson serialization/deserialization in Adapters - Fix reflection issue with data.time.* classes. * WIP custom adapters/dto to use date.time.* classes with Gson * WIP custom adapters/dto to use date.time.* classes with Gson * fix revert change build.gradle * Junit for Json Adapters at serialization/deserialization * Remove comment * Remove unused Junit + Rename refactor * inner classes for Json adapters --------- Co-authored-by: Sebastian Zubrinic <[email protected]>
|
Hi Zubri, when do you anticipate this being released? |
We normally made new releases for SRU updates available via Maven Central by October each year. |
… containing several jaxb bugfixes
WalkthroughWalkthroughThe recent changes enhance the project by updating the Java version to 11, transitioning from Java EE to Jakarta EE for improved compatibility, and restructuring various classes for better maintainability and readability. Key improvements include modifications to import statements, access modifiers of static variables, and the addition of essential methods for object representation and comparison. Additionally, the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Application
participant Database
User->>Application: Request data
Application->>Database: Query for data
Database-->>Application: Return data
Application-->>User: Send data response
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (105)
Files not processed due to max files limit (58)
Files not reviewed due to server errors (9)
Additional context usedPath-based instructions (44)
Markdownlint
Additional comments not posted (217)
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| @Test | ||
| void test() { | ||
| try { | ||
| String xml = "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>" |
There was a problem hiding this comment.
Enhance test coverage with assertions.
The test currently checks for exceptions but does not verify the correctness of the parsed MxSwiftMessage. Consider adding assertions to validate the properties of the resulting object.
// Example assertion to verify a property
// assertEquals(expectedValue, mxSwiftMessage.someProperty());| * (PW-1875) Fixed the `ZuluDateTimeAdapter` to convert the datetime to UTC offset if needed | ||
|
|
||
| #### 9.4.5 - May 2024 | ||
| #### 10.1.7 - August 2024 |
There was a problem hiding this comment.
Incorrect heading level.
The heading level should increment by one level at a time. Change the heading level from #### to ###.
-#### 10.1.7 - August 2024
+### 10.1.7 - August 2024Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #### 10.1.7 - August 2024 | |
| ### 10.1.7 - August 2024 |
Tools
Markdownlint
3-3: Expected: h2; Actual: h4
Heading levels should only increment by one level at a time(MD001, heading-increment)
3-3: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
| #### 10.1.7 - August 2024 | ||
| * (PW-1958) Fixed the `DefaultMxMetadataStrategy` NPE issue when the amount values are null | ||
|
|
||
| #### 10.1.6 - May 2024 |
There was a problem hiding this comment.
Missing blank line after heading.
Headings should be surrounded by blank lines for better readability.
-#### 10.1.6 - May 2024
+#### 10.1.6 - May 2024
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #### 10.1.6 - May 2024 | |
| #### 10.1.6 - May 2024 | |
Tools
Markdownlint
6-6: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
|
|
||
| #### 9.4.5 - May 2024 | ||
| #### 10.1.7 - August 2024 | ||
| * (PW-1958) Fixed the `DefaultMxMetadataStrategy` NPE issue when the amount values are null |
There was a problem hiding this comment.
Unordered list indentation.
The list item is not properly indented. Ensure consistent indentation for unordered list items.
- * (PW-1958) Fixed the `DefaultMxMetadataStrategy` NPE issue when the amount values are null
+* (PW-1958) Fixed the `DefaultMxMetadataStrategy` NPE issue when the amount values are nullCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * (PW-1958) Fixed the `DefaultMxMetadataStrategy` NPE issue when the amount values are null | |
| * (PW-1958) Fixed the `DefaultMxMetadataStrategy` NPE issue when the amount values are null |
Tools
Markdownlint
4-4: Expected: 0; Actual: 2
Unordered list indentation(MD007, ul-indent)
4-4: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
| void testSerializationAndDeserialization() { | ||
| testSerializationAndDeserializationImpl("{\"year\":2019}"); | ||
| testSerializationAndDeserializationImpl("{\"year\":2019}"); | ||
| testSerializationAndDeserializationImpl("{\"year\":2019}"); | ||
| testSerializationAndDeserializationImpl("{\"year\":1983}"); |
There was a problem hiding this comment.
Reduce redundancy in test cases.
The testSerializationAndDeserialization method repeats the same test case multiple times. Consider using a parameterized test to avoid redundancy.
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
@ParameterizedTest
@ValueSource(strings = {"{\"year\":2019}", "{\"year\":1983}"})
void testSerializationAndDeserialization(String json) {
testSerializationAndDeserializationImpl(json);
}| public OffsetDateTime unmarshal(String value) { | ||
| return parseOffsetDateTime(value); | ||
| } |
There was a problem hiding this comment.
Handle null values gracefully in unmarshal method.
The unmarshal method currently returns null for null input. Consider adding a log entry or exception handling to inform about null input.
| private static OffsetDateTime parseOffsetDateTime(String value) { | ||
| if (value == null) { | ||
| return null; | ||
| } | ||
| try { | ||
| return OffsetDateTime.parse(value); | ||
| } catch (DateTimeParseException e) { | ||
| log.log(Level.FINEST, "Error parsing OffsetDateTime: " + e.getMessage()); | ||
| try { | ||
| // Attempt to parse as LocalDateTime and assume system default time zone | ||
| LocalDateTime localDateTime = LocalDateTime.parse(value); | ||
| return localDateTime.atZone(ZoneId.systemDefault()).toOffsetDateTime(); | ||
| } catch (DateTimeParseException e2) { | ||
| log.log(Level.FINEST, "Error parsing LocalDateTime: " + e2.getMessage()); | ||
| return null; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Improve error handling in parseOffsetDateTime method.
The method logs parsing errors at the FINEST level. Consider using a higher log level or throwing an exception for critical parsing failures.
| @Override | ||
| public String toString() { | ||
| return "ZuluOffsetDateTimeAdapter{ yyyy-MM-dd'T'HH:mm:ss[.SSS]Z }"; | ||
| } |
There was a problem hiding this comment.
Enhance toString method for better clarity.
The toString method provides a brief description of the adapter. Consider including more details about the formatting pattern used.
| public String marshal(OffsetDateTime offsetDateTime) throws Exception { | ||
| // Convert to Zulu time (UTC) before formatting | ||
| OffsetDateTime utcDateTime = offsetDateTime.withOffsetSameInstant(ZoneOffset.UTC); | ||
| String formatted = formatOffsetDateTime(this.marshalFormat, utcDateTime); | ||
|
|
||
| // Remove trailing fractional seconds if they are all zeros | ||
| Matcher matcher = FRACTIONAL_SECONDS_PATTERN.matcher(formatted); | ||
| if (matcher.find()) { | ||
| formatted = formatted.replace(matcher.group(), "Z"); | ||
| } | ||
|
|
||
| return formatted; | ||
| } |
There was a problem hiding this comment.
Optimize fractional seconds removal logic.
The marshal method uses a regex to remove trailing zeros in fractional seconds. Consider optimizing this by checking if the fractional part is zero before formatting.
- Matcher matcher = FRACTIONAL_SECONDS_PATTERN.matcher(formatted);
- if (matcher.find()) {
- formatted = formatted.replace(matcher.group(), "Z");
- }
+ if (offsetDateTime.getNano() == 0) {
+ formatted = formatted.replaceAll("\\.0+Z", "Z");
+ }Committable suggestion was skipped due to low confidence.
| import jakarta.xml.bind.annotation.XmlAccessType; | ||
| import jakarta.xml.bind.annotation.XmlAccessorType; | ||
| import jakarta.xml.bind.annotation.XmlElement; | ||
| import jakarta.xml.bind.annotation.XmlRootElement; | ||
| import jakarta.xml.bind.annotation.XmlType; |
There was a problem hiding this comment.
Tip
Codebase Verification
Incomplete Transition to Jakarta EE
The codebase still contains several javax imports that need to be updated to their jakarta equivalents to ensure full compatibility with Jakarta EE. Please review and update the following files:
model-xsys-types/src/generated/java/com/prowidesoftware/swift/model/mx/sys/dic/SwIntMixedAnyPayload.javamodel-common-types/src/main/java/com/prowidesoftware/swift/model/mx/adapters/IsoDateAdapter.javamodel-common-types/src/main/java/com/prowidesoftware/swift/model/mx/adapters/IsoDateTimeAdapter.javaiso20022-core/src/main/java/com/prowidesoftware/swift/model/mx/MxReadImpl.javaiso20022-core/src/main/java/com/prowidesoftware/swift/model/mx/MxWriteImpl.javaiso20022-core/src/test/java/com/prowidesoftware/swift/model/mx/MxXsysModelTest.javaiso20022-core/src/main/java/com/prowidesoftware/swift/model/mx/LegacyAppHdr.javaiso20022-core/src/main/java/com/prowidesoftware/swift/model/mx/BusinessAppHdrV01.javaiso20022-core/src/main/java/com/prowidesoftware/swift/model/mx/BusinessAppHdrV03.javaiso20022-core/src/main/java/com/prowidesoftware/swift/model/mx/BusinessAppHdrV02.javaiso20022-core/src/main/java/com/prowidesoftware/swift/model/mx/AbstractMX.javaiso20022-core/src/main/java/com/prowidesoftware/swift/model/mx/XmlEventWriter.javaiso20022-core/src/main/java/com/prowidesoftware/swift/model/mx/NamespaceReader.javaiso20022-core/src/main/java/com/prowidesoftware/swift/model/mx/SupplementaryDataUtils.javaiso20022-core/src/main/java/com/prowidesoftware/swift/model/mx/MxParseUtils.javaiso20022-core/src/main/java/com/prowidesoftware/swift/model/mx/AppHdrParser.javaiso20022-core/src/test/java/com/prowidesoftware/swift/model/mx/AppHdrTest.java
Analysis chain
Transition to Jakarta EE.
The import statements have been updated from javax.xml.bind.annotation.* to jakarta.xml.bind.annotation.*. This transition is crucial for compatibility with Jakarta EE. Ensure that all dependencies and environments are compatible with Jakarta EE.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify that all imports from `javax.xml.bind.annotation` have been updated to `jakarta.xml.bind.annotation`.
rg --type java 'javax.xml.bind.annotation' -A 0
Length of output: 47
Script:
#!/bin/bash
# Search for any remaining `javax` imports in the Java files to ensure a complete transition to Jakarta EE.
rg --type java '^import javax\.' -A 0
Length of output: 4833
Summary by CodeRabbit
New Features
OffsetDateTimeforcreationDatemethods.Bug Fixes
DefaultMxMetadataStrategy.Documentation
Chores
.gitignoreto exclude.java-versionfiles.