Add Date to LocalDateTime conversion methods to bridge legacy and modern date APIs #1385

yuyu1025 · 2025-05-17T13:06:09Z

Background and Motivation

With the widespread adoption of Java 8+, the project has gradually moved from the traditional java.util.Date to modern date-time APIs (such as LocalDateTime), but a large amount of existing code still relies on Date. To reduce migration costs and improve development efficiency, it is necessary to provide officially supported conversion tools in Apache Commons Lang to avoid developers repeating the implementation of underlying logic.

Core Change Content

New Conversion Methods
- toLocalDateTime(Date date)：Uses the system's default time zone, simplifying common conversion scenarios.
- toLocalDateTime(Date date, TimeZone tz)：Explicitly specifies the time zone, meeting internationalization requirements (e.g., converting UTC time to local time in a specific time zone).
- Implementation principle: Converts using Instant and ZoneId, ensuring time zone accuracy (including daylight saving time, handling of extreme time zones).
Improve test coverage
- Covers scenarios such as default time zones, specified time zones, null value validation, daylight saving time (e.g., New York in the United States), extreme time zones (e.g., Kiribati with UTC+14) to ensure code robustness.

Value Added

Smooth Migration: Allows developers to gradually introduce LocalDateTime without modifying the existing Date interface, reducing refactoring risks.
Consistency and Reliability: Provides Apache's official conversion logic, avoiding potential bugs in third-party implementations, and enhancing project stability.
Feature Extension: Enhance DateUtils modern support to adapt to Java 8+ ecosystem, responding to the community's need for bridging new and old APIs.

Compatibility Note

No breaking changes, new methods are independent extensions, do not affect existing DateUtils functionality.
Depends on Java 8+ (consistent with Apache Commons Lang 3.18+).

garydgregory

-1: The implementation doesn't make sense. I suppose the question is: Do we really need a utility method for:

LocalDateTime.ofInstant(date.toInstant(), timeZone.toZoneId());

yuyu1025 · 2025-05-17T13:38:46Z

-1: The implementation doesn't make sense. I suppose the question is: Do we really need a utility method for:
LocalDateTime.ofInstant(date.toInstant(), timeZone.toZoneId());

For experienced developers, this indeed doesn't make much sense. However, for newbies who know nothing about LocalDateTime, it can often be of great help. In project engineering, it can also reduce a lot of repetitive logic.

ppkarwasz · 2025-05-17T14:53:37Z

I suppose the question is: Do we really need a utility method for:
LocalDateTime.ofInstant(date.toInstant(), timeZone.toZoneId());

If you also consider java.sql.Date and java.sql.Timestamp—both subclasses of java.util.Date—the conversion becomes less straightforward:

java.sql.Date.toInstant() throws an exception.
Instead, there are now dedicated methods: java.sql.Date.toLocalDate() and java.sql.Timestamp.toLocalDateTime().

When you have to deal with legacy code that uses Date, having conversion helpers is quite useful.

yuyu1025 · 2025-05-17T15:31:50Z

I suppose the question is: Do we really need a utility method for:
LocalDateTime.ofInstant(date.toInstant(), timeZone.toZoneId());
If you also consider java.sql.Date and java.sql.Timestamp—both subclasses of java.util.Date—the conversion becomes less straightforward:

java.sql.Date.toInstant() throws an exception.

Instead, there are now dedicated methods: java.sql.Date.toLocalDate() and java.sql.Timestamp.toLocalDateTime().

When you have to deal with legacy code that uses Date, having conversion helpers is quite useful.

You raised a very good question, I have handled all subclasses and completed the unit tests, which will greatly improve efficiency for the development of projects involving both old and new systems.

src/main/java/org/apache/commons/lang3/time/DateUtils.java

ppkarwasz

@yuyu1025,

Thank you so much for the updates you’ve made to this pull request—I really appreciate the effort you’ve put in so far. Before we dive into the points Gary raised about the method’s utility, let’s take a moment to clean things up a bit more. Once that’s done, we’ll be in a better place to evaluate the next steps together.

src/main/java/org/apache/commons/lang3/time/DateUtils.java

src/test/java/org/apache/commons/lang3/time/DateUtilsTest.java

ppkarwasz · 2025-05-17T21:14:14Z

@yuyu1025,

I noticed you've enabled "Vigilant mode", but it looks like your GPG key isn’t added to your GitHub account—so your commits are showing as “Unverified.” That’s probably not what you intended.

To resolve this, add your GPG key to your GitHub account to verify that you are the author of your commits.

