Keep the image ledger per account

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

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<String, Record>()
+    private val memoryBlobs = HashMap<String, String>()
     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<Record> {
         ensureLoaded()
@@ -161,7 +225,7 @@ class ImageBlobStore(context: Context) {
     fun forget(imageId: String) {
         ensureLoaded()
         ledger.remove(imageId)
-        runCatching { blobFile(imageId).delete() }
+        deleteBlob(imageId)
         persist()
     }
 

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<Context>().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()"))
+    }
+}