address second review: shuffle cache invalidation, locking, ended latch

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

@@ -109,11 +109,8 @@ class QueueRepository(
                 }
             }
         val (entries, order, shuffled) = loaded
-        if (shuffled && order == null) {
-            setShuffle(false, null)
-            return QueueSnapshot(entries, null, queueVersion)
-        }
-        return QueueSnapshot(entries, order, queueVersion)
+        val valid = if (shuffled && order == null) null else order
+        return QueueSnapshot(entries, valid, queueVersion)
     }
 
     suspend fun modes(): QueueModes {
@@ -126,15 +123,24 @@ class QueueRepository(
         order: LongArray?,
     ) {
         val seed = if (enabled && order != null) System.nanoTime() else 0L
-        dao.clearShuffleOrder()
-        if (enabled && order != null) {
-            dao.insertShuffleOrder(order.mapIndexed { index, flat -> ShuffleOrder(index.toLong(), flat) })
+        db.useWriterConnection { transactor ->
+            transactor.immediateTransaction {
+                dao.clearShuffleOrder()
+                if (enabled && order != null) {
+                    dao.insertShuffleOrder(order.mapIndexed { index, flat -> ShuffleOrder(index.toLong(), flat) })
+                }
+                dao.setCursor(cursorRow().copy(shuffleEnabled = enabled, shuffleSeed = seed))
+            }
         }
-        dao.setCursor(cursorRow().copy(shuffleEnabled = enabled, shuffleSeed = seed))
         cachedShuffleSeed = 0
         cachedShuffleOrder = null
     }
 
+    fun invalidateLibraryCache() {
+        cachedFlatIds = null
+        queueVersion++
+    }
+
     suspend fun setRepeat(mode: Int) = dao.setCursor(cursorRow().copy(repeatMode = mode))
 
     private suspend fun loadShuffleOrder(
@@ -271,7 +277,7 @@ class QueueRepository(
 
     private suspend fun flatIds(snapshot: QueueSnapshot): List<String> {
         val cached = cachedFlatIds
-        if (cached != null && cachedFlatVersion == snapshot.version) return cached
+        if (cached != null && cachedFlatVersion == snapshot.version && cached.size.toLong() == snapshot.size) return cached
         val ids = ArrayList<String>(snapshot.size.toInt())
         for (resolved in snapshot.entries) {
             val entry = resolved.entry

app/src/main/kotlin/com/subtracks/data/sync/SyncManager.kt

@@ -1,6 +1,7 @@
 package com.subtracks.data.sync
 
 import android.util.Log
+import com.subtracks.data.repo.QueueRepository
 import com.subtracks.data.repo.SourceRepository
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
@@ -23,6 +24,7 @@ sealed interface SyncStatus {
 
 class SyncManager(
     private val sourceRepository: SourceRepository,
+    private val queueRepository: QueueRepository,
 ) {
     private val scope = CoroutineScope(SupervisorJob() + Dispatchers.Default)
     private val _status = MutableStateFlow<SyncStatus>(SyncStatus.Idle)
@@ -36,7 +38,10 @@ class SyncManager(
         _status.value = SyncStatus.Running
         _status.value =
             sourceRepository.sync().fold(
-                onSuccess = { SyncStatus.Success },
+                onSuccess = {
+                    queueRepository.invalidateLibraryCache()
+                    SyncStatus.Success
+                },
                 onFailure = { error ->
                     Log.w(TAG, "Sync failed", error)
                     SyncStatus.Failed(error.message ?: "Sync failed")

app/src/main/kotlin/com/subtracks/di/AppModule.kt

@@ -34,7 +34,7 @@ fun appModule(
     single { SourceRepository(get(), get(), get()) }
     single { LibraryRepository(get(), get()) }
     single<ArtworkSeedStore> { ArtworkSeedRepository(get()) }
-    single { SyncManager(get()) }
+    single { SyncManager(get(), get()) }
     single { QueueRepository(get()) }
     single<PlayerConnection> { MediaSessionConnection(context.applicationContext, get()) }
     single { PlaybackController(get(), get(), get()) }

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

@@ -537,7 +537,15 @@ class PlaybackController(
             shuffleEnabled = modes.shuffle
             repeatMode = modes.repeat.toRepeatMode()
             player?.setRepeatOne(repeatMode == RepeatMode.One)
-            val snapshot = readSnapshot()
+            var snapshot = queueRepository.snapshot()
+            if (shuffleEnabled && !snapshot.shuffled && snapshot.size > 0L) {
+                val start = queueRepository.cursor().coerceIn(0, snapshot.size - 1)
+                queueRepository.setShuffle(true, shuffledOrder(snapshot.size, start))
+                queueRepository.setCursor(0)
+                snapshot = queueRepository.snapshot()
+            } else if (!snapshot.shuffled) {
+                shuffleEnabled = false
+            }
             this.snapshot = snapshot
             val queueSourceId =
                 snapshot.entries
@@ -600,22 +608,24 @@ class PlaybackController(
 
     private fun jumpTo(target: Long) {
         scope.launch {
-            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)
-            val wasEnded = player?.isEnded == true
-            if (dest == current && !wasEnded) return@launch
-            windowJob?.cancel()
-            queueRepository.setCursor(dest)
-            shiftWindow(dest)
-            val player = player ?: return@launch
-            player.ensurePrepared()
-            player.seekToIndex((dest - windowStart).toInt())
-            if (wasEnded) player.play()
-            endedHandled = false
-            refresh(dest)
+            startLock.withLock {
+                val snapshot = readSnapshot()
+                this@PlaybackController.snapshot = snapshot
+                if (snapshot.size == 0L) return@withLock
+                val current = currentPosition() ?: return@withLock
+                val dest = target.coerceIn(0, snapshot.size - 1)
+                val wasEnded = player?.isEnded == true
+                if (dest == current && !wasEnded) return@withLock
+                windowJob?.cancel()
+                queueRepository.setCursor(dest)
+                shiftWindowLocked(dest)
+                val player = player ?: return@withLock
+                player.ensurePrepared()
+                player.seekToIndex((dest - windowStart).toInt())
+                if (wasEnded) player.play()
+                endedHandled = false
+                refresh(dest)
+            }
         }
     }
 
@@ -702,7 +712,7 @@ class PlaybackController(
             }
 
             override fun onEvents() {
-                if (player?.isEnded == true) handleEnded()
+                if (player?.isEnded == true) handleEnded() else endedHandled = false
                 refresh()
             }
 
@@ -730,7 +740,10 @@ class PlaybackController(
 
     private suspend fun readSnapshot(): QueueSnapshot {
         val snapshot = queueRepository.snapshot()
-        if (!snapshot.shuffled) shuffleEnabled = false
+        if (!snapshot.shuffled) {
+            if (shuffleEnabled && snapshot.size > 0L) queueRepository.setShuffle(false, null)
+            shuffleEnabled = false
+        }
         return snapshot
     }
 

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

@@ -250,7 +250,7 @@ fun QueueRoute(
     sourceRepository: SourceRepository = koinInject(),
 ) {
     val playback by controller.state.collectAsStateWithLifecycle()
-    LaunchedEffect(Unit) { viewModel.open() }
+    LaunchedEffect(playback.shuffle) { viewModel.open() }
     QueueScreen(
         rows = viewModel.rows,
         ready = viewModel.ready,

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

@@ -321,7 +321,7 @@ class QueueRepositoryTest {
         (0 until snapshot.size).mapNotNull { repository.itemAt(snapshot, it)?.song?.id }
 
     @Test
-    fun aStaleShuffleOrderIsDroppedWhenTheQueueLengthChanges() =
+    fun aStaleShuffleOrderIsIgnoredWhenTheQueueLengthChanges() =
         runTest {
             seedLibrary()
             repository.replace(listOf(repository.albumEntry(1, "al1")))
@@ -331,7 +331,6 @@ class QueueRepositoryTest {
             db.libraryDao().upsertSongs(listOf(song("s6", "al1", track = 4, album = "First Album")))
 
             assertFalse(repository.snapshot().shuffled)
-            assertFalse(repository.modes().shuffle)
         }
 
     private suspend fun seedLibrary() {

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

@@ -819,6 +819,27 @@ class PlaybackControllerTest {
         )
     }
 
+    @Test
+    fun repeatAllLoopsAfterEndingWithRepeatOff() {
+        seedAlbum(2, sourceId = 1)
+
+        controller.playAlbum(1, "al1", 0)
+        await { controller.state.value.isPlaying }
+
+        handle.finish()
+        handle.emitEvents()
+        await { !controller.state.value.isPlaying }
+
+        controller.togglePlayPause()
+        await { controller.state.value.isPlaying }
+        controller.cycleRepeat()
+        assertEquals(RepeatMode.All, controller.state.value.repeat)
+
+        handle.finish()
+        handle.emitEvents()
+        await { controller.state.value.isPlaying }
+    }
+
     private fun seedAlbum(
         count: Int,
         sourceId: Long,