From a74ff3d67a5d6bd5465aa4fe952f7ebd0ec62e54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Resul=20=C3=96v=C3=BCr?= <92828257+ovurrsl@users.noreply.github.com> Date: Wed, 21 Oct 2026 03:05:05 +0300 Subject: [PATCH] fix: cancelling a paste stops the copy and keeps a move's source (#1029) --- .../launcher/files/FileManagerScreen.kt | 7 +- .../com/droiddeck/launcher/files/FileOps.kt | 66 ++++++- .../launcher/files/FileOpsCopyTest.kt | 172 ++++++++++++++++++ 3 files changed, 234 insertions(+), 11 deletions(-) create mode 100644 app/src/test/java/com/droiddeck/launcher/files/FileOpsCopyTest.kt diff --git a/app/src/main/java/com/droiddeck/launcher/files/FileManagerScreen.kt b/app/src/main/java/com/droiddeck/launcher/files/FileManagerScreen.kt index 03b4aa277..97fe393e4 100644 --- a/app/src/main/java/com/droiddeck/launcher/files/FileManagerScreen.kt +++ b/app/src/main/java/com/droiddeck/launcher/files/FileManagerScreen.kt @@ -90,6 +90,7 @@ import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp import com.droiddeck.launcher.session.SessionPrefs import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ensureActive import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import java.io.File @@ -268,6 +269,7 @@ fun FileManagerScreen( operationDeterminate = true operationLabel = context.getString(if (cut) R.string.fm_op_moving else R.string.fm_op_copying) isOperationRunning = true + val job = coroutineContext[kotlinx.coroutines.Job] var applyToAll: ConflictChoice? = null var failed = 0 @@ -303,6 +305,7 @@ fun FileManagerScreen( } var lastPct = -1 val onProgress = FileOps.ProgressCallback { copied, total -> + job?.ensureActive() val pct = if (total > 0) ((copied * 100) / total).toInt() else 100 if (pct != lastPct) { lastPct = pct @@ -311,8 +314,8 @@ fun FileManagerScreen( } val target = dst val ok = withContext(Dispatchers.IO) { - if (cut) FileOps.moveWithProgress(src, target, onProgress) - else FileOps.copyWithProgress(src, target, onProgress) + if (cut) FileOps.moveWithProgress(src, target, onProgress) { job?.isActive == false } + else FileOps.copyWithProgress(src, target, onProgress) { job?.isActive == false } } if (ok) done++ else failed++ } diff --git a/app/src/main/java/com/droiddeck/launcher/files/FileOps.kt b/app/src/main/java/com/droiddeck/launcher/files/FileOps.kt index 04b83abaa..9007e6d89 100644 --- a/app/src/main/java/com/droiddeck/launcher/files/FileOps.kt +++ b/app/src/main/java/com/droiddeck/launcher/files/FileOps.kt @@ -45,22 +45,25 @@ object FileOps { return f.length() } - fun moveWithProgress(src: File, dst: File, progress: ProgressCallback?): Boolean { + fun moveWithProgress(src: File, dst: File, progress: ProgressCallback?, isCancelled: () -> Boolean = { false }): Boolean { if (sameFile(src, dst)) return true + if (isCancelled()) return false if (!dst.exists() && src.renameTo(dst)) { progress?.onProgress(1, 1) return true } - if (!copyWithProgress(src, dst, progress)) return false + if (!copyWithProgress(src, dst, progress, isCancelled)) return false + if (isCancelled()) return false return delete(src) } - fun copyWithProgress(src: File, dst: File, progress: ProgressCallback?): Boolean { + fun copyWithProgress(src: File, dst: File, progress: ProgressCallback?, isCancelled: () -> Boolean = { false }): Boolean { val total = maxOf(totalSize(src), 1L) - return copyWithProgress(src, dst, longArrayOf(0), total, progress) + return copyWithProgress(src, dst, longArrayOf(0), total, progress, isCancelled) } - private fun copyWithProgress(src: File, dst: File, done: LongArray, total: Long, progress: ProgressCallback?): Boolean { + private fun copyWithProgress(src: File, dst: File, done: LongArray, total: Long, progress: ProgressCallback?, isCancelled: () -> Boolean): Boolean { + if (isCancelled()) throw java.util.concurrent.CancellationException("Copy cancelled: $src") if (isSymlink(src)) return true if (sameFile(src, dst)) return true if (src.isDirectory && isWithin(dst, src)) { @@ -71,7 +74,7 @@ object FileOps { if (!dst.exists() && !dst.mkdirs()) return false var allOk = true src.list()?.forEach { name -> - if (!copyWithProgress(File(src, name), File(dst, name), done, total, progress)) { + if (!copyWithProgress(File(src, name), File(dst, name), done, total, progress, isCancelled)) { Log.e(TAG, "Failed to copy: ${File(src, name)}") allOk = false } @@ -81,13 +84,41 @@ object FileOps { val parent = dst.parentFile if (!src.exists() || (parent != null && !parent.exists() && !parent.mkdirs())) return false val chunk = 4L * 1024 * 1024 + var staging: File? = null + var created = false + var output: java.nio.channels.FileChannel? = null try { + output = try { + java.nio.channels.FileChannel.open(dst.toPath(), java.nio.file.StandardOpenOption.WRITE, java.nio.file.StandardOpenOption.CREATE_NEW) + .also { created = true } + } catch (e: java.nio.file.FileAlreadyExistsException) { + null + } + val target = if (!created && isSymlink(dst)) dst.canonicalFile else dst + var permissions: Set? = null + if (output == null) { + if (isSymlink(target) || target.parentFile?.isDirectory != true) { + throw IOException("Destination symlink target is not a writable directory entry: $dst") + } + if (target.isDirectory) throw IOException("Destination is a directory: $target") + permissions = try { + Files.getPosixFilePermissions(target.toPath()) + } catch (e: java.nio.file.NoSuchFileException) { + null + } catch (e: UnsupportedOperationException) { + null + } + if (target.exists() && !target.canWrite()) throw IOException("Destination is not writable: $target") + val directory = Files.createTempDirectory(checkNotNull(target.absoluteFile.parentFile).toPath(), ".droiddeck-copy-") + staging = directory.toFile() + output = FileOutputStream(File(staging, "file")).channel + } FileInputStream(src).channel.use { input -> - FileOutputStream(dst).channel.use { output -> + checkNotNull(output).use { channel -> val size = input.size() var position = 0L while (position < size) { - val transferred = input.transferTo(position, minOf(chunk, size - position), output) + val transferred = input.transferTo(position, minOf(chunk, size - position), channel) if (transferred <= 0) break position += transferred done[0] += transferred @@ -96,11 +127,28 @@ object FileOps { if (position < size) throw IOException("Incomplete copy: $position/$size bytes") } } + staging?.let { directory -> + val copied = File(directory, "file").toPath() + if (permissions != null && Files.getPosixFilePermissions(copied) != permissions) { + Files.setPosixFilePermissions(copied, permissions) + } + if (isCancelled()) throw java.util.concurrent.CancellationException("Copy cancelled: $src") + Files.move(copied, target.toPath(), java.nio.file.StandardCopyOption.ATOMIC_MOVE) + } return true + } catch (e: java.util.concurrent.CancellationException) { + if (created) dst.delete() + throw e } catch (e: IOException) { Log.e(TAG, "Failed to copy file: $src to $dst", e) - dst.delete() + if (created) dst.delete() return false + } finally { + output?.close() + staging?.let { directory -> + File(directory, "file").delete() + directory.delete() + } } } diff --git a/app/src/test/java/com/droiddeck/launcher/files/FileOpsCopyTest.kt b/app/src/test/java/com/droiddeck/launcher/files/FileOpsCopyTest.kt new file mode 100644 index 000000000..ac13ac781 --- /dev/null +++ b/app/src/test/java/com/droiddeck/launcher/files/FileOpsCopyTest.kt @@ -0,0 +1,172 @@ +package com.droiddeck.launcher.files + +import java.io.File +import java.util.concurrent.CancellationException +import java.nio.file.Files +import java.nio.file.attribute.PosixFilePermissions +import org.junit.Assume.assumeTrue +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertThrows +import org.junit.Assert.assertTrue +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +class FileOpsCopyTest { + @get:Rule val tmp = TemporaryFolder() + + @Test fun cancellingAnOverwritePreservesTheDestinationAndSource() { + val src = tmp.newFile("source").apply { writeText("replacement") } + val dst = tmp.newFile("dest").apply { writeText("original user data") } + assertThrows(CancellationException::class.java) { + FileOps.copyWithProgress(src, dst, { _, _ -> throw CancellationException("cancel") }) + } + assertTrue("original destination was removed", dst.exists()) + assertEquals("original user data", dst.readText()) + assertEquals("replacement", src.readText()) + assertEquals(setOf("source", "dest"), tmp.root.list()!!.toSet()) + } + + @Test fun cancellingANewCopyLeavesNoPartialDestination() { + val src = tmp.newFile("source").apply { writeText("replacement") } + val dst = File(tmp.root, "dest") + assertThrows(CancellationException::class.java) { + FileOps.copyWithProgress(src, dst, { _, _ -> throw CancellationException("cancel") }) + } + assertFalse(dst.exists()) + assertEquals("replacement", src.readText()) + assertEquals(setOf("source"), tmp.root.list()!!.toSet()) + } + + @Test fun cancellingAMoveFallbackPreservesBothFiles() { + val src = tmp.newFile("source").apply { writeText("replacement") } + val dst = tmp.newFile("dest").apply { writeText("original user data") } + assertThrows(CancellationException::class.java) { + FileOps.moveWithProgress(src, dst, { _, _ -> throw CancellationException("cancel") }) + } + assertTrue("original destination was removed", dst.exists()) + assertEquals("original user data", dst.readText()) + assertEquals("replacement", src.readText()) + } + + @Test fun cancellingStopsATreeOfEmptyFiles() { + val src = tmp.newFolder("source") + for (name in listOf("a", "b", "c")) File(src, name).createNewFile() + val dst = File(tmp.root, "dest") + var checks = 0 + assertThrows(CancellationException::class.java) { + FileOps.copyWithProgress(src, dst, null) { checks++ > 1 } + } + assertEquals(1, dst.list()!!.size) + assertEquals(3, src.list()!!.size) + } + + @Test fun cancellingStopsATreeOfEmptyFolders() { + val src = tmp.newFolder("source") + for (name in listOf("a", "b", "c")) File(src, name).mkdir() + val dst = File(tmp.root, "dest") + var checks = 0 + assertThrows(CancellationException::class.java) { + FileOps.copyWithProgress(src, dst, null) { checks++ > 1 } + } + assertEquals(1, dst.list()!!.size) + } + + @Test fun aCompletedCopyReplacesTheDestination() { + val src = tmp.newFile("source").apply { writeText("replacement") } + val dst = tmp.newFile("dest").apply { writeText("original user data") } + assertTrue(FileOps.copyWithProgress(src, dst, null)) + assertEquals("replacement", dst.readText()) + assertEquals("replacement", src.readText()) + assertEquals(setOf("source", "dest"), tmp.root.list()!!.toSet()) + } + + @Test fun aLongDestinationNameDoesNotOverflowTheStagingName() { + val src = tmp.newFile("source").apply { writeText("replacement") } + val dst = File(tmp.root, "d".repeat(240)) + assertTrue(FileOps.copyWithProgress(src, dst, null)) + assertEquals("replacement", dst.readText()) + } + + @Test fun anOverwritePreservesTheTargetsExecutablePermissions() { + assumeTrue(Files.getFileStore(tmp.root.toPath()).supportsFileAttributeView("posix")) + val src = tmp.newFile("source").apply { writeText("replacement") } + val dst = tmp.newFile("dest").apply { writeText("original") } + val permissions = PosixFilePermissions.fromString("rwxr-x---") + Files.setPosixFilePermissions(dst.toPath(), permissions) + assertTrue(FileOps.copyWithProgress(src, dst, null)) + assertEquals("replacement", dst.readText()) + assertEquals(permissions, Files.getPosixFilePermissions(dst.toPath())) + } + + @Test fun aMoveFallbackPreservesTheTargetsExecutablePermissions() { + assumeTrue(Files.getFileStore(tmp.root.toPath()).supportsFileAttributeView("posix")) + val src = tmp.newFile("source").apply { writeText("replacement") } + val dst = tmp.newFile("dest").apply { writeText("original") } + val permissions = PosixFilePermissions.fromString("rwxr-x---") + Files.setPosixFilePermissions(dst.toPath(), permissions) + assertTrue(FileOps.moveWithProgress(src, dst, null)) + assertFalse(src.exists()) + assertEquals("replacement", dst.readText()) + assertEquals(permissions, Files.getPosixFilePermissions(dst.toPath())) + } + + @Test fun anOverwriteStillRefusesAReadOnlyDestination() { + assumeTrue(Files.getFileStore(tmp.root.toPath()).supportsFileAttributeView("posix")) + val src = tmp.newFile("source").apply { writeText("replacement") } + val dst = tmp.newFile("dest").apply { writeText("original") } + Files.setPosixFilePermissions(dst.toPath(), PosixFilePermissions.fromString("r--r--r--")) + try { + assertFalse(FileOps.copyWithProgress(src, dst, null)) + assertEquals("original", dst.readText()) + assertEquals("replacement", src.readText()) + } finally { + dst.setWritable(true) + } + } + + @Test fun anEmptyFileCanReplaceAnExistingFile() { + val src = tmp.newFile("source") + val dst = tmp.newFile("dest").apply { writeText("original user data") } + var callbacks = 0 + assertTrue(FileOps.copyWithProgress(src, dst, { _, _ -> callbacks++ })) + assertEquals(0, callbacks) + assertEquals(0L, dst.length()) + assertTrue(src.exists()) + } + + @Test fun aFailedPublicationPreservesTheDestinationAndCleansStaging() { + val src = tmp.newFile("source").apply { writeText("replacement") } + val dst = tmp.newFolder("dest") + File(dst, "keep").writeText("original user data") + assertFalse(FileOps.copyWithProgress(src, dst, null)) + assertEquals("original user data", File(dst, "keep").readText()) + assertEquals("replacement", src.readText()) + assertEquals(setOf("source", "dest"), tmp.root.list()!!.toSet()) + } + + @Test fun aSuccessfulMoveFallbackDeletesTheSourceOnlyAfterCopying() { + val src = tmp.newFile("source").apply { writeText("replacement") } + val dst = tmp.newFile("dest").apply { writeText("original user data") } + assertTrue(FileOps.moveWithProgress(src, dst, null)) + assertFalse(src.exists()) + assertEquals("replacement", dst.readText()) + assertEquals(setOf("dest"), tmp.root.list()!!.toSet()) + } + + @Test fun overwritingThroughASymlinkKeepsTheLinkAndUpdatesItsTarget() { + assumeTrue(Files.getFileStore(tmp.root.toPath()).supportsFileAttributeView("posix")) + val src = tmp.newFile("source").apply { writeText("replacement") } + val realTarget = tmp.newFile("real-target").apply { writeText("original user data") } + val link = File(tmp.root, "dest-link") + Files.createSymbolicLink(link.toPath(), realTarget.toPath()) + + assertTrue(FileOps.copyWithProgress(src, link, null)) + assertTrue("destination symlink was replaced instead of written through", Files.isSymbolicLink(link.toPath())) + assertEquals("replacement", realTarget.readText()) + } +}