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
commit 7a7b1418528a05e4633955205d4fffca451b8c4a Author: Gary Gregory <[email protected]> AuthorDate: Wed Feb 10 09:34:23 2021 -0500 Apply, refactor, clean up https://github.com/apache/commons-io/pull/203 by Andrew Shcheglov (ashcheglov on GitHub). --- src/changes/changes.xml | 3 + .../commons/io/file/CopyDirectoryVisitor.java | 20 +++- .../org/apache/commons/io/file/PathUtilsTest.java | 113 +++++++++++++++++++++ src/test/resources/org/apache/commons/io/test.jar | Bin 0 -> 1101 bytes 4 files changed, 133 insertions(+), 3 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 778d339..dc7699c 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -111,6 +111,9 @@ The <action> type attribute can be add,update,fix,remove. <action issue="IO-705" dev="ggregory" type="fix" due-to="Hao Zhong, Gary Gregory"> Fix infinite loops in ObservableInputStream read(*) when an exception is caught but not re-thrown. </action> + <action issue="IO-719" dev="ggregory" type="fix" due-to="Andrew Shcheglov, Gary Gregory"> + Fixed error of copying directories between different file systems #203. + </action> <!-- ADD --> <action dev="ggregory" type="add" due-to="Gary Gregory"> Add FileSystemProviders class. diff --git a/src/main/java/org/apache/commons/io/file/CopyDirectoryVisitor.java b/src/main/java/org/apache/commons/io/file/CopyDirectoryVisitor.java index c2fdf8b..920f51b 100644 --- a/src/main/java/org/apache/commons/io/file/CopyDirectoryVisitor.java +++ b/src/main/java/org/apache/commons/io/file/CopyDirectoryVisitor.java @@ -22,6 +22,7 @@ import java.nio.file.CopyOption; import java.nio.file.FileVisitResult; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.ProviderMismatchException; import java.nio.file.attribute.BasicFileAttributes; import java.util.Arrays; import java.util.Objects; @@ -99,7 +100,7 @@ public class CopyDirectoryVisitor extends CountingPathVisitor { } final CopyDirectoryVisitor other = (CopyDirectoryVisitor) obj; return Arrays.equals(copyOptions, other.copyOptions) && Objects.equals(sourceDirectory, other.sourceDirectory) - && Objects.equals(targetDirectory, other.targetDirectory); + && Objects.equals(targetDirectory, other.targetDirectory); } /** @@ -144,16 +145,29 @@ public class CopyDirectoryVisitor extends CountingPathVisitor { @Override public FileVisitResult preVisitDirectory(final Path directory, final BasicFileAttributes attributes) throws IOException { - final Path newTargetDir = targetDirectory.resolve(sourceDirectory.relativize(directory)); + final Path newTargetDir = resolveRelativeAsString(directory); if (Files.notExists(newTargetDir)) { Files.createDirectory(newTargetDir); } return super.preVisitDirectory(directory, attributes); } + /** + * Relativizes against {@code sourceDirectory}, then resolves against {@code targetDirectory}. + * + * We have to call {@link Path#toString()} relative value because we cannot use paths belonging to different + * FileSystems in the Path methods, usually this leads to {@link ProviderMismatchException}. + * + * @param directory the directory to relativize. + * @return a new path, relativized against sourceDirectory, then resolved against targetDirectory. + */ + private Path resolveRelativeAsString(final Path directory) { + return targetDirectory.resolve(sourceDirectory.relativize(directory).toString()); + } + @Override public FileVisitResult visitFile(final Path sourceFile, final BasicFileAttributes attributes) throws IOException { - final Path targetFile = targetDirectory.resolve(sourceDirectory.relativize(sourceFile)); + final Path targetFile = resolveRelativeAsString(sourceFile); copy(sourceFile, targetFile); return super.visitFile(targetFile, attributes); } diff --git a/src/test/java/org/apache/commons/io/file/PathUtilsTest.java b/src/test/java/org/apache/commons/io/file/PathUtilsTest.java index cfc5765..70c8346 100644 --- a/src/test/java/org/apache/commons/io/file/PathUtilsTest.java +++ b/src/test/java/org/apache/commons/io/file/PathUtilsTest.java @@ -22,11 +22,16 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.IOException; +import java.net.URI; import java.nio.file.DirectoryStream; +import java.nio.file.FileSystem; +import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.HashMap; import java.util.Iterator; +import java.util.Map; import org.apache.commons.io.filefilter.NameFileFilter; import org.junit.jupiter.api.Test; @@ -37,6 +42,10 @@ import org.junit.jupiter.api.io.TempDir; */ public class PathUtilsTest extends TestArguments { + private static final String TEST_JAR_NAME = "test.jar"; + + private static final String TEST_JAR_PATH = "src/test/resources/org/apache/commons/io/test.jar"; + private static final String PATH_FIXTURE = "NOTICE.txt"; /** @@ -45,6 +54,110 @@ public class PathUtilsTest extends TestArguments { @TempDir public Path tempDir; + private FileSystem openArchive(final Path p, final boolean createNew) throws IOException { + final FileSystem archive; + if (createNew) { + final Map<String, String> env = new HashMap<>(); + env.put("create", "true"); + final URI fileUri = p.toAbsolutePath().toUri(); + final URI uri = URI.create("jar:" + fileUri.toASCIIString()); + archive = FileSystems.newFileSystem(uri, env, null); + } else { + archive = FileSystems.newFileSystem(p, (ClassLoader) null); + } + return archive; + } + + @Test + public void testCopyDirectoryForDifferentFilesystemsWithAbsolutePath() throws IOException { + final Path tempDir = Files.createTempDirectory(getClass().getCanonicalName()).toAbsolutePath(); + try { + final Path archivePath = Paths.get(TEST_JAR_PATH); + try (final FileSystem archive = openArchive(archivePath, false)) { + // relative jar -> absolute dir + Path sourceDir = archive.getPath("dir1"); + PathUtils.copyDirectory(sourceDir, tempDir); + assertTrue(Files.exists(tempDir.resolve("f1"))); + + // absolute jar -> absolute dir + sourceDir = archive.getPath("/next"); + PathUtils.copyDirectory(sourceDir, tempDir); + assertTrue(Files.exists(tempDir.resolve("dir"))); + } + } finally { + PathUtils.deleteDirectory(tempDir); + } + } + + @Test + public void testCopyDirectoryForDifferentFilesystemsWithAbsolutePathReverse() throws IOException { + final Path tempDir = Files.createTempDirectory(getClass().getCanonicalName()); + try { + try (final FileSystem archive = openArchive(tempDir.resolve(TEST_JAR_NAME), true)) { + // absolute dir -> relative jar + Path targetDir = archive.getPath("target"); + Files.createDirectory(targetDir); + final Path sourceDir = Paths.get("src/test/resources/org/apache/commons/io/dirs-2-file-size-2") + .toAbsolutePath(); + PathUtils.copyDirectory(sourceDir, targetDir); + assertTrue(Files.exists(targetDir.resolve("dirs-a-file-size-1"))); + + // absolute dir -> absolute jar + targetDir = archive.getPath("/"); + PathUtils.copyDirectory(sourceDir, targetDir); + assertTrue(Files.exists(targetDir.resolve("dirs-a-file-size-1"))); + } + } finally { + PathUtils.deleteDirectory(tempDir); + } + } + + @Test + public void testCopyDirectoryForDifferentFilesystemsWithRelativePath() throws IOException { + final Path tempDir = Files.createTempDirectory(getClass().getCanonicalName()); + try { + final Path archivePath = Paths.get(TEST_JAR_PATH); + try (final FileSystem archive = openArchive(archivePath, false); + final FileSystem targetArchive = openArchive(tempDir.resolve(TEST_JAR_NAME), true)) { + final Path targetDir = targetArchive.getPath("targetDir"); + Files.createDirectory(targetDir); + // relative jar -> relative dir + Path sourceDir = archive.getPath("next"); + PathUtils.copyDirectory(sourceDir, targetDir); + assertTrue(Files.exists(targetDir.resolve("dir"))); + + // absolute jar -> relative dir + sourceDir = archive.getPath("/dir1"); + PathUtils.copyDirectory(sourceDir, targetDir); + assertTrue(Files.exists(targetDir.resolve("f1"))); + } + } finally { + PathUtils.deleteDirectory(tempDir); + } + } + + @Test + public void testCopyDirectoryForDifferentFilesystemsWithRelativePathReverse() throws IOException { + final Path tempDir = Files.createTempDirectory(getClass().getCanonicalName()); + try { + try (final FileSystem archive = openArchive(tempDir.resolve(TEST_JAR_NAME), true)) { + // relative dir -> relative jar + Path targetDir = archive.getPath("target"); + Files.createDirectory(targetDir); + final Path sourceDir = Paths.get("src/test/resources/org/apache/commons/io/dirs-2-file-size-2"); + PathUtils.copyDirectory(sourceDir, targetDir); + assertTrue(Files.exists(targetDir.resolve("dirs-a-file-size-1"))); + + // relative dir -> absolute jar + targetDir = archive.getPath("/"); + PathUtils.copyDirectory(sourceDir, targetDir); + assertTrue(Files.exists(targetDir.resolve("dirs-a-file-size-1"))); + } + } finally { + PathUtils.deleteDirectory(tempDir); + } + } + @Test public void testCopyFile() throws IOException { final Path tempDir = Files.createTempDirectory(getClass().getCanonicalName()); diff --git a/src/test/resources/org/apache/commons/io/test.jar b/src/test/resources/org/apache/commons/io/test.jar new file mode 100644 index 0000000..05b6d3d Binary files /dev/null and b/src/test/resources/org/apache/commons/io/test.jar differ
