diff --git a/app/src/main/java/com/pombo/android/AppViewModel.kt b/app/src/main/java/com/pombo/android/AppViewModel.kt index 5d7ed5f..b170402 100644 --- a/app/src/main/java/com/pombo/android/AppViewModel.kt +++ b/app/src/main/java/com/pombo/android/AppViewModel.kt @@ -227,6 +227,7 @@ class AppViewModel(app: Application) : AndroidViewModel(app), PomboBridge.Listen _contacts.value = emptyList() settingsStore.blockedPeers = emptySet() settingsStore.syncBase = null + blobStore.clearAccount() disconnect() toast("Account deleted", com.pombo.android.ui.ToastKind.INFO) } @@ -1629,6 +1630,8 @@ class AppViewModel(app: Application) : AndroidViewModel(app), PomboBridge.Listen sentDmStore.memoryOnly = guest failedOutbox.scopeAddress = address failedOutbox.memoryOnly = guest + blobStore.scopeAddress = address + blobStore.memoryOnly = guest sentReactionsStore.scopeAddress = address sentReactionsStore.memoryOnly = guest pushRegistry.scopeAddress = address @@ -1677,6 +1680,7 @@ class AppViewModel(app: Application) : AndroidViewModel(app), PomboBridge.Listen inviteStore.scopeAddress = store.address sentDmStore.scopeAddress = store.address failedOutbox.scopeAddress = store.address + blobStore.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/core/ImageBlobStore.kt b/app/src/main/java/com/pombo/android/core/ImageBlobStore.kt index 106b3a9..074e559 100644 --- a/app/src/main/java/com/pombo/android/core/ImageBlobStore.kt +++ b/app/src/main/java/com/pombo/android/core/ImageBlobStore.kt @@ -17,24 +17,77 @@ import java.io.File * * Payloads travel as data URLs because that is what the web stores and pushes; * keeping the same representation is what makes the two clients interoperable. + * + * One directory per account: blob sync pushes every unsynced record to the + * active account, so images kept for one account must never sit where another + * account's store reads. */ class ImageBlobStore(context: Context) { // filesDir, not cacheDir: Android evicts cacheDir aggressively and these // are the only remaining copy once storage retention drops the chunks. - private val dir = File(context.filesDir, "image-blobs").apply { mkdirs() } - private val ledgerFile = File(context.filesDir, "image-ledger.json") + private val root = File(context.filesDir, "image-blobs-v2") + // The device-wide layout: its images belong to no account, so nothing may read them. + private val deviceWide = listOf( + File(context.filesDir, "image-blobs"), + File(context.filesDir, "image-ledger.json"), + File(context.filesDir, "image-ledger.json.tmp") + ) data class Record(val imageId: String, val streamId: String, val synced: Boolean) private val ledger = LinkedHashMap() + private val memoryBlobs = HashMap() private var loaded = false + private var deviceWideDropped = false + + /** The account whose images this store reads and writes; changing it reloads. */ + @Volatile var scopeAddress: String? = null + set(value) = synchronized(this) { field = value?.lowercase(); reset() } + + /** Guest: images live in memory for the session, nothing reaches the disk. */ + @Volatile var memoryOnly: Boolean = false + set(value) = synchronized(this) { field = value; reset() } + + private fun reset() { + ledger.clear() + memoryBlobs.clear() + loaded = false + } + + private fun onDisk() = !memoryOnly && scopeAddress != null + private fun accountDir() = File(root, scopeAddress ?: "none") + private fun ledgerFile() = File(accountDir(), "ledger.json") + private fun blobFile(imageId: String) = + File(File(accountDir(), "blobs"), imageId.replace(Regex("[^A-Za-z0-9_.-]"), "_")) + + private fun readBlob(imageId: String): String? = + if (onDisk()) runCatching { blobFile(imageId).readText() }.getOrNull() else memoryBlobs[imageId] + + private fun hasBlob(imageId: String): Boolean = + if (onDisk()) blobFile(imageId).exists() else memoryBlobs.containsKey(imageId) + + private fun writeBlob(imageId: String, dataUrl: String) { + if (!onDisk()) { memoryBlobs[imageId] = dataUrl; return } + val file = blobFile(imageId) + file.parentFile?.mkdirs() + file.writeText(dataUrl) + } + + private fun deleteBlob(imageId: String) { + if (onDisk()) runCatching { blobFile(imageId).delete() } else memoryBlobs.remove(imageId) + } @Synchronized private fun ensureLoaded() { + if (!deviceWideDropped) { + deviceWideDropped = true + deviceWide.forEach { runCatching { it.deleteRecursively() } } + } if (loaded) return loaded = true - val raw = runCatching { ledgerFile.readText() }.getOrNull() ?: return + if (!onDisk()) return + val raw = runCatching { ledgerFile().readText() }.getOrNull() ?: return runCatching { val arr = JSONArray(raw) for (i in 0 until arr.length()) { @@ -51,6 +104,9 @@ class ImageBlobStore(context: Context) { @Synchronized private fun persist() { + if (!onDisk()) return + val ledgerFile = ledgerFile() + ledgerFile.parentFile?.mkdirs() val arr = JSONArray() ledger.values.forEach { arr.put( @@ -71,19 +127,19 @@ class ImageBlobStore(context: Context) { } } - private fun blobFile(imageId: String) = File(dir, imageId.replace(Regex("[^A-Za-z0-9_.-]"), "_")) - /** Stores a blob. Returns false when it was already present. */ suspend fun save(imageId: String, streamId: String, dataUrl: String, synced: Boolean): Boolean = withContext(Dispatchers.IO) { - ensureLoaded() - if (ledger.containsKey(imageId) && blobFile(imageId).exists()) return@withContext false - runCatching { blobFile(imageId).writeText(dataUrl) }.getOrElse { return@withContext false } + // Blob and ledger under the same lock as the scope: an account + // switch in between would file the image under the next account. synchronized(this@ImageBlobStore) { + ensureLoaded() + if (ledger.containsKey(imageId) && hasBlob(imageId)) return@withContext false + runCatching { writeBlob(imageId, dataUrl) }.getOrElse { return@withContext false } ledger[imageId] = Record(imageId, streamId, synced) evictOverCap() + persist() } - persist() true } @@ -99,22 +155,22 @@ class ImageBlobStore(context: Context) { for (victim in evictable) { if (ledger.size <= MAX_RECORDS) break ledger.remove(victim.imageId) - runCatching { blobFile(victim.imageId).delete() } + deleteBlob(victim.imageId) } } suspend fun load(imageId: String): String? = withContext(Dispatchers.IO) { - ensureLoaded() - val data = runCatching { blobFile(imageId).readText() }.getOrNull() - if (data != null) { - // Reinsertion moves the record to the tail of the LinkedHashMap — - // that order is what evictOverCap and the persisted ledger use as - // recency, so reads keep a blob alive (LRU). - synchronized(this@ImageBlobStore) { + synchronized(this@ImageBlobStore) { + ensureLoaded() + val data = readBlob(imageId) + if (data != null) { + // Reinsertion moves the record to the tail of the LinkedHashMap — + // that order is what evictOverCap and the persisted ledger use as + // recency, so reads keep a blob alive (LRU). ledger.remove(imageId)?.let { ledger[imageId] = it } } + data } - data } /** @@ -128,12 +184,20 @@ class ImageBlobStore(context: Context) { val victims = ledger.values.filter { it.streamId == streamId } victims.forEach { ledger.remove(it.imageId) - runCatching { blobFile(it.imageId).delete() } + deleteBlob(it.imageId) } } persist() } + /** Erases every image of the active account (the account is being deleted). */ + suspend fun clearAccount(): Unit = withContext(Dispatchers.IO) { + synchronized(this@ImageBlobStore) { + if (onDisk()) runCatching { accountDir().deleteRecursively() } + reset() + } + } + @Synchronized fun unsynced(): List { ensureLoaded() @@ -161,7 +225,7 @@ class ImageBlobStore(context: Context) { fun forget(imageId: String) { ensureLoaded() ledger.remove(imageId) - runCatching { blobFile(imageId).delete() } + deleteBlob(imageId) persist() } diff --git a/app/src/test/java/com/pombo/android/core/ImageBlobStoreTest.kt b/app/src/test/java/com/pombo/android/core/ImageBlobStoreTest.kt new file mode 100644 index 0000000..37bd842 --- /dev/null +++ b/app/src/test/java/com/pombo/android/core/ImageBlobStoreTest.kt @@ -0,0 +1,128 @@ +package com.pombo.android.core + +import android.content.Context +import io.mockk.every +import io.mockk.mockk +import java.io.File +import java.nio.file.Files +import kotlinx.coroutines.runBlocking +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 + +/** + * Images are kept per account: blob sync pushes every unsynced record to the + * active account, so one account must never read another's ledger. + */ +class ImageBlobStoreTest { + + private val filesDir: File = Files.createTempDirectory("pombo-blobs").toFile() + private val alice = "0xAAAA000000000000000000000000000000000001" + private val bob = "0xbbbb000000000000000000000000000000000002" + + @After fun tearDown() { filesDir.deleteRecursively() } + + private fun store(scope: String? = alice, guest: Boolean = false) = + ImageBlobStore(mockk().also { every { it.filesDir } returns filesDir }).apply { + scopeAddress = scope + memoryOnly = guest + } + + private fun allFiles() = filesDir.walkTopDown().filter { it.isFile }.toList() + + @Test + fun `one account never sees another account's images`() = runBlocking { + val s = store(alice) + s.save("img-a", "room-1", "data:a", synced = false) + + s.scopeAddress = bob + assertTrue(s.unsynced().isEmpty()) + assertTrue(s.allRecords().isEmpty()) + assertNull(s.load("img-a")) + s.save("img-b", "room-2", "data:b", synced = false) + + s.scopeAddress = alice + assertEquals(listOf("img-a"), s.unsynced().map { it.imageId }) + assertEquals("data:a", s.load("img-a")) + } + + @Test + fun `an account's images survive a restart, under that account only`() = runBlocking { + store(alice).save("img-a", "room-1", "data:a", synced = true) + + val restarted = store(alice.lowercase()) + assertEquals("data:a", restarted.load("img-a")) + assertTrue(restarted.allRecords().single().synced) + assertNull(store(bob).load("img-a")) + } + + @Test + fun `a guest keeps images for the session and writes nothing to disk`() = runBlocking { + val s = store(scope = null, guest = true) + s.save("img-g", "room-1", "data:g", synced = false) + + assertEquals("data:g", s.load("img-g")) + assertTrue(allFiles().isEmpty()) + s.scopeAddress = alice + assertNull(s.load("img-g")) + } + + @Test + fun `without an account nothing reaches the disk`() = runBlocking { + store(scope = null).save("img-x", "room-1", "data:x", synced = false) + + assertTrue(allFiles().isEmpty()) + } + + @Test + fun `deleting an account erases its images and leaves the others`() = runBlocking { + store(bob).save("img-b", "room-2", "data:b", synced = false) + val s = store(alice) + s.save("img-a", "room-1", "data:a", synced = false) + + s.clearAccount() + + assertNull(s.load("img-a")) + assertTrue(store(alice).allRecords().isEmpty()) + assertEquals("data:b", store(bob).load("img-b")) + } + + @Test + fun `leaving a conversation drops its images in this account only`() = runBlocking { + val s = store(alice) + s.save("img-1", "room-1", "data:1", synced = false) + s.save("img-2", "room-2", "data:2", synced = false) + + s.clearForStream("room-1") + + assertNull(s.load("img-1")) + assertEquals("data:2", s.load("img-2")) + assertFalse(store(alice).allRecords().any { it.imageId == "img-1" }) + } + + @Test + fun `the device-wide store is dropped, and the accounts' stores are not`() = runBlocking { + store(alice).save("img-a", "room-1", "data:a", synced = false) + File(filesDir, "image-blobs").apply { mkdirs() }.resolve("old-img").writeText("data:old") + File(filesDir, "image-ledger.json").writeText("""[{"imageId":"old-img","streamId":"room-1","synced":false}]""") + + val restarted = store(alice) + assertTrue(restarted.unsynced().none { it.imageId == "old-img" }) + + assertFalse(File(filesDir, "image-blobs").exists()) + assertFalse(File(filesDir, "image-ledger.json").exists()) + assertEquals("data:a", restarted.load("img-a")) + } + + @Test + fun `account deletion clears the images before the account's scope goes`() { + val vm = File("src/main/java/com/pombo/android/AppViewModel.kt").readText() + val body = vm.substring(vm.indexOf("fun deleteAccount()"), vm.indexOf("fun blockPeer(")) + val clear = body.indexOf("blobStore.clearAccount()") + assertTrue("deleteAccount no longer clears the image store", clear >= 0) + assertTrue("the image store must be cleared before disconnect()", clear < body.lastIndexOf("disconnect()")) + } +}