diff --git a/app/shared/src/commonMain/kotlin/com/linroid/ketch/app/ui/dialog/RemoveDownloadDialog.kt b/app/shared/src/commonMain/kotlin/com/linroid/ketch/app/ui/dialog/RemoveDownloadDialog.kt new file mode 100644 index 00000000..c90726e6 --- /dev/null +++ b/app/shared/src/commonMain/kotlin/com/linroid/ketch/app/ui/dialog/RemoveDownloadDialog.kt @@ -0,0 +1,86 @@ +package com.linroid.ketch.app.ui.dialog + +import androidx.compose.foundation.clickable +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.padding +import androidx.compose.material3.AlertDialog +import androidx.compose.material3.Checkbox +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.material3.TextButton +import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.text.font.FontWeight +import androidx.compose.ui.text.style.TextOverflow +import androidx.compose.ui.unit.dp +import com.linroid.ketch.app.util.formatBytes + +@Composable +fun RemoveDownloadDialog( + fileName: String, + totalBytes: Long?, + onDismiss: () -> Unit, + onConfirm: (deleteFiles: Boolean) -> Unit, +) { + var deleteFiles by remember { mutableStateOf(false) } + + AlertDialog( + onDismissRequest = onDismiss, + title = { Text("Remove download?") }, + text = { + Column { + Text( + text = fileName, + fontWeight = FontWeight.Medium, + maxLines = 2, + overflow = TextOverflow.Ellipsis, + ) + if (totalBytes != null && totalBytes > 0) { + Text( + text = formatBytes(totalBytes), + style = MaterialTheme.typography.labelSmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } + Spacer(Modifier.height(12.dp)) + Row( + modifier = Modifier + .fillMaxWidth() + .clickable { deleteFiles = !deleteFiles } + .padding(vertical = 4.dp), + verticalAlignment = Alignment.CenterVertically, + horizontalArrangement = Arrangement.spacedBy(4.dp), + ) { + Checkbox( + checked = deleteFiles, + onCheckedChange = { deleteFiles = it }, + ) + Text("Also delete downloaded file") + } + } + }, + confirmButton = { + TextButton( + onClick = { + onConfirm(deleteFiles) + onDismiss() + }, + ) { + Text("Remove") + } + }, + dismissButton = { + TextButton(onClick = onDismiss) { Text("Cancel") } + }, + ) +} diff --git a/app/shared/src/commonMain/kotlin/com/linroid/ketch/app/ui/list/DownloadListItem.kt b/app/shared/src/commonMain/kotlin/com/linroid/ketch/app/ui/list/DownloadListItem.kt index 0a5aba21..189ffc73 100644 --- a/app/shared/src/commonMain/kotlin/com/linroid/ketch/app/ui/list/DownloadListItem.kt +++ b/app/shared/src/commonMain/kotlin/com/linroid/ketch/app/ui/list/DownloadListItem.kt @@ -44,6 +44,7 @@ import com.linroid.ketch.app.ui.common.SpeedLimitPanel import com.linroid.ketch.app.ui.common.StatusIndicator import com.linroid.ketch.app.ui.common.TaskSettingsIcon import com.linroid.ketch.app.ui.common.TaskSettingsPanel +import com.linroid.ketch.app.ui.dialog.RemoveDownloadDialog import com.linroid.ketch.app.util.extractFilename import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch @@ -72,6 +73,7 @@ fun DownloadListItem( state is DownloadState.Queued || state is DownloadState.Scheduled var expanded by remember { mutableStateOf(ExpandedPanel.None) } + var showRemoveDialog by remember { mutableStateOf(false) } Card( onClick = { @@ -228,7 +230,7 @@ fun DownloadListItem( } Spacer(modifier = Modifier.weight(1f)) IconButton( - onClick = { scope.launch { task.remove() } }, + onClick = { showRemoveDialog = true }, modifier = Modifier.size(32.dp), ) { Icon( @@ -241,6 +243,22 @@ fun DownloadListItem( } } + if (showRemoveDialog) { + val totalBytes = when (val s = state) { + is DownloadState.Downloading -> s.progress.totalBytes + is DownloadState.Paused -> s.progress.totalBytes + else -> null + } + RemoveDownloadDialog( + fileName = fileName, + totalBytes = totalBytes, + onDismiss = { showRemoveDialog = false }, + onConfirm = { deleteFiles -> + scope.launch { task.remove(deleteFiles = deleteFiles) } + }, + ) + } + if (showToggles) { // Expanded panel below icons AnimatedContent( diff --git a/docs/api.md b/docs/api.md index cab34378..a8da3ab5 100644 --- a/docs/api.md +++ b/docs/api.md @@ -64,6 +64,43 @@ interface KetchApi { } ``` +`KetchApi.download(...)` returns a `DownloadTask` for controlling an +individual download. Tasks expose reactive state and per-task actions: + +```kotlin +interface DownloadTask { + val taskId: String + val request: DownloadRequest + val state: StateFlow + val segments: StateFlow> + + suspend fun pause() + suspend fun resume(destination: Destination? = null) + suspend fun cancel() + suspend fun setSpeedLimit(limit: SpeedLimit) + suspend fun setPriority(priority: DownloadPriority) + suspend fun setConnections(connections: Int) + suspend fun reschedule( + schedule: DownloadSchedule, + conditions: List = emptyList(), + ) + + /** + * Cancels the download and removes it from the task list. + * + * @param deleteFiles when `true`, also delete the data this task + * wrote to disk (partial bytes, completed file, or a torrent's + * save path). Deletion is best-effort — failures are logged + * and do not prevent the task record from being removed. + * Defaults to `false`. + */ + suspend fun remove(deleteFiles: Boolean = false) + + /** Suspends until the task reaches a terminal state. */ + suspend fun await(): Result +} +``` + ### `library:core` The in-process download engine. Depends on an `HttpEngine` interface (no HTTP client dependency): diff --git a/library/api/src/commonMain/kotlin/com/linroid/ketch/api/DownloadTask.kt b/library/api/src/commonMain/kotlin/com/linroid/ketch/api/DownloadTask.kt index c5ef12cc..1e2a3f04 100644 --- a/library/api/src/commonMain/kotlin/com/linroid/ketch/api/DownloadTask.kt +++ b/library/api/src/commonMain/kotlin/com/linroid/ketch/api/DownloadTask.kt @@ -78,8 +78,13 @@ interface DownloadTask { /** * Cancels the download and removes it from the task store and tasks list. + * + * @param deleteFiles when `true`, also delete the downloaded data + * (partial or completed). Deletion is best-effort: failures are + * logged and do not prevent the task record from being removed. + * Defaults to `false` for backward compatibility. */ - suspend fun remove() + suspend fun remove(deleteFiles: Boolean = false) /** * Suspends until the download reaches a terminal state. diff --git a/library/core/src/commonMain/kotlin/com/linroid/ketch/core/Ketch.kt b/library/core/src/commonMain/kotlin/com/linroid/ketch/core/Ketch.kt index f0c8eb4a..31cfc615 100644 --- a/library/core/src/commonMain/kotlin/com/linroid/ketch/core/Ketch.kt +++ b/library/core/src/commonMain/kotlin/com/linroid/ketch/core/Ketch.kt @@ -35,6 +35,7 @@ import com.linroid.ketch.core.task.TaskHandle import com.linroid.ketch.core.task.TaskRecord import com.linroid.ketch.core.task.TaskState import com.linroid.ketch.core.task.TaskStore +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.cancel @@ -226,12 +227,21 @@ class Ketch( coordinator.cancel(handle) } - override suspend fun remove(handle: TaskHandle) { + override suspend fun remove(handle: TaskHandle, deleteFiles: Boolean) { val taskId = handle.taskId - log.i { "Removing task: taskId=$taskId" } + log.i { "Removing task: taskId=$taskId, deleteFiles=$deleteFiles" } scheduler.cancel(taskId) queue.dequeue(taskId) coordinator.cancel(handle) + if (deleteFiles) { + try { + coordinator.cleanup(handle) + } catch (e: CancellationException) { + throw e + } catch (e: Throwable) { + log.w(e) { "Cleanup failed for taskId=$taskId" } + } + } taskStore.remove(taskId) tasksMutex.withLock { _tasks.value = _tasks.value.filter { it.taskId != taskId } diff --git a/library/core/src/commonMain/kotlin/com/linroid/ketch/core/engine/DownloadCoordinator.kt b/library/core/src/commonMain/kotlin/com/linroid/ketch/core/engine/DownloadCoordinator.kt index 25494106..76718f83 100644 --- a/library/core/src/commonMain/kotlin/com/linroid/ketch/core/engine/DownloadCoordinator.kt +++ b/library/core/src/commonMain/kotlin/com/linroid/ketch/core/engine/DownloadCoordinator.kt @@ -9,6 +9,8 @@ import com.linroid.ketch.api.DownloadConfig import com.linroid.ketch.api.log.KetchLogger import com.linroid.ketch.core.KetchDispatchers import com.linroid.ketch.core.file.FileNameResolver +import com.linroid.ketch.core.file.NoOpFileAccessor +import com.linroid.ketch.core.file.createFileAccessor import com.linroid.ketch.core.task.TaskHandle import com.linroid.ketch.core.task.TaskState import kotlinx.coroutines.CancellationException @@ -16,6 +18,7 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job import kotlinx.coroutines.NonCancellable import kotlinx.coroutines.cancel +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.launch import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock @@ -142,11 +145,17 @@ internal class DownloadCoordinator( suspend fun cancel(handle: TaskHandle) { val taskId = handle.taskId log.i { "Canceling download for taskId=$taskId" } - mutex.withLock { + val job = mutex.withLock { val entry = activeDownloads[taskId] entry?.job?.cancel() activeDownloads.remove(taskId) + entry?.job } + // Await the job's finally chain (including FileAccessor.close) + // before returning, so callers can safely follow up with file + // cleanup. Joining must happen outside the mutex because the + // job's own finally re-acquires it to clean up activeDownloads. + job?.join() handle.mutableState.value = DownloadState.Canceled handle.record.update { it.copy( @@ -232,6 +241,56 @@ internal class DownloadCoordinator( ) } + /** + * Deletes any data produced for the given task by dispatching to its + * [DownloadSource.cleanup]. The caller must have cancelled the active + * download (if any) before invoking this. No-op if the task has no + * recorded sourceType or outputPath (nothing to clean up). + */ + suspend fun cleanup(handle: TaskHandle) { + val record = handle.record.value + val sourceType = record.sourceType + val outputPath = record.outputPath + if (sourceType == null || outputPath == null) { + log.d { + "Skipping cleanup for taskId=${handle.taskId} " + + "(sourceType=$sourceType, outputPath=$outputPath)" + } + return + } + val source = try { + sourceResolver.resolveByType(sourceType) + } catch (e: KetchError) { + log.w(e) { + "Skipping cleanup for taskId=${handle.taskId}: " + + "unknown source type '$sourceType'" + } + return + } + val fa = if (source.managesOwnFileIo) { + NoOpFileAccessor + } else { + createFileAccessor(outputPath, dispatchers.io) + } + val ctx = DownloadContext( + taskId = handle.taskId, + url = handle.request.url, + request = handle.request, + fileAccessor = fa, + segments = MutableStateFlow(handle.mutableSegments.value), + onProgress = { _, _ -> }, + throttle = { _ -> }, + headers = handle.request.headers, + ) + try { + source.cleanup(ctx, record.sourceResumeState) + } finally { + if (!source.managesOwnFileIo) { + fa.close() + } + } + } + fun close() { scope.cancel() } diff --git a/library/core/src/commonMain/kotlin/com/linroid/ketch/core/engine/DownloadSource.kt b/library/core/src/commonMain/kotlin/com/linroid/ketch/core/engine/DownloadSource.kt index ee950001..5d1dca96 100644 --- a/library/core/src/commonMain/kotlin/com/linroid/ketch/core/engine/DownloadSource.kt +++ b/library/core/src/commonMain/kotlin/com/linroid/ketch/core/engine/DownloadSource.kt @@ -1,6 +1,7 @@ package com.linroid.ketch.core.engine import com.linroid.ketch.api.ResolvedSource +import kotlinx.coroutines.CancellationException /** * Abstraction for pluggable download source types. @@ -92,4 +93,37 @@ interface DownloadSource { suspend fun updateResumeState( context: DownloadContext, ): SourceResumeState? = null + + /** + * Deletes any data this source wrote for the given task. Called by + * the engine when a task is removed with `deleteFiles = true`, + * after the active download (if any) has been cancelled. + * + * Best-effort: implementations should log and swallow non-fatal + * errors rather than throwing. [CancellationException] must still + * propagate. + * + * The default implementation deletes [DownloadContext.fileAccessor], + * which is the right behavior for sources that write to a single + * output file. Sources with `managesOwnFileIo = true` (e.g. + * torrents) should override this and use [resumeState] to locate + * their internal handle (such as an info hash) for cleanup. + * + * @param context the download context, with a [DownloadContext.fileAccessor] + * pointing at the persisted output path + * @param resumeState the source-specific resume state persisted in the + * task record, or `null` if the task never produced one + */ + suspend fun cleanup( + context: DownloadContext, + resumeState: SourceResumeState?, + ) { + try { + context.fileAccessor.delete() + } catch (e: CancellationException) { + throw e + } catch (_: Throwable) { + // best-effort; caller is responsible for logging + } + } } diff --git a/library/core/src/commonMain/kotlin/com/linroid/ketch/core/task/RealDownloadTask.kt b/library/core/src/commonMain/kotlin/com/linroid/ketch/core/task/RealDownloadTask.kt index c839d7b1..70298c6e 100644 --- a/library/core/src/commonMain/kotlin/com/linroid/ketch/core/task/RealDownloadTask.kt +++ b/library/core/src/commonMain/kotlin/com/linroid/ketch/core/task/RealDownloadTask.kt @@ -84,7 +84,7 @@ internal class RealDownloadTask( controller.reschedule(this, schedule, conditions) } - override suspend fun remove() { - controller.remove(this) + override suspend fun remove(deleteFiles: Boolean) { + controller.remove(this, deleteFiles) } } diff --git a/library/core/src/commonMain/kotlin/com/linroid/ketch/core/task/TaskController.kt b/library/core/src/commonMain/kotlin/com/linroid/ketch/core/task/TaskController.kt index e9fe3a0f..8986ec57 100644 --- a/library/core/src/commonMain/kotlin/com/linroid/ketch/core/task/TaskController.kt +++ b/library/core/src/commonMain/kotlin/com/linroid/ketch/core/task/TaskController.kt @@ -16,7 +16,7 @@ internal interface TaskController { suspend fun pause(taskId: String) suspend fun resume(handle: TaskHandle, destination: Destination? = null) suspend fun cancel(handle: TaskHandle) - suspend fun remove(handle: TaskHandle) + suspend fun remove(handle: TaskHandle, deleteFiles: Boolean) suspend fun setSpeedLimit(taskId: String, limit: SpeedLimit) suspend fun setConnections(taskId: String, connections: Int) suspend fun setPriority(taskId: String, priority: DownloadPriority) diff --git a/library/core/src/commonTest/kotlin/com/linroid/ketch/engine/DownloadCoordinatorCancelTest.kt b/library/core/src/commonTest/kotlin/com/linroid/ketch/engine/DownloadCoordinatorCancelTest.kt new file mode 100644 index 00000000..26a07bf3 --- /dev/null +++ b/library/core/src/commonTest/kotlin/com/linroid/ketch/engine/DownloadCoordinatorCancelTest.kt @@ -0,0 +1,147 @@ +package com.linroid.ketch.engine + +import com.linroid.ketch.api.Destination +import com.linroid.ketch.api.DownloadConfig +import com.linroid.ketch.api.DownloadRequest +import com.linroid.ketch.api.DownloadState +import com.linroid.ketch.api.ResolvedSource +import com.linroid.ketch.api.Segment +import com.linroid.ketch.core.KetchDispatchers +import com.linroid.ketch.core.engine.DownloadContext +import com.linroid.ketch.core.engine.DownloadCoordinator +import com.linroid.ketch.core.engine.DownloadSource +import com.linroid.ketch.core.engine.SourceResolver +import com.linroid.ketch.core.engine.SourceResumeState +import com.linroid.ketch.core.file.DefaultFileNameResolver +import com.linroid.ketch.core.task.AtomicSaver +import com.linroid.ketch.core.task.TaskHandle +import com.linroid.ketch.core.task.TaskRecord +import com.linroid.ketch.core.task.TaskState +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.awaitCancellation +import kotlinx.coroutines.cancel +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.withContext +import kotlinx.coroutines.withTimeout +import kotlin.test.Test +import kotlin.test.assertTrue +import kotlin.time.Clock +import kotlin.time.Duration.Companion.seconds + +/** + * Verifies that [DownloadCoordinator.cancel] does not return until the + * active download job's `finally` chain (including [FileAccessor.close]) + * has completed. Without that guarantee, callers that follow `cancel` + * with file-cleanup work (e.g. `remove(deleteFiles = true)`) race against + * the still-running writer and may fail on lock-enforcing platforms. + */ +class DownloadCoordinatorCancelTest { + + private class SuspendingSource : DownloadSource { + val downloadStarted = CompletableDeferred() + var downloadFinallyRan: Boolean = false + + override val type: String = "suspend" + override fun canHandle(url: String): Boolean = true + + override suspend fun resolve( + url: String, + properties: Map, + ): ResolvedSource = ResolvedSource( + url = url, + sourceType = type, + totalBytes = 16L, + supportsResume = false, + suggestedFileName = "file.bin", + maxSegments = 1, + ) + + override suspend fun download(context: DownloadContext) { + downloadStarted.complete(Unit) + try { + awaitCancellation() + } finally { + downloadFinallyRan = true + } + } + + override suspend fun resume( + context: DownloadContext, + resumeState: SourceResumeState, + ) = Unit + + override fun buildResumeState( + resolved: ResolvedSource, + totalBytes: Long, + ): SourceResumeState = SourceResumeState(type, "") + } + + private fun createHandle(destination: String): TaskHandle { + val now = Clock.System.now() + val request = DownloadRequest( + url = "https://example.com/file.bin", + destination = Destination(destination), + ) + val record = TaskRecord( + taskId = "task-cancel", + request = request, + state = TaskState.QUEUED, + createdAt = now, + updatedAt = now, + ) + return object : TaskHandle { + override val taskId = "task-cancel" + override val request = request + override val createdAt = now + override val mutableState = + MutableStateFlow(DownloadState.Queued) + override val mutableSegments = + MutableStateFlow>(emptyList()) + override val record = AtomicSaver(record) {} + } + } + + private fun createCoordinator(source: DownloadSource): DownloadCoordinator = + DownloadCoordinator( + sourceResolver = SourceResolver(listOf(source)), + config = DownloadConfig(), + fileNameResolver = DefaultFileNameResolver(), + dispatchers = KetchDispatchers( + main = Dispatchers.Default, + network = Dispatchers.Default, + io = Dispatchers.Default, + ), + ) + + @Test + fun cancel_waitsForDownloadFinally() = runTest { + withContext(Dispatchers.Default) { + val scope = CoroutineScope(SupervisorJob() + Dispatchers.Default) + try { + val source = SuspendingSource() + val coordinator = createCoordinator(source) + val handle = createHandle("/tmp/ketch-cancel-test/") + + // start() launches the job and returns; the job calls + // source.download() concurrently and suspends. + coordinator.start(handle) + withTimeout(5.seconds) { source.downloadStarted.await() } + + coordinator.cancel(handle) + + assertTrue( + source.downloadFinallyRan, + "cancel() returned before the download's finally ran — race " + + "between cancellation propagation and follow-up cleanup work", + ) + } finally { + scope.cancel() + } + } + } +} diff --git a/library/core/src/commonTest/kotlin/com/linroid/ketch/engine/DownloadCoordinatorCleanupTest.kt b/library/core/src/commonTest/kotlin/com/linroid/ketch/engine/DownloadCoordinatorCleanupTest.kt new file mode 100644 index 00000000..853e0037 --- /dev/null +++ b/library/core/src/commonTest/kotlin/com/linroid/ketch/engine/DownloadCoordinatorCleanupTest.kt @@ -0,0 +1,168 @@ +package com.linroid.ketch.engine + +import com.linroid.ketch.api.Destination +import com.linroid.ketch.api.DownloadRequest +import com.linroid.ketch.api.DownloadState +import com.linroid.ketch.api.ResolvedSource +import com.linroid.ketch.api.Segment +import com.linroid.ketch.api.DownloadConfig +import com.linroid.ketch.core.KetchDispatchers +import com.linroid.ketch.core.engine.DownloadContext +import com.linroid.ketch.core.engine.DownloadCoordinator +import com.linroid.ketch.core.engine.DownloadSource +import com.linroid.ketch.core.engine.SourceResolver +import com.linroid.ketch.core.engine.SourceResumeState +import com.linroid.ketch.core.file.DefaultFileNameResolver +import com.linroid.ketch.core.task.AtomicSaver +import com.linroid.ketch.core.task.TaskHandle +import com.linroid.ketch.core.task.TaskRecord +import com.linroid.ketch.core.task.TaskState +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.test.runTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.time.Clock +import kotlin.time.Instant + +class DownloadCoordinatorCleanupTest { + + private class RecordingSource( + override val type: String = "rec", + override val managesOwnFileIo: Boolean = false, + ) : DownloadSource { + var cleanupCalls = 0 + var lastResumeState: SourceResumeState? = null + var lastFileAccessorType: String? = null + + override fun canHandle(url: String): Boolean = true + override suspend fun resolve( + url: String, + properties: Map, + ): ResolvedSource = error("not used") + override suspend fun download(context: DownloadContext) = Unit + override suspend fun resume( + context: DownloadContext, + resumeState: SourceResumeState, + ) = Unit + override fun buildResumeState( + resolved: ResolvedSource, + totalBytes: Long, + ): SourceResumeState = SourceResumeState(type, "") + override suspend fun cleanup( + context: DownloadContext, + resumeState: SourceResumeState?, + ) { + cleanupCalls++ + lastResumeState = resumeState + lastFileAccessorType = + context.fileAccessor::class.simpleName + } + } + + private fun createCoordinator(source: DownloadSource): DownloadCoordinator = + DownloadCoordinator( + sourceResolver = SourceResolver(listOf(source)), + config = DownloadConfig(), + fileNameResolver = DefaultFileNameResolver(), + dispatchers = KetchDispatchers( + main = Dispatchers.Default, + network = Dispatchers.Default, + io = Dispatchers.Default, + ), + ) + + private fun createHandle( + sourceType: String?, + outputPath: String?, + resumeState: SourceResumeState? = null, + ): TaskHandle { + val now = Clock.System.now() + val request = DownloadRequest( + url = "https://example.com/file.zip", + destination = Destination("/tmp/"), + ) + val record = TaskRecord( + taskId = "task-1", + request = request, + outputPath = outputPath, + state = TaskState.COMPLETED, + sourceType = sourceType, + sourceResumeState = resumeState, + createdAt = now, + updatedAt = now, + ) + return object : TaskHandle { + override val taskId = "task-1" + override val request = request + override val createdAt: Instant = now + override val mutableState = + MutableStateFlow(DownloadState.Completed(outputPath ?: "")) + override val mutableSegments = + MutableStateFlow>(emptyList()) + override val record = AtomicSaver(record) {} + } + } + + @Test + fun cleanup_invokesSourceWithResumeState() = runTest { + val source = RecordingSource() + val coordinator = createCoordinator(source) + val resume = SourceResumeState("rec", "payload") + val handle = createHandle( + sourceType = "rec", + outputPath = "/tmp/file.zip", + resumeState = resume, + ) + + coordinator.cleanup(handle) + + assertEquals(1, source.cleanupCalls) + assertEquals(resume, source.lastResumeState) + } + + @Test + fun cleanup_managesOwnFileIo_passesNoOpAccessor() = runTest { + val source = RecordingSource(managesOwnFileIo = true) + val coordinator = createCoordinator(source) + val handle = createHandle( + sourceType = "rec", + outputPath = "/tmp/file.zip", + ) + + coordinator.cleanup(handle) + + assertEquals(1, source.cleanupCalls) + assertEquals("NoOpFileAccessor", source.lastFileAccessorType) + } + + @Test + fun cleanup_unknownSourceType_isNoOp() = runTest { + val source = RecordingSource() + val coordinator = createCoordinator(source) + val handle = createHandle( + sourceType = null, + outputPath = "/tmp/file.zip", + ) + + coordinator.cleanup(handle) + + assertEquals(0, source.cleanupCalls) + } + + @Test + fun cleanup_missingOutputPath_isNoOp() = runTest { + val source = RecordingSource() + val coordinator = createCoordinator(source) + val handle = createHandle( + sourceType = "rec", + outputPath = null, + ) + + coordinator.cleanup(handle) + + assertEquals(0, source.cleanupCalls) + } +} diff --git a/library/core/src/commonTest/kotlin/com/linroid/ketch/engine/DownloadSourceCleanupTest.kt b/library/core/src/commonTest/kotlin/com/linroid/ketch/engine/DownloadSourceCleanupTest.kt new file mode 100644 index 00000000..c3b61443 --- /dev/null +++ b/library/core/src/commonTest/kotlin/com/linroid/ketch/engine/DownloadSourceCleanupTest.kt @@ -0,0 +1,101 @@ +package com.linroid.ketch.engine + +import com.linroid.ketch.api.Destination +import com.linroid.ketch.api.DownloadRequest +import com.linroid.ketch.api.ResolvedSource +import com.linroid.ketch.core.engine.DownloadContext +import com.linroid.ketch.core.engine.DownloadSource +import com.linroid.ketch.core.engine.SourceResumeState +import com.linroid.ketch.core.file.FileAccessor +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.test.runTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +class DownloadSourceCleanupTest { + + private class RecordingFileAccessor( + private val onDelete: suspend () -> Unit = {}, + ) : FileAccessor { + var deleteCount = 0 + override suspend fun writeAt(offset: Long, data: ByteArray) = Unit + override suspend fun flush() = Unit + override fun close() = Unit + override suspend fun delete() { + deleteCount++ + onDelete() + } + override suspend fun size(): Long = 0 + override suspend fun preallocate(size: Long) = Unit + } + + private class StubSource : DownloadSource { + override val type = "stub" + override fun canHandle(url: String): Boolean = true + override suspend fun resolve( + url: String, + properties: Map, + ): ResolvedSource = error("not used") + override suspend fun download(context: DownloadContext) = Unit + override suspend fun resume( + context: DownloadContext, + resumeState: SourceResumeState, + ) = Unit + override fun buildResumeState( + resolved: ResolvedSource, + totalBytes: Long, + ): SourceResumeState = SourceResumeState(type, "") + } + + private fun context(accessor: FileAccessor): DownloadContext = + DownloadContext( + taskId = "t", + url = "https://example.com/f", + request = DownloadRequest( + url = "https://example.com/f", + destination = Destination("/tmp/"), + ), + fileAccessor = accessor, + segments = MutableStateFlow(emptyList()), + onProgress = { _, _ -> }, + throttle = { _ -> }, + headers = emptyMap(), + ) + + @Test + fun defaultCleanup_deletesFileAccessor() = runTest { + val accessor = RecordingFileAccessor() + val source = StubSource() + + source.cleanup(context(accessor), resumeState = null) + + assertEquals(1, accessor.deleteCount) + } + + @Test + fun defaultCleanup_swallowsDeleteFailure() = runTest { + val accessor = RecordingFileAccessor( + onDelete = { throw RuntimeException("permission denied") }, + ) + val source = StubSource() + + // Should not throw — best-effort cleanup. + source.cleanup(context(accessor), resumeState = null) + + assertEquals(1, accessor.deleteCount) + } + + @Test + fun defaultCleanup_ignoresResumeState() = runTest { + val accessor = RecordingFileAccessor() + val source = StubSource() + + source.cleanup( + context = context(accessor), + resumeState = SourceResumeState("stub", "{}"), + ) + + assertEquals(1, accessor.deleteCount) + } +} diff --git a/library/core/src/commonTest/kotlin/com/linroid/ketch/task/RealDownloadTaskRemoveTest.kt b/library/core/src/commonTest/kotlin/com/linroid/ketch/task/RealDownloadTaskRemoveTest.kt new file mode 100644 index 00000000..a2be40ab --- /dev/null +++ b/library/core/src/commonTest/kotlin/com/linroid/ketch/task/RealDownloadTaskRemoveTest.kt @@ -0,0 +1,103 @@ +package com.linroid.ketch.task + +import com.linroid.ketch.api.Destination +import com.linroid.ketch.api.DownloadCondition +import com.linroid.ketch.api.DownloadPriority +import com.linroid.ketch.api.DownloadRequest +import com.linroid.ketch.api.DownloadSchedule +import com.linroid.ketch.api.DownloadState +import com.linroid.ketch.api.SpeedLimit +import com.linroid.ketch.core.task.RealDownloadTask +import com.linroid.ketch.core.task.TaskController +import com.linroid.ketch.core.task.TaskHandle +import com.linroid.ketch.core.task.TaskRecord +import com.linroid.ketch.core.task.TaskState +import kotlinx.coroutines.test.runTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue +import kotlin.time.Clock + +class RealDownloadTaskRemoveTest { + + private class CapturingController : TaskController { + var lastRemoveDeleteFiles: Boolean? = null + + override suspend fun pause(taskId: String) = Unit + override suspend fun resume(handle: TaskHandle, destination: Destination?) = Unit + override suspend fun cancel(handle: TaskHandle) = Unit + override suspend fun remove(handle: TaskHandle, deleteFiles: Boolean) { + lastRemoveDeleteFiles = deleteFiles + } + override suspend fun setSpeedLimit(taskId: String, limit: SpeedLimit) = Unit + override suspend fun setConnections(taskId: String, connections: Int) = Unit + override suspend fun setPriority(taskId: String, priority: DownloadPriority) = Unit + override suspend fun reschedule( + handle: TaskHandle, + schedule: DownloadSchedule, + conditions: List, + ) = Unit + } + + private fun createTask(controller: TaskController): RealDownloadTask { + val now = Clock.System.now() + val request = DownloadRequest( + url = "https://example.com/file.zip", + destination = Destination("/tmp/"), + ) + val record = TaskRecord( + taskId = "task-1", + request = request, + state = TaskState.QUEUED, + createdAt = now, + updatedAt = now, + ) + return RealDownloadTask( + taskId = "task-1", + request = request, + createdAt = now, + initialState = DownloadState.Queued, + initialSegments = emptyList(), + controller = controller, + taskStore = NoopTaskStore, + record = record, + ) + } + + @Test + fun remove_defaultsToFalse() = runTest { + val controller = CapturingController() + val task = createTask(controller) + + task.remove() + + assertEquals(false, controller.lastRemoveDeleteFiles) + } + + @Test + fun remove_withDeleteFilesTrue_forwardsFlag() = runTest { + val controller = CapturingController() + val task = createTask(controller) + + task.remove(deleteFiles = true) + + assertEquals(true, controller.lastRemoveDeleteFiles) + } + + @Test + fun remove_withDeleteFilesFalse_forwardsFlag() = runTest { + val controller = CapturingController() + val task = createTask(controller) + + task.remove(deleteFiles = false) + + assertEquals(false, controller.lastRemoveDeleteFiles) + } + + private object NoopTaskStore : com.linroid.ketch.core.task.TaskStore { + override suspend fun save(record: TaskRecord) = Unit + override suspend fun load(taskId: String): TaskRecord? = null + override suspend fun loadAll(): List = emptyList() + override suspend fun remove(taskId: String) = Unit + } +} diff --git a/library/remote/src/commonMain/kotlin/com/linroid/ketch/remote/RemoteDownloadTask.kt b/library/remote/src/commonMain/kotlin/com/linroid/ketch/remote/RemoteDownloadTask.kt index 621a9573..3bdc260b 100644 --- a/library/remote/src/commonMain/kotlin/com/linroid/ketch/remote/RemoteDownloadTask.kt +++ b/library/remote/src/commonMain/kotlin/com/linroid/ketch/remote/RemoteDownloadTask.kt @@ -20,6 +20,7 @@ import io.ktor.client.call.body import io.ktor.client.plugins.resources.delete import io.ktor.client.plugins.resources.post import io.ktor.client.plugins.resources.put +import io.ktor.client.request.parameter import io.ktor.client.request.setBody import io.ktor.http.ContentType import io.ktor.http.contentType @@ -84,9 +85,11 @@ internal class RemoteDownloadTask( update(response.body()) } - override suspend fun remove() { - log.d { "Remove taskId=$taskId" } - val response = httpClient.delete(byId) + override suspend fun remove(deleteFiles: Boolean) { + log.d { "Remove taskId=$taskId, deleteFiles=$deleteFiles" } + val response = httpClient.delete(byId) { + if (deleteFiles) parameter("deleteFiles", "true") + } checkSuccess(response) onRemoved(taskId) } diff --git a/library/server/src/main/kotlin/com/linroid/ketch/server/api/DownloadRoutes.kt b/library/server/src/main/kotlin/com/linroid/ketch/server/api/DownloadRoutes.kt index 23545954..60a1f36f 100644 --- a/library/server/src/main/kotlin/com/linroid/ketch/server/api/DownloadRoutes.kt +++ b/library/server/src/main/kotlin/com/linroid/ketch/server/api/DownloadRoutes.kt @@ -118,7 +118,9 @@ internal fun Route.downloadRoutes(ketch: KetchApi) { } delete { resource -> - log.d { "DELETE /api/tasks/${resource.id}" } + val deleteFiles = call.request.queryParameters["deleteFiles"] + ?.toBooleanStrictOrNull() ?: false + log.d { "DELETE /api/tasks/${resource.id} deleteFiles=$deleteFiles" } val task = ketch.tasks.value.find { it.taskId == resource.id } @@ -132,8 +134,8 @@ internal fun Route.downloadRoutes(ketch: KetchApi) { ) return@delete } - task.remove() - log.d { "Removed taskId=${resource.id}" } + task.remove(deleteFiles = deleteFiles) + log.d { "Removed taskId=${resource.id} deleteFiles=$deleteFiles" } call.respond(HttpStatusCode.NoContent) } diff --git a/library/server/src/test/kotlin/com/linroid/ketch/server/DownloadRoutesTest.kt b/library/server/src/test/kotlin/com/linroid/ketch/server/DownloadRoutesTest.kt index 868aa30f..e9368bb9 100644 --- a/library/server/src/test/kotlin/com/linroid/ketch/server/DownloadRoutesTest.kt +++ b/library/server/src/test/kotlin/com/linroid/ketch/server/DownloadRoutesTest.kt @@ -280,6 +280,94 @@ class DownloadRoutesTest { assertEquals(HttpStatusCode.NotFound, getResponse.status) } + @Test + fun `DELETE with deleteFiles=true forwards flag to task`() = testApplication { + val ketch = createKetch() + val recording = RecordingKetchApi(ketch) + application { + val server = createTestServer(ketch = recording) + with(server) { configureServer() } + } + val client = createClient { + install(ContentNegotiation) { json(json) } + } + val created = json.decodeFromString( + client.post("/api/tasks") { + contentType(ContentType.Application.Json) + setBody( + DownloadRequest( + url = "https://example.com/file.zip", + destination = Destination("/tmp/"), + ) + ) + }.bodyAsText() + ) + + val response = client.delete( + "/api/tasks/${created.taskId}?deleteFiles=true" + ) + assertEquals(HttpStatusCode.NoContent, response.status) + assertEquals(listOf(true), recording.removeCalls) + } + + @Test + fun `DELETE without deleteFiles defaults to false`() = testApplication { + val ketch = createKetch() + val recording = RecordingKetchApi(ketch) + application { + val server = createTestServer(ketch = recording) + with(server) { configureServer() } + } + val client = createClient { + install(ContentNegotiation) { json(json) } + } + val created = json.decodeFromString( + client.post("/api/tasks") { + contentType(ContentType.Application.Json) + setBody( + DownloadRequest( + url = "https://example.com/file.zip", + destination = Destination("/tmp/"), + ) + ) + }.bodyAsText() + ) + + val response = client.delete("/api/tasks/${created.taskId}") + assertEquals(HttpStatusCode.NoContent, response.status) + assertEquals(listOf(false), recording.removeCalls) + } + + @Test + fun `DELETE with invalid deleteFiles value defaults to false`() = testApplication { + val ketch = createKetch() + val recording = RecordingKetchApi(ketch) + application { + val server = createTestServer(ketch = recording) + with(server) { configureServer() } + } + val client = createClient { + install(ContentNegotiation) { json(json) } + } + val created = json.decodeFromString( + client.post("/api/tasks") { + contentType(ContentType.Application.Json) + setBody( + DownloadRequest( + url = "https://example.com/file.zip", + destination = Destination("/tmp/"), + ) + ) + }.bodyAsText() + ) + + val response = client.delete( + "/api/tasks/${created.taskId}?deleteFiles=banana" + ) + assertEquals(HttpStatusCode.NoContent, response.status) + assertEquals(listOf(false), recording.removeCalls) + } + @Test fun `pause on nonexistent task returns 404`() = testApplication { diff --git a/library/server/src/test/kotlin/com/linroid/ketch/server/TestHelpers.kt b/library/server/src/test/kotlin/com/linroid/ketch/server/TestHelpers.kt index 01fc3a19..2142e0ea 100644 --- a/library/server/src/test/kotlin/com/linroid/ketch/server/TestHelpers.kt +++ b/library/server/src/test/kotlin/com/linroid/ketch/server/TestHelpers.kt @@ -1,11 +1,30 @@ package com.linroid.ketch.server +import com.linroid.ketch.api.Destination +import com.linroid.ketch.api.DownloadCondition +import com.linroid.ketch.api.DownloadConfig +import com.linroid.ketch.api.DownloadPriority +import com.linroid.ketch.api.DownloadRequest +import com.linroid.ketch.api.DownloadSchedule +import com.linroid.ketch.api.DownloadState +import com.linroid.ketch.api.DownloadTask import com.linroid.ketch.api.KetchApi import com.linroid.ketch.api.KetchError -import com.linroid.ketch.api.DownloadConfig +import com.linroid.ketch.api.KetchStatus +import com.linroid.ketch.api.ResolvedSource +import com.linroid.ketch.api.Segment +import com.linroid.ketch.api.SpeedLimit import com.linroid.ketch.core.Ketch import com.linroid.ketch.core.engine.HttpEngine import com.linroid.ketch.core.engine.ServerInfo +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.stateIn +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel internal class NoOpHttpEngine : HttpEngine { override suspend fun head( @@ -46,3 +65,73 @@ internal fun createTestServer( ): KetchServer { return KetchServer(ketch) } + +/** + * Wraps a [KetchApi], routing all calls through to the delegate but + * substituting [DownloadTask] instances with [RecordingTask] proxies + * that capture `remove(deleteFiles)` calls. + */ +internal class RecordingKetchApi( + private val delegate: KetchApi, +) : KetchApi { + val removeCalls = mutableListOf() + private val scope = CoroutineScope(SupervisorJob() + Dispatchers.Default) + + override val backendLabel: String get() = delegate.backendLabel + + override val tasks: StateFlow> = + delegate.tasks + .map { list -> list.map { RecordingTask(it, removeCalls) } } + .stateIn( + scope, + kotlinx.coroutines.flow.SharingStarted.Eagerly, + delegate.tasks.value.map { RecordingTask(it, removeCalls) }, + ) + + override suspend fun download(request: DownloadRequest): DownloadTask = + RecordingTask(delegate.download(request), removeCalls) + + override suspend fun resolve( + url: String, + properties: Map, + ): ResolvedSource = delegate.resolve(url, properties) + + override suspend fun start() = delegate.start() + override suspend fun status(): KetchStatus = delegate.status() + override suspend fun updateConfig(config: DownloadConfig) = + delegate.updateConfig(config) + override fun close() { + scope.cancel() + delegate.close() + } +} + +private class RecordingTask( + private val delegate: DownloadTask, + private val removeCalls: MutableList, +) : DownloadTask { + override val taskId: String get() = delegate.taskId + override val request: DownloadRequest get() = delegate.request + override val createdAt = delegate.createdAt + override val state: StateFlow get() = delegate.state + override val segments: StateFlow> get() = delegate.segments + + override suspend fun pause() = delegate.pause() + override suspend fun resume(destination: Destination?) = + delegate.resume(destination) + override suspend fun cancel() = delegate.cancel() + override suspend fun setSpeedLimit(limit: SpeedLimit) = + delegate.setSpeedLimit(limit) + override suspend fun setPriority(priority: DownloadPriority) = + delegate.setPriority(priority) + override suspend fun setConnections(connections: Int) = + delegate.setConnections(connections) + override suspend fun reschedule( + schedule: DownloadSchedule, + conditions: List, + ) = delegate.reschedule(schedule, conditions) + override suspend fun remove(deleteFiles: Boolean) { + removeCalls.add(deleteFiles) + delegate.remove(deleteFiles) + } +} diff --git a/library/torrent/src/commonMain/kotlin/com/linroid/ketch/torrent/TorrentDownloadSource.kt b/library/torrent/src/commonMain/kotlin/com/linroid/ketch/torrent/TorrentDownloadSource.kt index 9369e58a..b4471e07 100644 --- a/library/torrent/src/commonMain/kotlin/com/linroid/ketch/torrent/TorrentDownloadSource.kt +++ b/library/torrent/src/commonMain/kotlin/com/linroid/ketch/torrent/TorrentDownloadSource.kt @@ -311,6 +311,34 @@ class TorrentDownloadSource( } } + override suspend fun cleanup( + context: DownloadContext, + resumeState: SourceResumeState?, + ) { + if (resumeState == null) { + log.d { "Skipping torrent cleanup: no resume state" } + return + } + val state = try { + Json.decodeFromString(resumeState.data) + } catch (e: CancellationException) { + throw e + } catch (e: Throwable) { + log.w(e) { "Skipping torrent cleanup: corrupt resume state" } + return + } + try { + getEngine().removeTorrent(state.infoHash, deleteFiles = true) + log.i { "Removed torrent and files: infoHash=${state.infoHash}" } + } catch (e: CancellationException) { + throw e + } catch (e: Throwable) { + log.w(e) { + "Failed to remove torrent infoHash=${state.infoHash}" + } + } + } + /** * Monitors torrent download progress and maps it to Ketch * segment progress until download completes or is canceled. diff --git a/library/torrent/src/commonTest/kotlin/com/linroid/ketch/torrent/TorrentDownloadSourceCleanupTest.kt b/library/torrent/src/commonTest/kotlin/com/linroid/ketch/torrent/TorrentDownloadSourceCleanupTest.kt new file mode 100644 index 00000000..56891c68 --- /dev/null +++ b/library/torrent/src/commonTest/kotlin/com/linroid/ketch/torrent/TorrentDownloadSourceCleanupTest.kt @@ -0,0 +1,124 @@ +package com.linroid.ketch.torrent + +import com.linroid.ketch.api.Destination +import com.linroid.ketch.api.DownloadRequest +import com.linroid.ketch.core.engine.DownloadContext +import com.linroid.ketch.core.engine.SourceResumeState +import com.linroid.ketch.core.file.FileAccessor +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.test.runTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +class TorrentDownloadSourceCleanupTest { + + private val infoHash = "aabbccddee11223344556677889900aabbccddee" + + private fun buildSource(engine: TorrentEngine): TorrentDownloadSource { + val source = TorrentDownloadSource() + source.engineFactory = { engine } + return source + } + + private object StubFileAccessor : FileAccessor { + override suspend fun writeAt(offset: Long, data: ByteArray) = Unit + override suspend fun flush() = Unit + override fun close() = Unit + override suspend fun delete() = Unit + override suspend fun size(): Long = 0 + override suspend fun preallocate(size: Long) = Unit + } + + private fun buildContext(): DownloadContext = DownloadContext( + taskId = "t", + url = "magnet:?xt=urn:btih:$infoHash", + request = DownloadRequest( + url = "magnet:?xt=urn:btih:$infoHash", + destination = Destination("/tmp/"), + ), + fileAccessor = StubFileAccessor, + segments = MutableStateFlow(emptyList()), + onProgress = { _, _ -> }, + throttle = { _ -> }, + headers = emptyMap(), + ) + + private fun resumeStateFor(infoHash: String): SourceResumeState = + TorrentDownloadSource.buildResumeState( + infoHash = infoHash, + totalBytes = 1024L, + resumeData = ByteArray(0), + selectedFileIds = setOf("0"), + savePath = "/tmp/torrent", + ) + + @Test + fun cleanup_callsRemoveTorrentWithDeleteFilesTrue() = runTest { + val engine = FakeTorrentEngine() + val source = buildSource(engine) + + source.cleanup(buildContext(), resumeStateFor(infoHash)) + + assertEquals(1, engine.removedTorrents.size) + assertEquals(infoHash, engine.removedTorrents[0].first) + assertEquals(true, engine.removedTorrents[0].second) + } + + @Test + fun cleanup_nullResumeState_isNoOp() = runTest { + val engine = FakeTorrentEngine() + val source = buildSource(engine) + + source.cleanup(buildContext(), resumeState = null) + + assertTrue(engine.removedTorrents.isEmpty()) + } + + @Test + fun cleanup_corruptResumeState_isNoOp() = runTest { + val engine = FakeTorrentEngine() + val source = buildSource(engine) + val corrupt = SourceResumeState("torrent", "not-json") + + source.cleanup(buildContext(), corrupt) + + assertTrue(engine.removedTorrents.isEmpty()) + } + + @Test + fun cleanup_engineThrows_swallowsException() = runTest { + val engine = ThrowingFakeEngine() + val source = buildSource(engine) + + // Must not propagate. + source.cleanup(buildContext(), resumeStateFor(infoHash)) + + assertEquals(1, engine.removeAttempts) + } + + private class ThrowingFakeEngine : TorrentEngine { + var removeAttempts = 0 + override val isRunning: Boolean = true + override suspend fun start() = Unit + override suspend fun stop() = Unit + override suspend fun fetchMetadata(magnetUri: String): TorrentMetadata? = null + override suspend fun addTorrent( + infoHash: String, + savePath: String, + magnetUri: String?, + torrentData: ByteArray?, + selectedFileIndices: Set, + resumeData: ByteArray?, + ): TorrentSession = error("not used") + override suspend fun removeTorrent( + infoHash: String, + deleteFiles: Boolean, + ) { + removeAttempts++ + throw RuntimeException("engine failure") + } + override fun setDownloadRateLimit(bytesPerSecond: Long) = Unit + override fun setUploadRateLimit(bytesPerSecond: Long) = Unit + } +}