[IO-808] Rationalize and unify checking for existence of files and directories (#529)

* Rationalize and unify checking for existence of files and directories

* fix sizeOfDirectoryAsBigInteger

Author: elharo

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

@@ -292,13 +292,12 @@ public static String byteCountToDisplaySize(final Number size) {
      * @return the checksum specified, updated with the content of the file
      * @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} does not exist or is not a file.
+     * @throws IllegalArgumentException if the given {@link File} is not a file.
      * @throws IOException if an IO error occurs reading the file.
      * @since 1.3
      */
     public static Checksum checksum(final File file, final Checksum checksum) throws IOException {
-        requireExistsChecked(file, "file");
-        requireFile(file, "file");
+        checkFileExists(file, "file");
         Objects.requireNonNull(checksum, "checksum");
         try (InputStream inputStream = new CheckedInputStream(Files.newInputStream(file.toPath()), checksum)) {
             IOUtils.consume(inputStream);
@@ -378,8 +377,8 @@ public static boolean contentEquals(final File file1, final File file2) throws I
             return true;
         }
 
-        requireFile(file1, "file1");
-        requireFile(file2, "file2");
+        checkFileExists(file1, "file1");
+        checkFileExists(file2, "file2");
 
         if (file1.length() != file2.length()) {
             // lengths differ, cannot be equal
@@ -431,8 +430,8 @@ public static boolean contentEqualsIgnoreEOL(final File file1, final File file2,
             return true;
         }
 
-        requireFile(file1, "file1");
-        requireFile(file2, "file2");
+        checkFileExists(file1, "file1");
+        checkFileExists(file2, "file2");
 
         if (file1.getCanonicalFile().equals(file2.getCanonicalFile())) {
             // same file
@@ -665,7 +664,7 @@ public static void copyDirectory(final File srcDir, final File destDir, final Fi
     public static void copyDirectory(final File srcDir, final File destDir, final FileFilter fileFilter, final boolean preserveFileDate,
         final CopyOption... copyOptions) throws IOException {
         requireFileCopy(srcDir, destDir);
-        requireDirectory(srcDir, "srcDir");
+        requireDirectoryExists(srcDir, "srcDir");
         requireCanonicalPathsNotEquals(srcDir, destDir);
 
         // Cater for destination being directory within the source directory (see IO-141)
@@ -797,11 +796,12 @@ public static void copyFile(final File srcFile, final File destFile, final boole
      */
     public static void copyFile(final File srcFile, final File destFile, final boolean preserveFileDate, final CopyOption... copyOptions) throws IOException {
         requireFileCopy(srcFile, destFile);
-        requireFile(srcFile, "srcFile");
+        checkFileExists(srcFile, "srcFile");
         requireCanonicalPathsNotEquals(srcFile, destFile);
         createParentDirectories(destFile);
-        requireFileIfExists(destFile, "destFile");
+        Objects.requireNonNull(destFile, "destFile");
         if (destFile.exists()) {
+            checkFileExists(destFile, "destFile");
             requireCanWrite(destFile, "destFile");
         }
         Files.copy(srcFile.toPath(), destFile.toPath(), copyOptions);
@@ -1259,8 +1259,8 @@ public static boolean deleteQuietly(final File file) {
      * @param child     the file to consider as the child.
      * @return true is the candidate leaf is under by the specified composite. False otherwise.
      * @throws IOException              if an IO error occurs while checking the files.
-     * @throws NullPointerException if the given {@link File} is {@code null}.
-     * @throws IllegalArgumentException if the given {@link File} does not exist or is not a directory.
+     * @throws NullPointerException if the parent is {@code null}.
+     * @throws IllegalArgumentException if the parent is not a directory.
      * @see FilenameUtils#directoryContains(String, String)
      * @since 2.2
      */
@@ -1657,10 +1657,9 @@ public static boolean isFileNewer(final File file, final Date date) {
      * @return true if the {@link File} exists and has been modified more
      * recently than 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 UncheckedIOException if the reference file doesn't exist.
      */
     public static boolean isFileNewer(final File file, final File reference) {
-        requireExists(reference, "reference");
         return Uncheck.get(() -> PathUtils.isNewer(file.toPath(), reference.toPath()));
     }
 
@@ -1862,8 +1861,7 @@ public static boolean isFileOlder(final File file, final Date date) {
      * @throws NullPointerException if the file or reference file is {@code null}.
      * @throws IllegalArgumentException if the reference file doesn't exist.
      */
-    public static boolean isFileOlder(final File file, final File reference) {
-        requireExists(reference, "reference");
+    public static boolean isFileOlder(final File file, final File reference) throws FileNotFoundException {
         return Uncheck.get(() -> PathUtils.isOlder(file.toPath(), reference.toPath()));
     }
 
@@ -2307,7 +2305,7 @@ private static File mkdirs(final File directory) throws IOException {
      */
     public static void moveDirectory(final File srcDir, final File destDir) throws IOException {
         validateMoveParameters(srcDir, destDir);
-        requireDirectory(srcDir, "srcDir");
+        requireDirectoryExists(srcDir, "srcDir");
         requireAbsent(destDir, "destDir");
         if (!srcDir.renameTo(destDir)) {
             if (destDir.getCanonicalPath().startsWith(srcDir.getCanonicalPath() + File.separator)) {
@@ -2394,7 +2392,7 @@ public static void moveFile(final File srcFile, final File destFile) throws IOEx
      */
     public static void moveFile(final File srcFile, final File destFile, final CopyOption... copyOptions) throws IOException {
         validateMoveParameters(srcFile, destFile);
-        requireFile(srcFile, "srcFile");
+        checkFileExists(srcFile, "srcFile");
         requireAbsent(destFile, "destFile");
         final boolean rename = srcFile.renameTo(destFile);
         if (!rename) {
@@ -2431,8 +2429,7 @@ public static void moveFileToDirectory(final File srcFile, final File destDir, f
         if (!destDir.exists() && createDestDir) {
             mkdirs(destDir);
         }
-        requireExistsChecked(destDir, "destDir");
-        requireDirectory(destDir, "destDir");
+        requireDirectoryExists(destDir, "destDir");
         moveFile(srcFile, new File(destDir, srcFile.getName()));
     }
 
@@ -2558,7 +2555,7 @@ public static FileOutputStream openOutputStream(final File file) throws IOExcept
     public static FileOutputStream openOutputStream(final File file, final boolean append) throws IOException {
         Objects.requireNonNull(file, "file");
         if (file.exists()) {
-            requireFile(file, "file");
+            checkFileExists(file, "file");
             requireCanWrite(file, "file");
         } else {
             createParentDirectories(file);
@@ -2721,81 +2718,52 @@ private static void requireCanWrite(final File file, final String name) {
     }
 
     /**
-     * Requires that the given {@link File} is a directory.
+     * Requires that the given {@link File} exists and is a directory.
      *
      * @param directory The {@link File} to check.
      * @param name The parameter name to use in the exception message in case of null input or if the file is not a directory.
-     * @return the given directory.
      * @throws NullPointerException if the given {@link File} is {@code null}.
-     * @throws IllegalArgumentException if the given {@link File} does not exist or is not a directory.
+     * @throws FileNotFoundException if the given {@link File} does not exist
+     * @throws IllegalArgumentException if the given {@link File} is not a directory
      */
-    private static File requireDirectory(final File directory, final String name) {
+    private static void requireDirectoryExists(final File directory, final String name) throws FileNotFoundException {
         Objects.requireNonNull(directory, name);
         if (!directory.isDirectory()) {
-            throw new IllegalArgumentException("Parameter '" + name + "' is not a directory: '" + directory + "'");
+            if (directory.exists()) {
+                throw new IllegalArgumentException("Parameter '" + name + "' is not a directory: '" + directory + "'");
+            }
+            throw new FileNotFoundException("Directory '" + directory + "' does not exist.");
         }
-        return directory;
-    }
-
-    /**
-     * Requires that the given {@link File} exists and is a directory.
-     *
-     * @param directory The {@link File} to check.
-     * @param name The parameter name to use in the exception message in case of null input.
-     * @return the given directory.
-     * @throws NullPointerException if the given {@link File} is {@code null}.
-     * @throws IllegalArgumentException if the given {@link File} does not exist or is not a directory.
-     */
-    private static File requireDirectoryExists(final File directory, final String name) {
-        requireExists(directory, name);
-        requireDirectory(directory, name);
-        return directory;
     }
 
     /**
      * Requires that the given {@link File} is a directory if it exists.
      *
      * @param directory The {@link File} to check.
      * @param name The parameter name to use in the exception message in case of null input.
-     * @return the given directory.
+     * @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 File requireDirectoryIfExists(final File directory, final String name) {
+    private static void requireDirectoryIfExists(final File directory, final String name) throws FileNotFoundException {
         Objects.requireNonNull(directory, name);
         if (directory.exists()) {
-            requireDirectory(directory, name);
-        }
-        return directory;
-    }
-
-    /**
-     * Requires that the given {@link File} exists and throws an {@link IllegalArgumentException} 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 IllegalArgumentException if the given {@link File} does not exist.
-     */
-    private static File requireExists(final File file, final String fileParamName) {
-        Objects.requireNonNull(file, fileParamName);
-        if (!file.exists()) {
-            throw new IllegalArgumentException("File system element for parameter '" + fileParamName + "' does not exist: '" + file + "'");
+            requireDirectoryExists(directory, name);
         }
-        return file;
     }
 
     /**
-     * Requires that the given {@link File} exists and throws an {@link FileNotFoundException} if it doesn't.
+     * 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 requireExistsChecked(final File file, final String fileParamName) throws FileNotFoundException {
+    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 + "'");
@@ -2804,18 +2772,25 @@ private static File requireExistsChecked(final File file, final String fileParam
     }
 
     /**
-     * Requires that the given {@link File} is a file.
+     * 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} does not exist or is not a file.
+     * @throws IllegalArgumentException if the given {@link File} is not a file.
      */
-    private static File requireFile(final File file, final String name) {
+    private static File checkFileExists(final File file, final String name) throws FileNotFoundException {
         Objects.requireNonNull(file, name);
         if (!file.isFile()) {
-            throw new IllegalArgumentException("Parameter '" + name + "' is not a file: " + file);
+            if (file.exists()) {
+                throw new IllegalArgumentException("Parameter '" + name + "' is not a file: " + file);
+            }
+            throw new FileNotFoundException("Source '" + file + "' does not exist");
         }
         return file;
     }
@@ -2829,24 +2804,10 @@ private static File requireFile(final File file, final String name) {
      * @throws FileNotFoundException if the source does not exist.
      */
     private static void requireFileCopy(final File source, final File destination) throws FileNotFoundException {
-        requireExistsChecked(source, "source");
+        checkFileObjectExists(source, "source");
         Objects.requireNonNull(destination, "destination");
     }
 
-    /**
-     * Requires that the given {@link File} is a file if it exists.
-     *
-     * @param file The {@link File} to check.
-     * @param name The parameter name to use in the exception message in case of null input.
-     * @return the given directory.
-     * @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 File requireFileIfExists(final File file, final String name) {
-        Objects.requireNonNull(file, name);
-        return file.exists() ? requireFile(file, name) : file;
-    }
-
     /**
      * Sets file lastModifiedTime, lastAccessTime and creationTime to match source file
      *
@@ -2900,7 +2861,6 @@ private static boolean setTimes(final File sourceFile, final File targetFile) {
      * @since 2.0
      */
     public static long sizeOf(final File file) {
-        requireExists(file, "file");
         return Uncheck.get(() -> PathUtils.sizeOf(file.toPath()));
     }
 
@@ -2923,7 +2883,6 @@ public static long sizeOf(final File file) {
      * @since 2.4
      */
     public static BigInteger sizeOfAsBigInteger(final File file) {
-        requireExists(file, "file");
         return Uncheck.get(() -> PathUtils.sizeOfAsBigInteger(file.toPath()));
     }
 
@@ -2942,7 +2901,11 @@ public static BigInteger sizeOfAsBigInteger(final File file) {
      * @throws UncheckedIOException if an IO error occurs.
      */
     public static long sizeOfDirectory(final File directory) {
-        requireDirectoryExists(directory, "directory");
+        try {
+            requireDirectoryExists(directory, "directory");
+        } catch (FileNotFoundException e) {
+            throw new UncheckedIOException(e);
+        }
         return Uncheck.get(() -> PathUtils.sizeOfDirectory(directory.toPath()));
     }
 
@@ -2956,7 +2919,11 @@ public static long sizeOfDirectory(final File directory) {
      * @since 2.4
      */
     public static BigInteger sizeOfDirectoryAsBigInteger(final File directory) {
-        requireDirectoryExists(directory, "directory");
+        try {
+            requireDirectoryExists(directory, "directory");
+        } catch (FileNotFoundException e) {
+            throw new UncheckedIOException(e);
+        }
         return Uncheck.get(() -> PathUtils.sizeOfDirectoryAsBigInteger(directory.toPath()));
     }
 

src/test/java/org/apache/commons/io/FileUtilsDirectoryContainsTest.java

@@ -21,6 +21,7 @@
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.io.File;
+import java.io.FileNotFoundException;
 import java.io.IOException;
 
 import org.junit.jupiter.api.BeforeEach;
@@ -115,7 +116,7 @@ public void testDirectoryDoesNotContainsDirectory() throws IOException {
     public void testDirectoryDoesNotExist() {
         final File dir = new File("DOESNOTEXIST");
         assertFalse(dir.exists());
-        assertThrows(IllegalArgumentException.class, () -> FileUtils.directoryContains(dir, file1));
+        assertThrows(FileNotFoundException.class, () -> FileUtils.directoryContains(dir, file1));
     }
 
     @Test
@@ -168,6 +169,6 @@ public void testUnrealizedContainment() {
         final File file = new File(dir, "DOESNOTEXIST2");
         assertFalse(dir.exists());
         assertFalse(file.exists());
-        assertThrows(IllegalArgumentException.class, () -> FileUtils.directoryContains(dir, file));
+        assertThrows(FileNotFoundException.class, () -> FileUtils.directoryContains(dir, file));
     }
 }