This commit enhances the DateUtils class by adding two new methods to convert java.util.Date to java.time.LocalDateTime, supporting both default and specified time zones. The implementation leverages Java 8's date-time API for accurate time zone handling, including: - `toLocalDateTime(Date date)`: Uses the system's default time zone - `toLocalDateTime(Date date, TimeZone tz)`: Explicitly specifies time zone via TimeZone parameter Key improvements: - Proper null checking with Objects.requireNonNull() - Direct conversion via Instant and ZoneId for timezone accuracy - Maintains parity with existing DateUtils method naming conventions Test cases added in DateUtilsTest: - Default time zone conversion (Asia/Shanghai example) - Specified time zone conversion (America/New_York) - Null input validation (throws NullPointerException) - Daylight saving time handling (America/New_York DST case) - Extreme time zone test (Pacific/Kiritimati, UTC+14) These changes enable seamless integration between legacy Date APIs and modern LocalDateTime, while ensuring robust timezone handling across different environments.

…atible subclasses (such as java.sql.Date).

…stamp **What** 1. Added extensive unit tests covering all java.sql date/time subclasses (Date, Time, Timestamp) 2. Implemented nanosecond precision support for java.sql.Timestamp conversions 3. Enhanced timezone handling to ensure correct conversion across different time zones 4. Added daylight saving time (DST) edge case testing **Why** 1. Existing tests lacked coverage for sql subclasses and edge cases 2. Timestamp conversions were losing nanosecond precision 3. Timezone handling was inconsistent between Date and Timestamp types

Co-authored-by: Piotr P. Karwasz <[email protected]>

@test

…rized tests - Remove multiple individual @test methods for SQL date/time/timestamp conversions - Replace with 2 parameterized tests using @ParameterizedTest and @MethodSource - Introduce test data providers: - dateConversionProvider() for basic conversions (SQL Date/Time/Timestamp) - dateWithTimeZoneProvider() for timezone-aware conversions - Maintain full test coverage for: - Epoch date (1970-01-01) - Max time values (23:59:59) - Nano precision timestamps - Default/system time zone handling - Specific time zones (America/New_York, Asia/Shanghai, Pacific/Kiritimati) - Daylight Saving Time transitions - Improve test maintainability by reducing code duplication - Add null argument validation test for time zone parameter Signed-off-by: finger <[email protected]>

…dule dependency - Introduce static reflection initialization for Timestamp class/method - Use isTimestamp() to check instance type without direct dependency - Extract nanoseconds via reflection in extractNanosFromSqlTimestamp() - Gracefully handle absence of java.sql module with null checks and exception handling - Maintain existing functionality for converting Timestamps to LocalDateTime with nanosecond precision - Ensure compatibility with both modular (JPMS) and non-modular environments - Remove unused Instant import in test class for cleaner imports Signed-off-by: finger <[email protected]>

yuyu1025 · 2025-05-18T01:57:21Z

@yuyu1025,

I noticed you've enabled "Vigilant mode", but it looks like your GPG key isn’t added to your GitHub account—so your commits are showing as “Unverified.” That’s probably not what you intended.

To resolve this, add your GPG key to your GitHub account to verify that you are the author of your commits.

I understand, thank you for the reminder.

ppkarwasz · 2025-05-18T09:29:26Z

pom.xml

+          <!--
+            ~ Modifies the inherited Moditect configuration to add `java.sql` as `static` (optional) dependency.
+            -->
+          <plugin>
+            <groupId>org.moditect</groupId>
+            <artifactId>moditect-maven-plugin</artifactId>
+            <executions>
+              <execution>
+                <id>add-module-infos</id>
+                <configuration>
+                  <module>
+                    <moduleInfo>
+                      <requires>
+                        static java.sql;
+                        *;
+                      </requires>
+                    </moduleInfo>
+                  </module>
+                </configuration>
+              </execution>
+            </executions>
+          </plugin>


I attempted to update the inherited Moditect Maven Plugin configuration to include the following directive in the generated module descriptor:

requires static java.sql;

However, due to moditect/moditect#262, this change currently has no effect.

@garydgregory, should we keep the directive in place in anticipation of the issue being fixed, or remove it for now and reintroduce it once the plugin supports it correctly?

@ppkarwasz
Well, no, since we shouldn't depend on java[x].sql in the first place.

All,
Maybe this kind of this-to-that conversion code belongs in Commons BeanUtils.

In Lang, DateUtils has only one kind of conversion ATM with 'toCalendar'(...). I'm not sure we want to open the door to more convertions, then there would be endless combinations due to the scale of the Java Time package.

Signed-off-by: finger <[email protected]>

ppkarwasz

This looks good to me!

I would just make sure that each test has a test case with java.util.Date, java.sql.Date and java.sql.Timestamp.

ppkarwasz · 2025-05-18T10:10:27Z

src/test/java/org/apache/commons/lang3/time/DateUtilsTest.java

