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 4810d9c FileUtils.copyURLToFile should create target parent
directories and overwrite target file #319.
4810d9c is described below
commit 4810d9c57276b584942cf93ae297f10a12ab128c
Author: Gary Gregory <[email protected]>
AuthorDate: Sun Jan 30 09:16:42 2022 -0500
FileUtils.copyURLToFile should create target parent directories and
overwrite target file #319.
- Based on PR #39 but different.
- New assert method leaks input stream; this bug was copied from
existing code into a new method.
- Use NIO.
- Tests don't need to schedule files for deletion on exit when JUnit
already manages the directory.
---
src/changes/changes.xml | 3 +
src/main/java/org/apache/commons/io/FileUtils.java | 4 +-
.../java/org/apache/commons/io/FileUtilsTest.java | 96 +++++++++-------------
.../io/file/PathUtilsContentEqualsTest.java | 3 -
4 files changed, 47 insertions(+), 59 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 53ef159..5aaca08 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -135,6 +135,9 @@ The <action> type attribute can be add,update,fix,remove.
<action dev="ggregory" type="fix" due-to="Gary Gregory">
WriterOutputStream maps null Charset, Charset name, and CharsetEncoder
name to the platform default instead of throwing a NullPointerException.
</action>
+ <action dev="ggregory" type="fix" due-to="Chad Wilson, Gary Gregory">
+ FileUtils.copyURLToFile should create target parent directories and
overwrite target file #319.
+ </action>
<!-- ADD -->
<action issue="IO-726" dev="ggregory" type="fix" due-to="shollander,
Gary Gregory">
Add MemoryMappedFileInputStream #215.
diff --git a/src/main/java/org/apache/commons/io/FileUtils.java
b/src/main/java/org/apache/commons/io/FileUtils.java
index 0f18096..339a814 100644
--- a/src/main/java/org/apache/commons/io/FileUtils.java
+++ b/src/main/java/org/apache/commons/io/FileUtils.java
@@ -1060,7 +1060,9 @@ public class FileUtils {
*/
public static void copyURLToFile(final URL source, final File destination)
throws IOException {
try (final InputStream stream = source.openStream()) {
- Files.copy(stream, destination.toPath());
+ final Path path = destination.toPath();
+ PathUtils.createParentDirectories(path);
+ Files.copy(stream, path, StandardCopyOption.REPLACE_EXISTING);
}
}
diff --git a/src/test/java/org/apache/commons/io/FileUtilsTest.java
b/src/test/java/org/apache/commons/io/FileUtilsTest.java
index 1cf9026..1e75d3f 100644
--- a/src/test/java/org/apache/commons/io/FileUtilsTest.java
+++ b/src/test/java/org/apache/commons/io/FileUtilsTest.java
@@ -167,6 +167,15 @@ public class FileUtilsTest extends AbstractTempDirTest {
private long testFile2Size;
+ private void assertContentMatchesAfterCopyURLToFileFor(final String
resourceName, final File destination) throws IOException {
+ FileUtils.copyURLToFile(getClass().getResource(resourceName),
destination);
+
+ try (InputStream fis = Files.newInputStream(destination.toPath());
+ InputStream expected =
getClass().getResourceAsStream(resourceName)) {
+ assertTrue(IOUtils.contentEquals(expected, fis), "Content is not
equal.");
+ }
+ }
+
private void backDateFile10Minutes(final File testFile) throws IOException
{
final long mins10 = 1000 * 60 * 10;
final long lastModified1 = getLastModifiedMillis(testFile);
@@ -616,21 +625,18 @@ public class FileUtilsTest extends AbstractTempDirTest {
// Different files
final File objFile1 =
new File(tempDirFile, getName() + ".object");
- objFile1.deleteOnExit();
FileUtils.copyURLToFile(
getClass().getResource("/java/lang/Object.class"),
objFile1);
final File objFile1b =
new File(tempDirFile, getName() + ".object2");
- objFile1.deleteOnExit();
FileUtils.copyURLToFile(
getClass().getResource("/java/lang/Object.class"),
objFile1b);
final File objFile2 =
new File(tempDirFile, getName() + ".collection");
- objFile2.deleteOnExit();
FileUtils.copyURLToFile(
getClass().getResource("/java/util/Collection.class"),
objFile2);
@@ -650,8 +656,6 @@ public class FileUtilsTest extends AbstractTempDirTest {
assertTrue(FileUtils.contentEquals(file, file2));
}
- // toFiles
-
@Test
public void testContentEqualsIgnoreEOL() throws Exception {
// Non-existent files
@@ -672,15 +676,12 @@ public class FileUtilsTest extends AbstractTempDirTest {
// Different files
final File tfile1 = new File(tempDirFile, getName() + ".txt1");
- tfile1.deleteOnExit();
FileUtils.write(tfile1, "123\r");
final File tfile2 = new File(tempDirFile, getName() + ".txt2");
- tfile1.deleteOnExit();
FileUtils.write(tfile2, "123\n");
final File tfile3 = new File(tempDirFile, getName() + ".collection");
- tfile3.deleteOnExit();
FileUtils.write(tfile3, "123\r\n2");
assertTrue(FileUtils.contentEqualsIgnoreEOL(tfile1, tfile1, null));
@@ -839,7 +840,25 @@ public class FileUtilsTest extends AbstractTempDirTest {
assertTrue(expectedSize > 0, "Size > 0");
}
- // toURLs
+// @Test public void testToURLs2() throws Exception {
+// File[] files = new File[] {
+// new File(temporaryFolder, "file1.txt"),
+// null,
+// };
+// URL[] urls = FileUtils.toURLs(files);
+//
+// assertEquals(files.length, urls.length);
+// assertTrue(urls[0].toExternalForm().startsWith("file:"));
+// assertTrue(urls[0].toExternalForm().indexOf("file1.txt") > 0);
+// assertEquals(null, urls[1]);
+// }
+//
+// @Test public void testToURLs3() throws Exception {
+// File[] files = null;
+// URL[] urls = FileUtils.toURLs(files);
+//
+// assertEquals(0, urls.length);
+// }
@Test
public void testCopyDirectoryToDirectory_NonExistingDest() throws
Exception {
@@ -885,26 +904,6 @@ public class FileUtilsTest extends AbstractTempDirTest {
FileUtils.deleteDirectory(destDir);
}
-// @Test public void testToURLs2() throws Exception {
-// File[] files = new File[] {
-// new File(temporaryFolder, "file1.txt"),
-// null,
-// };
-// URL[] urls = FileUtils.toURLs(files);
-//
-// assertEquals(files.length, urls.length);
-// assertTrue(urls[0].toExternalForm().startsWith("file:"));
-// assertTrue(urls[0].toExternalForm().indexOf("file1.txt") > 0);
-// assertEquals(null, urls[1]);
-// }
-//
-// @Test public void testToURLs3() throws Exception {
-// File[] files = null;
-// URL[] urls = FileUtils.toURLs(files);
-//
-// assertEquals(0, urls.length);
-// }
-
@Test
public void testCopyDirectoryToExistingDest() throws Exception {
if (!testFile1.getParentFile().exists()) {
@@ -946,8 +945,6 @@ public class FileUtilsTest extends AbstractTempDirTest {
assertTrue(new File(destDir, "sub/A.txt").exists());
}
- // contentEquals
-
/* Test for IO-141 */
@Test
public void testCopyDirectoryToGrandChild() throws Exception {
@@ -973,8 +970,6 @@ public class FileUtilsTest extends AbstractTempDirTest {
assertEquals(1, LIST_WALKER.list(dir).size());
}
- // copyURLToFile
-
@Test
public void testCopyDirectoryToNonExistingDest() throws Exception {
if (!testFile1.getParentFile().exists()) {
@@ -1029,8 +1024,6 @@ public class FileUtilsTest extends AbstractTempDirTest {
assertEquals(getLastModifiedMillis(testFile1),
getLastModifiedMillis(destination), "Check last modified date preserved");
}
- // forceMkdir
-
@Test
public void testCopyFile1ToDir() throws Exception {
final File directory = new File(tempDirFile, "subdir");
@@ -1062,8 +1055,6 @@ public class FileUtilsTest extends AbstractTempDirTest {
assertEquals(getLastModifiedMillis(testFile1) ,
getLastModifiedMillis(destination), "Check last modified date preserved");
}
- // sizeOfDirectory
-
@Test
public void testCopyFile2ToDir() throws Exception {
final File directory = new File(tempDirFile, "subdir");
@@ -1189,8 +1180,6 @@ public class FileUtilsTest extends AbstractTempDirTest {
() -> FileUtils.copyToDirectory(new File(tempDirFile,
"doesNotExists"), tempDirFile));
}
- // copyFile
-
@Test
public void testCopyToDirectoryWithFileSourceIsNull() {
assertThrows(NullPointerException.class, () ->
FileUtils.copyToDirectory((File) null, tempDirFile));
@@ -1242,25 +1231,27 @@ public class FileUtilsTest extends AbstractTempDirTest {
public void testCopyURLToFile() throws Exception {
// Creates file
final File file = new File(tempDirFile, getName());
- file.deleteOnExit();
+ assertContentMatchesAfterCopyURLToFileFor("/java/lang/Object.class",
file);
+ //TODO Maybe test copy to itself like for copyFile()
+ }
- // Loads resource
- final String resourceName = "/java/lang/Object.class";
- FileUtils.copyURLToFile(getClass().getResource(resourceName), file);
+ @Test
+ public void testCopyURLToFileCreatesParentDirs() throws Exception {
+ final File file =
managedTempDirPath.resolve("subdir").resolve(getName()).toFile();
+ assertContentMatchesAfterCopyURLToFileFor("/java/lang/Object.class",
file);
+ }
- // Tests that resuorce was copied correctly
- try (InputStream fis = Files.newInputStream(file.toPath())) {
-
assertTrue(IOUtils.contentEquals(getClass().getResourceAsStream(resourceName),
fis),
- "Content is not equal.");
- }
- //TODO Maybe test copy to itself like for copyFile()
+ @Test
+ public void testCopyURLToFileReplacesExisting() throws Exception {
+ final File file = new File(tempDirFile, getName());
+ assertContentMatchesAfterCopyURLToFileFor("/java/lang/Object.class",
file);
+ assertContentMatchesAfterCopyURLToFileFor("/java/lang/String.class",
file);
}
@Test
public void testCopyURLToFileWithTimeout() throws Exception {
// Creates file
final File file = new File(tempDirFile,
"testCopyURLToFileWithTimeout");
- file.deleteOnExit();
// Loads resource
final String resourceName = "/java/lang/Object.class";
@@ -1495,7 +1486,6 @@ public class FileUtilsTest extends AbstractTempDirTest {
// Creates test file
final File testFile = new File(tempDirFile, getName());
- testFile.deleteOnExit();
testFile.createNewFile();
assertTrue(testFile.exists(), "Test file does not exist.");
@@ -2489,7 +2479,6 @@ public class FileUtilsTest extends AbstractTempDirTest {
// Creates file
file.createNewFile();
- file.deleteOnExit();
// New file
assertEquals(0, FileUtils.sizeOf(file));
@@ -2513,7 +2502,6 @@ public class FileUtilsTest extends AbstractTempDirTest {
// Creates file
file.createNewFile();
- file.deleteOnExit();
// New file
assertEquals(BigInteger.ZERO, FileUtils.sizeOfAsBigInteger(file));
@@ -2564,7 +2552,6 @@ public class FileUtilsTest extends AbstractTempDirTest {
// Creates file
file.createNewFile();
- file.deleteOnExit();
// Existing file
assertThrows(IllegalArgumentException.class, () ->
FileUtils.sizeOfDirectoryAsBigInteger(file));
@@ -2589,7 +2576,6 @@ public class FileUtilsTest extends AbstractTempDirTest {
} finally {
IOUtils.closeQuietly(output);
}
- nonEmptyFile.deleteOnExit();
assertEquals(TEST_DIRECTORY_SIZE_GT_ZERO_BI,
FileUtils.sizeOfDirectoryAsBigInteger(file), "Unexpected directory size");
diff --git
a/src/test/java/org/apache/commons/io/file/PathUtilsContentEqualsTest.java
b/src/test/java/org/apache/commons/io/file/PathUtilsContentEqualsTest.java
index 6fed6e8..71203b6 100644
--- a/src/test/java/org/apache/commons/io/file/PathUtilsContentEqualsTest.java
+++ b/src/test/java/org/apache/commons/io/file/PathUtilsContentEqualsTest.java
@@ -175,15 +175,12 @@ public class PathUtilsContentEqualsTest {
// Different files
final Path objFile1 = Paths.get(temporaryFolder.getAbsolutePath(),
getName() + ".object");
- objFile1.toFile().deleteOnExit();
PathUtils.copyFile(getClass().getResource("/java/lang/Object.class"),
objFile1);
final Path objFile1b = Paths.get(temporaryFolder.getAbsolutePath(),
getName() + ".object2");
- objFile1b.toFile().deleteOnExit();
PathUtils.copyFile(getClass().getResource("/java/lang/Object.class"),
objFile1b);
final Path objFile2 = Paths.get(temporaryFolder.getAbsolutePath(),
getName() + ".collection");
- objFile2.toFile().deleteOnExit();
PathUtils.copyFile(getClass().getResource("/java/util/Collection.class"),
objFile2);
assertFalse(PathUtils.fileContentEquals(objFile1, objFile2));
