[GitHub] [commons-compress] andrebrait commented on a change in pull request #256: COMPRESS-614: Use FileTime in SevenZArchiveEntry

2022-03-21 Thread GitBox


andrebrait commented on a change in pull request #256:
URL: https://github.com/apache/commons-compress/pull/256#discussion_r831407866



##
File path: src/main/java/org/apache/commons/compress/archivers/zip/ZipUtil.java
##
@@ -30,11 +33,32 @@
  * @Immutable
  */
 public abstract class ZipUtil {
+
+/**
+ * https://msdn.microsoft.com/en-us/library/windows/desktop/ms724290%28v=vs.85%29.aspx;>Windows
 File Times
+ * 

Review comment:
   Fixed and moved this to TimeUtils




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [commons-compress] andrebrait commented on a change in pull request #256: COMPRESS-614: Use FileTime in SevenZArchiveEntry

2022-03-21 Thread GitBox


andrebrait commented on a change in pull request #256:
URL: https://github.com/apache/commons-compress/pull/256#discussion_r831407462



##
File path: 
src/main/java/org/apache/commons/compress/archivers/zip/X000A_NTFS.java
##
@@ -244,6 +245,36 @@ public Date getCreateJavaTime() {
 return zipToDate(createTime);
 }
 
+/**
+ * Returns the modify time as as a {@link FileTime}

Review comment:
   Fixed

##
File path: 
src/test/java/org/apache/commons/compress/archivers/sevenz/SevenZFileTest.java
##
@@ -836,4 +857,37 @@ private void checkHelloWorld(final String filename) throws 
Exception {
 private static boolean isStrongCryptoAvailable() throws 
NoSuchAlgorithmException {
 return Cipher.getMaxAllowedKeyLength("AES/ECB/PKCS5Padding") >= 256;
 }
+
+private void assertDates(SevenZArchiveEntry e, String modified, String 
access, String creation) {
+if (modified != null) {
+assertTrue(e.getHasLastModifiedDate());
+FileTime time = FileTime.from(Instant.parse(modified));

Review comment:
   Fixed




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [commons-compress] andrebrait commented on a change in pull request #256: COMPRESS-614: Use FileTime in SevenZArchiveEntry

2022-03-21 Thread GitBox


andrebrait commented on a change in pull request #256:
URL: https://github.com/apache/commons-compress/pull/256#discussion_r831407044



##
File path: 
src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZArchiveEntry.java
##
@@ -198,14 +223,27 @@ public void setHasLastModifiedDate(final boolean 
hasLastModifiedDate) {
 
 /**
  * Gets the last modified date.
- * @throws UnsupportedOperationException if the entry hasn't got a
- * last modified date.
+ * This is equivalent to {@link SevenZArchiveEntry#getLastModifiedTime()}, 
but precision is truncated to milliseconds.
+ *
+ * @throws UnsupportedOperationException if the entry hasn't got a last 
modified date.
  * @return the last modified date
+ * @see SevenZArchiveEntry#getLastModifiedTime()
  */
 @Override
 public Date getLastModifiedDate() {
+return new Date(getLastModifiedTime().toMillis());

Review comment:
   I created a TimeUtils utility class that will also be used for another 
improvement I have down the line.

##
File path: 
src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZArchiveEntry.java
##
@@ -217,17 +255,29 @@ public Date getLastModifiedDate() {
  * @param ntfsLastModifiedDate the last modified date
  */
 public void setLastModifiedDate(final long ntfsLastModifiedDate) {
-this.lastModifiedDate = ntfsLastModifiedDate;
+this.lastModifiedDate = 
ZipUtil.ntfsTimeToFileTime(ntfsLastModifiedDate);
 }
 
 /**
- * Sets the last modified date,
- * @param lastModifiedDate the last modified date
+ * Sets the last modified date.
+ *
+ * @param lastModifiedDate the new last modified date
+ * @see SevenZArchiveEntry#setLastModifiedTime(FileTime)
  */
 public void setLastModifiedDate(final Date lastModifiedDate) {
-hasLastModifiedDate = lastModifiedDate != null;
+setLastModifiedTime(toFileTime(lastModifiedDate));
+}
+
+/**
+ * Sets the last modified date.

Review comment:
   Fixed. Thanks.

##
File path: 
src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZArchiveEntry.java
##
@@ -198,14 +223,27 @@ public void setHasLastModifiedDate(final boolean 
hasLastModifiedDate) {
 
 /**
  * Gets the last modified date.
- * @throws UnsupportedOperationException if the entry hasn't got a
- * last modified date.
+ * This is equivalent to {@link SevenZArchiveEntry#getLastModifiedTime()}, 
but precision is truncated to milliseconds.
+ *
+ * @throws UnsupportedOperationException if the entry hasn't got a last 
modified date.
  * @return the last modified date
+ * @see SevenZArchiveEntry#getLastModifiedTime()
  */
 @Override
 public Date getLastModifiedDate() {
+return new Date(getLastModifiedTime().toMillis());
+}
+
+/**
+ * Gets the last modified date.

Review comment:
   Fixed

##
File path: 
src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZArchiveEntry.java
##
@@ -249,13 +299,26 @@ public void setHasAccessDate(final boolean hasAcessDate) {
 
 /**
  * Gets the access date.
- * @throws UnsupportedOperationException if the entry hasn't got a
- * access date.
+ * This is equivalent to {@link SevenZArchiveEntry#getAccessTime()}, but 
precision is truncated to milliseconds.
+ *
+ * @throws UnsupportedOperationException if the entry hasn't got an access 
date.
  * @return the access date
+ * @see SevenZArchiveEntry#getAccessTime()
  */
 public Date getAccessDate() {
+return new Date(getAccessTime().toMillis());
+}
+
+/**
+ * Gets the access date.

Review comment:
   Fixed




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [commons-compress] andrebrait commented on a change in pull request #256: COMPRESS-614: Use FileTime in SevenZArchiveEntry

2022-03-21 Thread GitBox


andrebrait commented on a change in pull request #256:
URL: https://github.com/apache/commons-compress/pull/256#discussion_r831222311



##
File path: 
src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZOutputFile.java
##
@@ -178,10 +184,18 @@ public SevenZArchiveEntry createArchiveEntry(final Path 
inputPath,
 final SevenZArchiveEntry entry = new SevenZArchiveEntry();
 entry.setDirectory(Files.isDirectory(inputPath, options));
 entry.setName(entryName);
-entry.setLastModifiedDate(new 
Date(Files.getLastModifiedTime(inputPath, options).toMillis()));
+fillDates(inputPath, entry, options);
 return entry;
 }
 
+private void fillDates(final Path inputPath, final SevenZArchiveEntry 
entry,
+final LinkOption... options) throws IOException {
+BasicFileAttributes attributes = Files.readAttributes(inputPath, 
BasicFileAttributes.class, options);
+entry.setLastModifiedTime(attributes.lastModifiedTime());

Review comment:
   This is what TAR does now. 7-Zip only adds those dates if you pass an 
argument to it. Let me know what you think is best to do.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org