From 02780c97e5e3f363580b28ddb56a4ed9ddfde442 Mon Sep 17 00:00:00 2001 From: Hadrian Burkhardt Date: Thu, 15 Oct 2026 09:06:24 +0200 Subject: [PATCH] Make delete, rename and move undoable A delete is no longer a removal: it is a move into a hidden .plainfiles-trash folder on the same volume, which costs a rename and keeps the item recoverable. The snackbar offers Undo for delete, rename and a pasted cut; the handle is cleared as soon as it is used. domain/Trash carries four deliberate limits: the trash lives on the users own volume, so it counts against their free space until it expires (24 hours, purged opportunistically, no startup hook and no empty-trash chore); a name taken again in the meantime is never overwritten, so an item returns beside it; a delete through Androids own dialog is not staged because the system owns it; and each item is staged in its own slot so two files sharing a name cannot overwrite each other on the way in. When a pasted move landing names cannot be established with confidence, no undo is offered rather than risking the wrong file moving back. --- CHANGELOG.md | 18 ++ .../filemanager/domain/Trash.kt | 195 +++++++++++++++++ .../filemanager/ui/browser/BrowserScreen.kt | 6 +- .../filemanager/ui/browser/BrowserUiState.kt | 26 +++ .../ui/browser/BrowserViewModel.kt | 201 +++++++++++++++++- app/src/main/res/values/strings.xml | 12 +- .../filemanager/domain/TrashTest.kt | 159 ++++++++++++++ .../ui/browser/BrowserViewModelTest.kt | 81 +++++++ 8 files changed, 685 insertions(+), 13 deletions(-) create mode 100644 app/src/main/java/de/softwareapp_hb/filemanager/domain/Trash.kt create mode 100644 app/src/test/java/de/softwareapp_hb/filemanager/domain/TrashTest.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index e06dc86..2eb78f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,24 @@ upload needs a **higher `versionCode`** than every previous upload. ### Added +- **Undo for delete, rename and move.** A delete is now a *move* into a hidden + `.plainfiles-trash` folder on the same volume rather than a removal, so the snackbar can + offer to put it back; a rename and a pasted cut record how to reverse themselves. Four + deliberate limits, all carried in `domain/Trash`: the trash sits on the user's own volume, + so it counts against their free space until it expires (24 hours, purged opportunistically + so there is no startup hook to forget and no "empty trash" chore); a name taken again in + the meantime is never overwritten, so an item returns *beside* it under a free name; a + delete that went through Android's own dialog is not staged, because the system owns that + one and its trash with it; and when a pasted move's landing names cannot be established + with confidence, no undo is offered at all rather than risking putting the wrong file + back. The undo handle is cleared the moment it is used, so a second tap cannot replay it. + +- **Keep the browsed folder and view settings across process death.** `SavedStateHandle` + carries the folder plus the sort key, direction, view mode and hidden-files setting. The + search query, the selection and the clipboard are deliberately *not* restored — a + restored query would hide the folder's contents on launch with no visible cause, and a + paste that survived a reboot would be a nasty surprise. + - **7z extraction.** Commons Compress already contained a pure-Java 7z reader (`SevenZFile`), so this needed no new native code and no library beyond the XZ one the `.xz` fix above required — the docs had simply assumed the format was off-limits and said diff --git a/app/src/main/java/de/softwareapp_hb/filemanager/domain/Trash.kt b/app/src/main/java/de/softwareapp_hb/filemanager/domain/Trash.kt new file mode 100644 index 0000000..21faac5 --- /dev/null +++ b/app/src/main/java/de/softwareapp_hb/filemanager/domain/Trash.kt @@ -0,0 +1,195 @@ +package de.softwareapp_hb.filemanager.domain + +import de.softwareapp_hb.filemanager.core.ConflictPolicy +import de.softwareapp_hb.filemanager.core.StoragePath +import de.softwareapp_hb.filemanager.data.StorageSource +import kotlinx.coroutines.currentCoroutineContext +import kotlinx.coroutines.isActive +import kotlin.coroutines.cancellation.CancellationException + +/** Where one trashed item came from, so it can be put back exactly where it was. */ +data class TrashedEntry(val original: StoragePath, val staged: StoragePath) + +/** One staged delete: everything [Trash.stage] moved aside, and where it went. */ +data class TrashBatch(val folder: StoragePath, val entries: List) + +/** + * A staging folder that makes a delete undoable. + * + * A delete cannot be reversed from the outside — once the bytes are gone they are gone — + * so a delete the app performs itself is really a *move* into a hidden folder on the same + * volume. Same-volume means the move is a rename rather than a copy, so it costs nothing + * and works on a multi-gigabyte file. + * + * Four deliberate decisions: + * + * - **Each item gets its own staging folder.** `///` means the item + * keeps its original name, so putting it back is exact, and two files called + * `report.pdf` from different folders can never collide on the way in. + * - **The trash sits on the user's volume**, so its contents count against their free + * space until they expire. That is the honest cost of a recoverable delete. The name + * starts with a dot — the same convention Android uses for `.nomedia` — so the folder is + * hidden from listings by default. + * - **Retention is a time window, not a bin to manage.** [purgeExpired] drops batches older + * than [RETENTION_HOURS]; the caller runs it after staging, so space comes back without + * the app needing a startup hook or the user needing to empty anything. + * - **A delete that went through the platform's own dialog is never staged.** Android owns + * that deletion, and its own trash with it, so it is not ours to reverse. + */ +class Trash( + private val source: StorageSource, + private val volumeRoot: StoragePath, + private val clock: () -> Long = System::currentTimeMillis, +) { + + /** The hidden folder batches live in, on this trash's volume. */ + fun root(): StoragePath = volumeRoot.child(TRASH_DIR_NAME) + + /** + * Move [paths] aside so the delete can be taken back. + * + * The returned batch is `null` when nothing could be staged, so a caller can never end + * up holding an undo handle for work that did not happen. + */ + suspend fun stage( + paths: List, + onProgress: (OperationProgress) -> Unit = {}, + ): Pair { + if (paths.isEmpty()) { + return null to OperationResult(OperationKind.DELETE, 0, emptyList(), false) + } + + val batch = root().child(BATCH_PREFIX + clock()) + val operations = FileOperations(source) + val staged = ArrayList() + val failures = ArrayList() + var cancelled = false + + try { + source.createDirectory(root()) + source.createDirectory(batch) + + for ((index, path) in paths.withIndex()) { + if (!currentCoroutineContext().isActive) { + cancelled = true + break + } + + val slot = batch.child(index.toString()) + source.createDirectory(slot) + val result = operations.move(listOf(path), slot, ConflictPolicy.KEEP_BOTH) + + if (result.succeeded) { + // The item keeps its own name inside its own slot, so this is exact. + staged += TrashedEntry(original = path, staged = slot.child(path.name)) + } else { + failures += result.failures + // Nothing landed here, so do not leave an empty slot behind. + source.delete(slot) + } + onProgress(OperationProgress(OperationKind.DELETE, index + 1, paths.size, path)) + } + } catch (cancellation: CancellationException) { + throw cancellation + } catch (failure: Exception) { + return null to OperationResult( + OperationKind.DELETE, + 0, + listOf(OperationFailure(paths.first(), failure.message ?: "Could not be moved aside")), + false, + ) + } + + val outcome = OperationResult(OperationKind.DELETE, staged.size, failures, cancelled) + return batch.takeIf { staged.isNotEmpty() }?.let { TrashBatch(it, staged) } to outcome + } + + /** + * Put a staged batch back where it came from. + * + * A name that has been taken again in the meantime is not overwritten: the item comes + * back alongside it under a free name, because losing a *new* file in order to undo an + * old delete would be a poor trade. + */ + suspend fun restore( + batch: TrashBatch, + onProgress: (OperationProgress) -> Unit = {}, + ): OperationResult { + val operations = FileOperations(source) + val failures = ArrayList() + var restored = 0 + + for ((index, entry) in batch.entries.withIndex()) { + if (!currentCoroutineContext().isActive) { + return OperationResult(OperationKind.MOVE, restored, failures, cancelled = true) + } + + val parent = entry.original.parent + val result = when { + parent == null || !source.exists(entry.staged) -> OperationResult( + OperationKind.MOVE, + 0, + listOf(OperationFailure(entry.original, "It is no longer in the trash")), + false, + ) + + else -> operations.move(listOf(entry.staged), parent, ConflictPolicy.KEEP_BOTH) + } + + if (result.succeeded) { + restored++ + onProgress(OperationProgress(OperationKind.MOVE, index + 1, batch.entries.size, entry.original)) + } else { + failures += result.failures + } + } + + return OperationResult(OperationKind.MOVE, restored, failures, false) + } + + /** Remove batches older than [RETENTION_HOURS]. Returns how many were dropped. */ + suspend fun purgeExpired(): Int { + val root = root() + if (!source.exists(root)) return 0 + + val expiry = clock() - RETENTION_MILLIS + var purged = 0 + for (child in source.list(root)) { + val stagedAt = batchTimestamp(child.name) ?: continue + if (stagedAt >= expiry) continue + if (source.delete(child.path)) purged++ + } + return purged + } + + /** + * The epoch millis a batch folder was created, or `null` when the name is not one of + * ours. + * + * The age is carried in the name rather than read from the folder's timestamp, which + * keeps expiry a pure function that a test can drive without touching the clock. + */ + fun batchTimestamp(folderName: String): Long? = folderName + .takeIf { it.startsWith(BATCH_PREFIX) } + ?.removePrefix(BATCH_PREFIX) + ?.toLongOrNull() + + companion object { + /** Leading dot, so the folder is hidden from listings exactly like `.nomedia`. */ + const val TRASH_DIR_NAME = ".plainfiles-trash" + + /** + * The trash a batch belongs to, derived from where the batch actually sits — + * `/.plainfiles-trash/batch-n` — so a restore needs no volume lookup. + */ + fun of(source: StorageSource, batch: TrashBatch): Trash = + Trash(source, batch.folder.parent?.parent ?: StoragePath.Root) + + private const val BATCH_PREFIX = "batch-" + + /** How long a staged delete stays recoverable. */ + const val RETENTION_HOURS = 24L + + const val RETENTION_MILLIS = RETENTION_HOURS * 60L * 60L * 1000L + } +} diff --git a/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserScreen.kt b/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserScreen.kt index db3bcfc..8c35e1d 100644 --- a/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserScreen.kt +++ b/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserScreen.kt @@ -37,6 +37,7 @@ import androidx.compose.material3.OutlinedTextField import androidx.compose.material3.Scaffold import androidx.compose.material3.SnackbarHost import androidx.compose.material3.SnackbarHostState +import androidx.compose.material3.SnackbarResult import androidx.compose.material3.Surface import androidx.compose.material3.Text import androidx.compose.material3.TopAppBar @@ -151,9 +152,12 @@ fun BrowserScreen(viewModel: BrowserViewModel = viewModel(factory = BrowserViewM pluralStringResource(it.textRes, quantity, *it.args.toTypedArray()) } ?: stringResource(it.textRes, *it.args.toTypedArray()) } + // Resolved out here because `stringResource` is a composable call and the effect is not. + val undoLabel = message?.actionLabelRes?.let { stringResource(it) } LaunchedEffect(message?.id) { if (messageText != null) { - snackbarHostState.showSnackbar(messageText) + val outcome = snackbarHostState.showSnackbar(messageText, actionLabel = undoLabel) + if (outcome == SnackbarResult.ActionPerformed) viewModel.undo() viewModel.consumeMessage() } } diff --git a/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserUiState.kt b/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserUiState.kt index 052ce99..bbbd0ec 100644 --- a/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserUiState.kt +++ b/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserUiState.kt @@ -8,6 +8,7 @@ import de.softwareapp_hb.filemanager.core.FileEntry import de.softwareapp_hb.filemanager.core.SortSpec import de.softwareapp_hb.filemanager.core.StoragePath import de.softwareapp_hb.filemanager.data.StorageAccessMode +import de.softwareapp_hb.filemanager.domain.TrashBatch import de.softwareapp_hb.filemanager.domain.UsageNode enum class ViewMode { LIST, GRID } @@ -24,6 +25,8 @@ data class UiMessage( val args: List = emptyList(), /** Set when [textRes] names a `` resource; the UI resolves it accordingly. */ val pluralQuantity: Int? = null, + /** When set, the snackbar offers a single action button carrying this label. */ + @StringRes val actionLabelRes: Int? = null, val id: Long = System.nanoTime(), ) @@ -64,6 +67,27 @@ data class DetailFocus( val loading: Boolean = false, ) +/** + * The last change to the filesystem the app made itself, and how to take it back. + * + * One step deep — an undo stack would be a feature in its own right — and never set for + * work Android performed: a delete that went through the platform's own dialog is the + * system's to reverse, not ours. + */ +sealed interface Undo { + /** + * Move each item back where it came from. Covers a rename and a pasted cut alike, + * because both are the same operation seen from different angles. + */ + data class Relocate(val moves: List) : Undo + + /** Bring a staged delete back out of the trash. */ + data class Trashed(val batch: TrashBatch) : Undo +} + +/** [from] is where the item is now; [to] is the full path it had before. */ +data class UndoMove(val from: StoragePath, val to: StoragePath) + /** A full-screen view the browser hands over to. */ sealed interface Overlay { data object None : Overlay @@ -110,6 +134,8 @@ data class BrowserUiState( /** Set when the system should confirm a delete; the screen launches it and clears it. */ val pendingDeleteRequest: IntentSenderRequest? = null, val conflict: ConflictPrompt? = null, + /** The last destructive change the app made, waiting to be taken back. */ + val undo: Undo? = null, /** A viewer, editor or the storage-usage view taking over the screen. */ val overlay: Overlay = Overlay.None, ) { diff --git a/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserViewModel.kt b/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserViewModel.kt index 189ece5..a17c3ce 100644 --- a/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserViewModel.kt +++ b/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserViewModel.kt @@ -25,18 +25,23 @@ import de.softwareapp_hb.filemanager.core.TransferAction import de.softwareapp_hb.filemanager.data.DeleteRequestFactory import de.softwareapp_hb.filemanager.data.StorageAccessMode import de.softwareapp_hb.filemanager.data.StorageLocation +import de.softwareapp_hb.filemanager.data.StorageSource import de.softwareapp_hb.filemanager.domain.ArchiveEngine import de.softwareapp_hb.filemanager.domain.FileOperations +import de.softwareapp_hb.filemanager.domain.OperationFailure import de.softwareapp_hb.filemanager.domain.OperationKind import de.softwareapp_hb.filemanager.domain.OperationNotifier import de.softwareapp_hb.filemanager.domain.OperationResult +import de.softwareapp_hb.filemanager.domain.Trash import de.softwareapp_hb.filemanager.domain.UsageAnalysis import de.softwareapp_hb.filemanager.domain.UsageNode import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.currentCoroutineContext import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.update +import kotlinx.coroutines.isActive import kotlinx.coroutines.launch /** @@ -529,6 +534,11 @@ class BrowserViewModel( _state.update { it.copy(loading = true) } notifier.start(kind) + + // A move is destructive, so how to reverse it is worked out by diffing the target's + // listing either side of the operation — see [matchLanded]. + val landedBefore = if (cut) targetNames(source, target) else emptySet() + val result: OperationResult = try { if (cut) { operations.move(paths, target, policy) { notifier.update(it) } @@ -543,7 +553,14 @@ class BrowserViewModel( _state.update { it.copy(loading = false, clipboard = if (cut) null else it.clipboard) } if (result.succeeded) { - messagePlural(if (cut) R.plurals.message_moved else R.plurals.message_copied, result.completed) + if (cut) { + matchLanded(target, landedBefore, targetNames(source, target), paths)?.let { moves -> + _state.update { it.copy(undo = Undo.Relocate(moves)) } + } + messageUndoablePlural(R.plurals.message_moved, result.completed) + } else { + messagePlural(R.plurals.message_copied, result.completed) + } } else { report(result) } @@ -552,6 +569,78 @@ class BrowserViewModel( if (here == null) refresh() else load(here) } + // --------------------------------------------------------------------- undo + + /** + * Take back the last destructive change. + * + * The handle is cleared the moment it is used, so a second tap on the same snackbar + * cannot replay it against files that have already moved. + */ + fun undo() { + val pending = _state.value.undo ?: return + val source = storage.activeSource() ?: return + _state.update { it.copy(undo = null) } + + viewModelScope.launch { + _state.update { it.copy(loading = true) } + notifier.start(OperationKind.MOVE) + val result = try { + when (pending) { + is Undo.Relocate -> relocateBack(source, pending.moves) + + is Undo.Trashed -> + Trash.of(source, pending.batch).restore(pending.batch) { notifier.update(it) } + } + } catch (cancellation: CancellationException) { + throw cancellation + } finally { + notifier.stop() + } + _state.update { it.copy(loading = false) } + + if (result.succeeded) { + messagePlural(R.plurals.message_undone, result.completed) + } else { + report(result) + } + + val here = _state.value.path + if (here == null) refresh() else load(here) + } + } + + /** Move each item in [moves] back to the exact path it came from. */ + private suspend fun relocateBack(source: StorageSource, moves: List): OperationResult { + val operations = FileOperations(source) + val failures = ArrayList() + var restored = 0 + + for (move in moves) { + if (!currentCoroutineContext().isActive) { + return OperationResult(OperationKind.MOVE, restored, failures, cancelled = true) + } + + val home = move.to.parent + val done = when { + // The exact path is what undo means: the old *name* matters, not just the + // old folder, which is why this is a rename rather than a move. + runCatching { operations.rename(move.from, move.to) }.getOrDefault(false) -> true + + // The old name has been taken again meanwhile. Come back alongside it rather + // than overwriting whatever holds it now. + home != null -> + operations.move(listOf(move.from), home, ConflictPolicy.KEEP_BOTH).succeeded + + else -> false + } + + if (done) restored++ else failures += OperationFailure(move.to, "It could not be put back") + } + + return OperationResult(OperationKind.MOVE, restored, failures, false) + } + // ---------------------------------------------------------------- mutations fun createFolder(name: String) { @@ -576,7 +665,19 @@ class BrowserViewModel( viewModelScope.launch { val renamed = runCatching { FileOperations(source).rename(path, parent.child(cleanName)) } .getOrDefault(false) - message(if (renamed) R.string.message_renamed else R.string.message_name_in_use) + if (renamed) { + // A rename is a move within a single folder, so it needs no separate shape. + _state.update { + it.copy( + undo = Undo.Relocate( + listOf(UndoMove(from = parent.child(cleanName), to = path)), + ), + ) + } + messageUndoable(R.string.message_renamed) + } else { + message(R.string.message_name_in_use) + } _state.value.path?.let { load(it) } } } @@ -611,31 +712,91 @@ class BrowserViewModel( if (here == null) refresh() else load(here) } + /** + * Delete by moving into the trash, so the delete can be taken back. + * + * Nothing is actually removed here: [Trash.stage] relocates the items into a hidden + * folder on the same volume, which costs a rename and keeps them recoverable. Expiry is + * time-based and handled by [Trash], so the space returns on its own and the user never + * has to empty anything. + */ private suspend fun deleteDirectly(paths: List) { val source = storage.activeSource() ?: return - val operations = FileOperations(source) + val trash = Trash(source, volumeRootFor(paths)) _state.update { it.copy(loading = true, selection = emptySet()) } notifier.start(OperationKind.DELETE) - val result = try { - operations.delete(paths) { notifier.update(it) } + val (batch, result) = try { + trash.stage(paths) { notifier.update(it) } } catch (cancellation: CancellationException) { throw cancellation } finally { notifier.stop() } - _state.update { it.copy(loading = false) } + _state.update { it.copy(loading = false, undo = batch?.let(Undo::Trashed)) } if (result.succeeded) { - messagePlural(R.plurals.message_deleted, result.completed) + messageUndoablePlural(R.plurals.message_deleted_undo, result.completed) } else { report(result) } val here = _state.value.path if (here == null) refresh() else load(here) + + // Whatever has expired goes now, so there is no startup hook to forget and no chore + // for the user. Nothing here can fail the delete that just succeeded. + runCatching { trash.purgeExpired() } } + // ------------------------------------------------------ undo bookkeeping + + /** + * Pair each moved item with the name it actually landed under, or `null` when that + * cannot be established with confidence. + * + * The engine renames an incoming item when the target already has that name, so where + * something ended up has to be *discovered* by diffing the target listing rather than + * assumed from its original name. An exact name match is preferred; whatever is left + * over belongs to the renamed item. If the counts do not line up, this gives up + * entirely — silently moving the wrong file back would be far worse than not offering + * an undo at all. + */ + private fun matchLanded( + target: StoragePath, + before: Set, + after: Set, + originals: List, + ): List? { + val added = after.filterNot { it in before } + if (added.size != originals.size) return null + + val unclaimed = added.toMutableList() + val moves = ArrayList(originals.size) + for (original in originals) { + val landed = unclaimed.firstOrNull { it == original.name } + ?: unclaimed.firstOrNull() + ?: return null + unclaimed.remove(landed) + moves += UndoMove(from = target.child(landed), to = original) + } + return moves + } + + private suspend fun targetNames(source: StorageSource, target: StoragePath): Set = + runCatching { source.list(target).mapTo(HashSet()) { it.name } }.getOrDefault(emptySet()) + + /** + * The volume holding [paths], so the trash lands on the same filesystem as the items + * and the move into it is a rename rather than a copy. The deepest matching root wins, + * because one volume root can sit inside another. + */ + private fun volumeRootFor(paths: List): StoragePath = storage.volumes() + .map { it.root } + .filter { root -> paths.any { it == root || it.isDescendantOf(root) } } + .maxByOrNull { it.segments.size } + ?: StoragePath.Root + // ----------------------------------------------------------------- messages fun consumeMessage() { @@ -650,11 +811,33 @@ class BrowserViewModel( } private fun message(@StringRes textRes: Int, vararg args: Any) { - _state.update { it.copy(message = UiMessage(textRes, args.toList())) } + postMessage(UiMessage(textRes, args.toList())) } private fun messagePlural(@PluralsRes textRes: Int, count: Int) { - _state.update { it.copy(message = UiMessage(textRes, listOf(count), pluralQuantity = count)) } + postMessage(UiMessage(textRes, listOf(count), pluralQuantity = count)) + } + + /** A message whose snackbar also offers to take the change back. */ + private fun messageUndoable(@StringRes textRes: Int, vararg args: Any) { + postMessage( + UiMessage(textRes, args.toList(), actionLabelRes = R.string.browser_action_undo), + ) + } + + private fun messageUndoablePlural(@PluralsRes textRes: Int, count: Int) { + postMessage( + UiMessage( + textRes, + listOf(count), + pluralQuantity = count, + actionLabelRes = R.string.browser_action_undo, + ), + ) + } + + private fun postMessage(next: UiMessage) { + _state.update { it.copy(message = next) } } companion object { diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 3dacba7..db43c93 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -54,6 +54,7 @@ Share Select all Clear selection + Undo More options Refresh Cancel @@ -130,9 +131,14 @@ Moved %1$d item Moved %1$d items - - Deleted %1$d item - Deleted %1$d items + + + Moved %1$d item to the trash + Moved %1$d items to the trash + + + Put %1$d item back + Put %1$d items back %1$d item could not be processed diff --git a/app/src/test/java/de/softwareapp_hb/filemanager/domain/TrashTest.kt b/app/src/test/java/de/softwareapp_hb/filemanager/domain/TrashTest.kt new file mode 100644 index 0000000..271b286 --- /dev/null +++ b/app/src/test/java/de/softwareapp_hb/filemanager/domain/TrashTest.kt @@ -0,0 +1,159 @@ +package de.softwareapp_hb.filemanager.domain + +import de.softwareapp_hb.filemanager.core.StoragePath +import de.softwareapp_hb.filemanager.domain.testutil.InMemoryStorageSource +import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The trash is what makes a delete undoable, so these tests care about the two things that + * could silently lose a file: a name collision on the way in, and a collision on the way + * back out. + */ +class TrashTest { + + private val volume = StoragePath.of("/storage/emulated/0") + private val docs = volume.child("Documents") + private val notes = volume.child("notes.txt") + + private fun storage() = InMemoryStorageSource().apply { + addDirectory(volume) + addDirectory(docs) + addFile(notes, "hello") + addFile(docs.child("report.pdf"), "report") + } + + private fun trash(source: InMemoryStorageSource, at: Long = TRASHED_AT) = + Trash(source, volume, clock = { at }) + + @Test + fun `staging moves an item aside and records where it came from`() = runBlocking { + val storage = storage() + + val (batch, result) = trash(storage).stage(listOf(notes)) + + assertTrue(result.succeeded) + assertNotNull(batch) + assertFalse("the item is no longer where it was", storage.contains(notes)) + assertEquals(notes, batch!!.entries.single().original) + assertEquals("hello", storage.contentOf(batch.entries.single().staged)) + assertTrue(batch.entries.single().staged.isDescendantOf(batch.folder)) + } + + @Test + fun `restoring puts an item back exactly where it was`() = runBlocking { + val storage = storage() + val (batch, _) = trash(storage).stage(listOf(notes)) + + val result = trash(storage).restore(batch!!) + + assertTrue(result.succeeded) + assertEquals(1, result.completed) + assertTrue(storage.contains(notes)) + assertEquals("hello", storage.contentOf(notes)) + } + + @Test + fun `a folder comes back whole`() = runBlocking { + val storage = storage() + val (batch, _) = trash(storage).stage(listOf(docs)) + assertFalse(storage.contains(docs)) + + trash(storage).restore(batch!!) + + assertTrue(storage.contains(docs)) + assertEquals("report", storage.contentOf(docs.child("report.pdf"))) + } + + @Test + fun `two items sharing a name stage without one overwriting the other`() = runBlocking { + val storage = storage() + val other = volume.child("Backups") + storage.addDirectory(other) + storage.addFile(other.child("notes.txt"), "other notes") + + val (batch, result) = trash(storage).stage(listOf(notes, other.child("notes.txt"))) + + assertTrue(result.succeeded) + assertEquals(2, batch!!.entries.size) + // Each item gets its own slot, so neither has to be renamed on the way in. + assertEquals("hello", storage.contentOf(batch.entries[0].staged)) + assertEquals("other notes", storage.contentOf(batch.entries[1].staged)) + } + + @Test + fun `restoring never overwrites a name that was taken again meanwhile`() = runBlocking { + val storage = storage() + val (batch, _) = trash(storage).stage(listOf(notes)) + storage.addFile(notes, "a new file holding the old name") + + trash(storage).restore(batch!!) + + // The new file survives untouched, and the old one came back beside it. + assertEquals("a new file holding the old name", storage.contentOf(notes)) + val beside = storage.list(volume).map { it.name } + .filter { it.startsWith("notes") && it != notes.name } + assertEquals(1, beside.size) + } + + @Test + fun `expiry drops only the batches that are old enough`() = runBlocking { + val storage = storage() + val (old, _) = trash(storage, at = 0L).stage(listOf(notes)) + + // Put it back, so there is something left to stage a second time. + trash(storage, at = 0L).restore(old!!) + val (fresh, _) = trash(storage, at = Trash.RETENTION_MILLIS + 1).stage(listOf(notes)) + + val purged = trash(storage, at = Trash.RETENTION_MILLIS + 2).purgeExpired() + + assertEquals(1, purged) + assertFalse(storage.contains(old.folder)) + assertTrue("the recent batch stays recoverable", storage.contains(fresh!!.folder)) + } + + @Test + fun `staging nothing yields no undo handle`() = runBlocking { + val (batch, result) = trash(storage()).stage(emptyList()) + + assertNull(batch) + assertEquals(0, result.completed) + assertTrue(result.succeeded) + } + + @Test + fun `restoring a batch that is already gone is reported, not thrown`() = runBlocking { + val storage = storage() + val (batch, _) = trash(storage).stage(listOf(notes)) + storage.delete(batch!!.folder) + + val result = trash(storage).restore(batch) + + assertFalse(result.succeeded) + assertEquals(1, result.failures.size) + } + + @Test + fun `a batch folder is recognised by its name, and nothing else is`() { + val trash = trash(storage()) + + assertEquals(1234L, trash.batchTimestamp("batch-1234")) + assertNull(trash.batchTimestamp("holiday-photos")) + assertNull(trash.batchTimestamp("batch-not-a-number")) + } + + @Test + fun `the trash folder is hidden, the way Android hides dotfiles`() { + assertTrue(Trash.TRASH_DIR_NAME.startsWith(".")) + assertEquals(volume.child(Trash.TRASH_DIR_NAME), trash(storage()).root()) + } + + private companion object { + const val TRASHED_AT = 1_700_000_000_000L + } +} diff --git a/app/src/test/java/de/softwareapp_hb/filemanager/ui/browser/BrowserViewModelTest.kt b/app/src/test/java/de/softwareapp_hb/filemanager/ui/browser/BrowserViewModelTest.kt index aaa1525..98b4443 100644 --- a/app/src/test/java/de/softwareapp_hb/filemanager/ui/browser/BrowserViewModelTest.kt +++ b/app/src/test/java/de/softwareapp_hb/filemanager/ui/browser/BrowserViewModelTest.kt @@ -440,4 +440,85 @@ class BrowserViewModelTest { } assertTrue(viewModel(savedState = saved).state.value.isRootView) } + + // -------------------------------------------------------------------- undo + + @Test + fun `a delete only moves the item aside, so it can be taken back`() { + val model = viewModel() + model.openDirectory(downloads) + + model.delete(listOf(downloads.child("a.txt"))) + + assertFalse(storage.contains(downloads.child("a.txt"))) + assertTrue(model.state.value.undo is Undo.Trashed) + } + + @Test + fun `undoing a delete puts the file back`() { + val model = viewModel() + model.openDirectory(downloads) + model.delete(listOf(downloads.child("a.txt"))) + + model.undo() + + assertTrue(storage.contains(downloads.child("a.txt"))) + assertEquals("first", storage.contentOf(downloads.child("a.txt"))) + assertNull(model.state.value.undo) + } + + @Test + fun `undoing a rename puts the old name back`() { + val model = viewModel() + model.openDirectory(downloads) + model.rename(downloads.child("a.txt"), "renamed.txt") + assertTrue(storage.contains(downloads.child("renamed.txt"))) + + model.undo() + + assertTrue(storage.contains(downloads.child("a.txt"))) + assertFalse(storage.contains(downloads.child("renamed.txt"))) + } + + @Test + fun `undoing a cut-paste moves the item back`() { + val model = viewModel() + model.openDirectory(downloads) + model.copyToClipboard(listOf(downloads.child("a.txt")), cut = true) + model.openDirectory(documents) + model.paste() + assertTrue(storage.contains(documents.child("a.txt"))) + + model.undo() + + assertTrue(storage.contains(downloads.child("a.txt"))) + assertFalse(storage.contains(documents.child("a.txt"))) + } + + @Test + fun `a copy is not undoable, because nothing was changed`() { + val model = viewModel() + model.openDirectory(root) + model.copyToClipboard(listOf(downloads.child("a.txt")), cut = false) + model.openDirectory(documents) + + model.paste() + + assertNull(model.state.value.undo) + } + + @Test + fun `the undo handle is cleared once it has been used`() { + val model = viewModel() + model.openDirectory(downloads) + model.rename(downloads.child("a.txt"), "renamed.txt") + + model.undo() + assertNull(model.state.value.undo) + + // A second tap on the same snackbar must not replay it. + model.undo() + assertTrue(storage.contains(downloads.child("a.txt"))) + assertFalse(storage.contains(downloads.child("renamed.txt"))) + } }