diff --git a/src/main/java/org/apache/commons/io/FileUtils.java b/src/main/java/org/apache/commons/io/FileUtils.java index b4794b61439..b8ab62a396a 100644 --- a/src/main/java/org/apache/commons/io/FileUtils.java +++ b/src/main/java/org/apache/commons/io/FileUtils.java @@ -23,6 +23,7 @@ import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.FileOutputStream; +import java.io.FilenameFilter; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; @@ -57,6 +58,7 @@ import java.time.chrono.ChronoLocalDateTime; import java.time.chrono.ChronoZonedDateTime; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Date; @@ -2311,6 +2313,22 @@ public static Collection listFiles(final File directory, final IOFileFilte return toList(visitor.getFileList().stream().map(Path::toFile)); } + @SuppressWarnings("null") + private static void listFiles(final File directory, final List files, final boolean recursive, final FilenameFilter filter) { + // Only allocate if you must. + final List dirs = recursive ? new ArrayList<>() : null; + Arrays.stream(directory.listFiles()).forEach(f -> { + if (recursive && f.isDirectory()) { + dirs.add(f); + } else if (f.isFile() && filter.accept(directory, f.getName())) { + files.add(f); + } + }); + if (recursive) { + dirs.forEach(d -> listFiles(d, files, true, filter)); + } + } + /** * Lists files within a given directory (and optionally its subdirectories) * which match an array of extensions. @@ -2322,9 +2340,11 @@ public static Collection listFiles(final File directory, final IOFileFilte * @return a collection of {@link File} with the matching files */ public static Collection listFiles(final File directory, final String[] extensions, final boolean recursive) { - try (Stream fileStream = Uncheck.get(() -> streamFiles(directory, recursive, extensions))) { - return toList(fileStream); - } + // IO-856: Don't use NIO to path walk, allocate as little as possible while traversing. + final List files = new ArrayList<>(); + final FilenameFilter filter = extensions != null ? new SuffixFileFilter(extensions) : TrueFileFilter.INSTANCE; + listFiles(directory, files, recursive, filter); + return files; } /** diff --git a/src/test/java/org/apache/commons/io/FileUtilsListFilesTest.java b/src/test/java/org/apache/commons/io/FileUtilsListFilesTest.java index 20e3c8d77a1..2c26f301205 100644 --- a/src/test/java/org/apache/commons/io/FileUtilsListFilesTest.java +++ b/src/test/java/org/apache/commons/io/FileUtilsListFilesTest.java @@ -18,6 +18,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -44,8 +45,6 @@ import org.apache.commons.lang3.function.Consumers; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.condition.EnabledOnOs; -import org.junit.jupiter.api.condition.OS; import org.junit.jupiter.api.io.TempDir; /** @@ -204,12 +203,12 @@ public void testListFilesByExtension() { files = FileUtils.listFiles(temporaryFolder, extensions, true); fileNames = filesToFilenames(files); - assertEquals(4, fileNames.size()); + assertEquals(4, fileNames.size(), fileNames::toString); assertTrue(fileNames.contains("dummy-file.txt")); assertFalse(fileNames.contains("dummy-index.html")); files = FileUtils.listFiles(temporaryFolder, null, false); - assertEquals(2, files.size()); + assertEquals(2, files.size(), files::toString); fileNames = filesToFilenames(files); assertTrue(fileNames.contains("dummy-build.xml")); assertTrue(fileNames.contains("README")); @@ -238,7 +237,6 @@ public void testListFilesWithDeletion() throws IOException { * Tests IO-856 ListFiles should not fail on vanishing files. */ @Test - @EnabledOnOs(value = OS.WINDOWS) public void testListFilesWithDeletionThreaded() throws ExecutionException, InterruptedException { // test for IO-856 // create random directory in tmp, create the directory if it does not exist @@ -248,14 +246,17 @@ public void testListFilesWithDeletionThreaded() throws ExecutionException, Inter fail("Could not create file path: " + tempDir.getAbsolutePath()); } final int waitTime = 10_000; + final int maxFiles = 500; final byte[] bytes = "TEST".getBytes(StandardCharsets.UTF_8); final CompletableFuture c1 = CompletableFuture.runAsync(() -> { final long endTime = System.currentTimeMillis() + waitTime; - while (System.currentTimeMillis() < endTime) { + int count = 0; + while (System.currentTimeMillis() < endTime && count < maxFiles) { final File file = new File(tempDir.getAbsolutePath(), UUID.randomUUID() + ".deletetester"); file.deleteOnExit(); try { Files.write(file.toPath(), bytes); + count++; } catch (final Exception e) { fail("Could not create test file: '" + file.getAbsolutePath() + "': " + e, e); } @@ -263,18 +264,24 @@ public void testListFilesWithDeletionThreaded() throws ExecutionException, Inter fail("Could not delete test file: '" + file.getAbsolutePath() + "'"); } } + // System.out.printf("Created %,d%n", count); }); final CompletableFuture c2 = CompletableFuture.runAsync(() -> { final long endTime = System.currentTimeMillis() + waitTime; + int max = 0; try { while (System.currentTimeMillis() < endTime) { - FileUtils.listFiles(tempDir, new String[] { "\\.deletetester" }, false); + final Collection files = FileUtils.listFiles(tempDir, new String[] { "\\.deletetester" }, false); + assertNotNull(files); + max = Math.max(max, files.size()); } } catch (final Exception e) { + System.out.printf("List size max %,d%n", max); fail("IO-856 test failure: " + e, e); // The exception can be hidden. e.printStackTrace(); } + // System.out.printf("List size max %,d%n", max); }); // wait for the threads to finish c1.get();