diff --git a/CHANGELOG.md b/CHANGELOG.md index cc9177a..9856ea0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,19 @@ All notable changes to the Sheaf Android client are recorded here. Format loosel follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); the project uses semantic versioning (`MAJOR.MINOR.PATCH`). +## [Unreleased] + +### Fixed + +- **"Delete unused files" no longer deletes files that are in use.** It worked + out which uploads were unused by looking at your system avatar and your + members' avatars, and nothing else, so banners, any image inside a bio or a + journal entry, and anything your edit history still holds were all listed as + unused and deleted on confirmation. The app now asks the server, which checks + all of those places, and still deletes one file at a time so System Safety's + image safeguard applies. If you lost an image this way it cannot be recovered + from the app. + ## [1.6.0] - 2026-10-23 ### Added diff --git a/sheaf/app/src/main/java/systems/lupine/sheaf/data/api/SheafApiService.kt b/sheaf/app/src/main/java/systems/lupine/sheaf/data/api/SheafApiService.kt index 41be0eb..a3dedc8 100644 --- a/sheaf/app/src/main/java/systems/lupine/sheaf/data/api/SheafApiService.kt +++ b/sheaf/app/src/main/java/systems/lupine/sheaf/data/api/SheafApiService.kt @@ -535,6 +535,22 @@ interface SheafApiService { @GET("/v1/files/list") suspend fun listFiles(): List + /** + * Which files the server considers unused, without deleting anything. + * + * Asked rather than worked out locally: the server checks avatars, banners, + * bios, journal entries and revision history, and a client that reimplements + * that list gets it wrong the moment the server learns a new place to + * reference a file. + * + * Deliberately the dry run rather than `POST /v1/files/cleanup`. The real + * cleanup deletes server-side in one shot and skips System Safety's image + * category; deleting the files one by one from here keeps that safeguard, + * which is the one thing this client was already doing right. + */ + @POST("/v1/files/cleanup/dry-run") + suspend fun previewFileCleanup(): FileCleanupPreview + @HTTP(method = "DELETE", path = "/v1/files/{id}", hasBody = true) suspend fun deleteFile( @Path("id") id: String, diff --git a/sheaf/app/src/main/java/systems/lupine/sheaf/data/model/Models.kt b/sheaf/app/src/main/java/systems/lupine/sheaf/data/model/Models.kt index baefb7d..c316e0c 100644 --- a/sheaf/app/src/main/java/systems/lupine/sheaf/data/model/Models.kt +++ b/sheaf/app/src/main/java/systems/lupine/sheaf/data/model/Models.kt @@ -992,6 +992,25 @@ data class FileUsage( @Json(name = "file_count") val fileCount: Int, ) +/** + * What `POST /v1/files/cleanup/dry-run` would delete. + * + * The server owns this question. Working it out on the client means knowing + * every place a file can be referenced, and this app knew about two of them: + * it counted avatars and nothing else, so banners, every image embedded in a + * bio or a journal entry, and everything held by revision history came back as + * unused. + * + * [keys] is absent when nothing is orphaned, hence the default. + */ +@JsonClass(generateAdapter = true) +data class FileCleanupPreview( + val orphaned: Int = 0, + @Json(name = "freed_bytes") val freedBytes: Long = 0, + @Json(name = "dry_run") val dryRun: Boolean = true, + val keys: List = emptyList(), +) + @JsonClass(generateAdapter = true) data class FileRead( val id: String, diff --git a/sheaf/app/src/main/java/systems/lupine/sheaf/ui/settings/SettingsViewModel.kt b/sheaf/app/src/main/java/systems/lupine/sheaf/ui/settings/SettingsViewModel.kt index 102e0b9..c9822d1 100644 --- a/sheaf/app/src/main/java/systems/lupine/sheaf/ui/settings/SettingsViewModel.kt +++ b/sheaf/app/src/main/java/systems/lupine/sheaf/ui/settings/SettingsViewModel.kt @@ -80,6 +80,23 @@ data class SettingsUiState( private const val CLIENT_ID = "android" +/** + * The listed files the server named as unused. + * + * Matched on [FileRead.key], which is what the server reports, and not on the + * serve URL: the URL depends on how the instance hands files out, so a CDN or + * signed-URL instance would match nothing and quietly report that everything + * is unused. + * + * The rows come from the listing rather than being built from the keys so each + * keeps its size and date, and so it can be deleted through the per-file path + * that honours System Safety. + */ +internal fun orphanFiles(files: List, orphanKeys: List): List { + val keys = orphanKeys.toSet() + return files.filter { it.key in keys } +} + @HiltViewModel class SettingsViewModel @Inject constructor( private val api: SheafApiService, @@ -508,14 +525,12 @@ class SettingsViewModel @Inject constructor( _state.update { it.copy(isCheckingFiles = true, fileError = null, orphanedFiles = null) } viewModelScope.launch { runCatching { - val files = api.listFiles() - val members = api.listMembers() - val system = api.getOwnSystem() - val usedUrls = buildSet { - system.avatarUrl?.let { add(it) } - members.forEach { m -> m.avatarUrl?.let { add(it) } } - } - files.filter { it.url !in usedUrls } + // The server decides what is unused. This used to be worked out + // here from the system avatar and member avatars alone, which + // meant banners, images embedded in bios and journal entries, + // and anything revision history still holds were all reported + // as unused, and the button then deleted them. + orphanFiles(api.listFiles(), api.previewFileCleanup().keys) }.onSuccess { orphans -> _state.update { it.copy(isCheckingFiles = false, orphanedFiles = orphans) } }.onFailure { e -> diff --git a/sheaf/app/src/test/java/systems/lupine/sheaf/ui/settings/OrphanFilesTest.kt b/sheaf/app/src/test/java/systems/lupine/sheaf/ui/settings/OrphanFilesTest.kt new file mode 100644 index 0000000..636f07b --- /dev/null +++ b/sheaf/app/src/test/java/systems/lupine/sheaf/ui/settings/OrphanFilesTest.kt @@ -0,0 +1,90 @@ +package systems.lupine.sheaf.ui.settings + +import systems.lupine.sheaf.data.model.FileCleanupPreview +import systems.lupine.sheaf.data.model.FileRead +import systems.lupine.sheaf.di.NetworkModule +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * Which files "Delete unused files" offers up. + * + * This used to be decided here, from the system avatar and member avatars and + * nothing else, so a banner, an image embedded in a bio or a journal entry, and + * anything revision history still held all came back as unused. The button then + * deleted them, and the server does no reference check of its own on a delete. + * + * The answer now comes from the server, which checks all of those places. These + * tests cover the matching, which is the part still done here, and the two + * response shapes the preview comes back as. + */ +class OrphanFilesTest { + + private fun file(id: String, key: String, url: String = "/v1/files/$key") = FileRead( + id = id, + key = key, + url = url, + purpose = "avatar", + contentType = "image/png", + sizeBytes = 1, + createdAt = "2026-01-01T00:00:00Z", + ) + + @Test fun `only the files the server named come back`() { + val files = listOf( + file("1", "avatars/sys/a.png"), + file("2", "banners/sys/b.png"), + file("3", "bios/sys/c.png"), + ) + val orphans = orphanFiles(files, listOf("bios/sys/c.png")) + assertEquals(listOf("3"), orphans.map { it.id }) + } + + @Test fun `a banner the system is using is not offered for deletion`() { + // The case found in the field: the account's only two files were a + // member avatar and the live system banner, and the old rule flagged + // the banner. The server reports no orphans at all here. + val files = listOf(file("1", "avatars/sys/a.png"), file("2", "banners/sys/b.png")) + assertTrue(orphanFiles(files, emptyList()).isEmpty()) + } + + @Test fun `matching is on the key, not the serve URL`() { + // The old rule compared serve URLs. How a file is served depends on the + // instance: a CDN or a signed URL means the listing's url does not look + // like anything the server reported, so url matching would come back + // with either nothing or everything. + val files = listOf( + file("1", "avatars/sys/a.png", url = "https://cdn.example.net/avatars/sys/a.png?token=x"), + ) + assertEquals(listOf("1"), orphanFiles(files, listOf("avatars/sys/a.png")).map { it.id }) + } + + @Test fun `a key the listing does not have is ignored`() { + // The two calls are not atomic, so the preview can name a file that has + // since gone. Nothing to delete rather than an invented row. + assertTrue(orphanFiles(listOf(file("1", "a.png")), listOf("gone.png")).isEmpty()) + } + + @Test fun `the no-orphans response has no keys at all`() { + // The server omits `keys` entirely when nothing is orphaned, so this + // has to decode to an empty list rather than failing or reading null. + val preview = NetworkModule.provideMoshi() + .adapter(FileCleanupPreview::class.java) + .fromJson("""{"orphaned":0,"freed_bytes":0,"dry_run":true}""")!! + assertEquals(0, preview.orphaned) + assertTrue(preview.keys.isEmpty()) + } + + @Test fun `a preview with orphans carries their keys`() { + val preview = NetworkModule.provideMoshi() + .adapter(FileCleanupPreview::class.java) + .fromJson( + """{"orphaned":2,"freed_bytes":2048,"dry_run":true,""" + + """"keys":["bios/sys/c.png","journals/sys/d.png"]}""", + )!! + assertEquals(2, preview.orphaned) + assertEquals(2048L, preview.freedBytes) + assertEquals(listOf("bios/sys/c.png", "journals/sys/d.png"), preview.keys) + } +}