address shuffle review findings

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

@@ -21,6 +21,7 @@ data class ResolvedQueueEntry(
 data class QueueSnapshot(
     val entries: List<ResolvedQueueEntry>,
     val shuffleOrder: LongArray? = null,
+    val version: Long = 0,
 ) {
     val size: Long = entries.sumOf { it.length }
 
@@ -96,20 +97,24 @@ class QueueRepository(
         dao.setCursor(cursorRow().copy(queuePosition = 0))
     }
 
-    suspend fun snapshot(): QueueSnapshot =
-        db.useReaderConnection { transactor ->
-            transactor.deferredTransaction {
-                val entries = dao.entries().map { ResolvedQueueEntry(it, it.resolvedLength()) }
-                val row = dao.cursor()
-                val order =
-                    if (row?.shuffleEnabled == true) {
-                        loadShuffleOrder(row.shuffleSeed, entries.sumOf { it.length })
-                    } else {
-                        null
-                    }
-                QueueSnapshot(entries, order)
+    suspend fun snapshot(): QueueSnapshot {
+        val loaded =
+            db.useReaderConnection { transactor ->
+                transactor.deferredTransaction {
+                    val entries = dao.entries().map { ResolvedQueueEntry(it, it.resolvedLength()) }
+                    val row = dao.cursor()
+                    val shuffled = row?.shuffleEnabled == true
+                    val order = if (shuffled) loadShuffleOrder(row.shuffleSeed, entries.sumOf { it.length }) else null
+                    Triple(entries, order, shuffled)
+                }
             }
+        val (entries, order, shuffled) = loaded
+        if (shuffled && order == null) {
+            setShuffle(false, null)
+            return QueueSnapshot(entries, null, queueVersion)
         }
+        return QueueSnapshot(entries, order, queueVersion)
+    }
 
     suspend fun modes(): QueueModes {
         val row = dao.cursor() ?: return QueueModes(shuffle = false, repeat = 0)
@@ -266,7 +271,7 @@ class QueueRepository(
 
     private suspend fun flatIds(snapshot: QueueSnapshot): List<String> {
         val cached = cachedFlatIds
-        if (cached != null && cachedFlatVersion == queueVersion) return cached
+        if (cached != null && cachedFlatVersion == snapshot.version) return cached
         val ids = ArrayList<String>(snapshot.size.toInt())
         for (resolved in snapshot.entries) {
             val entry = resolved.entry
@@ -282,7 +287,7 @@ class QueueRepository(
             ids.addAll(entryIds.subList(from, to))
         }
         cachedFlatIds = ids
-        cachedFlatVersion = queueVersion
+        cachedFlatVersion = snapshot.version
         return ids
     }
 

app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt

@@ -205,7 +205,7 @@ class PlaybackController(
             val playing = player.playWhenReady
             val removed = if (position == current) null else queueRepository.itemAt(snapshot, position)?.toQueueItem()
             queueRepository.removeAt(snapshot, position)
-            this.snapshot = queueRepository.snapshot()
+            this.snapshot = readSnapshot()
             windowJob?.cancel()
             lastEdit =
                 if (removed == null) {
@@ -252,7 +252,7 @@ class PlaybackController(
         val cursor = queueRepository.cursor()
         val entries = snapshot.entries.map { it.entry }
         if (!queueRepository.move(snapshot, from, to)) return@withLock
-        this.snapshot = queueRepository.snapshot()
+        this.snapshot = readSnapshot()
         val target = movedCursor(current, from, to)
         windowJob?.cancel()
         val fromInWindow = from in windowStart..windowEnd
@@ -303,7 +303,7 @@ class PlaybackController(
             val positionMs = player.currentPositionMs
             queueRepository.replace(undo.entries, undo.shuffleOrder)
             shuffleEnabled = undo.shuffleOrder != null
-            val restored = queueRepository.snapshot()
+            val restored = readSnapshot()
             this.snapshot = restored
             lastEdit = null
             if (restored.size == 0L) {
@@ -435,7 +435,7 @@ class PlaybackController(
     fun toggleShuffle() {
         scope.launch {
             startLock.withLock {
-                val snapshot = queueRepository.snapshot()
+                val snapshot = readSnapshot()
                 this@PlaybackController.snapshot = snapshot
                 if (snapshot.size == 0L) return@withLock
                 val player = player ?: return@withLock
@@ -443,17 +443,18 @@ class PlaybackController(
                 val positionMs = player.currentPositionMs
                 val playing = player.playWhenReady
                 val flat = snapshot.flatPosition(current) ?: return@withLock
+                windowJob?.cancel()
                 if (shuffleEnabled) {
                     queueRepository.setShuffle(false, null)
                     shuffleEnabled = false
                     queueRepository.setCursor(flat)
-                    this@PlaybackController.snapshot = queueRepository.snapshot()
+                    this@PlaybackController.snapshot = readSnapshot()
                     loadWindow(flat, autoplay = playing, startPositionMs = positionMs)
                 } else {
                     queueRepository.setShuffle(true, shuffledOrder(snapshot.size, flat))
                     shuffleEnabled = true
                     queueRepository.setCursor(0)
-                    this@PlaybackController.snapshot = queueRepository.snapshot()
+                    this@PlaybackController.snapshot = readSnapshot()
                     loadWindow(0, autoplay = playing, startPositionMs = positionMs)
                 }
                 endedHandled = false
@@ -522,7 +523,7 @@ class PlaybackController(
                 start
             }
         queueRepository.setCursor(position)
-        this.snapshot = queueRepository.snapshot()
+        this.snapshot = readSnapshot()
         loadWindow(position, autoplay = true)
     }
 
@@ -536,7 +537,7 @@ class PlaybackController(
             shuffleEnabled = modes.shuffle
             repeatMode = modes.repeat.toRepeatMode()
             player?.setRepeatOne(repeatMode == RepeatMode.One)
-            val snapshot = queueRepository.snapshot()
+            val snapshot = readSnapshot()
             this.snapshot = snapshot
             val queueSourceId =
                 snapshot.entries
@@ -599,17 +600,17 @@ class PlaybackController(
 
     private fun jumpTo(target: Long) {
         scope.launch {
-            val snapshot = queueRepository.snapshot()
+            val snapshot = readSnapshot()
             this@PlaybackController.snapshot = snapshot
             if (snapshot.size == 0L) return@launch
             val current = currentPosition() ?: return@launch
             val dest = target.coerceIn(0, snapshot.size - 1)
-            if (dest == current) return@launch
+            val wasEnded = player?.isEnded == true
+            if (dest == current && !wasEnded) return@launch
             windowJob?.cancel()
             queueRepository.setCursor(dest)
             shiftWindow(dest)
             val player = player ?: return@launch
-            val wasEnded = player.isEnded
             player.ensurePrepared()
             player.seekToIndex((dest - windowStart).toInt())
             if (wasEnded) player.play()
@@ -642,7 +643,7 @@ class PlaybackController(
 
     private suspend fun shiftWindowLocked(center: Long) {
         val player = player ?: return
-        val snapshot = queueRepository.snapshot()
+        val snapshot = readSnapshot()
         this.snapshot = snapshot
         if (snapshot.size == 0L) return
         val target = center.coerceIn(0, snapshot.size - 1)
@@ -727,6 +728,12 @@ class PlaybackController(
 
     private fun Int.toRepeatMode(): RepeatMode = RepeatMode.entries.getOrElse(this) { RepeatMode.Off }
 
+    private suspend fun readSnapshot(): QueueSnapshot {
+        val snapshot = queueRepository.snapshot()
+        if (!snapshot.shuffled) shuffleEnabled = false
+        return snapshot
+    }
+
     private fun contextAt(position: Long?): QueueContext? {
         val entry = position?.let { snapshot?.locate(it)?.first } ?: return null
         return QueueContext(entry.kind, entry.refId)

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

@@ -320,6 +320,20 @@ class QueueRepositoryTest {
     private suspend fun resolveAll(snapshot: QueueSnapshot): List<String> =
         (0 until snapshot.size).mapNotNull { repository.itemAt(snapshot, it)?.song?.id }
 
+    @Test
+    fun aStaleShuffleOrderIsDroppedWhenTheQueueLengthChanges() =
+        runTest {
+            seedLibrary()
+            repository.replace(listOf(repository.albumEntry(1, "al1")))
+            repository.setShuffle(true, longArrayOf(2, 0, 1))
+            assertTrue(repository.snapshot().shuffled)
+
+            db.libraryDao().upsertSongs(listOf(song("s6", "al1", track = 4, album = "First Album")))
+
+            assertFalse(repository.snapshot().shuffled)
+            assertFalse(repository.modes().shuffle)
+        }
+
     private suspend fun seedLibrary() {
         db.sourcesDao().upsertSource(
             Source(id = 1, name = "test", address = "http://localhost", isActive = true, createdAt = 0),

app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt

@@ -795,6 +795,30 @@ class PlaybackControllerTest {
         )
     }
 
+    @Test
+    fun repeatAllLoopsASingleTrackQueue() {
+        seedAlbum(1, sourceId = 1)
+
+        controller.playAlbum(1, "al1", 0)
+        await {
+            controller.state.value.item
+                ?.id == "s1" && controller.state.value.isPlaying
+        }
+
+        controller.cycleRepeat()
+        assertEquals(RepeatMode.All, controller.state.value.repeat)
+
+        handle.finish()
+        handle.emitEvents()
+        await { controller.state.value.isPlaying }
+
+        assertEquals(
+            "s1",
+            controller.state.value.item
+                ?.id,
+        )
+    }
+
     private fun seedAlbum(
         count: Int,
         sourceId: Long,