Jsr310Converters do not work correctly with LocalDate/LocalDateTime/LocalTime across timezones

[jandurovec]

Problem

The converters for LocalTime, LocalDate and LocalDateTime currently convert the local values to a Date (to be stored in the DB) using systemDefault() timezone, e.g.

spring-data-commons/src/main/java/org/springframework/data/convert/Jsr310Converters.java

Line 165 in 0ba14c1

return Date.from(source.atDate(LocalDate.now()).atZone(systemDefault()).toInstant());

This works OK if writing and reading of the value happens in the same JVM or in JVMs running in the same location.

However, for an application distributed across timezones (and with shared DB) it creates an issue, because when the data saved by one instance are read by another instance, they retrieve different information.

Verification

A sample test case would look something like this

@Test
void preservesLocalTimeWhenReadInDifferentSystemDefaultTimeZone() {

        var defaultTimeZone = TimeZone.getDefault();
        var source = LocalTime.of(12, 34, 56);

        try {
                TimeZone.setDefault(TimeZone.getTimeZone(ZoneId.of("Europe/London")));
                var stored = CONVERSION_SERVICE.convert(source, Date.class);

                TimeZone.setDefault(TimeZone.getTimeZone(ZoneId.of("America/New_York")));
                var read = CONVERSION_SERVICE.convert(stored, LocalTime.class);

                assertThat(read).isEqualTo(source);
        } finally {
                TimeZone.setDefault(defaultTimeZone);
        }
}

Currently, this case would fail.

Proposal

I would suggest using a fixed (UTC) timezone when converting LocalTime, LocalDate and LocalDateTime to Date values, which would bring consistency when being stored and read regardless of the timezone.

The issue is that this is a breaking change, and applications that already have some data in the database (stored in their timezone) will break when upgrading the lib version to the one with the fix, because they will suddenly read different information (unless they run in JVM with UTC timezone)

As a workaround, this can be solved by the clients by adding @Field(targetType = STRING) on their entities, but in this case StringToLocalTimeConverter should be added to this library, because it is missing (converters from String to LocalDate and LocalDateTime exist)

[adityaanikam]

Both parts check out on current main. All the systemDefault() call sites are still there including the exact line you linked, and confirmed StringToLocalTimeConverter really is missing while StringToLocalDateConverter and StringToLocalDateTimeConverter both exist and are registered.

Adding StringToLocalTimeConverter seems safe to just do, it closes an obvious gap in the existing STRING target type workaround with no behavior change for anyone not already using it. The systemDefault() to UTC switch is the bigger question though, you've already laid out the tradeoff yourself: correct going forward, but silently reinterprets whatever is already sitting in the database for anyone upgrading. That's not something I'd want to just PR without knowing whether the plan is a hard behavior change, an opt in flag, or a major version bump. Happy to open the PR for the missing converter now and leave the timezone question to whoever wants to weigh in on migration strategy.