diff --git a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt index df52bb25..09c1fba7 100644 --- a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt +++ b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt @@ -423,8 +423,9 @@ class PlaybackController( suspend fun removeAt( position: Long, resetVersion: Long? = null, - ) { - val requestedVersion = resetVersion ?: snapshot?.resetVersion ?: return + ): Boolean { + val requestedVersion = resetVersion ?: snapshot?.resetVersion ?: return false + var applied = false queueRepository.withMutationLock { val snapshot = this@PlaybackController.snapshot ?: return@withMutationLock if (snapshot.resetVersion != requestedVersion) return@withMutationLock @@ -436,6 +437,7 @@ class PlaybackController( if (snapshot.shuffled && !snapshot.isUpNext(position)) return@withMutationLock if (snapshot.size == 1L) { if (!removeAt(snapshot, position)) return@withMutationLock + applied = true if (detachedWindow && currentPosition() == null) { val updated = readSnapshot() this@PlaybackController.snapshot = updated @@ -471,6 +473,7 @@ class PlaybackController( val wasUpNext = snapshot.isUpNext(current) val removed = if (position == current) null else queueItemAt(snapshot, position) if (!removeAt(snapshot, position)) return@withMutationLock + applied = true var updated = readSnapshot() this@PlaybackController.snapshot = updated windowJob?.cancel() @@ -576,15 +579,17 @@ class PlaybackController( queueRepository.setCursor(target) refresh(target) } + return applied } suspend fun move( from: Long, to: Long, resetVersion: Long? = null, - ) { - if (from == to) return - val requestedVersion = resetVersion ?: snapshot?.resetVersion ?: return + ): Boolean { + if (from == to) return false + val requestedVersion = resetVersion ?: snapshot?.resetVersion ?: return false + var applied = false queueRepository.withMutationLock { val snapshot = this@PlaybackController.snapshot ?: return@withMutationLock if (snapshot.resetVersion != requestedVersion) return@withMutationLock @@ -596,6 +601,7 @@ class PlaybackController( val upNext = snapshot.upNext.map { it.entry } val wasUpNext = snapshot.isUpNext(current) if (!move(snapshot, from, to)) return@withMutationLock + applied = true val updated = readSnapshot() this@PlaybackController.snapshot = updated // The current track still exists, so derive its new position from the edit instead of @@ -669,6 +675,7 @@ class PlaybackController( queueRepository.setCursor(target) refresh(target) } + return applied } suspend fun undo() = diff --git a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt index a3bda73c..87ad3826 100644 --- a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt +++ b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt @@ -1899,7 +1899,7 @@ class PlaybackControllerTest { ?.id == "s1" } - runBlocking { controller.removeAt(3) } + assertTrue(runBlocking { controller.removeAt(3) }) await { handle.itemCount == 4 } assertEquals( @@ -1920,7 +1920,7 @@ class PlaybackControllerTest { ?.id == "s1" } - runBlocking { controller.move(0, 2) } + assertTrue(runBlocking { controller.move(0, 2) }) await { controller.state.value.position == 2L } assertEquals(listOf("s2", "s3", "s1", "s4"), handle.items.map { it.id }) @@ -1931,6 +1931,23 @@ class PlaybackControllerTest { ) } + @Test + fun staleMoveAndRemovalReturnFalseWithoutChangingTheQueue() { + seedAlbum(3, sourceId = 1) + controller.playAlbum(1, "al1", 0) + await { + controller.state.value.item + ?.id == "s1" + } + val before = resolvedIds() + val staleVersion = runBlocking { queues.snapshot().resetVersion } + 1 + + assertFalse(runBlocking { controller.move(0, 1, staleVersion) }) + assertFalse(runBlocking { controller.removeAt(1, staleVersion) }) + + assertEquals(before, resolvedIds()) + } + @Test fun movingALaterTrackBeforeTheCurrentShiftsTheCursor() { seedAlbum(4, sourceId = 1) @@ -4005,7 +4022,7 @@ class PlaybackControllerTest { runBlocking { queues.resetChangedOrigins(setOf(QueueOrigin(QueueKind.Playlist, 1, "p1")), point) } await { handle.items.map { it.id } == listOf("s3", "s4", "s5") } - runBlocking { controller.removeAt(3) } + assertTrue(runBlocking { controller.removeAt(3) }) assertEquals(listOf("s3", "s4"), handle.items.map { it.id }) assertEquals("s3", handle.currentItem?.id)