This is an automated email from the ASF dual-hosted git repository.
elharo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven-shared-utils.git
The following commit(s) were added to refs/heads/master by this push:
new ab2cc93 Test, fix, and deprecate buggy copyDirectory methods (#80)
ab2cc93 is described below
commit ab2cc9348b3f60c7fdcc8167432ab9d519346fba
Author: Elliotte Rusty Harold <elh...@users.noreply.github.com>
AuthorDate: Fri Feb 26 14:01:19 2021 +0000
Test, fix, and deprecate buggy copyDirectory methods (#80)
Unignore copyDirectory tests and fix broken behavior that caused them to be
ignored. Deprecate these methods in favor of better maintained and documented
Apache Commons IO methods from which they were copied and pasted 10+ years ago.
---
.../apache/maven/shared/utils/io/FileUtils.java | 58 +++++--
.../maven/shared/utils/io/FileUtilsTest.java | 189 ++++-----------------
2 files changed, 73 insertions(+), 174 deletions(-)
diff --git a/src/main/java/org/apache/maven/shared/utils/io/FileUtils.java
b/src/main/java/org/apache/maven/shared/utils/io/FileUtils.java
index f194782..4612b37 100644
--- a/src/main/java/org/apache/maven/shared/utils/io/FileUtils.java
+++ b/src/main/java/org/apache/maven/shared/utils/io/FileUtils.java
@@ -19,6 +19,7 @@ package org.apache.maven.shared.utils.io;
* under the License.
*/
+import org.apache.commons.io.IOUtils;
import org.apache.maven.shared.utils.Os;
import org.apache.maven.shared.utils.StringUtils;
@@ -52,13 +53,14 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
+import java.util.Objects;
import java.util.Random;
/**
* This class provides basic facilities for manipulating files and file paths.
- * <p/>
+ *
* <h3>Path-related methods</h3>
- * <p/>
+ *
* <p>Methods exist to retrieve the components of a typical file path. For
example
* <code>/www/hosted/mysite/index.html</code>, can be broken into:
* <ul>
@@ -68,13 +70,12 @@ import java.util.Random;
* </p>
* <p/>
* <h3>File-related methods</h3>
- * <p/>
+ *
* There are methods to create a {@link #toFile File from a URL}, copy a
* {@link #copyFile File to another File},
* copy a {@link #copyURLToFile URL's contents to a File},
* as well as methods to {@link #deleteDirectory(File) delete} and {@link
#cleanDirectory(File)
* clean} a directory.
- * </p>
* <p/>
* Common {@link java.io.File} manipulation routines.
* <p/>
@@ -89,7 +90,6 @@ import java.util.Random;
* @author <a href="mailto:christoph.r...@dlr.de";>Christoph.Reck</a>
* @author <a href="mailto:pe...@apache.org";>Peter Donald</a>
* @author <a href="mailto:je...@apache.org";>Jeff Turner</a>
- *
*/
public class FileUtils
{
@@ -633,7 +633,7 @@ public class FileUtils
try ( InputStream input1 = new FileInputStream( file1 );
InputStream input2 = new FileInputStream( file2 ) )
{
- return IOUtil.contentEquals( input1, input2 );
+ return IOUtils.contentEquals( input1, input2 );
}
}
@@ -750,7 +750,7 @@ public class FileUtils
{
if ( destinationDirectory.exists() &&
!destinationDirectory.isDirectory() )
{
- throw new IllegalArgumentException( "Destination is not a
directory" );
+ throw new IOException( "Destination is not a directory" );
}
copyFile( source, new File( destinationDirectory, source.getName() ) );
@@ -1022,9 +1022,9 @@ public class FileUtils
}
/**
- * Resolve a file <code>filename</code> to it's canonical form. If
<code>filename</code> is
- * relative (doesn't start with <code>/</code>), it will be resolved
relative to
- * <code>baseFile</code>, otherwise it is treated as a normal
root-relative path.
+ * Resolve a file <code>filename</code> to its canonical form. If
<code>filename</code> is
+ * relative (doesn't start with <code>/</code>), it is resolved relative to
+ * <code>baseFile</code>. Otherwise it is treated as a normal
root-relative path.
*
* @param baseFile where to resolve <code>filename</code> from, if
<code>filename</code> is relative
* @param filename absolute or relative file path to resolve
@@ -1605,14 +1605,30 @@ public class FileUtils
/**
* Copy the contents of a directory into another one.
*
- * @param sourceDirectory the source directory
- * @param destinationDirectory the target directory
+ * @param sourceDirectory the source directory. If the source does not
exist,
+ * the method simply returns.
+ * @param destinationDirectory the target directory; will be created if it
doesn't exist
* @throws IOException if any
+ * @deprecated use {@code org.apache.commons.io.FileUtils.copyDirectory()}
*/
+ @Deprecated
public static void copyDirectory( @Nonnull File sourceDirectory, @Nonnull
File destinationDirectory )
throws IOException
{
- copyDirectory( sourceDirectory, destinationDirectory, "**", null );
+ Objects.requireNonNull( sourceDirectory );
+ Objects.requireNonNull( destinationDirectory );
+ if ( destinationDirectory.equals( sourceDirectory ) )
+ {
+ throw new IOException( "Can't copy directory " + sourceDirectory +
" to itself." );
+ }
+ else if ( !destinationDirectory.exists() )
+ {
+ if ( !destinationDirectory.mkdirs() )
+ {
+ throw new IOException( "Can't create directory " +
destinationDirectory );
+ }
+ }
+ copyDirectoryStructure( sourceDirectory, destinationDirectory );
}
/**
@@ -1622,9 +1638,11 @@ public class FileUtils
* @param destinationDirectory the target directory
* @param includes Ant include pattern
* @param excludes Ant exclude pattern
- * @throws IOException if any
+ * @throws IOException if the source is a file or cannot be copied
* @see #getFiles(File, String, String)
+ * @deprecated use {@code org.apache.commons.io.FileUtils.copyDirectory()}
*/
+ @Deprecated
public static void copyDirectory( @Nonnull File sourceDirectory, @Nonnull
File destinationDirectory,
@Nullable String includes, @Nullable
String excludes )
throws IOException
@@ -1633,6 +1651,10 @@ public class FileUtils
{
return;
}
+ else if ( !sourceDirectory.isDirectory() )
+ {
+ throw new IOException( sourceDirectory + " is not a directory." );
+ }
List<File> files = getFiles( sourceDirectory, includes, excludes );
@@ -1651,8 +1673,8 @@ public class FileUtils
* <li>The <code>sourceDirectory</code> must exist.
* </ul>
*
- * @param sourceDirectory the source dir
- * @param destinationDirectory the target dir
+ * @param sourceDirectory the existing directory to be copied
+ * @param destinationDirectory the new directory to be created
* @throws IOException if any
* @deprecated use {@code org.apache.commons.io.FileUtils.copyDirectory()}
*/
@@ -1686,7 +1708,7 @@ public class FileUtils
if ( !sourceDirectory.exists() )
{
- throw new IOException( "Source directory doesn't exists (" +
sourceDirectory.getAbsolutePath() + ")." );
+ throw new IOException( "Source directory doesn't exist (" +
sourceDirectory.getAbsolutePath() + ")." );
}
File[] files = sourceDirectory.listFiles();
@@ -2123,7 +2145,7 @@ public class FileUtils
* @return the linked file
* @throws IOException in case of an error
* @see {@code java.nio.file.Files.createSymbolicLink(Path)} which creates
a new
- * symbolic link but does not replace exsiting symbolic links
+ * symbolic link but does not replace existing symbolic links
*/
@Nonnull public static File createSymbolicLink( @Nonnull File symlink,
@Nonnull File target )
throws IOException
diff --git a/src/test/java/org/apache/maven/shared/utils/io/FileUtilsTest.java
b/src/test/java/org/apache/maven/shared/utils/io/FileUtilsTest.java
index c692a38..4dfa4fa 100644
--- a/src/test/java/org/apache/maven/shared/utils/io/FileUtilsTest.java
+++ b/src/test/java/org/apache/maven/shared/utils/io/FileUtilsTest.java
@@ -122,7 +122,7 @@ public class FileUtilsTest
createFile( testFile2, testFile2Size );
}
- void createFile( File file, long size )
+ private static void createFile( File file, long size )
throws IOException
{
if ( !file.getParentFile().exists() )
@@ -140,7 +140,7 @@ public class FileUtilsTest
/**
* Assert that the content of a file is equal to that in a byte[].
*/
- void assertEqualContent( byte[] b0, File file )
+ private void assertEqualContent( byte[] b0, File file )
throws IOException
{
int count = 0, numRead = 0;
@@ -155,16 +155,16 @@ public class FileUtilsTest
assertThat( "Different number of bytes: ", count, is( b0.length )
);
for ( int i = 0; i < count; i++ )
{
- assertThat( "byte " + i + " differs", b1[i], is( b0[i] ) );
+ assertEquals( "byte " + i + " differs", b1[i], b0[i] );
}
}
}
- void deleteFile( File file )
+ private void deleteFile( File file )
{
if ( file.exists() )
{
- assertThat( "Couldn't delete file: " + file, file.delete(), is(
true ) );
+ assertTrue( "Couldn't delete file: " + file, file.delete() );
}
}
@@ -784,9 +784,7 @@ public class FileUtilsTest
}
@Test
- @Ignore( "Commons test case that is failing for plexus" )
- public void copyToSelf()
- throws Exception
+ public void copyToSelf() throws IOException
{
File destination = new File( tempFolder.getRoot(), "copy3.txt" );
//Prepare a test file
@@ -796,33 +794,6 @@ public class FileUtilsTest
}
@Test
- @Ignore( "Commons test case that is failing for plexus" )
- public void copyDirectoryToDirectory_NonExistingDest()
- throws Exception
- {
- createFile( testFile1, 1234 );
- createFile( testFile2, 4321 );
- File srcDir = tempFolder.getRoot();
- File subDir = new File( srcDir, "sub" );
- subDir.mkdir();
- File subFile = new File( subDir, "A.txt" );
- FileUtils.fileWrite( subFile, "UTF8", "HELLO WORLD" );
- File destDir = new File( System.getProperty( "java.io.tmpdir" ),
"tmp-FileUtilsTestCase" );
- FileUtils.deleteDirectory( destDir );
- File actualDestDir = new File( destDir, srcDir.getName() );
-
- FileUtils.copyDirectory( srcDir, destDir );
-
- assertThat( "Check exists", destDir.exists(), is( true ) );
- assertThat( "Check exists", actualDestDir.exists(), is( true ) );
- assertThat( "Check size", FileUtils.sizeOfDirectory( actualDestDir ),
- is( FileUtils.sizeOfDirectory( srcDir ) ) );
- assertThat( new File( actualDestDir, "sub/A.txt" ).exists(), is( true
) );
- FileUtils.deleteDirectory( destDir );
- }
-
- @Test
- @Ignore( "Commons test case that is failing for plexus" )
public void copyDirectoryToNonExistingDest()
throws Exception
{
@@ -838,130 +809,34 @@ public class FileUtilsTest
FileUtils.copyDirectory( srcDir, destDir );
- assertThat( "Check exists", destDir.exists(), is( true ) );
- assertThat( "Check size", FileUtils.sizeOfDirectory( destDir ), is(
FileUtils.sizeOfDirectory( srcDir ) ) );
- assertThat( new File( destDir, "sub/A.txt" ).exists(), is( true ) );
+ assertTrue( destDir.exists() );
+ assertEquals( FileUtils.sizeOfDirectory( destDir ),
FileUtils.sizeOfDirectory( srcDir ) );
+ assertTrue( new File( destDir, "sub/A.txt" ).exists() );
FileUtils.deleteDirectory( destDir );
}
@Test
- @Ignore( "Commons test case that is failing for plexus" )
- public void copyDirectoryToExistingDest()
- throws Exception
+ public void copyDirectoryToExistingDest() throws IOException
{
createFile( testFile1, 1234 );
createFile( testFile2, 4321 );
File srcDir = tempFolder.getRoot();
File subDir = new File( srcDir, "sub" );
- subDir.mkdir();
+ assertTrue( subDir.mkdir() );
File subFile = new File( subDir, "A.txt" );
FileUtils.fileWrite( subFile, "UTF8", "HELLO WORLD" );
File destDir = new File( System.getProperty( "java.io.tmpdir" ),
"tmp-FileUtilsTestCase" );
FileUtils.deleteDirectory( destDir );
- destDir.mkdirs();
+ assertTrue ( destDir.mkdirs() );
FileUtils.copyDirectory( srcDir, destDir );
- assertThat( FileUtils.sizeOfDirectory( destDir ), is(
FileUtils.sizeOfDirectory( srcDir ) ) );
- assertThat( new File( destDir, "sub/A.txt" ).exists(), is( true ) );
- }
-
- /**
- * Test for IO-141
- */
- @Test
- @Ignore( "Commons test case that is failing for plexus" )
- public void copyDirectoryToChild()
- throws Exception
- {
- File grandParentDir = new File( tempFolder.getRoot(), "grandparent" );
- File parentDir = new File( grandParentDir, "parent" );
- File childDir = new File( parentDir, "child" );
- createFilesForTestCopyDirectory( grandParentDir, parentDir, childDir );
-
- long expectedCount =
- FileUtils.getFileAndDirectoryNames( grandParentDir, null, null,
true, true, true, true ).size()
- + FileUtils.getFileAndDirectoryNames( parentDir, null, null,
true, true, true, true ).size();
- long expectedSize = FileUtils.sizeOfDirectory( grandParentDir ) +
FileUtils.sizeOfDirectory( parentDir );
- FileUtils.copyDirectory( parentDir, childDir );
- assertThat(
- 1L * FileUtils.getFileAndDirectoryNames( grandParentDir, null,
null, true, true, true, true ).size(),
- is( expectedCount ) );
- assertThat( FileUtils.sizeOfDirectory( grandParentDir ), is(
expectedSize ) );
- }
-
- /**
- * Test for IO-141
- */
- @Test
- @Ignore( "Commons test case that is failing for plexus" )
- public void copyDirectoryToGrandChild()
- throws Exception
- {
- File grandParentDir = new File( tempFolder.getRoot(), "grandparent" );
- File parentDir = new File( grandParentDir, "parent" );
- File childDir = new File( parentDir, "child" );
- createFilesForTestCopyDirectory( grandParentDir, parentDir, childDir );
-
- long expectedCount =
- ( FileUtils.getFileAndDirectoryNames( grandParentDir, null, null,
true, true, true, true ).size() * 2 );
- long expectedSize = ( FileUtils.sizeOfDirectory( grandParentDir ) * 2
);
- FileUtils.copyDirectory( grandParentDir, childDir );
- assertThat(
- 1L * FileUtils.getFileAndDirectoryNames( grandParentDir, null,
null, true, true, true, true ).size(),
- is( expectedCount ) );
- assertThat( FileUtils.sizeOfDirectory( grandParentDir ), is(
expectedSize ) );
- }
-
- /**
- * Test for IO-217 FileUtils.copyDirectoryToDirectory makes infinite loops
- */
- @Test
- public void copyDirectoryToItself()
- throws Exception
- {
- File dir = new File( tempFolder.getRoot(), "itself" );
- dir.mkdirs();
- FileUtils.copyDirectory( dir, dir );
- assertThat( FileUtils.getFileAndDirectoryNames( dir, null, null, true,
true, true, true ).size(), is( 1 ) );
- }
-
- private void createFilesForTestCopyDirectory( File grandParentDir, File
parentDir, File childDir )
- throws Exception
- {
- File childDir2 = new File( parentDir, "child2" );
- File grandChildDir = new File( childDir, "grandChild" );
- File grandChild2Dir = new File( childDir2, "grandChild2" );
- File file1 = new File( grandParentDir, "file1.txt" );
- File file2 = new File( parentDir, "file2.txt" );
- File file3 = new File( childDir, "file3.txt" );
- File file4 = new File( childDir2, "file4.txt" );
- File file5 = new File( grandChildDir, "file5.txt" );
- File file6 = new File( grandChild2Dir, "file6.txt" );
- FileUtils.deleteDirectory( grandParentDir );
- grandChildDir.mkdirs();
- grandChild2Dir.mkdirs();
- FileUtils.fileWrite( file1, "UTF8", "File 1 in grandparent" );
- FileUtils.fileWrite( file2, "UTF8", "File 2 in parent" );
- FileUtils.fileWrite( file3, "UTF8", "File 3 in child" );
- FileUtils.fileWrite( file4, "UTF8", "File 4 in child2" );
- FileUtils.fileWrite( file5, "UTF8", "File 5 in grandChild" );
- FileUtils.fileWrite( file6, "UTF8", "File 6 in grandChild2" );
+ assertEquals( FileUtils.sizeOfDirectory( destDir ),
FileUtils.sizeOfDirectory( srcDir ) );
+ assertTrue( new File( destDir, "sub/A.txt" ).exists() );
}
@Test
- @Ignore( "Commons test case that is failing for plexus" )
- public void copyDirectoryErrors()
- throws Exception
- {
- try
- {
- FileUtils.copyDirectory( null, null );
- fail();
- }
- catch ( NullPointerException ex )
- {
- }
+ public void copyDirectoryErrors_nullDestination() throws IOException {
try
{
FileUtils.copyDirectory( new File( "a" ), null );
@@ -970,41 +845,43 @@ public class FileUtilsTest
catch ( NullPointerException ex )
{
}
+ }
+
+ @Test
+ public void copyDirectoryErrors_copyToSelf() {
try
{
- FileUtils.copyDirectory( null, new File( "a" ) );
- fail();
- }
- catch ( NullPointerException ex )
- {
- }
- try
- {
- FileUtils.copyDirectory( new File( "doesnt-exist" ), new File( "a"
) );
+ FileUtils.copyDirectory( tempFolder.getRoot(),
tempFolder.getRoot() );
fail();
}
catch ( IOException ex )
{
}
+ }
+
+ @Test
+ public void copyDirectoryErrors()
+ throws IOException
+ {
try
{
- FileUtils.copyDirectory( testFile1, new File( "a" ) );
+ FileUtils.copyDirectory( null, null );
fail();
}
- catch ( IOException ex )
+ catch ( NullPointerException ex )
{
}
try
{
- FileUtils.copyDirectory( tempFolder.getRoot(), testFile1 );
+ FileUtils.copyDirectory( null, new File( "a" ) );
fail();
}
- catch ( IOException ex )
+ catch ( NullPointerException ex )
{
}
try
{
- FileUtils.copyDirectory( tempFolder.getRoot(),
tempFolder.getRoot() );
+ FileUtils.copyDirectory( tempFolder.getRoot(), testFile1 );
fail();
}
catch ( IOException ex )
@@ -1020,9 +897,9 @@ public class FileUtilsTest
{
File destination = new File( tempFolder.getRoot(), "copy1.txt" );
destination.createNewFile();
- assertThat( "Copy1.txt doesn't exist to delete", destination.exists(),
is( true ) );
+ assertTrue( "Copy1.txt doesn't exist to delete", destination.exists()
);
FileUtils.forceDelete( destination );
- assertThat( "Check No Exist", !destination.exists(), is( true ) );
+ assertFalse( destination.exists() );
}
@Test