+    private static Stream<Arguments> dateConversionProvider() {
+        return Stream.of(
+                Arguments.of(
+                        java.sql.Date.valueOf("2000-01-01"),
+                        LocalDateTime.of(2000, 1, 1, 0, 0, 0)
+                ),


Could you add some test cases with java.util.Date?

ppkarwasz · 2025-05-18T10:11:02Z

src/test/java/org/apache/commons/lang3/time/DateUtilsTest.java

+    private static Stream<Arguments> dateWithTimeZoneProvider() {
+        return Stream.of(
+                Arguments.of(
+                        java.sql.Timestamp.valueOf("2000-01-01 12:30:45"),
+                        TimeZone.getTimeZone("America/New_York"),
+                        LocalDateTime.ofInstant(
+                                java.sql.Timestamp.valueOf("2000-01-01 12:30:45").toInstant(),
+                                TimeZone.getTimeZone("America/New_York").toZoneId()
+                        )
+                ),


Could you add some test cases with java.sql.Date?

Could you add some test cases with java.sql.Date?

OK, I'll add some now.

Signed-off-by: finger <[email protected]>

garydgregory requested changes May 17, 2025

View reviewed changes

yuyu1025 force-pushed the feature/date_to_localdatetime branch from 27088ad to 4d69373 Compare May 17, 2025 15:11

yuyu1025 force-pushed the feature/date_to_localdatetime branch from fe3e661 to 6fb19d6 Compare May 17, 2025 16:34

yuyu1025 requested a review from garydgregory May 17, 2025 16:36

garydgregory reviewed May 17, 2025

View reviewed changes

src/main/java/org/apache/commons/lang3/time/DateUtils.java Outdated Show resolved Hide resolved

ppkarwasz requested changes May 17, 2025

View reviewed changes

src/main/java/org/apache/commons/lang3/time/DateUtils.java Outdated Show resolved Hide resolved

src/main/java/org/apache/commons/lang3/time/DateUtils.java Outdated Show resolved Hide resolved

src/test/java/org/apache/commons/lang3/time/DateUtilsTest.java Outdated Show resolved Hide resolved

yuyu1025 and others added 6 commits May 18, 2025 08:58

Manually convert date to Instant to avoid exceptions caused by incomp…

79b1099

…atible subclasses (such as java.sql.Date).

style: Add the final modifier to the variable

f7dcdb8

style: Remove an extra space

45003ab

style: Optimize readability

0d479ab

Co-authored-by: Piotr P. Karwasz <[email protected]>

yuyu1025 force-pushed the feature/date_to_localdatetime branch from 3044828 to 0d479ab Compare May 18, 2025 00:59

yuyu1025 force-pushed the feature/date_to_localdatetime branch from e8faf11 to f7928be Compare May 18, 2025 01:28

fix: add java.sql as optional dependency to Moditect

88b7114

ppkarwasz reviewed May 18, 2025

View reviewed changes

fix: Fix unit test errors caused by different default time zones

36eb042

Signed-off-by: finger <[email protected]>

ppkarwasz approved these changes May 18, 2025

View reviewed changes

yuyu1025 added 2 commits May 18, 2025 18:20

style: Optimize the format by building verification

5d20c0b

Signed-off-by: finger <[email protected]>

style: add final var

eb906ba

Signed-off-by: finger <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add Date to LocalDateTime conversion methods to bridge legacy and modern date APIs #1385

Add Date to LocalDateTime conversion methods to bridge legacy and modern date APIs #1385

Uh oh!

yuyu1025 commented May 17, 2025

Uh oh!

garydgregory left a comment

Uh oh!

yuyu1025 commented May 17, 2025

Uh oh!

ppkarwasz commented May 17, 2025

Uh oh!

yuyu1025 commented May 17, 2025

Uh oh!

Uh oh!

ppkarwasz left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ppkarwasz commented May 17, 2025

Uh oh!

yuyu1025 commented May 18, 2025

Uh oh!

ppkarwasz May 18, 2025

Uh oh!

garydgregory May 18, 2025

Uh oh!

ppkarwasz left a comment

Uh oh!

ppkarwasz May 18, 2025

Uh oh!

ppkarwasz May 18, 2025

Uh oh!

yuyu1025 May 18, 2025

Uh oh!

Uh oh!

Add Date to LocalDateTime conversion methods to bridge legacy and modern date APIs #1385

Are you sure you want to change the base?

Add Date to LocalDateTime conversion methods to bridge legacy and modern date APIs #1385

Uh oh!

Conversation

yuyu1025 commented May 17, 2025

Background and Motivation

Core Change Content

Value Added

Compatibility Note

Uh oh!

garydgregory left a comment

Choose a reason for hiding this comment

Uh oh!

yuyu1025 commented May 17, 2025

Uh oh!

ppkarwasz commented May 17, 2025

Uh oh!

yuyu1025 commented May 17, 2025

Uh oh!

Uh oh!

ppkarwasz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ppkarwasz commented May 17, 2025

Uh oh!

yuyu1025 commented May 18, 2025

Uh oh!

ppkarwasz May 18, 2025

Choose a reason for hiding this comment

Uh oh!

garydgregory May 18, 2025

Choose a reason for hiding this comment

Uh oh!

ppkarwasz left a comment

Choose a reason for hiding this comment

Uh oh!

ppkarwasz May 18, 2025

Choose a reason for hiding this comment

Uh oh!

ppkarwasz May 18, 2025

Choose a reason for hiding this comment

Uh oh!

yuyu1025 May 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!