This is an automated email from the ASF dual-hosted git repository. ggregory pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-io.git
The following commit(s) were added to refs/heads/master by this push: new b824205 Add and use FileUtils.lastModified[Unchecked](File) to workaround https://bugs.openjdk.java.net/browse/JDK-8177809. b824205 is described below commit b8242057cd8bbb5af2fe19a5354fec8e31b86032 Author: Gary Gregory <gardgreg...@gmail.com> AuthorDate: Wed Jan 27 11:30:11 2021 -0500 Add and use FileUtils.lastModified[Unchecked](File) to workaround https://bugs.openjdk.java.net/browse/JDK-8177809. --- src/changes/changes.xml | 3 ++ src/main/java/org/apache/commons/io/FileUtils.java | 63 ++++++++++++++++++---- .../io/comparator/LastModifiedFileComparator.java | 6 ++- .../commons/io/filefilter/AgeFileFilter.java | 2 +- .../java/org/apache/commons/io/input/Tailer.java | 6 +-- .../org/apache/commons/io/monitor/FileEntry.java | 35 ++++++------ .../commons/io/FileUtilsFileNewerTestCase.java | 15 +++--- .../org/apache/commons/io/FileUtilsTestCase.java | 12 ++--- .../comparator/LastModifiedFileComparatorTest.java | 28 +++++----- .../io/monitor/AbstractMonitorTestCase.java | 10 ++-- .../io/monitor/FileAlterationObserverTestCase.java | 13 +++-- 11 files changed, 125 insertions(+), 68 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 782791b..067747c 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -148,6 +148,9 @@ The <action> type attribute can be add,update,fix,remove. <action issue="IO-700" dev="ggregory" type="add" due-to="Gary Gregory"> Add FileUtils.isEmptyDirectory(File). </action> + <action dev="ggregory" type="add" due-to="Gary Gregory"> + Add FileUtils.lastModified[Unchecked](File) to workaround https://bugs.openjdk.java.net/browse/JDK-8177809. + </action> <!-- UPDATES --> <action dev="ggregory" type="update" due-to="Dependabot"> Update junit-jupiter from 5.6.2 to 5.7.0 #153. diff --git a/src/main/java/org/apache/commons/io/FileUtils.java b/src/main/java/org/apache/commons/io/FileUtils.java index d22a434..7a44b7a 100644 --- a/src/main/java/org/apache/commons/io/FileUtils.java +++ b/src/main/java/org/apache/commons/io/FileUtils.java @@ -36,6 +36,7 @@ import java.nio.charset.StandardCharsets; import java.nio.charset.UnsupportedCharsetException; import java.nio.file.CopyOption; import java.nio.file.Files; +import java.nio.file.LinkOption; import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.time.Instant; @@ -1629,7 +1630,7 @@ public class FileUtils { */ public static boolean isFileNewer(final File file, final File reference) { requireExists(reference, "reference"); - return isFileNewer(file, reference.lastModified()); + return isFileNewer(file, lastModifiedUnchecked(reference)); } /** @@ -1658,10 +1659,7 @@ public class FileUtils { */ public static boolean isFileNewer(final File file, final long timeMillis) { Objects.requireNonNull(file, "file"); - if (!file.exists()) { - return false; - } - return file.lastModified() > timeMillis; + return file.exists() ? lastModifiedUnchecked(file) > timeMillis : false; } /** @@ -1794,7 +1792,7 @@ public class FileUtils { */ public static boolean isFileOlder(final File file, final File reference) { requireExists(reference, "reference"); - return isFileOlder(file, reference.lastModified()); + return isFileOlder(file, lastModifiedUnchecked(reference)); } /** @@ -1822,10 +1820,7 @@ public class FileUtils { */ public static boolean isFileOlder(final File file, final long timeMillis) { Objects.requireNonNull(file, "file"); - if (!file.exists()) { - return false; - } - return file.lastModified() < timeMillis; + return file.exists() ? lastModifiedUnchecked(file) < timeMillis : false; } /** @@ -1921,6 +1916,52 @@ public class FileUtils { } /** + * Returns the last modification time in milliseconds via + * {@link java.nio.file.Files#getLastModifiedTime(Path, LinkOption...)}. + * <p> + * Use this method to avoid issues with {@link File#lastModified()} like + * <a href="https://bugs.openjdk.java.net/browse/JDK-8177809">JDK-8177809</a> where {@link File#lastModified()} is + * losing milliseconds (always ends in 000). This bug exists in OpenJDK 8 and 9, and is fixed in 10. + * </p> + * + * @param file The File to query. + * @return See {@link java.nio.file.attribute.FileTime#toMillis()}. + * @throws IOException if an I/O error occurs. + * @since 2.9.0 + */ + public static long lastModified(final File file) throws IOException { + // https://bugs.openjdk.java.net/browse/JDK-8177809 + // File.lastModified() is losing milliseconds (always ends in 000) + // This bug is in OpenJDK 8 and 9, and fixed in 10. + return Files.getLastModifiedTime(Objects.requireNonNull(file.toPath(), "file")).toMillis(); + } + + /** + * Returns the last modification time in milliseconds via + * {@link java.nio.file.Files#getLastModifiedTime(Path, LinkOption...)}. + * <p> + * Use this method to avoid issues with {@link File#lastModified()} like + * <a href="https://bugs.openjdk.java.net/browse/JDK-8177809">JDK-8177809</a> where {@link File#lastModified()} is + * losing milliseconds (always ends in 000). This bug exists in OpenJDK 8 and 9, and is fixed in 10. + * </p> + * + * @param file The File to query. + * @return See {@link java.nio.file.attribute.FileTime#toMillis()}. + * @throws IllegalArgumentException if an I/O error occurs. + * @since 2.9.0 + */ + public static long lastModifiedUnchecked(final File file) { + // https://bugs.openjdk.java.net/browse/JDK-8177809 + // File.lastModified() is losing milliseconds (always ends in 000) + // This bug is in OpenJDK 8 and 9, and fixed in 10. + try { + return lastModified(file); + } catch (IOException e) { + throw new IllegalArgumentException(file.toString(), e); + } + } + + /** * Returns an Iterator for the lines in a {@code File} using the default encoding for the VM. * * @param file the file to open for input, must not be {@code null} @@ -2697,7 +2738,7 @@ public class FileUtils { */ private static void setLastModified(final File sourceFile, final File targetFile) throws IOException { Objects.requireNonNull(sourceFile, "sourceFile"); - setLastModified(targetFile, sourceFile.lastModified()); + setLastModified(targetFile, lastModified(sourceFile)); } /** diff --git a/src/main/java/org/apache/commons/io/comparator/LastModifiedFileComparator.java b/src/main/java/org/apache/commons/io/comparator/LastModifiedFileComparator.java index 3d40e1b..b46f1e1 100644 --- a/src/main/java/org/apache/commons/io/comparator/LastModifiedFileComparator.java +++ b/src/main/java/org/apache/commons/io/comparator/LastModifiedFileComparator.java @@ -20,9 +20,11 @@ import java.io.File; import java.io.Serializable; import java.util.Comparator; +import org.apache.commons.io.FileUtils; + /** * Compare the <b>last modified date/time</b> of two files for order - * (see {@link File#lastModified()}). + * (see {@link FileUtils#lastModifiedUnchecked(File)}). * <p> * This comparator can be used to sort lists or arrays of files * by their last modified date/time. @@ -65,7 +67,7 @@ public class LastModifiedFileComparator extends AbstractFileComparator implement */ @Override public int compare(final File file1, final File file2) { - final long result = file1.lastModified() - file2.lastModified(); + final long result = FileUtils.lastModifiedUnchecked(file1) - FileUtils.lastModifiedUnchecked(file2); if (result < 0) { return -1; } else if (result > 0) { diff --git a/src/main/java/org/apache/commons/io/filefilter/AgeFileFilter.java b/src/main/java/org/apache/commons/io/filefilter/AgeFileFilter.java index 80dd9ad..8742f70 100644 --- a/src/main/java/org/apache/commons/io/filefilter/AgeFileFilter.java +++ b/src/main/java/org/apache/commons/io/filefilter/AgeFileFilter.java @@ -121,7 +121,7 @@ public class AgeFileFilter extends AbstractFileFilter implements Serializable { * cutoff). */ public AgeFileFilter(final File cutoffReference, final boolean acceptOlder) { - this(cutoffReference.lastModified(), acceptOlder); + this(FileUtils.lastModifiedUnchecked(cutoffReference), acceptOlder); } /** diff --git a/src/main/java/org/apache/commons/io/input/Tailer.java b/src/main/java/org/apache/commons/io/input/Tailer.java index 8eef493..e3e9124 100644 --- a/src/main/java/org/apache/commons/io/input/Tailer.java +++ b/src/main/java/org/apache/commons/io/input/Tailer.java @@ -424,7 +424,7 @@ public class Tailer implements Runnable { } else { // The current position in the file position = end ? file.length() : 0; - last = file.lastModified(); + last = FileUtils.lastModified(file); reader.seek(position); } } @@ -459,7 +459,7 @@ public class Tailer implements Runnable { if (length > position) { // The file has more content than it did last time position = readLines(reader); - last = file.lastModified(); + last = FileUtils.lastModified(file); } else if (newer) { /* * This can happen if the file is truncated or overwritten with the exact same length of @@ -470,7 +470,7 @@ public class Tailer implements Runnable { // Now we can read new lines position = readLines(reader); - last = file.lastModified(); + last = FileUtils.lastModified(file); } if (reOpen && reader != null) { reader.close(); diff --git a/src/main/java/org/apache/commons/io/monitor/FileEntry.java b/src/main/java/org/apache/commons/io/monitor/FileEntry.java index 8a53c9d..3d54420 100644 --- a/src/main/java/org/apache/commons/io/monitor/FileEntry.java +++ b/src/main/java/org/apache/commons/io/monitor/FileEntry.java @@ -17,7 +17,11 @@ package org.apache.commons.io.monitor; import java.io.File; +import java.io.IOException; import java.io.Serializable; +import java.nio.file.Files; + +import org.apache.commons.io.FileUtils; /** * The state of a file or directory, capturing the following {@link File} attributes at a point in time. @@ -25,7 +29,7 @@ import java.io.Serializable; * <li>File Name (see {@link File#getName()})</li> * <li>Exists - whether the file exists or not (see {@link File#exists()})</li> * <li>Directory - whether the file is a directory or not (see {@link File#isDirectory()})</li> - * <li>Last Modified Date/Time (see {@link File#lastModified()})</li> + * <li>Last Modified Date/Time (see {@link FileUtils#lastModifiedUnchecked(File)})</li> * <li>Length (see {@link File#length()}) - directories treated as zero</li> * <li>Children - contents of a directory (see {@link File#listFiles(java.io.FileFilter)})</li> * </ul> @@ -94,25 +98,26 @@ public class FileEntry implements Serializable { * @return {@code true} if the file has changed, otherwise {@code false} */ public boolean refresh(final File file) { - // cache original values - final boolean origExists = exists; - final long origLastModified = lastModified; - final boolean origDirectory = directory; - final long origLength = length; + final boolean origExists = exists; + final long origLastModified = lastModified; + final boolean origDirectory = directory; + final long origLength = length; // refresh the values - name = file.getName(); - exists = file.exists(); - directory = exists && file.isDirectory(); - lastModified = exists ? file.lastModified() : 0; - length = exists && !directory ? file.length() : 0; + name = file.getName(); + exists = Files.exists(file.toPath()); + directory = exists && file.isDirectory(); + try { + lastModified = exists ? FileUtils.lastModified(file) : 0; + } catch (IOException e) { + lastModified = 0; + } + length = exists && !directory ? file.length() : 0; // Return if there are changes - return exists != origExists || - lastModified != origLastModified || - directory != origDirectory || - length != origLength; + return exists != origExists || lastModified != origLastModified || directory != origDirectory + || length != origLength; } /** diff --git a/src/test/java/org/apache/commons/io/FileUtilsFileNewerTestCase.java b/src/test/java/org/apache/commons/io/FileUtilsFileNewerTestCase.java index 70b6cdd..4d6163f 100644 --- a/src/test/java/org/apache/commons/io/FileUtilsFileNewerTestCase.java +++ b/src/test/java/org/apache/commons/io/FileUtilsFileNewerTestCase.java @@ -69,18 +69,19 @@ public class FileUtilsFileNewerTestCase { /** * Tests the {@code isFileNewer(File, *)} methods which a "normal" file. + * @throws IOException * * @see FileUtils#isFileNewer(File, long) * @see FileUtils#isFileNewer(File, Date) * @see FileUtils#isFileNewer(File, File) */ @Test - public void testIsFileNewer() { + public void testIsFileNewer() throws IOException { if (!testFile1.exists()) { throw new IllegalStateException("The testFile1 should exist"); } - final long fileLastModified = testFile1.lastModified(); + final long fileLastModified = FileUtils.lastModified(testFile1); final long TWO_SECOND = 2000; testIsFileNewer("two second earlier is not newer" , testFile1, fileLastModified + TWO_SECOND, false); @@ -90,19 +91,20 @@ public class FileUtilsFileNewerTestCase { /** * Tests the {@code isFileNewer(File, *)} methods which a not existing file. + * @throws IOException if an I/O error occurs. * * @see FileUtils#isFileNewer(File, long) * @see FileUtils#isFileNewer(File, Date) * @see FileUtils#isFileNewer(File, File) */ @Test - public void testIsFileNewerImaginaryFile() { + public void testIsFileNewerImaginaryFile() throws IOException { final File imaginaryFile = new File(temporaryFolder, "imaginaryFile"); if (imaginaryFile.exists()) { throw new IllegalStateException("The imaginary File exists"); } - testIsFileNewer("imaginary file can be newer" , imaginaryFile, testFile2.lastModified(), false); + testIsFileNewer("imaginary file can be newer" , imaginaryFile, FileUtils.lastModified(testFile2), false); } /** @@ -123,19 +125,20 @@ public class FileUtilsFileNewerTestCase { * @param file the file of which the last modification date is compared * @param time the time reference measured in milliseconds since the epoch * @param wantedResult the expected result + * @throws IOException if an I/O error occurs. * * @see FileUtils#isFileNewer(File, long) * @see FileUtils#isFileNewer(File, Date) * @see FileUtils#isFileNewer(File, File) */ - protected void testIsFileNewer(final String description, final File file, final long time, final boolean wantedResult) { + protected void testIsFileNewer(final String description, final File file, final long time, final boolean wantedResult) throws IOException { assertEquals(wantedResult, FileUtils.isFileNewer(file, time), description + " - time"); assertEquals(wantedResult, FileUtils.isFileNewer(file, new Date(time)), description + " - date"); final File temporaryFile = testFile2; temporaryFile.setLastModified(time); - assertEquals(time, temporaryFile.lastModified(), "The temporary file hasn't the right last modification date"); + assertEquals(time, FileUtils.lastModified(temporaryFile), "The temporary file hasn't the right last modification date"); assertEquals(wantedResult, FileUtils.isFileNewer(file, temporaryFile), description + " - file"); } diff --git a/src/test/java/org/apache/commons/io/FileUtilsTestCase.java b/src/test/java/org/apache/commons/io/FileUtilsTestCase.java index f7e1ba5..68a1750 100644 --- a/src/test/java/org/apache/commons/io/FileUtilsTestCase.java +++ b/src/test/java/org/apache/commons/io/FileUtilsTestCase.java @@ -167,7 +167,7 @@ public class FileUtilsTestCase { private long testFile2Size; - private void backDateFile10Minutes(final File testFile) { + private void backDateFile10Minutes(final File testFile) throws IOException { final long mins10 = 1000 * 60 * 10; final long lastModified1 = getLastModifiedMillis(testFile); assertTrue(setLastModifiedMillis(testFile, lastModified1 - mins10)); @@ -216,10 +216,8 @@ public class FileUtilsTestCase { FileUtils.writeStringToFile(file6, "File 6 in grandChild2", "UTF8"); } - private long getLastModifiedMillis(final File file) { - return file.lastModified(); - //https://bugs.openjdk.java.net/browse/JDK-8177809 - //return Files.getLastModifiedTime(file.toPath()).toMillis(); + private long getLastModifiedMillis(final File file) throws IOException { + return FileUtils.lastModified(file); } private String getName() { @@ -1024,7 +1022,7 @@ public class FileUtilsTestCase { FileUtils.copyFileToDirectory(testFile1, directory); assertTrue(destination.exists(), "Check Exist"); assertEquals(testFile1Size, destination.length(), "Check Full copy"); - assertEquals(testFile1.lastModified(), destination.lastModified(), "Check last modified date preserved"); + assertEquals(FileUtils.lastModified(testFile1), FileUtils.lastModified(destination), "Check last modified date preserved"); assertThrows(IllegalArgumentException.class, () -> FileUtils.copyFileToDirectory(destination, directory), "Should not be able to copy a file into the same directory as itself"); @@ -1057,7 +1055,7 @@ public class FileUtilsTestCase { FileUtils.copyFileToDirectory(testFile1, directory); assertTrue(destination.exists(), "Check Exist"); assertEquals(testFile2Size, destination.length(), "Check Full copy"); - assertEquals(testFile1.lastModified(), destination.lastModified(), "Check last modified date preserved"); + assertEquals(FileUtils.lastModified(testFile1), FileUtils.lastModified(destination), "Check last modified date preserved"); } @Test diff --git a/src/test/java/org/apache/commons/io/comparator/LastModifiedFileComparatorTest.java b/src/test/java/org/apache/commons/io/comparator/LastModifiedFileComparatorTest.java index 80d65ca..c595f47 100644 --- a/src/test/java/org/apache/commons/io/comparator/LastModifiedFileComparatorTest.java +++ b/src/test/java/org/apache/commons/io/comparator/LastModifiedFileComparatorTest.java @@ -21,6 +21,7 @@ import java.io.File; import java.io.FileOutputStream; import java.io.IOException; +import org.apache.commons.io.FileUtils; import org.apache.commons.io.test.TestUtils; import org.junit.jupiter.api.BeforeEach; @@ -28,51 +29,46 @@ import org.junit.jupiter.api.BeforeEach; * Test case for {@link LastModifiedFileComparator}. */ public class LastModifiedFileComparatorTest extends ComparatorAbstractTestCase { + @BeforeEach public void setUp() throws Exception { comparator = (AbstractFileComparator) LastModifiedFileComparator.LASTMODIFIED_COMPARATOR; reverse = LastModifiedFileComparator.LASTMODIFIED_REVERSE; final File olderFile = new File(dir, "older.txt"); if (!olderFile.getParentFile().exists()) { - throw new IOException("Cannot create file " + olderFile - + " as the parent directory does not exist"); + throw new IOException("Cannot create file " + olderFile + " as the parent directory does not exist"); } - try (final BufferedOutputStream output2 = - new BufferedOutputStream(new FileOutputStream(olderFile))) { + try (final BufferedOutputStream output2 = new BufferedOutputStream(new FileOutputStream(olderFile))) { TestUtils.generateTestData(output2, 0); } final File equalFile = new File(dir, "equal.txt"); if (!equalFile.getParentFile().exists()) { - throw new IOException("Cannot create file " + equalFile - + " as the parent directory does not exist"); + throw new IOException("Cannot create file " + equalFile + " as the parent directory does not exist"); } - try (final BufferedOutputStream output1 = - new BufferedOutputStream(new FileOutputStream(equalFile))) { + try (final BufferedOutputStream output1 = new BufferedOutputStream(new FileOutputStream(equalFile))) { TestUtils.generateTestData(output1, 0); } do { TestUtils.sleepQuietly(300); equalFile.setLastModified(System.currentTimeMillis()); - } while( olderFile.lastModified() == equalFile.lastModified() ); + } while (FileUtils.lastModified(olderFile) == FileUtils.lastModified(equalFile)); final File newerFile = new File(dir, "newer.txt"); if (!newerFile.getParentFile().exists()) { - throw new IOException("Cannot create file " + newerFile - + " as the parent directory does not exist"); + throw new IOException("Cannot create file " + newerFile + " as the parent directory does not exist"); } - try (final BufferedOutputStream output = - new BufferedOutputStream(new FileOutputStream(newerFile))){ + try (final BufferedOutputStream output = new BufferedOutputStream(new FileOutputStream(newerFile))) { TestUtils.generateTestData(output, 0); } do { TestUtils.sleepQuietly(300); newerFile.setLastModified(System.currentTimeMillis()); - } while( equalFile.lastModified() == newerFile.lastModified() ); + } while (FileUtils.lastModified(equalFile) == FileUtils.lastModified(newerFile)); equalFile1 = equalFile; equalFile2 = equalFile; - lessFile = olderFile; - moreFile = newerFile; + lessFile = olderFile; + moreFile = newerFile; } } diff --git a/src/test/java/org/apache/commons/io/monitor/AbstractMonitorTestCase.java b/src/test/java/org/apache/commons/io/monitor/AbstractMonitorTestCase.java index 5b06469..e4ffc6e 100644 --- a/src/test/java/org/apache/commons/io/monitor/AbstractMonitorTestCase.java +++ b/src/test/java/org/apache/commons/io/monitor/AbstractMonitorTestCase.java @@ -18,10 +18,12 @@ package org.apache.commons.io.monitor; import static org.apache.commons.io.test.TestUtils.sleepQuietly; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; import java.io.File; import java.io.FileFilter; +import java.io.IOException; import org.apache.commons.io.FileUtils; import org.apache.commons.io.filefilter.FileFilterUtils; @@ -127,13 +129,15 @@ public abstract class AbstractMonitorTestCase { * * @param file The file to touch * @return The file + * @throws IOException if an I/O error occurs. */ - protected File touch(File file) { - final long lastModified = file.exists() ? file.lastModified() : 0; + protected File touch(File file) throws IOException { + final long lastModified = file.exists() ? FileUtils.lastModified(file) : 0; try { FileUtils.touch(file); + assertTrue(file.exists()); file = new File(file.getParent(), file.getName()); - while (lastModified == file.lastModified()) { + while (lastModified == FileUtils.lastModified(file)) { sleepQuietly(pauseTime); FileUtils.touch(file); file = new File(file.getParent(), file.getName()); diff --git a/src/test/java/org/apache/commons/io/monitor/FileAlterationObserverTestCase.java b/src/test/java/org/apache/commons/io/monitor/FileAlterationObserverTestCase.java index 61464c5..7ff09b8 100644 --- a/src/test/java/org/apache/commons/io/monitor/FileAlterationObserverTestCase.java +++ b/src/test/java/org/apache/commons/io/monitor/FileAlterationObserverTestCase.java @@ -22,6 +22,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.File; import java.io.FileFilter; +import java.io.IOException; import java.util.Iterator; import org.apache.commons.io.FileUtils; @@ -146,9 +147,10 @@ public class FileAlterationObserverTestCase extends AbstractMonitorTestCase { /** * Test checkAndNotify() creating + * @throws IOException if an I/O error occurs. */ @Test - public void testFileCreate() { + public void testFileCreate() throws IOException { checkAndNotify(); checkCollectionsEmpty("A"); File testDirA = new File(testDir, "test-dir-A"); @@ -205,9 +207,10 @@ public class FileAlterationObserverTestCase extends AbstractMonitorTestCase { /** * Test checkAndNotify() creating + * @throws IOException if an I/O error occurs. */ @Test - public void testFileUpdate() { + public void testFileUpdate() throws IOException { checkAndNotify(); checkCollectionsEmpty("A"); File testDirA = new File(testDir, "test-dir-A"); @@ -261,9 +264,10 @@ public class FileAlterationObserverTestCase extends AbstractMonitorTestCase { /** * Test checkAndNotify() deleting + * @throws IOException if an I/O error occurs. */ @Test - public void testFileDelete() { + public void testFileDelete() throws IOException { checkAndNotify(); checkCollectionsEmpty("A"); File testDirA = new File(testDir, "test-dir-A"); @@ -320,9 +324,10 @@ public class FileAlterationObserverTestCase extends AbstractMonitorTestCase { /** * Test checkAndNotify() method + * @throws IOException if an I/O error occurs. */ @Test - public void testObserveSingleFile() { + public void testObserveSingleFile() throws IOException { final File testDirA = new File(testDir, "test-dir-A"); File testDirAFile1 = new File(testDirA, "A-file1.java"); testDirA.mkdir();