Skip to content

Commit dcf0197

Browse files
garydgregorygarydgregory
committed
[IO-863] Recent incompatible change to FileUtils.listFiles re extensions
1 parent dee58ac commit dcf0197

File tree

3 files changed

+68
-46
lines changed

3 files changed

+68
-46
lines changed

‎src/changes/changes.xml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,8 @@ The <action> type attribute can be add,update,fix,remove.
5555
<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>
5656
<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>
5757
<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>
58-
<action dev="ggregory" type="fix" due-to="Éamonn McManus">Use Unicode escapes for superscript characters. #701.</action>
58+
<action dev="ggregory" type="fix" due-to="Éamonn McManus">Use Unicode escapes for superscript characters. #701.</action>
59+
<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>
5960
<!-- ADD -->
6061
<action dev="ggregory" type="add" due-to="Gary Gregory">Add @FunctionalInterface to ClassNameMatcher.</action>
6162
<action dev="ggregory" type="add" due-to="Gary Gregory">Add ValidatingObjectInputStream.Builder and ValidatingObjectInputStream.builder().</action>

‎src/main/java/org/apache/commons/io/FileUtils.java‎

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2319,17 +2319,20 @@ public static Collection<File> listFiles(final File directory, final IOFileFilte
23192319

23202320
@SuppressWarnings("null")
23212321
private static void listFiles(final File directory, final List<File> files, final boolean recursive, final FilenameFilter filter) {
2322-
// Only allocate if you must.
2323-
final List<File> dirs = recursive ? new ArrayList<>() : null;
2324-
Arrays.stream(directory.listFiles()).forEach(f -> {
2325-
if (recursive && f.isDirectory()) {
2326-
dirs.add(f);
2327-
} else if (f.isFile() && filter.accept(directory, f.getName())) {
2328-
files.add(f);
2322+
final File[] listFiles = directory.listFiles();
2323+
if (listFiles != null) {
2324+
// Only allocate if you must.
2325+
final List<File> dirs = recursive ? new ArrayList<>() : null;
2326+
Arrays.stream(listFiles).forEach(f -> {
2327+
if (recursive && f.isDirectory()) {
2328+
dirs.add(f);
2329+
} else if (f.isFile() && filter.accept(directory, f.getName())) {
2330+
files.add(f);
2331+
}
2332+
});
2333+
if (recursive) {
2334+
dirs.forEach(d -> listFiles(d, files, true, filter));
23292335
}
2330-
});
2331-
if (recursive) {
2332-
dirs.forEach(d -> listFiles(d, files, true, filter));
23332336
}
23342337
}
23352338

@@ -2346,7 +2349,7 @@ private static void listFiles(final File directory, final List<File> files, fina
23462349
public static Collection<File> listFiles(final File directory, final String[] extensions, final boolean recursive) {
23472350
// IO-856: Don't use NIO to path walk, allocate as little as possible while traversing.
23482351
final List<File> files = new ArrayList<>();
2349-
final FilenameFilter filter = extensions != null ? new SuffixFileFilter(extensions) : TrueFileFilter.INSTANCE;
2352+
final FilenameFilter filter = extensions != null ? toSuffixFileFilter(extensions) : TrueFileFilter.INSTANCE;
23502353
listFiles(directory, files, recursive, filter);
23512354
return files;
23522355
}
@@ -2983,7 +2986,7 @@ public static Stream<File> streamFiles(final File directory, final boolean recur
29832986
// @formatter:off
29842987
final IOFileFilter filter = extensions == null
29852988
? FileFileFilter.INSTANCE
2986-
: FileFileFilter.INSTANCE.and(new SuffixFileFilter(toSuffixes(extensions)));
2989+
: FileFileFilter.INSTANCE.and(toSuffixFileFilter(extensions));
29872990
// @formatter:on
29882991
return PathUtils.walk(directory.toPath(), filter, toMaxDepth(recursive), false, FileVisitOption.FOLLOW_LINKS).map(Path::toFile);
29892992
}
@@ -3079,7 +3082,11 @@ private static int toMaxDepth(final boolean recursive) {
30793082
* @throws NullPointerException if the parameter is null
30803083
*/
30813084
private static String[] toSuffixes(final String... extensions) {
3082-
return Stream.of(Objects.requireNonNull(extensions, "extensions")).map(e -> "." + e).toArray(String[]::new);
3085+
return Stream.of(Objects.requireNonNull(extensions, "extensions")).map(s -> s.charAt(0) == '.' ? s : "." + s).toArray(String[]::new);
3086+
}
3087+
3088+
private static SuffixFileFilter toSuffixFileFilter(final String... extensions) {
3089+
return new SuffixFileFilter(toSuffixes(extensions));
30833090
}
30843091

30853092
/**

‎src/test/java/org/apache/commons/io/FileUtilsListFilesTest.java‎

Lines changed: 46 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -55,23 +55,6 @@ public class FileUtilsListFilesTest {
5555
@TempDir
5656
public File temporaryFolder;
5757

58-
private Collection<String> filesToFilenames(final Collection<File> files) {
59-
return files.stream().map(File::getName).collect(Collectors.toList());
60-
}
61-
62-
/**
63-
* Consumes and closes the underlying stream.
64-
*
65-
* @param files The iterator to consume.
66-
* @return a new collection.
67-
*/
68-
private Collection<String> filesToFilenames(final Iterator<File> files) {
69-
final Collection<String> fileNames = new ArrayList<>();
70-
// Iterator.forEachRemaining() closes the underlying stream.
71-
files.forEachRemaining(f -> fileNames.add(f.getName()));
72-
return fileNames;
73-
}
74-
7558
@SuppressWarnings("ResultOfMethodCallIgnored")
7659
@BeforeEach
7760
public void setUp() throws Exception {
@@ -94,6 +77,8 @@ public void setUp() throws Exception {
9477
FileUtils.touch(file);
9578
file = new File(dir, "dummy-index.html");
9679
FileUtils.touch(file);
80+
file = new File(dir, "dummy-indexhtml");
81+
FileUtils.touch(file);
9782

9883
dir = dir.getParentFile();
9984
dir = new File(dir, "CVS");
@@ -110,7 +95,7 @@ public void testIterateFilesByExtension() {
11095

11196
Iterator<File> files = FileUtils.iterateFiles(temporaryFolder, extensions, false);
11297
try {
113-
final Collection<String> fileNames = filesToFilenames(files);
98+
final Collection<String> fileNames = toFileNames(files);
11499
assertEquals(1, fileNames.size());
115100
assertTrue(fileNames.contains("dummy-build.xml"));
116101
assertFalse(fileNames.contains("README"));
@@ -122,7 +107,7 @@ public void testIterateFilesByExtension() {
122107

123108
try {
124109
files = FileUtils.iterateFiles(temporaryFolder, extensions, true);
125-
final Collection<String> fileNames = filesToFilenames(files);
110+
final Collection<String> fileNames = toFileNames(files);
126111
assertEquals(4, fileNames.size());
127112
assertTrue(fileNames.contains("dummy-file.txt"));
128113
assertFalse(fileNames.contains("dummy-index.html"));
@@ -133,7 +118,7 @@ public void testIterateFilesByExtension() {
133118

134119
files = FileUtils.iterateFiles(temporaryFolder, null, false);
135120
try {
136-
final Collection<String> fileNames = filesToFilenames(files);
121+
final Collection<String> fileNames = toFileNames(files);
137122
assertEquals(2, fileNames.size());
138123
assertTrue(fileNames.contains("dummy-build.xml"));
139124
assertTrue(fileNames.contains("README"));
@@ -150,43 +135,43 @@ public void testListFiles() {
150135
Collection<String> fileNames;
151136
IOFileFilter fileFilter;
152137
IOFileFilter dirFilter;
153-
138+
//
154139
// First, find non-recursively
155140
fileFilter = FileFilterUtils.trueFileFilter();
156141
files = FileUtils.listFiles(temporaryFolder, fileFilter, null);
157-
fileNames = filesToFilenames(files);
142+
fileNames = toFileNames(files);
158143
assertTrue(fileNames.contains("dummy-build.xml"), "'dummy-build.xml' is missing");
159144
assertFalse(fileNames.contains("dummy-index.html"), "'dummy-index.html' shouldn't be found");
160145
assertFalse(fileNames.contains("Entries"), "'Entries' shouldn't be found");
161-
146+
//
162147
// Second, find recursively
163148
fileFilter = FileFilterUtils.trueFileFilter();
164149
dirFilter = FileFilterUtils.notFileFilter(FileFilterUtils.nameFileFilter("CVS"));
165150
files = FileUtils.listFiles(temporaryFolder, fileFilter, dirFilter);
166-
fileNames = filesToFilenames(files);
151+
fileNames = toFileNames(files);
167152
assertTrue(fileNames.contains("dummy-build.xml"), "'dummy-build.xml' is missing");
168153
assertTrue(fileNames.contains("dummy-index.html"), "'dummy-index.html' is missing");
169154
assertFalse(fileNames.contains("Entries"), "'Entries' shouldn't be found");
170-
155+
//
171156
// Do the same as above but now with the filter coming from FileFilterUtils
172157
fileFilter = FileFilterUtils.trueFileFilter();
173158
dirFilter = FileFilterUtils.makeCVSAware(null);
174159
files = FileUtils.listFiles(temporaryFolder, fileFilter, dirFilter);
175-
fileNames = filesToFilenames(files);
160+
fileNames = toFileNames(files);
176161
assertTrue(fileNames.contains("dummy-build.xml"), "'dummy-build.xml' is missing");
177162
assertTrue(fileNames.contains("dummy-index.html"), "'dummy-index.html' is missing");
178163
assertFalse(fileNames.contains("Entries"), "'Entries' shouldn't be found");
179-
164+
//
180165
// Again with the CVS filter but now with a non-null parameter
181166
fileFilter = FileFilterUtils.trueFileFilter();
182167
dirFilter = FileFilterUtils.prefixFileFilter("sub");
183168
dirFilter = FileFilterUtils.makeCVSAware(dirFilter);
184169
files = FileUtils.listFiles(temporaryFolder, fileFilter, dirFilter);
185-
fileNames = filesToFilenames(files);
170+
fileNames = toFileNames(files);
186171
assertTrue(fileNames.contains("dummy-build.xml"), "'dummy-build.xml' is missing");
187172
assertTrue(fileNames.contains("dummy-index.html"), "'dummy-index.html' is missing");
188173
assertFalse(fileNames.contains("Entries"), "'Entries' shouldn't be found");
189-
174+
// Edge case
190175
assertThrows(NullPointerException.class, () -> FileUtils.listFiles(temporaryFolder, null, null));
191176
}
192177

@@ -196,23 +181,35 @@ public void testListFilesByExtension() {
196181

197182
Collection<File> files = FileUtils.listFiles(temporaryFolder, extensions, false);
198183
assertEquals(1, files.size());
199-
Collection<String> fileNames = filesToFilenames(files);
184+
Collection<String> fileNames = toFileNames(files);
200185
assertTrue(fileNames.contains("dummy-build.xml"));
201186
assertFalse(fileNames.contains("README"));
202187
assertFalse(fileNames.contains("dummy-file.txt"));
203188

204189
files = FileUtils.listFiles(temporaryFolder, extensions, true);
205-
fileNames = filesToFilenames(files);
190+
fileNames = toFileNames(files);
206191
assertEquals(4, fileNames.size(), fileNames::toString);
207192
assertTrue(fileNames.contains("dummy-file.txt"));
208193
assertFalse(fileNames.contains("dummy-index.html"));
209194

210195
files = FileUtils.listFiles(temporaryFolder, null, false);
211196
assertEquals(2, files.size(), files::toString);
212-
fileNames = filesToFilenames(files);
197+
fileNames = toFileNames(files);
213198
assertTrue(fileNames.contains("dummy-build.xml"));
214199
assertTrue(fileNames.contains("README"));
215200
assertFalse(fileNames.contains("dummy-file.txt"));
201+
202+
final File directory = new File(temporaryFolder, "subdir1/subsubdir1");
203+
files = FileUtils.listFiles(directory, new String[] { "html" }, false);
204+
fileNames = toFileNames(files);
205+
assertFalse(files.isEmpty(), directory::toString);
206+
assertTrue(fileNames.contains("dummy-index.html"));
207+
assertFalse(fileNames.contains("dummy-indexhtml"));
208+
files = FileUtils.listFiles(temporaryFolder, new String[] { "html" }, true);
209+
fileNames = toFileNames(files);
210+
assertFalse(files.isEmpty(), temporaryFolder::toString);
211+
assertTrue(fileNames.contains("dummy-index.html"));
212+
assertFalse(fileNames.contains("dummy-indexhtml"));
216213
}
217214

218215
@Test
@@ -288,4 +285,21 @@ public void testListFilesWithDeletionThreaded() throws ExecutionException, Inter
288285
c2.get();
289286
}
290287

288+
private Collection<String> toFileNames(final Collection<File> files) {
289+
return files.stream().map(File::getName).collect(Collectors.toList());
290+
}
291+
292+
/**
293+
* Consumes and closes the underlying stream.
294+
*
295+
* @param files The iterator to consume.
296+
* @return a new collection.
297+
*/
298+
private Collection<String> toFileNames(final Iterator<File> files) {
299+
final Collection<String> fileNames = new ArrayList<>();
300+
// Iterator.forEachRemaining() closes the underlying stream.
301+
files.forEachRemaining(f -> fileNames.add(f.getName()));
302+
return fileNames;
303+
}
304+
291305
}

0 commit comments

Comments
 (0)