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 dcf0197fb [IO-863] Recent incompatible change to FileUtils.listFiles 
re extensions
dcf0197fb is described below

commit dcf0197fbb04019d66c47e8db47a869cb3bf51af
Author: Gary Gregory <garydgreg...@gmail.com>
AuthorDate: Wed Nov 13 10:25:41 2024 -0500

    [IO-863] Recent incompatible change to FileUtils.listFiles re extensions
---
 src/changes/changes.xml                            |  3 +-
 src/main/java/org/apache/commons/io/FileUtils.java | 33 +++++----
 .../apache/commons/io/FileUtilsListFilesTest.java  | 78 +++++++++++++---------
 3 files changed, 68 insertions(+), 46 deletions(-)

diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index cb625e2fa..84e0b4bd5 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -55,7 +55,8 @@ The <action> type attribute can be add,update,fix,remove.
       <action dev="ggregory" type="fix" issue="IO-860" due-to="Stefan 
Feenstra, Gary Gregory">Missing reserved file names in FileSystem.WINDOWS 
(superscript digits for COM and LPT).</action>
       <action dev="ggregory" type="fix" issue="IO-856" due-to="Thomas Hartwig, 
Gary Gregory">FileUtils.listFiles(final File, String[], boolean) can throw 
NoSuchFileException #697, #699.</action>
       <action dev="ggregory" type="fix" issue="IO-859" due-to="JD Dean, Gary 
Gregory">FileUtils.forceDelete on non-existent file on Windows throws 
IOException rather than FileNotFoundException.</action>
-      <action dev="ggregory" type="fix"                due-to="Éamonn 
McManus">Use Unicode escapes for superscript characters. #701.</action> 
+      <action dev="ggregory" type="fix"                due-to="Éamonn 
McManus">Use Unicode escapes for superscript characters. #701.</action>
+      <action dev="ggregory" type="fix" issue="IO-863" due-to="Éamonn McManus, 
Gary Gregory">Recent incompatible change to FileUtils.listFiles re extensions, 
see IO-856.</action>
       <!-- ADD -->
       <action dev="ggregory" type="add"                due-to="Gary 
Gregory">Add @FunctionalInterface to ClassNameMatcher.</action>
       <action dev="ggregory" type="add"                due-to="Gary 
Gregory">Add ValidatingObjectInputStream.Builder and 
ValidatingObjectInputStream.builder().</action>
diff --git a/src/main/java/org/apache/commons/io/FileUtils.java 
b/src/main/java/org/apache/commons/io/FileUtils.java
index 88f1898c9..55a717c6b 100644
--- a/src/main/java/org/apache/commons/io/FileUtils.java
+++ b/src/main/java/org/apache/commons/io/FileUtils.java
@@ -2319,17 +2319,20 @@ public class FileUtils {
 
     @SuppressWarnings("null")
     private static void listFiles(final File directory, final List<File> 
files, final boolean recursive, final FilenameFilter filter) {
-        // Only allocate if you must.
-        final List<File> 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);
+        final File[] listFiles = directory.listFiles();
+        if (listFiles != null) {
+            // Only allocate if you must.
+            final List<File> dirs = recursive ? new ArrayList<>() : null;
+            Arrays.stream(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));
             }
-        });
-        if (recursive) {
-            dirs.forEach(d -> listFiles(d, files, true, filter));
         }
     }
 
