fix: cancelling a paste stops the copy and keeps a move's source (#1029)

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)
+                    if (cut) FileOps.moveWithProgress(src, target, onProgress) { job?.isActive == false }
-                    else FileOps.copyWithProgress(src, target, onProgress)
+                    else FileOps.copyWithProgress(src, target, onProgress) { job?.isActive == false }
                 }
                 if (ok) done++ else failed++
             }

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<java.nio.file.attribute.PosixFilePermission>? = 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()
+            }
         }
     }
 

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())
+    }
+}