Skip to content

Commit

Permalink
Use ZoneId rather than ZoneOffset for conversion (#9620)
Browse files Browse the repository at this point in the history
Use ZoneId rather than ZoneOffset for conversion between model (Date) and presentation of LocalDateTime.

LocalDateToDateConverter (correctly) uses ZoneId, whereas LocalDateTimeToDateConverter (incorrectly) used a ZoneOffset. This fix aligns the two Converter implementations and makes the latter one more robust.

A ZoneOffset is a fixed time difference, e.g., +07:00, whereas a time zone - represented by a ZoneId - is more dynamic, including features like Daylight-Savings Time. A ZoneId returns one or more ZoneOffsets via its ZoneRules method. (ZoneOffsets have trivial ZoneRules that simply return the ZoneOffset.)

Since the date/time being displayed may be from any date on the calendar, the ZoneOffset imposes a negative limitation. Using ZoneId instead gets us past that limitation and allows a more robust set of conversion rules.

Fixes #9594
  • Loading branch information
eepstein authored and hesara committed Jul 3, 2017
1 parent 840e45b commit 7d6408a
Showing 1 changed file with 10 additions and 16 deletions.
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@


import java.time.Instant; import java.time.Instant;
import java.time.LocalDateTime; import java.time.LocalDateTime;
import java.time.ZoneOffset; import java.time.ZoneId;
import java.util.Date; import java.util.Date;
import java.util.Objects; import java.util.Objects;


Expand All @@ -35,41 +35,35 @@
* @author Vaadin Ltd * @author Vaadin Ltd
* @since 8.0 * @since 8.0
*/ */
public class LocalDateTimeToDateConverter public class LocalDateTimeToDateConverter implements Converter<LocalDateTime, Date> {
implements Converter<LocalDateTime, Date> {


private ZoneOffset zoneOffset; private ZoneId zoneId;


/** /**
* Creates a new converter using the given time zone. * Creates a new converter using the given time zone.
* *
* @param zoneOffset * @param zoneId the time zone to use, not <code>null</code>
* the time zone offset to use, not <code>null</code>
*/ */
public LocalDateTimeToDateConverter(ZoneOffset zoneOffset) { public LocalDateTimeToDateConverter(ZoneId zoneId) {
this.zoneOffset = Objects.requireNonNull(zoneOffset, this.zoneId = Objects.requireNonNull(zoneId, "Zone identifier cannot be null");
"Zone offset cannot be null");
} }


@Override @Override
public Result<Date> convertToModel(LocalDateTime localDate, public Result<Date> convertToModel(LocalDateTime localDate, ValueContext context) {
ValueContext context) {
if (localDate == null) { if (localDate == null) {
return Result.ok(null); return Result.ok(null);
} }


return Result.ok(Date.from(localDate.toInstant(zoneOffset))); return Result.ok(Date.from(localDate.atZone(zoneId).toInstant()));
} }


@Override @Override
public LocalDateTime convertToPresentation(Date date, public LocalDateTime convertToPresentation(Date date, ValueContext context) {
ValueContext context) {
if (date == null) { if (date == null) {
return null; return null;
} }


return Instant.ofEpochMilli(date.getTime()).atZone(zoneOffset) return Instant.ofEpochMilli(date.getTime()).atZone(zoneId).toLocalDateTime();
.toLocalDateTime();
} }


} }

0 comments on commit 7d6408a

Please sign in to comment.