From 7d58a5e2f22985396418983240578b0e202da8da Mon Sep 17 00:00:00 2001 From: Ilya Gorbunov Date: Thu, 8 Oct 2020 16:07:53 +0300 Subject: [PATCH] Simplify Path.copyTo implementation Delegate most checks to the platform Files.copy. This changes the exception type thrown in one case, so document when it happens. Also 'copyTo' no longer creates target parent directory if it doesn't exist. #KT-19192 --- .../stdlib/jdk7/src/kotlin/io/PathUtils.kt | 48 ++++++++----------- .../stdlib/jdk7/test/PathExtensionsTest.kt | 21 +++++--- 2 files changed, 34 insertions(+), 35 deletions(-) diff --git a/libraries/stdlib/jdk7/src/kotlin/io/PathUtils.kt b/libraries/stdlib/jdk7/src/kotlin/io/PathUtils.kt index a28d163a222..bfb666a8bdf 100644 --- a/libraries/stdlib/jdk7/src/kotlin/io/PathUtils.kt +++ b/libraries/stdlib/jdk7/src/kotlin/io/PathUtils.kt @@ -120,7 +120,12 @@ internal object PathRelativizer { /** * Copies this path to the given [target] path. * - * If some directories on a way to the [target] are missing, then they will be created. + * Unlike `File.copyTo`, if some directories on a way to the [target] are missing, then they won't be created automatically. + * You can use the following approach to ensure that required intermediate directories are created: + * ``` + * sourcePath.copyTo(destinationPath.apply { parent?.createDirectories() }) + * ``` + * * If the [target] path already exists, this function will fail unless [overwrite] argument is set to `true`. * * When [overwrite] is `true` and [target] is a directory, it is replaced only if it is empty. @@ -134,6 +139,8 @@ internal object PathRelativizer { * @return the [target] path. * @throws NoSuchFileException if the source path doesn't exist. * @throws FileAlreadyExistsException if the destination path already exists and [overwrite] argument is set to `false`. + * @throws DirectoryNotEmptyException if the destination path point to an existing directory and [overwrite] argument is `true`, + * when the directory being replaced is not empty. * @throws IOException if any errors occur while copying. */ @SinceKotlin("1.4") @@ -146,15 +153,20 @@ public fun Path.copyTo(target: Path, overwrite: Boolean = false): Path { /** * Copies this path to the given [target] path. * - * If some directories on a way to the [target] are missing, then they will be created. + * Unlike `File.copyTo`, if some directories on a way to the [target] are missing, then they won't be created automatically. + * You can use the following approach to ensure that required intermediate directories are created: + * ``` + * sourcePath.copyTo(destinationPath.apply { parent?.createDirectories() }) + * ``` + * * If the [target] path already exists, this function will fail unless the * [REPLACE_EXISTING][StandardCopyOption.REPLACE_EXISTING] is option is used. * * When [REPLACE_EXISTING][StandardCopyOption.REPLACE_EXISTING] is used and [target] is a directory, * it is replaced only if it is empty. * - * If this path is a directory, it is copied without its content, i.e. an empty [target] directory is created. - * If you want to copy directory including its contents, use [copyRecursively]. + * If this path is a directory, it is copied *without* its content, i.e. an empty [target] directory is created. + * If you want to copy a directory including its contents, use [copyRecursively]. * * The operation doesn't preserve copied file attributes such as creation/modification date, * permissions, etc. unless [COPY_ATTRIBUTES][StandardCopyOption.COPY_ATTRIBUTES] is used. @@ -163,36 +175,14 @@ public fun Path.copyTo(target: Path, overwrite: Boolean = false): Path { * @return the [target] path. * @throws NoSuchFileException if the source path doesn't exist. * @throws FileAlreadyExistsException if the destination path already exists and [REPLACE_EXISTING][StandardCopyOption.REPLACE_EXISTING] is not used. + * @throws DirectoryNotEmptyException if the destination path point to an existing directory and [REPLACE_EXISTING][StandardCopyOption.REPLACE_EXISTING] is used, + * when the directory being replaced is not empty. * @throws IOException if any errors occur while copying. */ @SinceKotlin("1.4") @ExperimentalStdlibApi public fun Path.copyTo(target: Path, vararg options: CopyOption): Path { - if (this.notExists()) { - throw NoSuchFileException(toString(), null, "The source path doesn't exist.") - } - - if (target.exists() && StandardCopyOption.REPLACE_EXISTING !in options) { - throw FileAlreadyExistsException(toString(), null, "The destination path already exists.") - } - - if (this.isDirectory()) { - if (target.isDirectory() && Files.newDirectoryStream(target).use { it.firstOrNull() } != null) { - throw FileAlreadyExistsException(toString(), null, "The destination path already exists.") - } - try { - Files.createDirectories(target) - } catch (_: FileAlreadyExistsException) { - // File already exists and is not a directory - Files.delete(target) - Files.createDirectories(target) - } - } else { - target.parent?.let { Files.createDirectories(it) } - Files.copy(this, target, *options) - } - - return target + return Files.copy(this, target, *options) } /** diff --git a/libraries/stdlib/jdk7/test/PathExtensionsTest.kt b/libraries/stdlib/jdk7/test/PathExtensionsTest.kt index 82a1ec089b4..6540f435ff7 100644 --- a/libraries/stdlib/jdk7/test/PathExtensionsTest.kt +++ b/libraries/stdlib/jdk7/test/PathExtensionsTest.kt @@ -40,7 +40,7 @@ class PathExtensionsTest { val dstFile = Files.createTempFile(null, null) try { srcFile.writeText("Hello, World!") - assertFailsWith(FileAlreadyExistsException::class, "copy do not overwrite existing file") { + assertFailsWith("copy do not overwrite existing file") { srcFile.copyTo(dstFile) } @@ -48,15 +48,24 @@ class PathExtensionsTest { assertSame(dst, dstFile) compareFiles(srcFile, dst, "copy with overwrite over existing file") + srcFile.copyTo(srcFile) + srcFile.copyTo(srcFile, overwrite = true) + compareFiles(dst, srcFile, "copying file to itself leaves it intact") + assertTrue(Files.deleteIfExists(dstFile)) dst = srcFile.copyTo(dstFile) compareFiles(srcFile, dst, "copy to new file") + val subDst = dstFile.resolve("foo/bar") + assertFailsWith { srcFile.copyTo(subDst) } + assertFailsWith { srcFile.copyTo(subDst, overwrite = true) } assertTrue(Files.deleteIfExists(dstFile)) + assertFailsWith { srcFile.copyTo(subDst) } + Files.createDirectory(dstFile) val child = dstFile.resolve("child") Files.createFile(child) - assertFailsWith(DirectoryNotEmptyException::class, "copy with overwrite do not overwrite non-empty dir") { + assertFailsWith( "copy with overwrite do not overwrite non-empty dir") { srcFile.copyTo(dstFile, overwrite = true) } Files.delete(child) @@ -67,21 +76,21 @@ class PathExtensionsTest { assertTrue(Files.deleteIfExists(srcFile)) assertTrue(Files.deleteIfExists(dstFile)) - assertFailsWith(NoSuchFileException::class) { + assertFailsWith { srcFile.copyTo(dstFile) } Files.createDirectory(srcFile) srcFile.resolve("somefile").writeText("some content") dstFile.writeText("") - assertFailsWith(FileAlreadyExistsException::class, "copy dir do not overwrite file") { + assertFailsWith("copy dir do not overwrite file") { srcFile.copyTo(dstFile) } srcFile.copyTo(dstFile, overwrite = true) assertTrue(dstFile.isDirectory()) assertTrue(dstFile.listDirectoryEntries().isEmpty(), "only directory is copied, but not its content") - assertFailsWith(FileAlreadyExistsException::class, "copy dir do not overwrite dir") { + assertFailsWith("copy dir do not overwrite dir") { srcFile.copyTo(dstFile) } @@ -90,7 +99,7 @@ class PathExtensionsTest { assertTrue(dstFile.listDirectoryEntries().isEmpty(), "only directory is copied, but not its content") dstFile.resolve("somefile2").writeText("some content2") - assertFailsWith(FileAlreadyExistsException::class, "copy dir do not overwrite non-empty dir") { + assertFailsWith("copy dir do not overwrite non-empty dir") { srcFile.copyTo(dstFile, overwrite = true) } } finally {