[
https://issues.apache.org/jira/browse/COMPRESS-613?focusedWorklogId=836166&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-836166
]
ASF GitHub Bot logged work on COMPRESS-613:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 29/Dec/22 21:11
Start Date: 29/Dec/22 21:11
Worklog Time Spent: 10m
Work Description: garydgregory commented on code in PR #345:
URL: https://github.com/apache/commons-compress/pull/345#discussion_r1059143999
##########
src/main/java/org/apache/commons/compress/archivers/zip/X000A_NTFS.java:
##########
@@ -68,7 +68,14 @@
* @NotThreadSafe
*/
public class X000A_NTFS implements ZipExtraField {
- private static final ZipShort HEADER_ID = new ZipShort(0x000a);
+
+ /**
+ * The header ID for this extra field.
+ *
+ * @since 1.23
+ */
+ public static final ZipShort HEADER_ID = new ZipShort(0x000a);
Review Comment:
Why is this public now? Would package-private suffice? Let's make sure to
avoid increasing the API surface unless we must.
##########
src/main/java/org/apache/commons/compress/utils/TimeUtils.java:
##########
@@ -46,6 +46,62 @@ public final class TimeUtils {
*/
static final long WINDOWS_EPOCH_OFFSET = -116444736000000000L;
+ /**
+ * Converts "standard UNIX time" (in seconds, UTC/GMT) to {@link FileTime}.
Review Comment:
Why is "standard UNIX time" in quotes?
##########
src/main/java/org/apache/commons/compress/utils/TimeUtils.java:
##########
@@ -46,6 +46,62 @@ public final class TimeUtils {
*/
static final long WINDOWS_EPOCH_OFFSET = -116444736000000000L;
+ /**
+ * Converts "standard UNIX time" (in seconds, UTC/GMT) to {@link FileTime}.
+ *
+ * @param time UNIX timestamp
+ * @return the corresponding FileTime
+ */
+ public static FileTime unixTimeToFileTime(final long time) {
+ return FileTime.from(time, TimeUnit.SECONDS);
+ }
+
+ /**
+ * Converts {@link FileTime} to "standard UNIX time".
+ *
+ * @param time the original FileTime
+ * @return the UNIX timestamp
+ */
+ public static long fileTimeToUnixTime(final FileTime time) {
+ return time.to(TimeUnit.SECONDS);
+ }
+
+ /**
+ * Converts Java time (milliseconds since Epoch) to "standard UNIX time".
+ *
+ * @param time the original Java time
+ * @return the UNIX timestamp
+ */
+ public static long javaTimeToUnixTime(final long time) {
Review Comment:
The method name is good here because the parameter and return type are long.
##########
src/main/java/org/apache/commons/compress/archivers/zip/X5455_ExtendedTimestamp.java:
##########
@@ -130,16 +155,20 @@ private static Date zipLongToDate(final ZipLong unixTime)
{
return unixTime != null ? new Date(unixTime.getIntValue() * 1000L) :
null;
}
+ private static FileTime zipLongToFileTime(final ZipLong unixTime) {
Review Comment:
The "zipLong" method name prefix is redundant with the argument type
`zipLongToFileTime` -> `toFileTime` or probably better `zipLongToFileTime` ->
`unixToFileTime`
##########
src/main/java/org/apache/commons/compress/archivers/zip/X000A_NTFS.java:
##########
@@ -97,13 +105,13 @@ private static FileTime zipToFileTime(final
ZipEightByteInteger z) {
}
return TimeUtils.ntfsTimeToFileTime(z.getLongValue());
}
-
private ZipEightByteInteger modifyTime = ZipEightByteInteger.ZERO;
private ZipEightByteInteger accessTime = ZipEightByteInteger.ZERO;
private ZipEightByteInteger createTime = ZipEightByteInteger.ZERO;
+
Review Comment:
Remove extra empty line.
##########
src/main/java/org/apache/commons/compress/archivers/zip/X5455_ExtendedTimestamp.java:
##########
@@ -242,6 +285,20 @@ public Date getCreateJavaTime() {
return zipLongToDate(createTime);
}
+ /**
+ * Returns the create time as a {@link FileTime}
+ * of this zip entry, or null if no such timestamp exists in the zip entry.
+ * The milliseconds are always zeroed out, since the underlying data
+ * offers only per-second precision.
+ *
+ * @return modify time as {@link FileTime} or null.
+ *
Review Comment:
Remove extra line b/w Javadoc tags return and since.
##########
src/main/java/org/apache/commons/compress/utils/TimeUtils.java:
##########
@@ -46,6 +46,62 @@ public final class TimeUtils {
*/
static final long WINDOWS_EPOCH_OFFSET = -116444736000000000L;
+ /**
+ * Converts "standard UNIX time" (in seconds, UTC/GMT) to {@link FileTime}.
+ *
+ * @param time UNIX timestamp
+ * @return the corresponding FileTime
+ */
+ public static FileTime unixTimeToFileTime(final long time) {
+ return FileTime.from(time, TimeUnit.SECONDS);
+ }
+
+ /**
+ * Converts {@link FileTime} to "standard UNIX time".
+ *
+ * @param time the original FileTime
+ * @return the UNIX timestamp
+ */
+ public static long fileTimeToUnixTime(final FileTime time) {
+ return time.to(TimeUnit.SECONDS);
+ }
+
+ /**
+ * Converts Java time (milliseconds since Epoch) to "standard UNIX time".
+ *
+ * @param time the original Java time
+ * @return the UNIX timestamp
+ */
+ public static long javaTimeToUnixTime(final long time) {
+ return time / 1000L;
+ }
+
+ /**
+ * Checks whether a FileTime exceeds the minimum or maximum for the
"standard UNIX time".
+ * If the FileTime is null, this method always returns false.
+ *
+ * @param time the FileTime to evaluate, can be null
+ * @return true if the time exceeds the minimum or maximum UNIX time,
false otherwise
+ */
+ public static boolean exceedsUnixTime(final FileTime time) {
Review Comment:
The word "exceeds" is quite unusual in APIs IMO, also implies a greater than
relationship, which is not what happens here. I would just is the more standard
"is" prefix for names like "isUnixTime()". For "is", method, I've been using
the starting sentence as "Tests...".
##########
src/main/java/org/apache/commons/compress/utils/TimeUtils.java:
##########
@@ -46,6 +46,62 @@ public final class TimeUtils {
*/
static final long WINDOWS_EPOCH_OFFSET = -116444736000000000L;
+ /**
+ * Converts "standard UNIX time" (in seconds, UTC/GMT) to {@link FileTime}.
+ *
+ * @param time UNIX timestamp
+ * @return the corresponding FileTime
+ */
+ public static FileTime unixTimeToFileTime(final long time) {
+ return FileTime.from(time, TimeUnit.SECONDS);
+ }
+
+ /**
+ * Converts {@link FileTime} to "standard UNIX time".
+ *
+ * @param time the original FileTime
+ * @return the UNIX timestamp
+ */
+ public static long fileTimeToUnixTime(final FileTime time) {
Review Comment:
The method prefix "fileTime" is redundant with the argument type,
`fileTimeToUnixTime` -> `toUnixTime`. Since Unix time is always in seconds (as
a primitive long here), we should not need to include the scale in the name.
##########
src/main/java/org/apache/commons/compress/archivers/zip/X000A_NTFS.java:
##########
@@ -97,13 +105,13 @@ private static FileTime zipToFileTime(final
ZipEightByteInteger z) {
}
return TimeUtils.ntfsTimeToFileTime(z.getLongValue());
}
-
Review Comment:
Keep this empty line.
Issue Time Tracking
-------------------
Worklog Id: (was: 836166)
Time Spent: 0.5h (was: 20m)
> Write ZIP extra time fields automatically
> -----------------------------------------
>
> Key: COMPRESS-613
> URL: https://issues.apache.org/jira/browse/COMPRESS-613
> Project: Commons Compress
> Issue Type: Improvement
> Components: Archivers
> Affects Versions: 1.21
> Reporter: Andre Brait
> Priority: Major
> Labels: zip
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> When writing to a Zip file through ZipArchiveOutputStream, setting creation
> and access times in a ZipArchiveEntry does not cause these to be reflected as
> X5455 or X000A extra fields in the resulting zip file. This also happens for
> modification times that do not fit into an MS-DOS time.
> As a consequence, the date range is reduced, as well as the granularity (form
> 100ns intervals to seconds).
> ZipEntry and standard java.util.zip facilities do that automatically, but
> that's missing here.
> My proposal is to use the same logic the java.util.zip do and add those extra
> fields automatically, if situation requires them.
> See my existing logic for this here:
> [https://github.com/andrebrait/DATROMTool/blob/86a4f4978bab250ca54d047c58b4f91e7dbbcc7f/core/src/main/java/io/github/datromtool/io/FileCopier.java#L1425]
> It's (almost) the same logic from java.util.zip, but adapted to be used with
> ZipArchiveEntry.
> If you're ok with it, I can send a PR.
> Actual logic will be more like
> {{{}java.util.zip.ZipOutputStream#writeLOC(XEntry){}}}, represented below:
> {code:java}
> int elenEXTT = 0; // info-zip extended timestamp
> int flagEXTT = 0;
> long umtime = -1;
> long uatime = -1;
> long uctime = -1;
> if (e.mtime != null) {
> elenEXTT += 4;
> flagEXTT |= EXTT_FLAG_LMT;
> umtime = fileTimeToUnixTime(e.mtime);
> }
> if (e.atime != null) {
> elenEXTT += 4;
> flagEXTT |= EXTT_FLAG_LAT;
> uatime = fileTimeToUnixTime(e.atime);
> }
> if (e.ctime != null) {
> elenEXTT += 4;
> flagEXTT |= EXTT_FLAT_CT;
> uctime = fileTimeToUnixTime(e.ctime);
> }
> if (flagEXTT != 0) {
> // to use ntfs time if any m/a/ctime is beyond unixtime upper
> bound
> if (umtime > UPPER_UNIXTIME_BOUND ||
> uatime > UPPER_UNIXTIME_BOUND ||
> uctime > UPPER_UNIXTIME_BOUND) {
> elen += 36; // NTFS time, total 36 bytes
> } else {
> elen += (elenEXTT + 5); // headid(2) + size(2) + flag(1) +
> data
> }
> }
> writeShort(elen);
> writeBytes(nameBytes, 0, nameBytes.length);
> if (hasZip64) {
> writeShort(ZIP64_EXTID);
> writeShort(16);
> writeLong(e.size);
> writeLong(e.csize);
> }
> if (flagEXTT != 0) {
> if (umtime > UPPER_UNIXTIME_BOUND ||
> uatime > UPPER_UNIXTIME_BOUND ||
> uctime > UPPER_UNIXTIME_BOUND) {
> writeShort(EXTID_NTFS); // id
> writeShort(32); // data size
> writeInt(0); // reserved
> writeShort(0x0001); // NTFS attr tag
> writeShort(24);
> writeLong(e.mtime == null ? WINDOWS_TIME_NOT_AVAILABLE
> : fileTimeToWinTime(e.mtime));
> writeLong(e.atime == null ? WINDOWS_TIME_NOT_AVAILABLE
> : fileTimeToWinTime(e.atime));
> writeLong(e.ctime == null ? WINDOWS_TIME_NOT_AVAILABLE
> : fileTimeToWinTime(e.ctime));
> } else {
> writeShort(EXTID_EXTT);
> writeShort(elenEXTT + 1); // flag + data
> writeByte(flagEXTT);
> if (e.mtime != null)
> writeInt(umtime);
> if (e.atime != null)
> writeInt(uatime);
> if (e.ctime != null)
> writeInt(uctime);
> }
> }
> writeExtra(e.extra);
> locoff = written;
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
