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 a616108f..94faace8 100644 --- a/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt @@ -80,6 +80,13 @@ data class QueueRow( val upNext: Boolean = false, ) +// Both must match: the position alone can transiently point at the row above while the list +// reconciles, and the song id alone lights up a duplicate elsewhere in the queue. +internal fun QueueRow.isPlaying( + currentPosition: Long?, + currentSongId: String?, +): Boolean = position == currentPosition && song.song.id == currentSongId + class QueueViewModel( private val queueRepository: QueueRepository, private val playbackController: PlaybackController, @@ -436,7 +443,7 @@ fun QueueScreen( ) { isDragging -> QueueRowItem( row = row, - isPlaying = row.position == currentPosition && row.song.song.id == currentSongId, + isPlaying = row.isPlaying(currentPosition, currentSongId), floating = isDragging, coverArt = coverArt, dragHandle = diff --git a/app/src/test/kotlin/com/subtracks/data/repo/QueueRepositoryTest.kt b/app/src/test/kotlin/com/subtracks/data/repo/QueueRepositoryTest.kt index aec8bab3..01906f84 100644 --- a/app/src/test/kotlin/com/subtracks/data/repo/QueueRepositoryTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/repo/QueueRepositoryTest.kt @@ -692,6 +692,48 @@ class QueueRepositoryTest { assertEquals((0L until after.contextSize).toList(), (0 until after.contextSize).map { after.flatContext(it)!! }.sorted()) } + @Test + fun removingAnyPositionWhileShuffledKeepsTheCoordinatesConsistent() = + runTest { + seedLibrary() + for (seed in listOf(1L, 42L, 999L)) { + for (position in 0L..4L) { + repository.replace(listOf(repository.albumEntry(1, "al1"))) + repository.setShuffle(seed) + repository.addUpNext( + repository.snapshot(), + currentPosition = 0, + entry = repository.songEntry(1, "s4"), + playNext = false, + ) + val before = repository.snapshot() + if (position >= before.size) continue + + repository.removeAt(before, position) + + val after = repository.snapshot() + assertTrue("seed=$seed position=$position stopped being shuffled", after.shuffled) + assertEquals(before.size - 1, after.size) + val flats = (0 until after.contextSize).map { after.flatContext(it)!! } + assertEquals((0L until after.contextSize).toList(), flats.sorted()) + for (play in 0 until after.contextSize) { + assertEquals(play, after.playContext(after.flatContext(play)!!)) + } + for (p in 0 until after.size) { + if (after.isUpNext(p)) continue + assertEquals(p, after.combined(after.contextPlay(p))) + } + val block = (0 until after.size).filter { after.isUpNext(it) } + assertEquals( + (after.anchorPlay + 1..after.anchorPlay + after.upNextSize).toList(), + block, + ) + val ids = (0 until after.size).mapNotNull { repository.itemAt(after, it)?.song?.id } + assertEquals(ids.distinct().size, ids.size) + } + } + } + private suspend fun seedLibrary() { db.sourcesDao().upsertSource( Source(id = 1, name = "test", address = "http://localhost", isActive = true, createdAt = 0), diff --git a/app/src/test/kotlin/com/subtracks/data/repo/ShuffleTest.kt b/app/src/test/kotlin/com/subtracks/data/repo/ShuffleTest.kt index c6ab9846..296354d6 100644 --- a/app/src/test/kotlin/com/subtracks/data/repo/ShuffleTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/repo/ShuffleTest.kt @@ -1,6 +1,7 @@ package com.subtracks.data.repo import org.junit.Assert.assertEquals +import org.junit.Assert.assertThrows import org.junit.Assert.assertTrue import org.junit.Test @@ -103,6 +104,22 @@ class ShuffleTest { } } + @Test + fun handlesDegenerateAndOutOfRangeInputs() { + assertEquals(0L, Shuffle.toFlat(1L, 0L, 5L)) + assertEquals(0L, Shuffle.toSequence(1L, 0L, 5L)) + assertEquals(0L, Shuffle.toFlat(1L, 1L, 0L)) + assertEquals(10L, Shuffle.toFlat(1L, 10L, 10L)) + assertEquals(10L, Shuffle.toSequence(1L, 10L, 10L)) + } + + @Test + fun rejectsSizesBeyondThePermutationDomain() { + assertThrows(IllegalArgumentException::class.java) { + Shuffle.toFlat(1L, (1L shl 40) + 1L, 0L) + } + } + @Test fun consecutivePositionsAreNotASingleStride() { val size = 1000L diff --git a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt index 7e87ea52..94355911 100644 --- a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt +++ b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt @@ -28,6 +28,7 @@ import okhttp3.OkHttpClient import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotEquals import org.junit.Assert.assertNotNull import org.junit.Assert.assertTrue import org.junit.Before @@ -1731,6 +1732,73 @@ class PlaybackControllerTest { } } + @Test + fun undoingARemovalRestoresTheShuffledOrder() { + seedShuffledQueue(seed = 42L) + val orderBefore = resolvedIds() + val currentPosition = controller.state.value.position!! + val before = runBlocking { queues.snapshot() } + val removePosition = (0 until before.size).first { it != currentPosition } + + runBlocking { controller.removeAt(removePosition) } + assertNotEquals(orderBefore, resolvedIds()) + + runBlocking { controller.undo() } + + await { controller.state.value.shuffle && resolvedIds() == orderBefore } + assertEquals(orderBefore, resolvedIds()) + assertEquals(currentPosition, controller.state.value.position) + } + + @Test + fun removingANonCurrentTrackWhileShuffledKeepsTheCursorOnThePlayingTrack() { + seedShuffledQueue(seed = 12345L) + val currentId = + controller.state.value.item!! + .id + val before = runBlocking { queues.snapshot() } + val currentPosition = controller.state.value.position!! + val removePosition = (0 until before.size).first { it != currentPosition } + + runBlocking { controller.removeAt(removePosition) } + + val after = runBlocking { queues.snapshot() } + assertEquals( + currentId, + controller.state.value.item + ?.id, + ) + assertEquals(currentId, runBlocking { queues.itemAt(after, controller.state.value.position!!)?.song?.id }) + } + + @Test + fun removingANonCurrentTrackWhileShuffledKeepsTheBlockNext() { + seedAlbum(6, sourceId = 1) + seedSong("x1", "al2") + runBlocking { + queues.replace(listOf(queues.albumEntry(1, "al1"))) + queues.setShuffle(999L) + queues.setCursor(0) + } + controller.connect() + await { controller.state.value.item != null && controller.state.value.shuffle } + controller.addToQueue(1, QueueKind.Song, "x1") + await { runBlocking { controller.upcomingItem()?.id } == "x1" } + + val currentId = + controller.state.value.item!! + .id + val before = runBlocking { queues.snapshot() } + val currentPosition = controller.state.value.position!! + val removePosition = (0 until before.size).first { !before.isUpNext(it) && it != currentPosition } + + runBlocking { controller.removeAt(removePosition) } + + val after = runBlocking { queues.snapshot() } + assertEquals(currentId, runBlocking { queues.itemAt(after, controller.state.value.position!!)?.song?.id }) + await { runBlocking { controller.upcomingItem()?.id } == "x1" } + } + @Test fun movingContextTracksWhileShuffledIsIgnored() { seedAlbum(6, sourceId = 1) @@ -2376,6 +2444,28 @@ class PlaybackControllerTest { } } + // Loads a shuffled album under a fixed seed, so the permutation is deterministic and a test can + // assert exact positions rather than looping until a lucky seed exposes a bug. + private fun seedShuffledQueue( + seed: Long, + count: Int = 6, + ) { + seedAlbum(count, sourceId = 1) + runBlocking { + queues.replace(listOf(queues.albumEntry(1, "al1"))) + queues.setShuffle(seed) + queues.setCursor(0) + } + controller.connect() + await { controller.state.value.item != null && controller.state.value.shuffle } + } + + private fun resolvedIds(): List = + runBlocking { + val snapshot = queues.snapshot() + (0 until snapshot.size).mapNotNull { queues.itemAt(snapshot, it)?.song?.id } + } + private fun await(predicate: () -> Boolean) { val deadline = System.nanoTime() + 5_000_000_000L while (!predicate() && System.nanoTime() < deadline) Thread.sleep(10) 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 ac3a5c31..eaa1449f 100644 --- a/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt @@ -25,6 +25,7 @@ import kotlinx.coroutines.test.setMain import okhttp3.OkHttpClient import org.junit.After import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test @@ -85,6 +86,67 @@ class QueueViewModelTest { assertEquals((10L..189L).toList(), viewModel.rows.map { it.position }) } + @Test + fun thePlayingHighlightNeedsBothPositionAndSong() { + val playing = row(5) + + assertTrue(playing.isPlaying(5L, "s5")) + assertFalse("position lagged while the list reconciled", playing.isPlaying(6L, "s5")) + assertFalse("duplicate song elsewhere in the queue", playing.isPlaying(5L, "s6")) + assertFalse(playing.isPlaying(null, "s5")) + } + + @Test + fun openingAShuffledQueueStartsAtTheTop() { + runBlocking { seedSongs(20) } + runBlocking { + queues.replace(listOf(queues.albumEntry(1, "al1"))) + queues.setShuffle(12345L) + queues.setCursor(0) + } + controller.connect() + await { controller.state.value.item != null && controller.state.value.shuffle } + + viewModel.open() + await { viewModel.ready } + + assertEquals(0, viewModel.initialIndex) + assertEquals(0L, viewModel.rows.first().position) + assertTrue( + viewModel.rows.first().isPlaying( + 0L, + controller.state.value.item + ?.id, + ), + ) + } + + @Test + fun removingWhileShuffledReconcilesTheRows() { + runBlocking { seedSongs(20) } + runBlocking { + queues.replace(listOf(queues.albumEntry(1, "al1"))) + queues.setShuffle(12345L) + queues.setCursor(0) + } + controller.connect() + await { controller.state.value.item != null && controller.state.value.shuffle } + viewModel.open() + await { viewModel.ready } + + val before = runBlocking { queues.snapshot() } + val currentPosition = controller.state.value.position!! + val removePosition = (0 until before.size).first { it != currentPosition } + + viewModel.remove(removePosition) + + await { + val snapshot = runBlocking { queues.snapshot() } + viewModel.rows.isNotEmpty() && + viewModel.rows.all { runBlocking { queues.itemAt(snapshot, it.position)?.song?.id } == it.song.song.id } + } + } + @Test fun openingAtTheEndLoadsAWindowAndExtendsOlder() { runBlocking { seedSongs(200) }