mirror of
https://github.com/austinried/subtracks.git
synced 2026-11-15 00:42:54 +01:00
ui: validate queue moves against content revisions
This commit is contained in:
@@ -586,6 +586,7 @@ class PlaybackController(
|
|||||||
from: Long,
|
from: Long,
|
||||||
to: Long,
|
to: Long,
|
||||||
resetVersion: Long? = null,
|
resetVersion: Long? = null,
|
||||||
|
expectedContentVersion: Long? = null,
|
||||||
): Boolean {
|
): Boolean {
|
||||||
if (from == to) return false
|
if (from == to) return false
|
||||||
val requestedVersion = resetVersion ?: snapshot?.resetVersion ?: return false
|
val requestedVersion = resetVersion ?: snapshot?.resetVersion ?: return false
|
||||||
@@ -593,6 +594,7 @@ class PlaybackController(
|
|||||||
queueRepository.withMutationLock {
|
queueRepository.withMutationLock {
|
||||||
val snapshot = this@PlaybackController.snapshot ?: return@withMutationLock
|
val snapshot = this@PlaybackController.snapshot ?: return@withMutationLock
|
||||||
if (snapshot.resetVersion != requestedVersion) 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
|
if (from !in 0 until snapshot.size || to !in 0 until snapshot.size) return@withMutationLock
|
||||||
val player = player ?: return@withMutationLock
|
val player = player ?: return@withMutationLock
|
||||||
val current = currentQueuePosition() ?: return@withMutationLock
|
val current = currentQueuePosition() ?: return@withMutationLock
|
||||||
|
|||||||
@@ -195,7 +195,7 @@ class QueueViewModel(
|
|||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
val usedIds = updated.mapTo(HashSet()) { it.id }
|
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.clear()
|
||||||
rows.addAll(updated)
|
rows.addAll(updated)
|
||||||
rows.addAll(unresolvedTail)
|
rows.addAll(unresolvedTail)
|
||||||
@@ -302,12 +302,13 @@ class QueueViewModel(
|
|||||||
val resetVersion = rows.firstOrNull { it.position == from }?.resetVersion ?: return rejectMove(edit, onComplete)
|
val resetVersion = rows.firstOrNull { it.position == from }?.resetVersion ?: return rejectMove(edit, onComplete)
|
||||||
val targetResetVersion = targetResetVersion ?: rows.firstOrNull { it.position == to }?.resetVersion ?: resetVersion
|
val targetResetVersion = targetResetVersion ?: rows.firstOrNull { it.position == to }?.resetVersion ?: resetVersion
|
||||||
if (resetVersion != targetResetVersion) return rejectMove(edit, onComplete)
|
if (resetVersion != targetResetVersion) return rejectMove(edit, onComplete)
|
||||||
|
val expectedContentVersion = queueRepository.contentVersion()
|
||||||
reorderPending = true
|
reorderPending = true
|
||||||
viewModelScope.launch {
|
viewModelScope.launch {
|
||||||
try {
|
try {
|
||||||
val applied =
|
val applied =
|
||||||
mutex.withLock {
|
mutex.withLock {
|
||||||
val applied = playbackController.move(from, to, resetVersion)
|
val applied = playbackController.move(from, to, resetVersion, expectedContentVersion)
|
||||||
if (!applied) rollbackReorder(edit)
|
if (!applied) rollbackReorder(edit)
|
||||||
reconcileLocked()
|
reconcileLocked()
|
||||||
applied
|
applied
|
||||||
|
|||||||
@@ -552,6 +552,21 @@ class QueueViewModelTest {
|
|||||||
assertEquals(before.map { it.position }, rowsSnapshot().map { it.position })
|
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
|
@Test
|
||||||
fun staleRowMoveIsRejectedAfterControllerReconciliation() {
|
fun staleRowMoveIsRejectedAfterControllerReconciliation() {
|
||||||
runBlocking { seedSongs(3) }
|
runBlocking { seedSongs(3) }
|
||||||
@@ -768,6 +783,43 @@ class QueueViewModelTest {
|
|||||||
assertEquals(listOf(0L, 1L, 2L), rowsSnapshot().map { it.position })
|
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<Boolean>()
|
||||||
|
val moveCompletion = CompletableDeferred<Boolean>()
|
||||||
|
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
|
@Test
|
||||||
fun moveRejectsRowsFromDifferentResetGenerations() {
|
fun moveRejectsRowsFromDifferentResetGenerations() {
|
||||||
runOnMain {
|
runOnMain {
|
||||||
|
|||||||
Reference in New Issue
Block a user