playback: preserve null keys and ranged duplicate ranks

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

@@ -210,7 +210,7 @@ data class Reanchored(
 )
 
 private interface QueueKindResolver {
-    val supportsKeyset: Boolean get() = false
+    fun supportsKeyset(key: OrderKey): Boolean = false
 
     suspend fun length(
         dao: com.subtracks.data.db.QueueDao,
@@ -247,9 +247,9 @@ private interface QueueKindResolver {
     ): Long? = null
 }
 
-private fun resolverFor(kind: QueueKind): QueueKindResolver =
+private val queueKindResolvers =
-    when (kind) {
+    mapOf<QueueKind, QueueKindResolver>(
-        QueueKind.Song -> {
+        QueueKind.Song to
             object : QueueKindResolver {
                 override suspend fun length(
                     dao: com.subtracks.data.db.QueueDao,
@@ -268,12 +268,10 @@ private fun resolverFor(kind: QueueKind): QueueKindResolver =
                     entry: QueueEntry,
                     songId: String,
                 ) = if (entry.refId == songId && length(dao, entry) > 0) 0L else null
-            }
+            },
-        }
+        QueueKind.Playlist to
-
-        QueueKind.Playlist -> {
             object : QueueKindResolver {
-                override val supportsKeyset = true
+                override fun supportsKeyset(key: OrderKey) = key.position != null
 
                 override suspend fun length(
                     dao: com.subtracks.data.db.QueueDao,
@@ -308,12 +306,10 @@ private fun resolverFor(kind: QueueKind): QueueKindResolver =
                     entry: QueueEntry,
                     songId: String,
                 ) = dao.playlistRank(entry.sourceId, entry.refId, songId)
-            }
+            },
-        }
+        QueueKind.Album to
-
-        QueueKind.Album -> {
             object : QueueKindResolver {
-                override val supportsKeyset = true
+                override fun supportsKeyset(key: OrderKey) = key.disc != null && key.track != null
 
                 override suspend fun length(
                     dao: com.subtracks.data.db.QueueDao,
@@ -348,10 +344,8 @@ private fun resolverFor(kind: QueueKind): QueueKindResolver =
                     entry: QueueEntry,
                     songId: String,
                 ) = dao.albumRank(entry.sourceId, entry.refId, songId)
-            }
+            },
-        }
+        QueueKind.Artist to
-
-        QueueKind.Artist -> {
             object : QueueKindResolver {
                 override suspend fun length(
                     dao: com.subtracks.data.db.QueueDao,
@@ -364,10 +358,8 @@ private fun resolverFor(kind: QueueKind): QueueKindResolver =
                     offset: Long,
                     limit: Int,
                 ) = dao.artistSongs(entry.sourceId, entry.refId, offset, limit)
-            }
+            },
-        }
+        QueueKind.Genre to
-
-        QueueKind.Genre -> {
             object : QueueKindResolver {
                 override suspend fun length(
                     dao: com.subtracks.data.db.QueueDao,
@@ -380,10 +372,8 @@ private fun resolverFor(kind: QueueKind): QueueKindResolver =
                     offset: Long,
                     limit: Int,
                 ) = dao.genreSongs(entry.sourceId, entry.refId, offset, limit)
-            }
+            },
-        }
+        QueueKind.Downloaded to
-
-        QueueKind.Downloaded -> {
             object : QueueKindResolver {
                 override suspend fun length(
                     dao: com.subtracks.data.db.QueueDao,
@@ -396,10 +386,8 @@ private fun resolverFor(kind: QueueKind): QueueKindResolver =
                     offset: Long,
                     limit: Int,
                 ) = dao.downloadedSongs(entry.sourceId, offset, limit)
-            }
+            },
-        }
+        QueueKind.Starred to
-
-        QueueKind.Starred -> {
             object : QueueKindResolver {
                 override suspend fun length(
                     dao: com.subtracks.data.db.QueueDao,
@@ -412,9 +400,10 @@ private fun resolverFor(kind: QueueKind): QueueKindResolver =
                     offset: Long,
                     limit: Int,
                 ) = dao.starredSongs(entry.sourceId, offset, limit)
-            }
+            },
-        }
+    )
-    }
+
+private fun resolverFor(kind: QueueKind): QueueKindResolver = queueKindResolvers.getValue(kind)
 
 class QueueRepository(
     private val db: SubtracksDatabase,
@@ -858,10 +847,12 @@ class QueueRepository(
         var start = 0L
         for (resolved in entries) {
             val entry = resolved.entry
-            val rank = resolverFor(entry.kind).rank(dao, entry, songId) ?: fallbackRank(entry, songId)
+            val rank = resolverFor(entry.kind).rank(dao, entry, songId)
             if (rank != null && rank >= entry.offset && rank < entry.offset + resolved.length) {
                 return start + rank - entry.offset
             }
+            val fallback = fallbackRank(entry, resolved.length, songId)
+            if (fallback != null) return start + fallback - entry.offset
             start += resolved.length
         }
         return null
@@ -869,16 +860,20 @@ class QueueRepository(
 
     private suspend fun fallbackRank(
         entry: QueueEntry,
+        length: Long,
         songId: String,
     ): Long? {
-        var offset = 0L
+        var offset = entry.offset
-        while (true) {
+        val end = entry.offset + length
-            val batch = rows(entry, offset, QUEUE_CHUNK)
+        while (offset < end) {
+            val limit = minOf(QUEUE_CHUNK.toLong(), end - offset).toInt()
+            val batch = rows(entry, offset, limit)
             val index = batch.indexOfFirst { it.song.id == songId }
             if (index >= 0) return offset + index
-            if (batch.size < QUEUE_CHUNK) return null
+            if (batch.size < limit) return null
             offset += batch.size
         }
+        return null
     }
 
     private suspend fun contextPlayRange(
@@ -994,7 +989,7 @@ class QueueRepository(
         limit: Int,
         cursor: KeyCursor,
     ): List<SongItem>? {
-        if (!resolverFor(entry.kind).supportsKeyset) return null
+        if (!resolverFor(entry.kind).supportsKeyset(cursor.key)) return null
         val from = cursor.ordinal
         // A backward seek from a far cursor is not cheaper than the offset query: the album
         // predicate is not sargable (it has to include NULL disc/track), so it scans the partition

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

@@ -150,6 +150,18 @@ class QueueKeysetTest {
             assertTrue("backward keyset seeks", repository.keysetSeeks > beforeBackward)
         }
 
+    @Test
+    fun scanningAnAlbumWithNullDiscAndTrackForwardFallsBackToOffsetRows() =
+        runTest {
+            seedLibrary()
+            repository.replace(listOf(repository.albumEntry(1, "mixed")))
+            val snapshot = repository.snapshot()
+
+            val resolved = (0 until snapshot.size).map { repository.itemAt(snapshot, it)?.song?.id }
+
+            assertEquals(listOf("n1", "n2", "s1", "s2"), resolved)
+        }
+
     @Test
     fun scanningAnAlbumWithNullDiscAndTrackBackwardsResolvesEveryRow() =
         runTest {

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

@@ -895,6 +895,24 @@ class QueueRepositoryTest {
             assertEquals(1L, repository.combinedContextIndexOf(snapshot, "s1"))
         }
 
+    @Test
+    fun contextIndexFindsADuplicateInsideTheEntryRange() =
+        runTest {
+            seedLibrary()
+            db.libraryDao().upsertPlaylists(listOf(playlist("duplicates", "Duplicates")))
+            db.libraryDao().upsertPlaylistSongs(
+                listOf(
+                    PlaylistSong(1, "duplicates", "s2", 10),
+                    PlaylistSong(1, "duplicates", "s1", 20),
+                    PlaylistSong(1, "duplicates", "s2", 30),
+                ),
+            )
+            val entry = repository.playlistEntry(1, "duplicates").copy(rangeStart = 1, rangeEnd = 2)
+            val snapshot = repository.snapshotAfter(listOf(entry))
+
+            assertEquals(1L, repository.combinedContextIndexOf(snapshot, "s2"))
+        }
+
     @Test
     fun contextIndexFallsBackToBoundedRowsForGenre() =
         runTest {