bound the queue view and compact edited queue ranges

app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt

@@ -13,10 +13,21 @@ import com.subtracks.data.model.SongListItem
 import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.sync.Mutex
 import kotlinx.coroutines.sync.withLock
+import java.util.concurrent.ConcurrentHashMap
 import java.util.concurrent.atomic.AtomicLong
 
 const val QUEUE_CHUNK = 60
 
+private const val LENGTH_CACHE_LIMIT = 512
+
+private data class EntryLengthKey(
+    val sourceId: Long,
+    val kind: QueueKind,
+    val refId: String,
+    val offset: Long,
+    val count: Long?,
+)
+
 data class ResolvedQueueEntry(
     val entry: QueueEntry,
     val length: Long,
@@ -74,6 +85,8 @@ class QueueRepository(
     @Volatile
     private var cachedFlatVersion = -1L
 
+    private val lengthCache = ConcurrentHashMap<EntryLengthKey, Long>()
+
     private val cursorMutex = Mutex()
 
     fun playlistEntry(
@@ -134,7 +147,7 @@ class QueueRepository(
         val loaded =
             db.useReaderConnection { transactor ->
                 transactor.deferredTransaction {
-                    val entries = dao.entries().map { ResolvedQueueEntry(it, it.resolvedLength()) }
+                    val entries = dao.entries().map { ResolvedQueueEntry(it, lengthOf(it)) }
                     val row = dao.cursor()
                     val shuffled = row?.shuffleEnabled == true
                     val order = if (shuffled) loadShuffleOrder(row.shuffleSeed, entries.sumOf { it.length }) else null
@@ -173,6 +186,7 @@ class QueueRepository(
 
     fun invalidateLibraryCache() {
         cachedFlatIds = null
+        lengthCache.clear()
         queueVersion.incrementAndGet()
     }
 
@@ -272,11 +286,11 @@ class QueueRepository(
     ): QueueSnapshot {
         val order = snapshot.shuffleOrder
         if (order == null) {
-            write(removeEntry(snapshot.entries, position).map { it.entry })
+            write(compact(removeEntry(snapshot.entries, position)).map { it.entry })
             return snapshot()
         }
         val flat = order.getOrNull(position.toInt()) ?: return snapshot
-        write(removeEntry(snapshot.entries, flat).map { it.entry })
+        write(compact(removeEntry(snapshot.entries, flat)).map { it.entry })
         val reordered = order.toMutableList().apply { removeAt(position.toInt()) }
         for (index in reordered.indices) if (reordered[index] > flat) reordered[index] -= 1
         setShuffle(true, reordered.toLongArray())
@@ -299,8 +313,8 @@ class QueueRepository(
         val located = snapshot.locate(from) ?: return false
         val song = itemAt(snapshot, from) ?: return false
         val removed = removeEntry(snapshot.entries, from)
-        val moved = songEntry(located.first.sourceId, song.song.id)
+        val moved = ResolvedQueueEntry(songEntry(located.first.sourceId, song.song.id), 1L)
-        write(insertEntry(removed, to.coerceIn(0, removed.sumOf { it.length }), moved))
+        write(compact(insertEntry(removed, to.coerceIn(0, removed.sumOf { it.length }), moved)).map { it.entry })
         return true
     }
 
@@ -372,6 +386,24 @@ class QueueRepository(
             }
         }
 
+    private suspend fun lengthOf(entry: QueueEntry): Long {
+        val key = entry.lengthKey()
+        lengthCache[key]?.let { return it }
+        val length = entry.resolvedLength()
+        if (lengthCache.size >= LENGTH_CACHE_LIMIT) lengthCache.clear()
+        lengthCache[key] = length
+        return length
+    }
+
+    private fun QueueEntry.lengthKey() =
+        EntryLengthKey(
+            sourceId = sourceId,
+            kind = kind,
+            refId = refId,
+            offset = offset,
+            count = count,
+        )
+
     private suspend fun QueueEntry.resolvedLength(): Long {
         val total =
             when (kind) {
@@ -425,9 +457,9 @@ class QueueRepository(
     private fun insertEntry(
         entries: List<ResolvedQueueEntry>,
         index: Long,
-        newEntry: QueueEntry,
+        newEntry: ResolvedQueueEntry,
-    ): List<QueueEntry> {
+    ): List<ResolvedQueueEntry> {
-        val result = mutableListOf<QueueEntry>()
+        val result = mutableListOf<ResolvedQueueEntry>()
         var start = 0L
         var placed = false
         for (resolved in entries) {
@@ -435,21 +467,29 @@ class QueueRepository(
             when {
                 !placed && index <= start -> {
                     result += newEntry
-                    result += resolved.entry
+                    result += resolved
                     placed = true
                 }
 
                 !placed && index in start..end -> {
                     val cut = index - start
                     val offset = resolved.entry.offset
-                    if (cut > 0) result += resolved.entry.range(offset, offset + cut - 1)
+                    if (cut > 0) {
+                        result += ResolvedQueueEntry(resolved.entry.range(offset, offset + cut - 1), cut)
+                    }
                     result += newEntry
-                    if (cut < resolved.length) result += resolved.entry.range(offset + cut, offset + resolved.length - 1)
+                    if (cut < resolved.length) {
+                        result +=
+                            ResolvedQueueEntry(
+                                resolved.entry.range(offset + cut, offset + resolved.length - 1),
+                                resolved.length - cut,
+                            )
+                    }
                     placed = true
                 }
 
                 else -> {
-                    result += resolved.entry
+                    result += resolved
                 }
             }
             start += resolved.length
@@ -457,4 +497,27 @@ class QueueRepository(
         if (!placed) result += newEntry
         return result
     }
+
+    private fun compact(entries: List<ResolvedQueueEntry>): List<ResolvedQueueEntry> {
+        val merged = mutableListOf<ResolvedQueueEntry>()
+        for (resolved in entries) {
+            val previous = merged.lastOrNull()
+            if (previous != null && previous.contiguousWith(resolved)) {
+                merged[merged.size - 1] =
+                    ResolvedQueueEntry(
+                        previous.entry.range(previous.entry.offset, resolved.entry.offset + resolved.length - 1),
+                        previous.length + resolved.length,
+                    )
+            } else {
+                merged += resolved
+            }
+        }
+        return merged
+    }
+
+    private fun ResolvedQueueEntry.contiguousWith(next: ResolvedQueueEntry): Boolean =
+        entry.kind == next.entry.kind &&
+            entry.sourceId == next.entry.sourceId &&
+            entry.refId == next.entry.refId &&
+            entry.offset + length == next.entry.offset
 }

app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt

@@ -69,6 +69,8 @@ private const val LOAD_THRESHOLD = 5
 private const val OLDER_LOAD = 30
 private const val POSITION_TIMEOUT_MS = 1_000L
 
+internal const val QUEUE_WINDOW_ROWS = QUEUE_CHUNK * 3
+
 data class QueueRow(
     val id: Long,
     val position: Long,
@@ -92,9 +94,6 @@ class QueueViewModel(
 
     private var nextId = 0L
     private val mutex = Mutex()
-    private var size = 0L
-    private var first = 0L
-    private var last = -1L
 
     private fun newRow(item: QueueWindowItem) = QueueRow(nextId++, item.position, item.item)
 
@@ -114,21 +113,16 @@ class QueueViewModel(
         viewModelScope.launch {
             mutex.withLock {
                 val snapshot = queueRepository.snapshot()
-                size = snapshot.size
                 val reused = reusedIds()
                 rows.clear()
-                first = 0
+                if (snapshot.size > 0L) {
-                last = -1
-                if (size > 0L) {
                     val cursor =
                         withTimeoutOrNull(POSITION_TIMEOUT_MS) {
                             playbackController.state.first { it.position != null }.position
-                        }?.coerceIn(0, size - 1) ?: 0L
+                        }?.coerceIn(0, snapshot.size - 1) ?: 0L
                     val start = (cursor - OLDER_LOAD).coerceAtLeast(0)
-                    val end = (start + QUEUE_CHUNK - 1).coerceAtMost(size - 1)
+                    val end = (start + QUEUE_CHUNK - 1).coerceAtMost(snapshot.size - 1)
                     rows.addAll(queueRepository.range(snapshot, start, end).map { rowFor(it, reused) })
-                    first = rows.firstOrNull()?.position ?: 0
-                    last = rows.lastOrNull()?.position ?: -1
                     initialIndex = rows.indexOfFirst { it.position == cursor }.coerceAtLeast(0)
                 }
                 ready = true
@@ -138,34 +132,33 @@ class QueueViewModel(
     }
 
     fun loadOlder() {
-        if (first <= 0L) return
+        val top = rows.firstOrNull()?.position ?: return
+        if (top <= 0L) return
         viewModelScope.launch {
             mutex.withLock {
-                if (first <= 0L || rows.isEmpty()) return@withLock
+                val first = rows.firstOrNull()?.position ?: return@withLock
+                if (first <= 0L) return@withLock
                 val snapshot = queueRepository.snapshot()
-                size = snapshot.size
+                val loaded = queueRepository.range(snapshot, (first - QUEUE_CHUNK).coerceAtLeast(0), first - 1)
-                val from = (first - QUEUE_CHUNK).coerceAtLeast(0)
-                val loaded = queueRepository.range(snapshot, from, first - 1)
                 if (loaded.isEmpty()) return@withLock
                 rows.addAll(0, loaded.map(::newRow))
-                first = rows.first().position
+                while (rows.size > QUEUE_WINDOW_ROWS) rows.removeAt(rows.size - 1)
             }
         }
     }
 
     fun loadNewer() {
-        if (rows.isEmpty() || last >= size - 1) return
+        if (rows.lastOrNull() == null) return
         viewModelScope.launch {
             mutex.withLock {
-                if (rows.isEmpty() || last >= size - 1) return@withLock
+                val last = rows.lastOrNull()?.position ?: return@withLock
                 val snapshot = queueRepository.snapshot()
-                size = snapshot.size
+                if (last >= snapshot.size - 1) return@withLock
-                if (last >= size - 1) return@withLock
+                val to = (last + QUEUE_CHUNK).coerceAtMost(snapshot.size - 1)
-                val to = (last + QUEUE_CHUNK).coerceAtMost(size - 1)
                 val loaded = queueRepository.range(snapshot, last + 1, to)
                 if (loaded.isEmpty()) return@withLock
                 rows.addAll(loaded.map(::newRow))
-                last = rows.last().position
+                while (rows.size > QUEUE_WINDOW_ROWS) rows.removeAt(0)
             }
         }
     }
@@ -191,12 +184,15 @@ class QueueViewModel(
         viewModelScope.launch {
             playbackController.move(from, to)
             mutex.withLock {
+                val base = rows.firstOrNull()?.position ?: return@withLock
                 // The loaded order already reflects the move, so only the positions need updating.
-                for (i in rows.indices) {
+                val updated =
-                    val row = rows[i]
+                    rows.mapIndexed { index, row ->
-                    val position = first + i
+                        val position = base + index
-                    if (row.position != position) rows[i] = row.copy(position = position)
+                        if (row.position == position) row else row.copy(position = position)
                     }
+                rows.clear()
+                rows.addAll(updated)
             }
         }
     }
@@ -214,18 +210,15 @@ class QueueViewModel(
                 }
                 playbackController.removeAt(position)
                 val snapshot = queueRepository.snapshot()
-                size = snapshot.size
                 if (rows.isEmpty()) {
                     reload()
                     return@withLock
                 }
-                first = rows.first().position
+                val next = rows.last().position + 1
-                last = rows.last().position
+                if (next <= snapshot.size - 1) {
-                val next = last + 1
-                if (next <= size - 1) {
                     queueRepository.range(snapshot, next, next).firstOrNull()?.let { rows.add(newRow(it)) }
-                    last = rows.last().position
                 }
+                while (rows.size > QUEUE_WINDOW_ROWS) rows.removeAt(0)
             }
         }
     }
@@ -239,22 +232,17 @@ class QueueViewModel(
 
     private suspend fun reload() {
         val snapshot = queueRepository.snapshot()
-        size = snapshot.size
+        if (snapshot.size == 0L) {
-        if (size == 0L) {
             rows.clear()
-            first = 0
-            last = -1
             return
         }
-        val start = (rows.firstOrNull()?.position ?: 0L).coerceIn(0, size - 1)
+        val start = (rows.firstOrNull()?.position ?: 0L).coerceIn(0, snapshot.size - 1)
-        val end = (start + rows.size.coerceAtLeast(QUEUE_CHUNK) - 1).coerceAtMost(size - 1)
+        val end = (start + rows.size.coerceAtLeast(QUEUE_CHUNK) - 1).coerceAtMost(snapshot.size - 1)
         val loaded = queueRepository.range(snapshot, start, end)
         val reused = reusedIds()
         val updated = loaded.map { rowFor(it, reused) }
         rows.clear()
         rows.addAll(updated)
-        first = rows.firstOrNull()?.position ?: 0
-        last = rows.lastOrNull()?.position ?: -1
     }
 }
 

app/src/test/kotlin/com/subtracks/data/repo/QueueRepositoryTest.kt

@@ -299,6 +299,84 @@ class QueueRepositoryTest {
             assertEquals(listOf("s2", "s3", "s1"), resolveAll(repository.snapshot()))
         }
 
+    @Test
+    fun removingASongBetweenContiguousRangesMergesThem() =
+        runTest {
+            seedLibrary()
+            repository.replace(
+                listOf(
+                    albumRange(0, 0),
+                    repository.songEntry(1, "s4"),
+                    albumRange(1, 2),
+                ),
+            )
+            assertEquals(listOf("s1", "s4", "s2", "s3"), resolveAll(repository.snapshot()))
+
+            repository.removeAt(repository.snapshot(), 1)
+
+            val compacted = repository.snapshot()
+            assertEquals(1, compacted.entries.size)
+            assertEquals(listOf("s1", "s2", "s3"), resolveAll(compacted))
+        }
+
+    @Test
+    fun rangesSeparatedByAGapAreNotMerged() =
+        runTest {
+            seedLibrary()
+            repository.replace(
+                listOf(
+                    albumRange(0, 0),
+                    repository.songEntry(1, "s4"),
+                    albumRange(2, 2),
+                ),
+            )
+            assertEquals(listOf("s1", "s4", "s3"), resolveAll(repository.snapshot()))
+
+            repository.removeAt(repository.snapshot(), 1)
+
+            val kept = repository.snapshot()
+            assertEquals(2, kept.entries.size)
+            assertEquals(listOf("s1", "s3"), resolveAll(kept))
+        }
+
+    @Test
+    fun movingATrackOutAndBackCompactsTheQueue() =
+        runTest {
+            seedLibrary()
+            repository.snapshotAfter(listOf(repository.albumEntry(1, "al1"), repository.songEntry(1, "s4")))
+
+            repository.move(repository.snapshot(), from = 3, to = 1)
+            assertEquals(listOf("s1", "s4", "s2", "s3"), resolveAll(repository.snapshot()))
+
+            repository.move(repository.snapshot(), from = 1, to = 3)
+
+            val compacted = repository.snapshot()
+            assertEquals(listOf("s1", "s2", "s3", "s4"), resolveAll(compacted))
+            assertEquals(2, compacted.entries.size)
+        }
+
+    @Test
+    fun resolvedLengthsSurviveQueueEditsUntilTheLibraryIsInvalidated() =
+        runTest {
+            seedLibrary()
+            repository.replace(listOf(repository.albumEntry(1, "al1"), repository.songEntry(1, "s5")))
+            assertEquals(4, repository.snapshot().size)
+
+            repository.removeAt(repository.snapshot(), 3)
+            assertEquals(3, repository.snapshot().size)
+
+            db.libraryDao().upsertSongs(listOf(song("s6", "al1", track = 4, album = "First Album")))
+            assertEquals(3, repository.snapshot().size)
+
+            repository.invalidateLibraryCache()
+            assertEquals(4, repository.snapshot().size)
+        }
+
+    private fun albumRange(
+        start: Long,
+        end: Long,
+    ) = QueueEntry(position = 0, sourceId = 1, kind = QueueKind.Album, refId = "al1", rangeStart = start, rangeEnd = end)
+
     private suspend fun QueueRepository.snapshotAfter(entries: List<QueueEntry>): QueueSnapshot {
         replace(entries)
         return snapshot()
@@ -316,6 +394,7 @@ class QueueRepositoryTest {
             assertTrue(repository.snapshot().shuffled)
 
             db.libraryDao().upsertSongs(listOf(song("s6", "al1", track = 4, album = "First Album")))
+            repository.invalidateLibraryCache()
 
             assertFalse(repository.snapshot().shuffled)
         }

app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt

@@ -140,6 +140,138 @@ class QueueViewModelTest {
         assertEquals(listOf(71L, 72L, 70L), positions.take(3))
     }
 
+    @Test
+    fun scrollingNewerEvictsOlderRowsToBoundTheWindow() {
+        runBlocking { seedSongs(400) }
+        controller.playAlbum(1, "al1", 100)
+        await { controller.state.value.position == 100L }
+
+        viewModel.open()
+        await { viewModel.ready }
+        assertEquals((70L..129L).toList(), viewModel.rows.map { it.position })
+
+        extendToEnd()
+
+        assertEquals(QUEUE_WINDOW_ROWS, viewModel.rows.size)
+        assertEquals((220L..399L).toList(), viewModel.rows.map { it.position })
+    }
+
+    @Test
+    fun scrollingOlderAfterEvictionDropsNewerRows() {
+        runBlocking { seedSongs(400) }
+        controller.playAlbum(1, "al1", 100)
+        await { controller.state.value.position == 100L }
+
+        viewModel.open()
+        await { viewModel.ready }
+        extendToEnd()
+
+        viewModel.loadOlder()
+        awaitWindow(first = 160, last = 339)
+
+        assertEquals(QUEUE_WINDOW_ROWS, viewModel.rows.size)
+    }
+
+    @Test
+    fun movingARowAfterEvictionKeepsPositionsContiguous() {
+        runBlocking { seedSongs(400) }
+        controller.playAlbum(1, "al1", 100)
+        await { controller.state.value.position == 100L }
+
+        viewModel.open()
+        await { viewModel.ready }
+        extendToEnd()
+
+        val movedSongId =
+            viewModel.rows[10]
+                .song.song.id
+        val from = viewModel.rows[10].position
+        val to = viewModel.rows[13].position
+        viewModel.reorder(10, 13)
+        viewModel.move(from, to)
+        await {
+            viewModel.rows.size == QUEUE_WINDOW_ROWS &&
+                viewModel.rows.getOrNull(13)?.position == 233L
+        }
+
+        assertEquals(
+            movedSongId,
+            viewModel.rows[13]
+                .song.song.id,
+        )
+        assertEquals(QUEUE_WINDOW_ROWS, viewModel.rows.size)
+        assertContiguous()
+    }
+
+    @Test
+    fun removeAndUndoKeepTheBoundedWindowConsistent() {
+        runBlocking { seedSongs(400) }
+        controller.playAlbum(1, "al1", 100)
+        await { controller.state.value.position == 100L }
+
+        viewModel.open()
+        await { viewModel.ready }
+        val removedSongId =
+            viewModel.rows[30]
+                .song.song.id
+
+        viewModel.remove(viewModel.rows[30].position)
+        await {
+            viewModel.rows.size == 60 &&
+                viewModel.rows.firstOrNull()?.position == 70L &&
+                viewModel.rows
+                    .getOrNull(30)
+                    ?.song
+                    ?.song
+                    ?.id != removedSongId
+        }
+        assertEquals((70L..129L).toList(), viewModel.rows.map { it.position })
+
+        viewModel.undo()
+        await {
+            viewModel.rows.size == 60 &&
+                viewModel.rows.firstOrNull()?.position == 70L &&
+                viewModel.rows
+                    .getOrNull(30)
+                    ?.song
+                    ?.song
+                    ?.id == removedSongId
+        }
+        assertContiguous()
+    }
+
+    private fun assertContiguous() = assertTrue(isContiguous())
+
+    private fun isContiguous(): Boolean {
+        val positions = viewModel.rows.map { it.position }
+        return positions.isNotEmpty() && positions == (positions.first()..positions.last()).toList()
+    }
+
+    private fun awaitWindow(
+        first: Long,
+        last: Long,
+    ) = await {
+        viewModel.rows.size == (last - first + 1).toInt() &&
+            viewModel.rows.firstOrNull()?.position == first &&
+            viewModel.rows.lastOrNull()?.position == last
+    }
+
+    private fun extendTo(
+        position: Long,
+        first: Long,
+    ) {
+        viewModel.loadNewer()
+        awaitWindow(first = first, last = position)
+    }
+
+    private fun extendToEnd() {
+        extendTo(189, first = 70)
+        extendTo(249, first = 70)
+        extendTo(309, first = 130)
+        extendTo(369, first = 190)
+        extendTo(399, first = 220)
+    }
+
     private fun row(position: Int) =
         QueueRow(
             position.toLong(),
@@ -189,8 +321,10 @@ class QueueViewModelTest {
     }
 
     private fun await(predicate: () -> Boolean) {
-        val deadline = System.nanoTime() + 5_000_000_000L
+        repeat(500) {
-        while (!predicate() && System.nanoTime() < deadline) Thread.sleep(10)
+            if (predicate()) return
+            Thread.sleep(10)
+        }
         assertTrue("Timed out waiting for the expected state", predicate())
     }
 }