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; + } + }