From b95b8c356955f5b1bf0eac3355758186a4f12af5 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Sat, 14 Nov 2026 04:56:32 +0000 Subject: [PATCH] ui: validate queue moves against content revisions --- .../subtracks/playback/PlaybackController.kt | 2 + .../com/subtracks/ui/playback/QueueScreen.kt | 5 +- .../ui/playback/QueueViewModelTest.kt | 52 +++++++++++++++++++ 3 files changed, 57 insertions(+), 2 deletions(-) diff --git a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt index 09c1fba7..707c098b 100644 --- a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt +++ b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt @@ -586,6 +586,7 @@ class PlaybackController( from: Long, to: Long, resetVersion: Long? = null, + expectedContentVersion: Long? = null, ): Boolean { if (from == to) return false val requestedVersion = resetVersion ?: snapshot?.resetVersion ?: return false @@ -593,6 +594,7 @@ class PlaybackController( queueRepository.withMutationLock { val snapshot = this@PlaybackController.snapshot ?: return@withMutationLock if (snapshot.resetVersion != requestedVersion) return@withMutationLock + if (expectedContentVersion != null && snapshot.version != expectedContentVersion) return@withMutationLock if (from !in 0 until snapshot.size || to !in 0 until snapshot.size) return@withMutationLock val player = player ?: return@withMutationLock val current = currentQueuePosition() ?: return@withMutationLock diff --git a/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt b/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt index 4a35ee32..6ec0a3c1 100644 --- a/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt @@ -195,7 +195,7 @@ class QueueViewModel( } } else { val usedIds = updated.mapTo(HashSet()) { it.id } - val unresolvedTail = rows.filter { it.id !in usedIds } + val unresolvedTail = rows.filter { it.position > end && it.id !in usedIds } rows.clear() rows.addAll(updated) rows.addAll(unresolvedTail) @@ -302,12 +302,13 @@ class QueueViewModel( val resetVersion = rows.firstOrNull { it.position == from }?.resetVersion ?: return rejectMove(edit, onComplete) val targetResetVersion = targetResetVersion ?: rows.firstOrNull { it.position == to }?.resetVersion ?: resetVersion if (resetVersion != targetResetVersion) return rejectMove(edit, onComplete) + val expectedContentVersion = queueRepository.contentVersion() reorderPending = true viewModelScope.launch { try { val applied = mutex.withLock { - val applied = playbackController.move(from, to, resetVersion) + val applied = playbackController.move(from, to, resetVersion, expectedContentVersion) if (!applied) rollbackReorder(edit) reconcileLocked() applied diff --git a/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt b/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt index 0d7184d0..ee343c44 100644 --- a/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt @@ -552,6 +552,21 @@ class QueueViewModelTest { assertEquals(before.map { it.position }, rowsSnapshot().map { it.position }) } + @Test + fun reconciliationRemovesADeletedHeadRowFromSparseOfflineRows() { + openSparseOfflineQueue() + val point = runBlocking { queues.captureResetPoint() } + runBlocking { + db.libraryDao().deleteSongs(1, listOf("s1")) + queues.resetChangedOrigins(setOf(QueueOrigin(QueueKind.Album, 1, "al1")), point) + } + + viewModel.reconcile() + + await { rowsSnapshot().map { it.song.song.id } == listOf("s801", "s901") } + assertEquals(listOf(800L, 900L), rowsSnapshot().map { it.position }) + } + @Test fun staleRowMoveIsRejectedAfterControllerReconciliation() { runBlocking { seedSongs(3) } @@ -768,6 +783,43 @@ class QueueViewModelTest { assertEquals(listOf(0L, 1L, 2L), rowsSnapshot().map { it.position }) } + @Test + fun moveQueuedBehindRemovalRejectsItsInvalidatedPositions() { + runBlocking { seedSongs(4) } + controller.playAlbum(1, "al1", 0) + await { controller.state.value.position == 0L } + viewModel.open() + await { viewModel.ready } + val before = rowsSnapshot() + val paused = pauseQueueMutation() + val removalCompletion = CompletableDeferred() + val moveCompletion = CompletableDeferred() + try { + runOnMainSuspending { + viewModel.remove(before[0].position, removalCompletion::complete) + yield() + assertTrue( + viewModel.move( + before[1].position, + before[2].position, + before[1].resetVersion, + moveCompletion::complete, + ), + ) + yield() + } + } finally { + resumeQueueMutation(paused) + } + + assertTrue(awaitCompletion(removalCompletion)) + assertFalse(awaitCompletion(moveCompletion)) + println("DEBUG reverse rows=${rowsSnapshot().map { it.song.song.id to it.position }} queue=${resolvedQueueIds()}") + assertEquals(listOf("s2", "s3", "s4"), resolvedQueueIds()) + assertEquals(listOf("s2", "s3", "s4"), rowsSnapshot().map { it.song.song.id }) + assertEquals(listOf(0L, 1L, 2L), rowsSnapshot().map { it.position }) + } + @Test fun moveRejectsRowsFromDifferentResetGenerations() { runOnMain {