@@ -2346,7 +2349,7 @@ public class FileUtils {
     public static Collection<File> listFiles(final File directory, final 
String[] extensions, final boolean recursive) {
         // IO-856: Don't use NIO to path walk, allocate as little as possible 
while traversing.
         final List<File> files = new ArrayList<>();
-        final FilenameFilter filter = extensions != null ? new 
SuffixFileFilter(extensions) : TrueFileFilter.INSTANCE;
+        final FilenameFilter filter = extensions != null ? 
toSuffixFileFilter(extensions) : TrueFileFilter.INSTANCE;
         listFiles(directory, files, recursive, filter);
         return files;
     }
@@ -2983,7 +2986,7 @@ public class FileUtils {
         // @formatter:off
         final IOFileFilter filter = extensions == null
             ? FileFileFilter.INSTANCE
-            : FileFileFilter.INSTANCE.and(new 
SuffixFileFilter(toSuffixes(extensions)));
+            : FileFileFilter.INSTANCE.and(toSuffixFileFilter(extensions));
         // @formatter:on
         return PathUtils.walk(directory.toPath(), filter, 
toMaxDepth(recursive), false, FileVisitOption.FOLLOW_LINKS).map(Path::toFile);
     }
@@ -3079,7 +3082,11 @@ public class FileUtils {
      * @throws NullPointerException if the parameter is null
      */
     private static String[] toSuffixes(final String... extensions) {
-        return Stream.of(Objects.requireNonNull(extensions, 
"extensions")).map(e -> "." + e).toArray(String[]::new);
+        return Stream.of(Objects.requireNonNull(extensions, 
"extensions")).map(s -> s.charAt(0) == '.' ? s : "." + 
s).toArray(String[]::new);
+    }
+
+    private static SuffixFileFilter toSuffixFileFilter(final String... 
extensions) {
+        return new SuffixFileFilter(toSuffixes(extensions));
     }
 
     /**
diff --git a/src/test/java/org/apache/commons/io/FileUtilsListFilesTest.java 
b/src/test/java/org/apache/commons/io/FileUtilsListFilesTest.java
index 8907bd1fe..926f67fa3 100644
--- a/src/test/java/org/apache/commons/io/FileUtilsListFilesTest.java
+++ b/src/test/java/org/apache/commons/io/FileUtilsListFilesTest.java
@@ -55,23 +55,6 @@ public class FileUtilsListFilesTest {
     @TempDir
     public File temporaryFolder;
 
-    private Collection<String> filesToFilenames(final Collection<File> files) {
-        return files.stream().map(File::getName).collect(Collectors.toList());
-    }
-
-    /**
-     * Consumes and closes the underlying stream.
-     *
-     * @param files The iterator to consume.
-     * @return a new collection.
-     */
-    private Collection<String> filesToFilenames(final Iterator<File> files) {
-        final Collection<String> fileNames = new ArrayList<>();
-        // Iterator.forEachRemaining() closes the underlying stream.
-        files.forEachRemaining(f -> fileNames.add(f.getName()));
-        return fileNames;
-    }
-
     @SuppressWarnings("ResultOfMethodCallIgnored")
     @BeforeEach
     public void setUp() throws Exception {
@@ -94,6 +77,8 @@ public class FileUtilsListFilesTest {
         FileUtils.touch(file);
         file = new File(dir, "dummy-index.html");
         FileUtils.touch(file);
+        file = new File(dir, "dummy-indexhtml");
+        FileUtils.touch(file);
 
         dir = dir.getParentFile();
         dir = new File(dir, "CVS");
@@ -110,7 +95,7 @@ public class FileUtilsListFilesTest {
 
         Iterator<File> files = FileUtils.iterateFiles(temporaryFolder, 
extensions, false);
         try {
-            final Collection<String> fileNames = filesToFilenames(files);
+            final Collection<String> fileNames = toFileNames(files);
             assertEquals(1, fileNames.size());
             assertTrue(fileNames.contains("dummy-build.xml"));
             assertFalse(fileNames.contains("README"));
@@ -122,7 +107,7 @@ public class FileUtilsListFilesTest {
 
         try {
             files = FileUtils.iterateFiles(temporaryFolder, extensions, true);
-            final Collection<String> fileNames = filesToFilenames(files);
+            final Collection<String> fileNames = toFileNames(files);
             assertEquals(4, fileNames.size());
             assertTrue(fileNames.contains("dummy-file.txt"));
             assertFalse(fileNames.contains("dummy-index.html"));
@@ -133,7 +118,7 @@ public class FileUtilsListFilesTest {
 
         files = FileUtils.iterateFiles(temporaryFolder, null, false);
         try {
-            final Collection<String> fileNames = filesToFilenames(files);
+            final Collection<String> fileNames = toFileNames(files);
             assertEquals(2, fileNames.size());
             assertTrue(fileNames.contains("dummy-build.xml"));
             assertTrue(fileNames.contains("README"));
@@ -150,43 +135,43 @@ public class FileUtilsListFilesTest {
         Collection<String> fileNames;
         IOFileFilter fileFilter;
         IOFileFilter dirFilter;
-
+        //
         // First, find non-recursively
         fileFilter = FileFilterUtils.trueFileFilter();
         files = FileUtils.listFiles(temporaryFolder, fileFilter, null);
-        fileNames = filesToFilenames(files);
+        fileNames = toFileNames(files);
         assertTrue(fileNames.contains("dummy-build.xml"), "'dummy-build.xml' 
is missing");
         assertFalse(fileNames.contains("dummy-index.html"), 
"'dummy-index.html' shouldn't be found");
         assertFalse(fileNames.contains("Entries"), "'Entries' shouldn't be 
found");
-
+        //
         // Second, find recursively
         fileFilter = FileFilterUtils.trueFileFilter();
         dirFilter = 
FileFilterUtils.notFileFilter(FileFilterUtils.nameFileFilter("CVS"));
         files = FileUtils.listFiles(temporaryFolder, fileFilter, dirFilter);
-        fileNames = filesToFilenames(files);
+        fileNames = toFileNames(files);
         assertTrue(fileNames.contains("dummy-build.xml"), "'dummy-build.xml' 
is missing");
         assertTrue(fileNames.contains("dummy-index.html"), "'dummy-index.html' 
is missing");
         assertFalse(fileNames.contains("Entries"), "'Entries' shouldn't be 
found");
-
+        //
         // Do the same as above but now with the filter coming from 
FileFilterUtils
         fileFilter = FileFilterUtils.trueFileFilter();
         dirFilter = FileFilterUtils.makeCVSAware(null);
         files = FileUtils.listFiles(temporaryFolder, fileFilter, dirFilter);
-        fileNames = filesToFilenames(files);
+        fileNames = toFileNames(files);
         assertTrue(fileNames.contains("dummy-build.xml"), "'dummy-build.xml' 
is missing");
         assertTrue(fileNames.contains("dummy-index.html"), "'dummy-index.html' 
is missing");
         assertFalse(fileNames.contains("Entries"), "'Entries' shouldn't be 
found");
-
+        //
         // Again with the CVS filter but now with a non-null parameter
         fileFilter = FileFilterUtils.trueFileFilter();
         dirFilter = FileFilterUtils.prefixFileFilter("sub");
         dirFilter = FileFilterUtils.makeCVSAware(dirFilter);
         files = FileUtils.listFiles(temporaryFolder, fileFilter, dirFilter);
-        fileNames = filesToFilenames(files);
+        fileNames = toFileNames(files);
         assertTrue(fileNames.contains("dummy-build.xml"), "'dummy-build.xml' 
is missing");
         assertTrue(fileNames.contains("dummy-index.html"), "'dummy-index.html' 
is missing");
         assertFalse(fileNames.contains("Entries"), "'Entries' shouldn't be 
found");
-
+        // Edge case
         assertThrows(NullPointerException.class, () -> 
FileUtils.listFiles(temporaryFolder, null, null));
     }
 
@@ -196,23 +181,35 @@ public class FileUtilsListFilesTest {
 
         Collection<File> files = FileUtils.listFiles(temporaryFolder, 
extensions, false);
         assertEquals(1, files.size());
-        Collection<String> fileNames = filesToFilenames(files);
+        Collection<String> fileNames = toFileNames(files);
         assertTrue(fileNames.contains("dummy-build.xml"));
         assertFalse(fileNames.contains("README"));
         assertFalse(fileNames.contains("dummy-file.txt"));
 
         files = FileUtils.listFiles(temporaryFolder, extensions, true);
-        fileNames = filesToFilenames(files);
+        fileNames = toFileNames(files);
         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(), files::toString);
-        fileNames = filesToFilenames(files);
+        fileNames = toFileNames(files);
         assertTrue(fileNames.contains("dummy-build.xml"));
         assertTrue(fileNames.contains("README"));
         assertFalse(fileNames.contains("dummy-file.txt"));
+
+        final File directory = new File(temporaryFolder, "subdir1/subsubdir1");
+        files = FileUtils.listFiles(directory, new String[] { "html" }, false);
+        fileNames = toFileNames(files);
+        assertFalse(files.isEmpty(), directory::toString);
+        assertTrue(fileNames.contains("dummy-index.html"));
+        assertFalse(fileNames.contains("dummy-indexhtml"));
+        files = FileUtils.listFiles(temporaryFolder, new String[] { "html" }, 
true);
+        fileNames = toFileNames(files);
+        assertFalse(files.isEmpty(), temporaryFolder::toString);
+        assertTrue(fileNames.contains("dummy-index.html"));
+        assertFalse(fileNames.contains("dummy-indexhtml"));
     }
 
     @Test
@@ -288,4 +285,21 @@ public class FileUtilsListFilesTest {
         c2.get();
     }
 
+    private Collection<String> toFileNames(final Collection<File> files) {
+        return files.stream().map(File::getName).collect(Collectors.toList());
+    }
+
+    /**
+     * Consumes and closes the underlying stream.
+     *
+     * @param files The iterator to consume.
+     * @return a new collection.
+     */
+    private Collection<String> toFileNames(final Iterator<File> files) {
+        final Collection<String> fileNames = new ArrayList<>();
+        // Iterator.forEachRemaining() closes the underlying stream.
+        files.forEachRemaining(f -> fileNames.add(f.getName()));
+        return fileNames;
+    }
+
 }

Reply via email to