From 45c9cc4add1b6b3bd899280445c7d07cd545447c Mon Sep 17 00:00:00 2001 From: Ilya Gorbunov Date: Fri, 22 Jan 2016 07:21:36 +0300 Subject: [PATCH] Breaking: Change File.copyTo contract to be similar to Files.copy from JDK8. --- libraries/stdlib/src/kotlin/io/files/Utils.kt | 52 ++++++----- libraries/stdlib/test/io/Files.kt | 93 ++++++++++++------- 2 files changed, 87 insertions(+), 58 deletions(-) diff --git a/libraries/stdlib/src/kotlin/io/files/Utils.kt b/libraries/stdlib/src/kotlin/io/files/Utils.kt index dcc02c83e24..43ab4b65292 100644 --- a/libraries/stdlib/src/kotlin/io/files/Utils.kt +++ b/libraries/stdlib/src/kotlin/io/files/Utils.kt @@ -159,44 +159,48 @@ private fun File.toRelativeStringOrNull(base: File): String? { /** - * Copies this file to the given output [dst], returning the number of bytes copied. + * Copies this file to the given output [target], returning the number of bytes copied. * - * If some directories on a way to the [dst] are missing, then they will be created. - * If the [dst] file already exists, then this function will fail unless [overwrite] argument is set to `true`. - * Otherwise this file overwrites [dst] if it's a file to, or is written into [dst] if it's a directory. + * If some directories on a way to the [target] are missing, then they will be created. + * If the [target] file already exists, this function will fail unless [overwrite] argument is set to `true`. * - * Note: this function fails if you call it on a directory. - * If you want to copy directories, use 'copyRecursively' function instead. + * When [overwrite] is `true` and [target] is a directory, it is replaced only if it is empty. + * + * If this file 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]. * * @param overwrite `true` if destination overwrite is allowed. * @param bufferSize the buffer size to use when copying. - * @return the number of bytes copied + * @return the number of bytes copied or zero if the copied file was a directory. * @throws NoSuchFileException if the source file doesn't exist. * @throws FileAlreadyExistsException if the destination file already exists and 'rewrite' argument is set to `false`. * @throws IOException if any errors occur while copying. */ -public fun File.copyTo(dst: File, overwrite: Boolean = false, bufferSize: Int = DEFAULT_BUFFER_SIZE): Long { - if (!exists()) { +public fun File.copyTo(target: File, overwrite: Boolean = false, bufferSize: Int = DEFAULT_BUFFER_SIZE): Long { + if (!this.exists()) { throw NoSuchFileException(file = this, reason = "The source file doesn't exist") - } else if (isDirectory) { - throw IllegalArgumentException("Use copyRecursively to copy a directory $this") - } else if (dst.exists()) { - if (!overwrite) { + } + + if (target.exists()) { + val stillExists = if (!overwrite) true else !target.delete() + + if (stillExists) { throw FileAlreadyExistsException(file = this, - other = dst, + other = target, reason = "The destination file already exists") - } else if (dst.isDirectory && dst.listFiles().any()) { - // In this case file should be copied *into* this directory, - // no matter whether it is empty or not - return copyTo(dst.resolve(name), overwrite, bufferSize) } } - dst.parentFile?.mkdirs() - dst.delete() - val input = FileInputStream(this) - return input.use { - val output = FileOutputStream(dst) - output.use { + + if (this.isDirectory) { + if (!target.mkdirs()) + throw FileSystemException(file = this, other = target, reason = "Failed to create target directory") + return 0 + } + + target.parentFile?.mkdirs() + + return this.inputStream().use { input -> + target.outputStream().use { output -> input.copyTo(output, bufferSize) } } diff --git a/libraries/stdlib/test/io/Files.kt b/libraries/stdlib/test/io/Files.kt index be7cd3c2b71..e23e1dfb54c 100644 --- a/libraries/stdlib/test/io/Files.kt +++ b/libraries/stdlib/test/io/Files.kt @@ -356,42 +356,67 @@ class FilesTest { @test fun testCopyTo() { val srcFile = createTempFile() val dstFile = createTempFile() - srcFile.writeText("Hello, World!") - assertFailsWith(FileAlreadyExistsException::class) { - srcFile.copyTo(dstFile) + try { + srcFile.writeText("Hello, World!") + assertFailsWith(FileAlreadyExistsException::class, "copy do not overwrite existing file") { + srcFile.copyTo(dstFile) + } + + var len = srcFile.copyTo(dstFile, overwrite = true) + assertEquals(13L, len) + assertEquals(srcFile.readText(), dstFile.readText(Charsets.UTF_8), "copy with overwrite over existing file") + + assertTrue(dstFile.delete()) + len = srcFile.copyTo(dstFile) + assertEquals(13L, len) + assertEquals(srcFile.readText(Charsets.UTF_8), dstFile.readText(), "copy to new file") + + assertTrue(dstFile.delete()) + dstFile.mkdir() + val child = File(dstFile, "child") + child.createNewFile() + assertFailsWith(FileAlreadyExistsException::class, "copy with overwrite do not overwrite non-empty dir") { + srcFile.copyTo(dstFile, overwrite = true) + } + child.delete() + + srcFile.copyTo(dstFile, overwrite = true) + assertEquals(srcFile.readText(), dstFile.readText(), "copy with overwrite over empty dir") + + assertTrue(srcFile.delete()) + assertTrue(dstFile.delete()) + + assertFailsWith(NoSuchFileException::class) { + srcFile.copyTo(dstFile) + } + + srcFile.mkdir() + srcFile.resolve("somefile").writeText("some content") + dstFile.writeText("") + assertFailsWith(FileAlreadyExistsException::class, "copy dir do not overwrite file") { + srcFile.copyTo(dstFile) + } + srcFile.copyTo(dstFile, overwrite = true) + assertTrue(dstFile.isDirectory) + assertTrue(dstFile.listFiles()!!.isEmpty(), "only directory is copied, but not its content") + + assertFailsWith(FileAlreadyExistsException::class, "copy dir do not overwrite dir") { + srcFile.copyTo(dstFile) + } + + srcFile.copyTo(dstFile, overwrite = true) + assertTrue(dstFile.isDirectory) + assertTrue(dstFile.listFiles()!!.isEmpty(), "only directory is copied, but not its content") + + dstFile.resolve("somefile2").writeText("some content2") + assertFailsWith(FileAlreadyExistsException::class, "copy dir do not overwrite dir") { + srcFile.copyTo(dstFile, overwrite = true) + } } - - var len = srcFile.copyTo(dstFile, overwrite = true) - assertEquals(13L, len) - assertEquals(srcFile.readText(), dstFile.readText(Charsets.UTF_8)) - - assertTrue(dstFile.delete()) - len = srcFile.copyTo(dstFile) - assertEquals(13L, len) - assertEquals(srcFile.readText(Charsets.UTF_8), dstFile.readText()) - - assertTrue(dstFile.delete()) - dstFile.mkdir() - val child = File(dstFile, "child") - child.createNewFile() - srcFile.copyTo(dstFile, overwrite = true) - assertEquals(13L, len) - val copy = dstFile.resolve(srcFile.name) - assertEquals(srcFile.readText(), copy.readText()) - - assertTrue(srcFile.delete()) - assertTrue(child.delete() && copy.delete() && dstFile.delete()) - - assertFailsWith(NoSuchFileException::class) { - srcFile.copyTo(dstFile) + finally { + srcFile.deleteRecursively() + dstFile.deleteRecursively() } - - srcFile.mkdir() - - assertFailsWith(IllegalArgumentException::class) { - srcFile.copyTo(dstFile) - } - srcFile.delete() } @test fun copyToNameWithoutParent() {