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 f01754e Use the JRE CopyOption enum instead of asrc boolean. f01754e is described below commit f01754e820f98e7e99909b9ea3900000a68ad8e9 Author: Gary Gregory <gardgreg...@gmail.com> AuthorDate: Wed Jan 13 12:20:28 2021 -0500 Use the JRE CopyOption enum instead of asrc boolean. --- src/changes/changes.xml | 2 +- src/main/java/org/apache/commons/io/FileUtils.java | 150 +++++++++++---------- .../org/apache/commons/io/FileUtilsTestCase.java | 7 +- 3 files changed, 85 insertions(+), 74 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 059f68c..ba34b86 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -125,7 +125,7 @@ The <action> type attribute can be add,update,fix,remove. Add FileUtils.delete(File). </action> <action issue="IO-700" dev="ggregory" type="add" due-to="Gary Gregory"> - Add FileUtils.moveFile(File, File, boolean) #185. + Add FileUtils.moveFile(File, File, CopyOption...) #185. </action> <!-- UPDATES --> <action dev="ggregory" type="update" due-to="Dependabot"> diff --git a/src/main/java/org/apache/commons/io/FileUtils.java b/src/main/java/org/apache/commons/io/FileUtils.java index 2c25bac..f177fad 100644 --- a/src/main/java/org/apache/commons/io/FileUtils.java +++ b/src/main/java/org/apache/commons/io/FileUtils.java @@ -45,6 +45,7 @@ import java.time.chrono.ChronoLocalDate; import java.time.chrono.ChronoLocalDateTime; import java.time.chrono.ChronoZonedDateTime; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Date; @@ -674,7 +675,8 @@ public class FileUtils { } } } - doCopyDirectory(srcDir, destDir, fileFilter, preserveFileDate, exclusionList, copyOptions); + doCopyDirectory(srcDir, destDir, fileFilter, exclusionList, + preserveFileDate, preserveFileDate ? addCopyAttributes(copyOptions) : copyOptions); } /** @@ -730,7 +732,7 @@ public class FileUtils { * @see #copyFile(File, File, boolean) */ public static void copyFile(final File srcFile, final File destFile) throws IOException { - copyFile(srcFile, destFile, true); + copyFile(srcFile, destFile, StandardCopyOption.REPLACE_EXISTING); } /** @@ -757,15 +759,18 @@ public class FileUtils { */ public static void copyFile(final File srcFile, final File destFile, final boolean preserveFileDate) throws IOException { - copyFile(srcFile, destFile, preserveFileDate, StandardCopyOption.REPLACE_EXISTING); + copyFile(srcFile, destFile, + preserveFileDate + ? new CopyOption[] {StandardCopyOption.COPY_ATTRIBUTES, StandardCopyOption.REPLACE_EXISTING} + : new CopyOption[] {StandardCopyOption.REPLACE_EXISTING}); } /** * Copies a file to a new location. * <p> * This method copies the contents of the specified source file to the specified destination file. The directory - * holding the destination file is created if it does not exist. If the destination file exists, then this method - * will overwrite it. + * holding the destination file is created if it does not exist. If the destination file exists, you can overwrite + * it with {@link StandardCopyOption#REPLACE_EXISTING}. * </p> * <p> * <strong>Note:</strong> Setting <code>preserveFileDate</code> to {@code true} tries to preserve the file's last @@ -787,15 +792,61 @@ public class FileUtils { */ public static void copyFile(final File srcFile, final File destFile, final boolean preserveFileDate, final CopyOption... copyOptions) throws IOException { + copyFile(srcFile, destFile, preserveFileDate ? addCopyAttributes(copyOptions) : copyOptions); + } + + /** + * Copies a file to a new location. + * <p> + * This method copies the contents of the specified source file to the specified destination file. The directory + * holding the destination file is created if it does not exist. If the destination file exists, you can overwrite + * it if you use {@link StandardCopyOption#REPLACE_EXISTING}. + * </p> + * + * @param srcFile an existing file to copy, must not be {@code null}. + * @param destFile the new file, must not be {@code null}. + * @param copyOptions options specifying how the copy should be done, for example {@link StandardCopyOption}.. + * @throws NullPointerException if any of the given {@code File}s are {@code null}. + * @throws FileNotFoundException if the source does not exist. + * @throws IllegalArgumentException if source is not a file. + * @throws IOException if the output file length is not the same as the input file length after the copy completes. + * @throws IOException if an I/O error occurs. + * @see StandardCopyOption + * @since 2.9.0 + */ + public static void copyFile(final File srcFile, final File destFile, final CopyOption... copyOptions) + throws IOException { requireFileCopy(srcFile, destFile); requireFile(srcFile, "srcFile"); requireCanonicalPathsNotEquals(srcFile, destFile); createParentDirectories(destFile); - + requireFileIfExists(destFile, "destFile"); if (destFile.exists()) { requireCanWrite(destFile, "destFile"); } - doCopyFile(srcFile, destFile, preserveFileDate, copyOptions); + + // On Windows, the last modified time is copied by default. + Files.copy(srcFile.toPath(), destFile.toPath(), copyOptions); + + // TODO IO-386: Do we still need this check? + requireEqualSizes(srcFile, destFile, srcFile.length(), destFile.length()); + } + + /** + * Copies the given array and adds StandardCopyOption.COPY_ATTRIBUTES. + * + * @param copyOptions sorted copy options. + * @return a new array. + */ + private static CopyOption[] addCopyAttributes(final CopyOption... copyOptions) { + // Make a copy first since we don't want to sort the call site's version. + final CopyOption[] actual = Arrays.copyOf(copyOptions, copyOptions.length + 1); + Arrays.sort(actual, 0, copyOptions.length); + if (Arrays.binarySearch(copyOptions, 0, copyOptions.length, StandardCopyOption.COPY_ATTRIBUTES) >= 0) { + return copyOptions; + } + actual[actual.length - 1] = StandardCopyOption.COPY_ATTRIBUTES; + return actual; } /** @@ -861,14 +912,14 @@ public class FileUtils { * @throws NullPointerException if any of the given {@code File}s are {@code null}. * @throws IOException if an error occurs or setting the last-modified time didn't succeeded. * @throws IOException if the output file length is not the same as the input file length after the copy completes. - * @see #copyFile(File, File, boolean) + * @see #copyFile(File, File, CopyOption...) * @since 1.3 */ public static void copyFileToDirectory(final File sourceFile, final File destinationDir, final boolean preserveFileDate) throws IOException { + Objects.requireNonNull(sourceFile, "sourceFile"); requireDirectoryIfExists(destinationDir, "destinationDir"); - final File destFile = new File(destinationDir, sourceFile.getName()); - copyFile(sourceFile, destFile, preserveFileDate); + copyFile(sourceFile, new File(destinationDir, sourceFile.getName()), preserveFileDate); } /** @@ -1235,16 +1286,15 @@ public class FileUtils { * @param srcDir the validated source directory, must not be {@code null}. * @param destDir the validated destination directory, must not be {@code null}. * @param fileFilter the filter to apply, null means copy all directories and files. - * @param preserveFileDate whether to preserve the file date. * @param exclusionList List of files and directories to exclude from the copy, may be null. - * @param copyOptions options specifying how the copy should be done, for example {@link StandardCopyOption}. + * @param preserveDirDate preserve the directories last modified dates. + * @param copyOptions options specifying how the copy should be done, see {@link StandardCopyOption}. * @throws IOException if the directory was not created along with all its parent directories. * @throws IOException if the given file object is not a directory. */ private static void doCopyDirectory(final File srcDir, final File destDir, final FileFilter fileFilter, - final boolean preserveFileDate, final List<String> exclusionList, final CopyOption... copyOptions) - throws IOException { - // recurse + final List<String> exclusionList, boolean preserveDirDate, final CopyOption... copyOptions) throws IOException { + // recurse dirs, copy files. final File[] srcFiles = listFiles(srcDir, fileFilter); requireDirectoryIfExists(destDir, "destDir"); mkdirs(destDir); @@ -1253,53 +1303,19 @@ public class FileUtils { final File dstFile = new File(destDir, srcFile.getName()); if (exclusionList == null || !exclusionList.contains(srcFile.getCanonicalPath())) { if (srcFile.isDirectory()) { - doCopyDirectory(srcFile, dstFile, fileFilter, preserveFileDate, exclusionList, copyOptions); + doCopyDirectory(srcFile, dstFile, fileFilter, exclusionList, preserveDirDate, copyOptions); } else { - doCopyFile(srcFile, dstFile, preserveFileDate, copyOptions); + copyFile(srcFile, dstFile, copyOptions); } } } - // Do this last, as the above has probably affected directory metadata - if (preserveFileDate) { + if (preserveDirDate) { setLastModified(srcDir, destDir); } } /** - * Internal copy file method. This uses the original file length, and throws an IOException if the output file - * length is different from the current input file length. So it may fail if the file changes size. It may also fail - * with "IllegalArgumentException: Negative size" if the input file is truncated part way through copying the data - * and the new file size is less than the current position. - * - * @param srcFile the validated source file, must not be {@code null} - * @param destFile the validated destination file, must not be {@code null} - * @param preserveFileDate whether to preserve the file date - * @param copyOptions options specifying how the copy should be done, for example {@link StandardCopyOption}. - * @throws IOException if an error occurs or setting the last-modified time didn't succeeded. - * @throws IOException if the output file length is not the same as the input file length after the copy completes - * @throws IllegalArgumentException "Negative size" if the file is truncated so that the size is less than the - * position - */ - private static void doCopyFile(final File srcFile, final File destFile, final boolean preserveFileDate, final CopyOption... copyOptions) - throws IOException { - Objects.requireNonNull(srcFile, "srcFile"); - requireFileIfExists(destFile, "destFile"); - - final Path srcPath = srcFile.toPath(); - final Path destPath = destFile.toPath(); - // On Windows, the last modified time is copied by default. - Files.copy(srcPath, destPath, copyOptions); - - // TODO IO-386: Do we still need this check? - requireEqualSizes(srcFile, destFile, srcFile.length(), destFile.length()); - - if (preserveFileDate) { - setLastModified(srcFile, destFile); - } - } - - /** * Deletes a file or directory. For a directory, delete it and all sub-directories. * <p> * The difference between File.delete() and this method are: @@ -2155,14 +2171,12 @@ public class FileUtils { } /** - * Moves a file. + * Moves a file preserving attributes. * <p> - * When the destination file is on another file system, do a "copy and delete". + * Shorthand for {@code moveFile(srcFile, destFile, StandardCopyOption.COPY_ATTRIBUTES)}. * </p> * <p> - * <strong>Note:</strong> This method tries to preserve the file's last modified date/times using - * {@link File#setLastModified(long)} when destination is another file system, however it is not guaranteed that the - * operation will succeed. If the modification operation fails, the methods throws IOException. + * When the destination file is on another file system, do a "copy and delete". * </p> * * @param srcFile the file to be moved. @@ -2170,11 +2184,11 @@ public class FileUtils { * @throws NullPointerException if any of the given {@code File}s are {@code null}. * @throws FileExistsException if the destination file exists. * @throws IOException if source or destination is invalid. - * @throws IOException if an error occurs or setting the last-modified time didn't succeeded. + * @throws IOException if an error occurs. * @since 1.4 */ public static void moveFile(final File srcFile, final File destFile) throws IOException { - moveFile(srcFile, destFile, true); + moveFile(srcFile, destFile, StandardCopyOption.COPY_ATTRIBUTES); } /** @@ -2182,31 +2196,24 @@ public class FileUtils { * <p> * When the destination file is on another file system, do a "copy and delete". * </p> - * <p> - * <strong>Note:</strong> Setting <code>preserveFileDate</code> to {@code true} tries to preserve the files' last - * modified date/times using {@link File#setLastModified(long)} when destination is another file system, however it - * is not guaranteed that those operations will succeed. If the modification operation fails, the methods throws - * IOException. - * </p> * * @param srcFile the file to be moved. * @param destFile the destination file. - * @param preserveFileDate true if the file date of the "copy and delete" should be the same as the original when - * destination is on another file system. Param is not used if destination is on same file system. + * @param copyOptions Copy options. * @throws NullPointerException if any of the given {@code File}s are {@code null}. * @throws FileExistsException if the destination file exists. * @throws IOException if source or destination is invalid. * @throws IOException if an error occurs or setting the last-modified time didn't succeeded. * @since 2.9.0 */ - public static void moveFile(final File srcFile, final File destFile, final boolean preserveFileDate) + public static void moveFile(final File srcFile, final File destFile, final CopyOption... copyOptions) throws IOException { validateMoveParameters(srcFile, destFile); requireFile(srcFile, "srcFile"); requireAbsent(destFile, null); final boolean rename = srcFile.renameTo(destFile); if (!rename) { - copyFile(srcFile, destFile, preserveFileDate); + copyFile(srcFile, destFile, copyOptions); if (!srcFile.delete()) { FileUtils.deleteQuietly(destFile); throw new IOException("Failed to delete original file '" + srcFile + @@ -2505,7 +2512,8 @@ public class FileUtils { } /** - * Throws an {@link IllegalArgumentException} if the file is not writable. + * Throws an {@link IllegalArgumentException} if the file is not writable. This provides a more precise exception + * message than a plain access denied. * * @param file The file to test. * @param name The parameter name to use in the exception message. @@ -2515,7 +2523,7 @@ public class FileUtils { private static void requireCanWrite(final File file, String name) { Objects.requireNonNull(file, "file"); if (!file.canWrite()) { - throw new IllegalArgumentException("File parameter '" + name + " is non-writable: '" + file + "'"); + throw new IllegalArgumentException("File parameter '" + name + " is not writable: '" + file + "'"); } } diff --git a/src/test/java/org/apache/commons/io/FileUtilsTestCase.java b/src/test/java/org/apache/commons/io/FileUtilsTestCase.java index 327322f..efffbda 100644 --- a/src/test/java/org/apache/commons/io/FileUtilsTestCase.java +++ b/src/test/java/org/apache/commons/io/FileUtilsTestCase.java @@ -39,6 +39,8 @@ import java.math.BigInteger; import java.net.URL; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; +import java.nio.file.CopyOption; +import java.nio.file.StandardCopyOption; import java.time.Instant; import java.time.LocalDate; import java.time.LocalDateTime; @@ -58,6 +60,7 @@ import java.util.Map; import java.util.zip.CRC32; import java.util.zip.Checksum; +import org.apache.commons.io.file.PathUtils; import org.apache.commons.io.filefilter.IOFileFilter; import org.apache.commons.io.filefilter.NameFileFilter; import org.apache.commons.io.filefilter.WildcardFileFilter; @@ -2183,7 +2186,7 @@ public class FileUtilsTestCase { }; final long expected = getLastModifiedMillis(testFile1); - FileUtils.moveFile(src, destination, true); + FileUtils.moveFile(src, destination, StandardCopyOption.COPY_ATTRIBUTES); assertTrue(destination.exists(), "Check Exist"); assertTrue(!src.exists(), "Original deleted"); @@ -2213,7 +2216,7 @@ public class FileUtilsTestCase { }; final long unexpected = getLastModifiedMillis(testFile1); - FileUtils.moveFile(src, destination, false); + FileUtils.moveFile(src, destination, PathUtils.EMPTY_COPY_OPTIONS); assertTrue(destination.exists(), "Check Exist"); assertTrue(!src.exists(), "Original deleted");