Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent dates when using setters taking a java.time.Instant #588

Open
ingokegel opened this issue Nov 16, 2019 · 0 comments
Open

Inconsistent dates when using setters taking a java.time.Instant #588

ingokegel opened this issue Nov 16, 2019 · 0 comments

Comments

@ingokegel
Copy link

I just migrated from Charts 3.2.1 and wanted to convert to the setters that take a java.time.Instant instead of a java.util.Date - not least because all the setters with java.util.Date are deprecated now.

However, the logic for the converting a java.time.Instant to a Highchart time is not consistent with the previous behavior and other parts of the API. All the corresponding setters call Util::toHighchartsTS(Instant) which just returns the epoch milliseconds. Setters taking a java.util.Date call Util::toHighchartsTS(Date) which returns

date.getTime() - date.getTimezoneOffset() * 60000;

java.util.Date does not have a time zone, Date::getTimezoneOffset() just refers to the default time zone. Just like java.time.Instant, its getTime() method returns the same value as Instant::toEpochMilli(), namely the epoch in milliseconds. Adding the time zone offset is important, though, because Highchart will subtract it again to convert to UTC.

What happens now is this:

  1. With the Date-based setters, the chart will show server times. In my opinion, this is the desirable behavior and this is how it worked before 4.0.
  2. With the Instant-based setters, the chart will show UTC times. However, you cannot implement this consistently, because there are still (undeprecated) Date-based setters without an Instant-based equivalent, such as Axis::setMax/setMin and those are still transmitted with the timezone offset.

To solve this issue, the implementation of Util::toHighchartsTS(Instant) should be changed to

public static long toHighchartsTS(Instant date) {
    long epochMilli = date.toEpochMilli();
    return epochMilli + TimeZone.getDefault().getOffset(epochMilli);
}

To convince yourself of the relationships between Date and Instant, run the following code:

Instant instant = Instant.now();
Date date = Date.from(instant);
long epochMilli = instant.toEpochMilli();
long time = date.getTime();

System.out.println("instant epochMills = " + epochMilli);
System.out.println("date time = " + time);
System.out.println("equals " + (epochMilli == time));

long instantPlusOffset = epochMilli + TimeZone.getDefault().getOffset(epochMilli);
System.out.println("instant + offset = " + instantPlusOffset);
long datePlusOffset = time - date.getTimezoneOffset() * 60000;
System.out.println("date + offset = " + datePlusOffset);

System.out.println("equals " + (datePlusOffset == instantPlusOffset));
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant