Hi,
On 2015-06-30 23:11, Xueming Shen wrote:
Hi,
Please help review and comment on this rfe.
Issue: https://bugs.openjdk.java.net/browse/JDK-8075526
webrev: http://cr.openjdk.java.net/~sherman/8075526
I think this looks reasonable.
I think we could consolidate the LocalDateTime<->xdostime
conversions into ZipUtils along with and aligned with the code
to convert UTC instants (ZipUtils::dosToJavaTime/javaToDosTime),
which I've suggested should be converted into similar code
anyhow:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-February/031716.html
The get32S changes seems like an unrelated but welcome
optimization.
Background info:
The title of the RFE is a little "mis-leading". All the existing
set/get date-time
methods actually work with "UTC" time. Both the old pair
public void setTime(long time);
public long getTime();
and the newly introduced pair
pubic ZipEntry set/getLastModifiedTime(FileTime time);
public FileTime getLastModifiedTime();
are to set/get the last modified time in UTC time. However the ZIP
specification
clearly specifies that the "normal" date and time fields in the zip
file entry (loc and
cen) are defined as the date/time in dos format, which unfortunately
is a "local"
date-time. Therefor timezone conversion must be applied before/after
the utc time
can be set into/got from those fields (the UTC timestamps set/get by
the new pair
are therefor being set into/got from the "extended timestamp fields"
of the optional
extra data of each entry, those fields are specified as unix/utc
timestamp)
We did not have an "local-date-time" abstract before the
java.time.LocalDateTime
was introduced in jdk8, the epoc date/time is the only date/time
abstract in java
vm.
The proposed change here is to add yet another pair of set/get
modified time methods
public void setTimeLocale(LocalDateTime time);
public LocalDateTime getTimeLocal();
to use the java.time.LocalDateTime type to set/get the modified time
into zip entry's
dos timestamp fields directly WITHOUT involving the timezone
conversion (implied, with
default TimeZone).
Other than solving the pack/unpack problem raised in this RFE, it
should also help improve
the performance when local-date-time is actually desired when
interfacing with the ZipEntry
by eliminating the un-necessary/implied timezone conversion. For
example, in our jar tool,
currently we are "printing" the timestamp for zip entry "e" as
new Date(e.getTime()).toString();
in which we are converting the local-date-time (ms-dos-formatted in
zip entry) to utc time
by using the default timezone (in ZipEntry), and then converting the
utc time (in Date) back
to the printable "local date time" again.
It might be desired to format the "local-date-time" directly without
involving the timezone
conversion now via the proposed method
java.time.format.DateTimeFormatter.ofPattern("EEE MMM dd HH:mm:ss zzz
yyyy")
.withZone(java.time.ZoneId.systemDefault());
.format(e.getTimeLocal()
In above example, we still use the "system default timezone", however,
it is used
purely to output the zone name for the "zzz" (which the
Date.toString() does), not for conversion.
if the "zzz" is not required/needed, it can just be
java.time.format.DateTimeFormatter.ofPattern("EEE MMM dd HH:mm:ss
yyyy").format(e.getTimeLocal());
Since you're asking for opinions:
I think we should try not to propagate the problem we're working around
here
(that values are printed/represented in a local time with no way of
knowing what
timezone the time was local to). If we can print the local timezone at
little to no
cost, why not?
Thanks!
/Claes
Comment/Opinion please. If we agree the approach/webrev, I will submit
the CCC before integration.
Thanks,
-Sherman
