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 9aa8ef35 [IO-808] remove redundant checks; improve Javadoc (#533)
9aa8ef35 is described below
commit 9aa8ef35d805df0e2c0b635c62c31c0eb5fa2579
Author: Elliotte Rusty Harold <elh...@users.noreply.github.com>
AuthorDate: Thu Dec 21 17:17:21 2023 -0500
[IO-808] remove redundant checks; improve Javadoc (#533)
* remove redundant checks; improve Javadoc
* remove duplicate check
* update tests
* print failure message
* combin if statements to make PMD happy
* restore nested IOException
* remove unnecessary string concatenation
* fill in missing @throws tags
---
src/main/java/org/apache/commons/io/FileUtils.java | 121 ++++++++++-----------
.../org/apache/commons/io/function/IOConsumer.java | 2 +-
.../io/FileUtilsCopyDirectoryToDirectoryTest.java | 2 +-
.../io/FileUtilsDeleteDirectoryLinuxTest.java | 3 +-
4 files changed, 60 insertions(+), 68 deletions(-)
diff --git a/src/main/java/org/apache/commons/io/FileUtils.java
b/src/main/java/org/apache/commons/io/FileUtils.java
index 363ab69f..0c315204 100644
--- a/src/main/java/org/apache/commons/io/FileUtils.java
+++ b/src/main/java/org/apache/commons/io/FileUtils.java
@@ -293,6 +293,7 @@ public class FileUtils {
* @throws NullPointerException if the given {@link File} is {@code null}.
* @throws NullPointerException if the given {@link Checksum} is {@code
null}.
* @throws IllegalArgumentException if the given {@link File} is not a
file.
+ * @throws FileNotFoundException if the file does not exist
* @throws IOException if an IO error occurs reading the file.
* @since 1.3
*/
@@ -377,8 +378,12 @@ public class FileUtils {
return true;
}
- checkFileExists(file1, "file1");
- checkFileExists(file2, "file2");
+ if (!file1.isFile()) {
+ throw new IllegalArgumentException("Parameter 'file1' is not a
file: " + file1);
+ }
+ if (!file2.isFile()) {
+ throw new IllegalArgumentException("Parameter 'file2' is not a
file: " + file2);
+ }
if (file1.length() != file2.length()) {
// lengths differ, cannot be equal
@@ -663,7 +668,7 @@ public class FileUtils {
*/
public static void copyDirectory(final File srcDir, final File destDir,
final FileFilter fileFilter, final boolean preserveFileDate,
final CopyOption... copyOptions) throws IOException {
- requireFileCopy(srcDir, destDir);
+ Objects.requireNonNull(destDir, "destination");
requireDirectoryExists(srcDir, "srcDir");
requireCanonicalPathsNotEquals(srcDir, destDir);
@@ -709,7 +714,7 @@ public class FileUtils {
* @since 1.2
*/
public static void copyDirectoryToDirectory(final File sourceDir, final
File destinationDir) throws IOException {
- requireDirectoryIfExists(sourceDir, "sourceDir");
+ Objects.requireNonNull(sourceDir, "sourceDir");
requireDirectoryIfExists(destinationDir, "destinationDir");
copyDirectory(sourceDir, new File(destinationDir,
sourceDir.getName()), true);
}
@@ -795,11 +800,10 @@ public class FileUtils {
* @since 2.8.0
*/
public static void copyFile(final File srcFile, final File destFile, final
boolean preserveFileDate, final CopyOption... copyOptions) throws IOException {
- requireFileCopy(srcFile, destFile);
+ Objects.requireNonNull(destFile, "destination");
checkFileExists(srcFile, "srcFile");
requireCanonicalPathsNotEquals(srcFile, destFile);
createParentDirectories(destFile);
- Objects.requireNonNull(destFile, "destFile");
if (destFile.exists()) {
checkFileExists(destFile, "destFile");
requireCanWrite(destFile, "destFile");
@@ -1249,7 +1253,7 @@ public class FileUtils {
*
* Edge cases:
* <ul>
- * <li>A {@code directory} must not be null: if null, throw
IllegalArgumentException</li>
+ * <li>A {@code directory} must not be null: if null, throw
NullPointerException</li>
* <li>A {@code directory} must be a directory: if not a directory, throw
IllegalArgumentException</li>
* <li>A directory does not contain itself: return false</li>
* <li>A null child file is not contained in any parent: return false</li>
@@ -1267,7 +1271,7 @@ public class FileUtils {
public static boolean directoryContains(final File directory, final File
child) throws IOException {
requireDirectoryExists(directory, "directory");
- if (child == null || !directory.exists() || !child.exists()) {
+ if (child == null || !child.exists()) {
return false;
}
@@ -1328,14 +1332,15 @@ public class FileUtils {
*/
public static void forceDelete(final File file) throws IOException {
Objects.requireNonNull(file, "file");
+
final Counters.PathCounters deleteCounters;
try {
- deleteCounters = PathUtils.delete(file.toPath(),
PathUtils.EMPTY_LINK_OPTION_ARRAY,
- StandardDeleteOption.OVERRIDE_READ_ONLY);
- } catch (final IOException e) {
- throw new IOException("Cannot delete file: " + file, e);
+ deleteCounters = PathUtils.delete(
+ file.toPath(), PathUtils.EMPTY_LINK_OPTION_ARRAY,
+ StandardDeleteOption.OVERRIDE_READ_ONLY);
+ } catch (final IOException ex) {
+ throw new IOException("Cannot delete file: " + file, ex);
}
-
if (deleteCounters.getFileCounter().get() < 1 &&
deleteCounters.getDirectoryCounter().get() < 1) {
// didn't find a file to delete.
throw new FileNotFoundException("File does not exist: " + file);
@@ -1533,6 +1538,7 @@ public class FileUtils {
* @param chronoLocalDate the date reference.
* @return true if the {@link File} exists and has been modified after the
given
* {@link ChronoLocalDate} at the current time.
+ * @throws UncheckedIOException if an I/O error occurs
* @throws NullPointerException if the file or local date is {@code null}.
* @since 2.8.0
*/
@@ -1554,6 +1560,7 @@ public class FileUtils {
* @param localTime the time reference.
* @return true if the {@link File} exists and has been modified after the
given
* {@link ChronoLocalDate} at the given time.
+ * @throws UncheckedIOException if an I/O error occurs
* @throws NullPointerException if the file, local date or zone ID is
{@code null}.
* @since 2.8.0
*/
@@ -1572,6 +1579,7 @@ public class FileUtils {
* @param offsetTime the time reference
* @return true if the {@link File} exists and has been modified after the
given {@link ChronoLocalDate} at the given
* {@link OffsetTime}.
+ * @throws UncheckedIOException if an I/O error occurs
* @throws NullPointerException if the file, local date or zone ID is
{@code null}
* @since 2.12.0
*/
@@ -1594,6 +1602,7 @@ public class FileUtils {
* @param chronoLocalDateTime the date reference.
* @return true if the {@link File} exists and has been modified after the
given
* {@link ChronoLocalDateTime} at the system-default time zone.
+ * @throws UncheckedIOException if an I/O error occurs
* @throws NullPointerException if the file or local date time is {@code
null}.
* @since 2.8.0
*/
@@ -1610,6 +1619,7 @@ public class FileUtils {
* @param zoneId the time zone.
* @return true if the {@link File} exists and has been modified after the
given
* {@link ChronoLocalDateTime} at the given {@link ZoneId}.
+ * @throws UncheckedIOException if an I/O error occurs
* @throws NullPointerException if the file, local date time or zone ID is
{@code null}.
* @since 2.8.0
*/
@@ -1627,6 +1637,7 @@ public class FileUtils {
* @return true if the {@link File} exists and has been modified after the
given
* {@link ChronoZonedDateTime}.
* @throws NullPointerException if the file or zoned date time is {@code
null}.
+ * @throws UncheckedIOException if an I/O error occurs
* @since 2.8.0
*/
public static boolean isFileNewer(final File file, final
ChronoZonedDateTime<?> chronoZonedDateTime) {
@@ -1642,6 +1653,7 @@ public class FileUtils {
* @param date the date reference.
* @return true if the {@link File} exists and has been modified
* after the given {@link Date}.
+ * @throws UncheckedIOException if an I/O error occurs
* @throws NullPointerException if the file or date is {@code null}.
*/
public static boolean isFileNewer(final File file, final Date date) {
@@ -1685,6 +1697,7 @@ public class FileUtils {
* @param instant the date reference.
* @return true if the {@link File} exists and has been modified after the
given {@link Instant}.
* @throws NullPointerException if the file or instant is {@code null}.
+ * @throws UncheckedIOException if an I/O error occurs
* @since 2.8.0
*/
public static boolean isFileNewer(final File file, final Instant instant) {
@@ -1699,6 +1712,7 @@ public class FileUtils {
* @param timeMillis the time reference measured in milliseconds since the
* epoch (00:00:00 GMT, January 1, 1970).
* @return true if the {@link File} exists and has been modified after the
given time reference.
+ * @throws UncheckedIOException if an I/O error occurs
* @throws NullPointerException if the file is {@code null}.
*/
public static boolean isFileNewer(final File file, final long timeMillis) {
@@ -1712,6 +1726,7 @@ public class FileUtils {
* @param file the {@link File} of which the modification date must be
compared
* @param offsetDateTime the date reference
* @return true if the {@link File} exists and has been modified before
the given {@link OffsetDateTime}.
+ * @throws UncheckedIOException if an I/O error occurs
* @throws NullPointerException if the file or zoned date time is {@code
null}
* @since 2.12.0
*/
@@ -1735,6 +1750,7 @@ public class FileUtils {
* @return true if the {@link File} exists and has been modified before
the given
* {@link ChronoLocalDate} at the current time.
* @throws NullPointerException if the file or local date is {@code null}.
+ * @throws UncheckedIOException if an I/O error occurs
* @see ZoneId#systemDefault()
* @see LocalTime#now()
* @since 2.8.0
@@ -1757,6 +1773,7 @@ public class FileUtils {
* @param localTime the time reference.
* @return true if the {@link File} exists and has been modified before the
* given {@link ChronoLocalDate} at the specified time.
+ * @throws UncheckedIOException if an I/O error occurs
* @throws NullPointerException if the file, local date or local time is
{@code null}.
* @see ZoneId#systemDefault()
* @since 2.8.0
@@ -1777,6 +1794,7 @@ public class FileUtils {
* @return true if the {@link File} exists and has been modified after the
given {@link ChronoLocalDate} at the given
* {@link OffsetTime}.
* @throws NullPointerException if the file, local date or zone ID is
{@code null}
+ * @throws UncheckedIOException if an I/O error occurs
* @since 2.12.0
*/
public static boolean isFileOlder(final File file, final ChronoLocalDate
chronoLocalDate, final OffsetTime offsetTime) {
@@ -1799,6 +1817,7 @@ public class FileUtils {
* @return true if the {@link File} exists and has been modified before
the given
* {@link ChronoLocalDateTime} at the system-default time zone.
* @throws NullPointerException if the file or local date time is {@code
null}.
+ * @throws UncheckedIOException if an I/O error occurs
* @see ZoneId#systemDefault()
* @since 2.8.0
*/
@@ -1816,6 +1835,7 @@ public class FileUtils {
* @return true if the {@link File} exists and has been modified before
the given
* {@link ChronoLocalDateTime} at the given {@link ZoneId}.
* @throws NullPointerException if the file, local date time or zone ID is
{@code null}.
+ * @throws UncheckedIOException if an I/O error occurs
* @since 2.8.0
*/
public static boolean isFileOlder(final File file, final
ChronoLocalDateTime<?> chronoLocalDateTime, final ZoneId zoneId) {
@@ -1832,6 +1852,7 @@ public class FileUtils {
* @return true if the {@link File} exists and has been modified before
the given
* {@link ChronoZonedDateTime}.
* @throws NullPointerException if the file or zoned date time is {@code
null}.
+ * @throws UncheckedIOException if an I/O error occurs
* @since 2.8.0
*/
public static boolean isFileOlder(final File file, final
ChronoZonedDateTime<?> chronoZonedDateTime) {
@@ -1846,6 +1867,7 @@ public class FileUtils {
* @param date the date reference.
* @return true if the {@link File} exists and has been modified before
the given {@link Date}.
* @throws NullPointerException if the file or date is {@code null}.
+ * @throws UncheckedIOException if an I/O error occurs
*/
public static boolean isFileOlder(final File file, final Date date) {
Objects.requireNonNull(date, "date");
@@ -1859,7 +1881,8 @@ public class FileUtils {
* @param reference the {@link File} of which the modification date is
used.
* @return true if the {@link File} exists and has been modified before
the reference {@link File}.
* @throws NullPointerException if the file or reference file is {@code
null}.
- * @throws IllegalArgumentException if the reference file doesn't exist.
+ * @throws FileNotFoundException if the reference file doesn't exist.
+ * @throws UncheckedIOException if an I/O error occurs
*/
public static boolean isFileOlder(final File file, final File reference)
throws FileNotFoundException {
return Uncheck.get(() -> PathUtils.isOlder(file.toPath(),
reference.toPath()));
@@ -1902,6 +1925,7 @@ public class FileUtils {
* epoch (00:00:00 GMT, January 1, 1970).
* @return true if the {@link File} exists and has been modified before
the given time reference.
* @throws NullPointerException if the file is {@code null}.
+ * @throws UncheckedIOException if an I/O error occurs
*/
public static boolean isFileOlder(final File file, final long timeMillis) {
Objects.requireNonNull(file, "file");
@@ -2304,7 +2328,7 @@ public class FileUtils {
* @since 1.4
*/
public static void moveDirectory(final File srcDir, final File destDir)
throws IOException {
- validateMoveParameters(srcDir, destDir);
+ Objects.requireNonNull(destDir, "destination");
requireDirectoryExists(srcDir, "srcDir");
requireAbsent(destDir, "destDir");
if (!srcDir.renameTo(destDir)) {
@@ -2326,7 +2350,7 @@ public class FileUtils {
* If {@code createDestDir} is true, creates all destination parent
directories, including any necessary but non-existent parent directories.
* </p>
*
- * @param source the file to be moved.
+ * @param source the directory to be moved.
* @param destDir the destination file.
* @param createDestDir If {@code true} create the destination directory,
otherwise if {@code false} throw an
* IOException.
@@ -2391,7 +2415,7 @@ public class FileUtils {
* @since 2.9.0
*/
public static void moveFile(final File srcFile, final File destFile, final
CopyOption... copyOptions) throws IOException {
- validateMoveParameters(srcFile, destFile);
+ Objects.requireNonNull(destFile, "destination");
checkFileExists(srcFile, "srcFile");
requireAbsent(destFile, "destFile");
final boolean rename = srcFile.renameTo(destFile);
@@ -2406,15 +2430,15 @@ public class FileUtils {
}
/**
- * Moves a file to a directory.
+ * Moves a file into a directory.
* <p>
* If {@code createDestDir} is true, creates all destination parent
directories, including any necessary but non-existent parent directories.
* </p>
*
* @param srcFile the file to be moved.
- * @param destDir the destination file.
- * @param createDestDir If {@code true} create the destination directory,
otherwise if {@code false} throw an
- * IOException.
+ * @param destDir the directory to move the file into
+ * @param createDestDir if {@code true} create the destination directory.
If {@code false} throw an
+ * IOException if the destination directory does not already exist.
* @throws NullPointerException if any of the given {@link File}s are
{@code null}.
* @throws FileExistsException if the destination file exists.
* @throws FileNotFoundException if the source file does not exist.
@@ -2434,7 +2458,7 @@ public class FileUtils {
}
/**
- * Moves a file or directory to a destination directory.
+ * Moves a file or directory into a destination directory.
* <p>
* If {@code createDestDir} is true, creates all destination parent
directories, including any necessary but non-existent parent directories.
* </p>
@@ -2444,7 +2468,8 @@ public class FileUtils {
*
* @param src the file or directory to be moved.
* @param destDir the destination directory.
- * @param createDestDir If {@code true} create the destination directory,
otherwise if {@code false} throw an IOException.
+ * @param createDestDir if {@code true} create the destination directory.
If {@code false} throw an
+ * IOException if the destination directory does not already exist.
* @throws NullPointerException if any of the given {@link File}s are
{@code null}.
* @throws FileExistsException if the directory or file exists in the
destination directory.
* @throws FileNotFoundException if the source file does not exist.
@@ -2555,7 +2580,9 @@ public class FileUtils {
public static FileOutputStream openOutputStream(final File file, final
boolean append) throws IOException {
Objects.requireNonNull(file, "file");
if (file.exists()) {
- checkFileExists(file, "file");
+ if (!file.isFile()) {
+ throw new IllegalArgumentException("Parameter 'file' is not a
file: " + file);
+ }
requireCanWrite(file, "file");
} else {
createParentDirectories(file);
@@ -2711,7 +2738,6 @@ public class FileUtils {
* @throws IllegalArgumentException if the file is not writable.
*/
private static void requireCanWrite(final File file, final String name) {
- Objects.requireNonNull(file, "file");
if (!file.canWrite()) {
throw new IllegalArgumentException("File parameter '" + name + "
is not writable: '" + file + "'");
}
@@ -2741,50 +2767,29 @@ public class FileUtils {
*
* @param directory The {@link File} to check.
* @param name The parameter name to use in the exception message in case
of null input.
- * @throws FileNotFoundException if the given {@link File} does not exist
* @throws NullPointerException if the given {@link File} is {@code null}.
* @throws IllegalArgumentException if the given {@link File} exists but
is not a directory.
*/
private static void requireDirectoryIfExists(final File directory, final
String name) throws FileNotFoundException {
Objects.requireNonNull(directory, name);
- if (directory.exists()) {
- requireDirectoryExists(directory, name);
- }
- }
-
- /**
- * Requires that the given {@link File} object, which may be a file or a
directory,
- * points to an actually existing item in the file system,
- * and throws a {@link FileNotFoundException} if it doesn't.
- *
- * @param file The {@link File} to check.
- * @param fileParamName The parameter name to use in the exception message
in case of {@code null} input.
- * @return the given file.
- * @throws NullPointerException if the given {@link File} is {@code null}.
- * @throws FileNotFoundException if the given {@link File} does not exist.
- */
- private static File checkFileObjectExists(final File file, final String
fileParamName) throws FileNotFoundException {
- Objects.requireNonNull(file, fileParamName);
- if (!file.exists()) {
- throw new FileNotFoundException("File system element for parameter
'" + fileParamName + "' does not exist: '" + file + "'");
+ if (directory.exists() && !directory.isDirectory()) {
+ throw new IllegalArgumentException("Parameter '" + name + "' is
not a directory: '" + directory + "'");
}
- return file;
}
/**
- * Requires that the given {@link File} object,
+ * Requires that the given {@link File} object
* points to an actual file (not a directory) in the file system,
* and throws a {@link FileNotFoundException} if it doesn't.
* It throws an IllegalArgumentException if the object points to a
directory.
*
* @param file The {@link File} to check.
* @param name The parameter name to use in the exception message.
- * @return the given file.
* @throws FileNotFoundException if the file does not exist
* @throws NullPointerException if the given {@link File} is {@code null}.
* @throws IllegalArgumentException if the given {@link File} is not a
file.
*/
- private static File checkFileExists(final File file, final String name)
throws FileNotFoundException {
+ private static void checkFileExists(final File file, final String name)
throws FileNotFoundException {
Objects.requireNonNull(file, name);
if (!file.isFile()) {
if (file.exists()) {
@@ -2792,20 +2797,6 @@ public class FileUtils {
}
throw new FileNotFoundException("Source '" + file + "' does not
exist");
}
- return file;
- }
-
- /**
- * Requires parameter attributes for a file copy operation.
- *
- * @param source the source file
- * @param destination the destination
- * @throws NullPointerException if any of the given {@link File}s are
{@code null}.
- * @throws FileNotFoundException if the source does not exist.
- */
- private static void requireFileCopy(final File source, final File
destination) throws FileNotFoundException {
- checkFileObjectExists(source, "source");
- Objects.requireNonNull(destination, "destination");
}
/**
@@ -3520,7 +3511,7 @@ public class FileUtils {
/**
* Instances should NOT be constructed in standard programming.
- * @deprecated Will be private in 3.0.
+ * @deprecated Will be private in 4.0.
*/
@Deprecated
public FileUtils() { //NOSONAR
diff --git a/src/main/java/org/apache/commons/io/function/IOConsumer.java
b/src/main/java/org/apache/commons/io/function/IOConsumer.java
index e417178f..e35ad7ef 100644
--- a/src/main/java/org/apache/commons/io/function/IOConsumer.java
+++ b/src/main/java/org/apache/commons/io/function/IOConsumer.java
@@ -67,7 +67,7 @@ public interface IOConsumer<T> {
}
/**
- * Performs an action for each element of the array gathering any
exceptions.
+ * Performs an action for each element of the array, gathering any
exceptions.
*
* @param action The action to apply to each input element.
* @param array The input to stream.
diff --git
a/src/test/java/org/apache/commons/io/FileUtilsCopyDirectoryToDirectoryTest.java
b/src/test/java/org/apache/commons/io/FileUtilsCopyDirectoryToDirectoryTest.java
index e4032f86..cce762bd 100644
---
a/src/test/java/org/apache/commons/io/FileUtilsCopyDirectoryToDirectoryTest.java
+++
b/src/test/java/org/apache/commons/io/FileUtilsCopyDirectoryToDirectoryTest.java
@@ -77,7 +77,7 @@ public class FileUtilsCopyDirectoryToDirectoryTest {
try (TempFile srcDir = TempFile.create("notadirectory", null)) {
final File destDir = new File(temporaryFolder,
"destinationDirectory");
destDir.mkdirs();
- final String expectedMessage = String.format("Parameter
'sourceDir' is not a directory: '%s'", srcDir);
+ final String expectedMessage = String.format("Parameter 'srcDir'
is not a directory: '%s'", srcDir);
assertExceptionTypeAndMessage(srcDir.toFile(), destDir,
IllegalArgumentException.class, expectedMessage);
}
}
diff --git
a/src/test/java/org/apache/commons/io/FileUtilsDeleteDirectoryLinuxTest.java
b/src/test/java/org/apache/commons/io/FileUtilsDeleteDirectoryLinuxTest.java
index 60600ff5..ad50f2f8 100644
--- a/src/test/java/org/apache/commons/io/FileUtilsDeleteDirectoryLinuxTest.java
+++ b/src/test/java/org/apache/commons/io/FileUtilsDeleteDirectoryLinuxTest.java
@@ -88,7 +88,8 @@ public class FileUtilsDeleteDirectoryLinuxTest extends
AbstractFileUtilsDeleteDi
try {
// deleteDirectory calls forceDelete
final IOExceptionList ioExceptionList = (IOExceptionList)
assertThrows(IOException.class, () -> FileUtils.deleteDirectory(nested));
-
assertTrue(ioExceptionList.getCause(0).getMessage().endsWith("Cannot delete
file: " + file.getAbsolutePath()));
+ final String message = ioExceptionList.getCause(0).getMessage();
+ assertTrue(message.endsWith("Cannot delete file: " +
file.getAbsolutePath()), message);
} finally {
chmod(nested, 755, false);
FileUtils.deleteDirectory(nested);
