From e9a13034c0f609b71f521ddc24e22d2b13b8e74b Mon Sep 17 00:00:00 2001 From: Ocnrb Date: Tue, 6 Oct 2026 17:23:14 +0100 Subject: [PATCH] Keep last-message previews per account - A DM's preview key is the peer's inbox, so two accounts talking to the same peer read each other's decrypted last message from one device-wide file - A fetch or scan that started under another account no longer lands in this one - The old device-wide file is deleted on first start: channels get their preview back when read again, and a DM only from a message still in its inbox or the peer's next one Co-authored-by: Claude Opus 5.5 --- .../java/com/pombo/android/AppViewModel.kt | 5 + .../java/com/pombo/android/ChannelManager.kt | 3 +- .../pombo/android/core/LatestMessageStore.kt | 92 +++++++++-- .../pombo/android/DeleteAccountWiringTest.kt | 2 +- .../android/core/LatestMessageStoreTest.kt | 152 ++++++++++++++++++ 5 files changed, 236 insertions(+), 18 deletions(-) create mode 100644 app/src/test/java/com/pombo/android/core/LatestMessageStoreTest.kt diff --git a/app/src/main/java/com/pombo/android/AppViewModel.kt b/app/src/main/java/com/pombo/android/AppViewModel.kt index 15a95ae..8f3289e 100644 --- a/app/src/main/java/com/pombo/android/AppViewModel.kt +++ b/app/src/main/java/com/pombo/android/AppViewModel.kt @@ -239,6 +239,7 @@ class AppViewModel(app: Application) : AndroidViewModel(app), PomboBridge.Listen pushRegistry.clearAccount() walletTokenStore.clearAccount(store.address) blobStore.clearAccount() + previewStore.clearAccount() disconnect() toast("Account deleted", com.pombo.android.ui.ToastKind.INFO) } @@ -1643,6 +1644,9 @@ class AppViewModel(app: Application) : AndroidViewModel(app), PomboBridge.Listen failedOutbox.memoryOnly = guest blobStore.scopeAddress = address blobStore.memoryOnly = guest + previewStore.scopeAddress = address + previewStore.memoryOnly = guest + viewModelScope.launch { previewStore.warmUp() } sentReactionsStore.scopeAddress = address sentReactionsStore.memoryOnly = guest pushRegistry.scopeAddress = address @@ -1692,6 +1696,7 @@ class AppViewModel(app: Application) : AndroidViewModel(app), PomboBridge.Listen sentDmStore.scopeAddress = store.address failedOutbox.scopeAddress = store.address blobStore.scopeAddress = store.address + previewStore.scopeAddress = store.address sentReactionsStore.scopeAddress = store.address pushRegistry.scopeAddress = store.address settingsStore.scopeAddress = store.address diff --git a/app/src/main/java/com/pombo/android/ChannelManager.kt b/app/src/main/java/com/pombo/android/ChannelManager.kt index 8cafe29..a8cec4b 100644 --- a/app/src/main/java/com/pombo/android/ChannelManager.kt +++ b/app/src/main/java/com/pombo/android/ChannelManager.kt @@ -2561,6 +2561,7 @@ class ChannelManager( */ suspend fun scanChannelActivity(channel: Channel) { val streamId = channel.messageStreamId + val previewScope = previewStore.generation() try { bridge.awaitConnected() val args = JSONObject() @@ -2634,7 +2635,7 @@ class ChannelManager( if (isContent) newContent++ } - best?.let { previewStore.put(streamId, it) } + best?.let { previewStore.put(streamId, it, previewScope) } if (newContent > 0) unreadStore.add(streamId, newContent) if (maxTs > since) unreadStore.setWatermark(streamId, maxTs) } catch (e: Exception) { diff --git a/app/src/main/java/com/pombo/android/core/LatestMessageStore.kt b/app/src/main/java/com/pombo/android/core/LatestMessageStore.kt index 251c31c..78cf327 100644 --- a/app/src/main/java/com/pombo/android/core/LatestMessageStore.kt +++ b/app/src/main/java/com/pombo/android/core/LatestMessageStore.kt @@ -13,6 +13,9 @@ import java.io.File * Last-message previews for the channel list and Explore cards — the web's * ChannelLatestMessageManager, which persists them so a cold start paints * previews immediately instead of waiting on a resend per channel. + * + * One file per account: a DM's key is the peer's inbox, so two accounts + * talking to the same peer share the key, and previews hold decrypted text. */ /** How long a preview may be newer than storage before it stops being an echo. */ private const val ECHO_GRACE_MS = 5 * 60 * 1000L @@ -36,7 +39,39 @@ class LatestMessageStore(context: Context) { val previews: StateFlow> = _previews.asStateFlow() // Persistent, like the web's IndexedDB store — cacheDir gets purged. - private val file = File(context.filesDir, "channel-previews.json") + private val dir = File(context.filesDir, "channel-previews") + // The device-wide file: its previews belong to no account, so nothing may read them. + private val deviceWide = listOf( + File(context.filesDir, "channel-previews.json"), + File(context.filesDir, "channel-previews.json.tmp") + ) + private var deviceWideDropped = false + + private val lock = Any() + private var generation = 0 + + /** The account whose previews these are; changing it empties memory until [warmUp]. */ + @Volatile var scopeAddress: String? = null + set(value) = synchronized(lock) { field = value?.lowercase(); reset() } + + /** Guest: previews live in memory for the session, nothing reaches the disk. */ + @Volatile var memoryOnly: Boolean = false + set(value) = synchronized(lock) { field = value; reset() } + + private fun reset() { + generation++ + _previews.value = emptyMap() + synchronized(inflight) { inflight.clear() } + } + + private fun currentFile(): File? { + val scope = scopeAddress + if (memoryOnly || scope.isNullOrEmpty()) return null + return File(dir, "$scope.json") + } + + /** Captured before a fetch: a [put] carrying an older one belongs to another account. */ + fun generation(): Int = synchronized(lock) { generation } /** * In-flight de-duplication, matching the web manager: the sidebar and @@ -48,22 +83,31 @@ class LatestMessageStore(context: Context) { suspend fun dedup(streamId: String, fetch: suspend () -> Preview?): Preview? { val existing = synchronized(inflight) { inflight[streamId] } if (existing != null) return existing.await() + val startedAt = generation() val deferred = kotlinx.coroutines.CompletableDeferred() synchronized(inflight) { inflight[streamId] = deferred } return try { val result = fetch() - if (result != null) put(streamId, result) + if (result != null) put(streamId, result, startedAt) deferred.complete(result) result } catch (e: Exception) { deferred.complete(null) null } finally { - synchronized(inflight) { inflight.remove(streamId) } + synchronized(inflight) { if (inflight[streamId] === deferred) inflight.remove(streamId) } } } suspend fun warmUp() = withContext(Dispatchers.IO) { + val (startedAt, file) = synchronized(lock) { + if (!deviceWideDropped) { + deviceWideDropped = true + deviceWide.forEach { runCatching { it.delete() } } + } + generation to currentFile() + } + if (file == null) return@withContext try { if (!file.exists()) return@withContext val root = JSONObject(file.readText()) @@ -80,31 +124,47 @@ class LatestMessageStore(context: Context) { o.optString("sender"), text, o.optLong("ts"), o.optString("senderAddress") ) } - if (loaded.isNotEmpty()) _previews.value = loaded + synchronized(lock) { + // What arrived while the file was being read is newer than the file. + if (startedAt == generation && loaded.isNotEmpty()) _previews.value = loaded + _previews.value + } } catch (e: Exception) { /* corrupt cache — start empty */ } } fun cached(streamId: String): Preview? = _previews.value[streamId] - suspend fun put(streamId: String, preview: Preview) { - // Never let an older message overwrite a newer one (the live path in - // channels.js always wins over a resend that lands late) — unless the - // newer one has outlived what the stream serves, which is what pinned - // a refused message on a read-only card for good. - val existing = _previews.value[streamId] - if (existing != null && existing.ts > preview.ts - && System.currentTimeMillis() - existing.ts < ECHO_GRACE_MS) return - _previews.value = _previews.value + (streamId to preview) - persist() + suspend fun put(streamId: String, preview: Preview, startedAt: Int = generation()) { + val (snapshot, file) = synchronized(lock) { + if (startedAt != generation) return + // Never let an older message overwrite a newer one (the live path in + // channels.js always wins over a resend that lands late) — unless the + // newer one has outlived what the stream serves, which is what pinned + // a refused message on a read-only card for good. + val existing = _previews.value[streamId] + if (existing != null && existing.ts > preview.ts + && System.currentTimeMillis() - existing.ts < ECHO_GRACE_MS) return + _previews.value = _previews.value + (streamId to preview) + _previews.value to currentFile() + } + if (file != null) persist(snapshot, file, startedAt) + } + + /** Erases the previews of the account in scope (the account is being deleted). */ + suspend fun clearAccount(): Unit = withContext(Dispatchers.IO) { + val file = synchronized(lock) { currentFile().also { reset() } } ?: return@withContext + runCatching { file.delete() } + runCatching { File(file.parentFile, file.name + ".tmp").delete() } } - private suspend fun persist() = withContext(Dispatchers.IO) { + private suspend fun persist(snapshot: Map, file: File, startedAt: Int) = withContext(Dispatchers.IO) { + if (generation() != startedAt) return@withContext try { val root = JSONObject() - _previews.value.forEach { (k, v) -> + snapshot.forEach { (k, v) -> root.put(k, JSONObject().put("sender", v.sender).put("text", v.text) .put("ts", v.ts).put("senderAddress", v.senderAddress)) } + file.parentFile?.mkdirs() // Write-then-rename: a truncate-in-place interrupted mid-write // (process death, low battery kill) left a corrupt file behind. val tmp = File(file.parentFile, file.name + ".tmp") diff --git a/app/src/test/java/com/pombo/android/DeleteAccountWiringTest.kt b/app/src/test/java/com/pombo/android/DeleteAccountWiringTest.kt index e7dded6..130b9ed 100644 --- a/app/src/test/java/com/pombo/android/DeleteAccountWiringTest.kt +++ b/app/src/test/java/com/pombo/android/DeleteAccountWiringTest.kt @@ -32,7 +32,7 @@ class DeleteAccountWiringTest { listOf( "channelStore", "contactsStore", "inviteStore", "sentDmStore", "sentReactionsStore", "failedOutbox", "epochKeyStore", "unreadStore", "settingsStore", "syncStore", - "pushRegistry", "walletTokenStore", "blobStore" + "pushRegistry", "walletTokenStore", "blobStore", "previewStore" ).forEach { store -> val clear = body.indexOf("$store.clearAccount(") assertTrue("deleteAccount no longer clears $store", clear >= 0) diff --git a/app/src/test/java/com/pombo/android/core/LatestMessageStoreTest.kt b/app/src/test/java/com/pombo/android/core/LatestMessageStoreTest.kt new file mode 100644 index 0000000..5685c87 --- /dev/null +++ b/app/src/test/java/com/pombo/android/core/LatestMessageStoreTest.kt @@ -0,0 +1,152 @@ +package com.pombo.android.core + +import android.content.Context +import com.pombo.android.core.LatestMessageStore.Preview +import io.mockk.every +import io.mockk.mockk +import java.io.File +import java.nio.file.Files +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.async +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.yield +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Previews are kept per account: a DM's key is the peer's inbox, so two + * accounts talking to the same peer share the key, and previews hold + * decrypted text. + */ +class LatestMessageStoreTest { + + private val filesDir: File = Files.createTempDirectory("pombo-previews").toFile() + private val alice = "0xAAAA000000000000000000000000000000000001" + private val bob = "0xbbbb000000000000000000000000000000000002" + private val dmWithCarol = "0xcccc000000000000000000000000000000000003/Pombo-DM-1" + + @After fun tearDown() { filesDir.deleteRecursively() } + + private fun store(scope: String? = alice, guest: Boolean = false) = + LatestMessageStore(mockk().also { every { it.filesDir } returns filesDir }).apply { + scopeAddress = scope + memoryOnly = guest + } + + private fun preview(text: String, ts: Long = 1_000L) = Preview("You", text, ts, alice.lowercase()) + + private fun allFiles() = filesDir.walkTopDown().filter { it.isFile }.toList() + + @Test + fun `two accounts talking to the same peer never see each other's preview`() = runBlocking { + val s = store(alice) + s.put(dmWithCarol, preview("alice's secret")) + + s.scopeAddress = bob + s.warmUp() + assertNull(s.cached(dmWithCarol)) + s.put(dmWithCarol, preview("bob's secret")) + + s.scopeAddress = alice + s.warmUp() + assertEquals("alice's secret", s.cached(dmWithCarol)?.text) + } + + @Test + fun `an account's previews survive a restart, under that account only`() = runBlocking { + store(alice).put(dmWithCarol, preview("hello")) + + val restarted = store(alice.lowercase()).also { it.warmUp() } + assertEquals("hello", restarted.cached(dmWithCarol)?.text) + assertNull(store(bob).also { it.warmUp() }.cached(dmWithCarol)) + } + + @Test + fun `a guest keeps previews for the session and writes nothing to disk`() = runBlocking { + val s = store(scope = null, guest = true) + s.put(dmWithCarol, preview("guest")) + + assertEquals("guest", s.cached(dmWithCarol)?.text) + assertTrue(allFiles().isEmpty()) + } + + @Test + fun `without an account nothing reaches the disk`() = runBlocking { + store(scope = null).put(dmWithCarol, preview("nobody")) + + assertTrue(allFiles().isEmpty()) + } + + @Test + fun `deleting an account erases its previews and leaves the others`() = runBlocking { + store(bob).put(dmWithCarol, preview("bob")) + val s = store(alice) + s.put(dmWithCarol, preview("alice")) + + s.clearAccount() + + assertNull(s.cached(dmWithCarol)) + assertNull(store(alice).also { it.warmUp() }.cached(dmWithCarol)) + assertEquals("bob", store(bob).also { it.warmUp() }.cached(dmWithCarol)?.text) + } + + @Test + fun `the device-wide file is dropped, and the accounts' files are not`() = runBlocking { + store(alice).put(dmWithCarol, preview("alice")) + File(filesDir, "channel-previews.json").writeText("""{"$dmWithCarol":{"sender":"x","text":"old","ts":1}}""") + + val restarted = store(alice).also { it.warmUp() } + + assertFalse(File(filesDir, "channel-previews.json").exists()) + assertEquals("alice", restarted.cached(dmWithCarol)?.text) + } + + @Test + fun `a fetch that started under another account is dropped`() = runBlocking { + val s = store(alice) + val answer = CompletableDeferred() + val fetch = async { s.dedup(dmWithCarol) { answer.await() } } + yield() + + s.scopeAddress = bob + answer.complete(preview("alice's secret")) + fetch.await() + + assertNull(s.cached(dmWithCarol)) + assertTrue(allFiles().isEmpty()) + } + + @Test + fun `every scope change re-scopes the previews and the cold start scopes them before loading`() { + val vm = File("src/main/java/com/pombo/android/AppViewModel.kt").readText() + val start = vm.indexOf("private fun applyStorageScope(") + val apply = vm.substring(start, vm.indexOf("\n init {", start)) + listOf("previewStore.scopeAddress = address", "previewStore.memoryOnly = guest", "previewStore.warmUp()").forEach { + assertTrue("applyStorageScope no longer does $it", apply.contains(it)) + } + val scoped = vm.indexOf("previewStore.scopeAddress = store.address") + assertTrue("the cold start loads previews before scoping them", scoped in 0 until vm.lastIndexOf("previewStore.warmUp()")) + + val cm = File("src/main/java/com/pombo/android/ChannelManager.kt").readText() + val scan = cm.substring(cm.indexOf("suspend fun scanChannelActivity("), cm.indexOf("suspend fun fetchChannelMeta(")) + val captured = scan.indexOf("previewStore.generation()") + assertTrue("the activity scan no longer tags its preview with the scope it started in", captured >= 0) + assertTrue(captured < scan.indexOf("previewStore.put(streamId, it, previewScope)")) + } + + @Test + fun `a scan put carrying an old scope is dropped`() = runBlocking { + val s = store(alice) + val startedAt = s.generation() + + s.scopeAddress = bob + s.put(dmWithCarol, preview("alice's secret"), startedAt) + + assertNull(s.cached(dmWithCarol)) + assertTrue(allFiles().isEmpty()) + } +}