queue: make reset remapping origin-safe and bounded

This commit is contained in:
austinried
2026-11-14 12:05:29 +00:00
parent df8cf73385
commit 746446be86
3 changed files with 185 additions and 5 deletions
@@ -23,11 +23,13 @@ private const val ALBUM_SONGS_SQL = ALBUM_SONGS_SELECT + ALBUM_SONGS_ORDER
private const val ARTIST_SONG_IDS = private const val ARTIST_SONG_IDS =
"(SELECT songId FROM ($CREDITED_SONG_ROWS) WHERE artistId = :artistId)" "(SELECT songId FROM ($CREDITED_SONG_ROWS) WHERE artistId = :artistId)"
private const val ARTIST_SONGS_SELECT = private const val ARTIST_SONGS_FROM =
"SELECT songs.*, albums.coverArt AS coverArt FROM songs " + "FROM songs " +
"LEFT JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " + "LEFT JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " +
"WHERE songs.sourceId = :sourceId AND songs.id IN $ARTIST_SONG_IDS" "WHERE songs.sourceId = :sourceId AND songs.id IN $ARTIST_SONG_IDS"
private const val ARTIST_SONGS_SELECT = "SELECT songs.*, albums.coverArt AS coverArt $ARTIST_SONGS_FROM"
private const val ARTIST_SONGS_ORDER = private const val ARTIST_SONGS_ORDER =
" ORDER BY albums.id IS NULL, albums.year DESC, albums.sortName COLLATE NOCASE, albums.id, songs.disc, songs.track, songs.id" " ORDER BY albums.id IS NULL, albums.year DESC, albums.sortName COLLATE NOCASE, albums.id, songs.disc, songs.track, songs.id"
@@ -181,6 +183,17 @@ interface QueueDao {
limit: Int, limit: Int,
): List<AlbumSongItem> ): List<AlbumSongItem>
@Query(
"SELECT songRank FROM (" +
"SELECT songs.id AS songId, ROW_NUMBER() OVER ($ARTIST_SONGS_ORDER) - 1 AS songRank $ARTIST_SONGS_FROM" +
") WHERE songId = :songId ORDER BY songRank LIMIT 1",
)
suspend fun artistRank(
sourceId: Long,
artistId: String,
songId: String,
): Long?
@Query("$SONG_SQL LIMIT :limit OFFSET :offset") @Query("$SONG_SQL LIMIT :limit OFFSET :offset")
suspend fun song( suspend fun song(
sourceId: Long, sourceId: Long,
@@ -308,6 +321,20 @@ interface QueueDao {
limit: Int, limit: Int,
): List<AlbumSongItem> ): List<AlbumSongItem>
@Query(
"SELECT songRank FROM (" +
"SELECT songs.id AS songId, " +
"ROW_NUMBER() OVER (ORDER BY songs.sortName COLLATE NOCASE, songs.id) - 1 AS songRank " +
"FROM song_genres JOIN songs ON songs.sourceId = song_genres.sourceId AND songs.id = song_genres.songId " +
"WHERE song_genres.sourceId = :sourceId AND song_genres.genre = :genre" +
") WHERE songId = :songId ORDER BY songRank LIMIT 1",
)
suspend fun genreRank(
sourceId: Long,
genre: String,
songId: String,
): Long?
@Query( @Query(
"SELECT COUNT(*) FROM song_downloads d " + "SELECT COUNT(*) FROM song_downloads d " +
"JOIN songs ON songs.sourceId = d.sourceId AND songs.id = d.songId " + "JOIN songs ON songs.sourceId = d.sourceId AND songs.id = d.songId " +
@@ -342,4 +369,29 @@ interface QueueDao {
offset: Long, offset: Long,
limit: Int, limit: Int,
): List<AlbumSongItem> ): List<AlbumSongItem>
@Query(
"SELECT songRank FROM (" +
"SELECT songs.id AS songId, " +
"ROW_NUMBER() OVER (ORDER BY songs.starred DESC, songs.sortName COLLATE NOCASE, songs.id) - 1 AS songRank " +
"FROM songs WHERE songs.sourceId = :sourceId AND songs.starred IS NOT NULL" +
") WHERE songId = :songId ORDER BY songRank LIMIT 1",
)
suspend fun starredRank(
sourceId: Long,
songId: String,
): Long?
@Query(
"SELECT songRank FROM (" +
"SELECT songs.id AS songId, " +
"ROW_NUMBER() OVER (ORDER BY d.downloadedAt DESC, songs.sortName COLLATE NOCASE, songs.id) - 1 AS songRank " +
"FROM song_downloads d JOIN songs ON songs.sourceId = d.sourceId AND songs.id = d.songId " +
"WHERE d.sourceId = :sourceId AND d.status = 'Completed'" +
") WHERE songId = :songId ORDER BY songRank LIMIT 1",
)
suspend fun downloadedRank(
sourceId: Long,
songId: String,
): Long?
} }
@@ -356,6 +356,12 @@ private val queueKindResolvers =
offset: Long, offset: Long,
limit: Int, limit: Int,
) = dao.artistSongs(entry.sourceId, entry.refId, offset, limit) ) = dao.artistSongs(entry.sourceId, entry.refId, offset, limit)
override suspend fun rank(
dao: com.subtracks.data.db.QueueDao,
entry: QueueEntry,
songId: String,
) = dao.artistRank(entry.sourceId, entry.refId, songId)
}, },
QueueKind.Genre to QueueKind.Genre to
object : QueueKindResolver { object : QueueKindResolver {
@@ -370,6 +376,12 @@ private val queueKindResolvers =
offset: Long, offset: Long,
limit: Int, limit: Int,
) = dao.genreSongs(entry.sourceId, entry.refId, offset, limit) ) = dao.genreSongs(entry.sourceId, entry.refId, offset, limit)
override suspend fun rank(
dao: com.subtracks.data.db.QueueDao,
entry: QueueEntry,
songId: String,
) = dao.genreRank(entry.sourceId, entry.refId, songId)
}, },
QueueKind.Downloaded to QueueKind.Downloaded to
object : QueueKindResolver { object : QueueKindResolver {
@@ -384,6 +396,12 @@ private val queueKindResolvers =
offset: Long, offset: Long,
limit: Int, limit: Int,
) = dao.downloadedSongs(entry.sourceId, offset, limit) ) = dao.downloadedSongs(entry.sourceId, offset, limit)
override suspend fun rank(
dao: com.subtracks.data.db.QueueDao,
entry: QueueEntry,
songId: String,
) = dao.downloadedRank(entry.sourceId, songId)
}, },
QueueKind.Starred to QueueKind.Starred to
object : QueueKindResolver { object : QueueKindResolver {
@@ -398,6 +416,12 @@ private val queueKindResolvers =
offset: Long, offset: Long,
limit: Int, limit: Int,
) = dao.starredSongs(entry.sourceId, offset, limit) ) = dao.starredSongs(entry.sourceId, offset, limit)
override suspend fun rank(
dao: com.subtracks.data.db.QueueDao,
entry: QueueEntry,
songId: String,
) = dao.starredRank(entry.sourceId, songId)
}, },
) )
@@ -936,14 +960,13 @@ class QueueRepository(
val position = dao.playlistPositionForOccurrence(entry.sourceId, entry.refId, item.songId, item.occurrence) val position = dao.playlistPositionForOccurrence(entry.sourceId, entry.refId, item.songId, item.occurrence)
position?.let { dao.playlistRankAtPosition(entry.sourceId, entry.refId, it) } position?.let { dao.playlistRankAtPosition(entry.sourceId, entry.refId, it) }
} else { } else {
resolverFor(entry.kind).rank(dao, entry, item.songId) ?: fallbackRank(entry, resolved.length, item.songId) resolverFor(entry.kind).rank(dao, entry, item.songId)
} }
if (rank != null) { if (rank != null) {
val insideEntry = rank >= entry.offset && rank < entry.offset + resolved.length val insideEntry = rank >= entry.offset && rank < entry.offset + resolved.length
val flat = val flat =
when { when {
insideEntry -> group.starts[index] + rank - entry.offset insideEntry -> group.starts[index] + rank - entry.offset
entry.originEntryId == null -> rank
else -> continue else -> continue
} }
if (item.upNext) return snapshot.anchorPlay + 1 + flat if (item.upNext) return snapshot.anchorPlay + 1 + flat
@@ -1261,7 +1284,11 @@ class QueueRepository(
for (resolved in entries) { for (resolved in entries) {
val entry = resolved.entry val entry = resolved.entry
val rank = resolverFor(entry.kind).rank(dao, entry, songId) val rank = resolverFor(entry.kind).rank(dao, entry, songId)
if (rank != null && rank >= entry.offset && rank < entry.offset + resolved.length) { if (rank == null) {
start += resolved.length
continue
}
if (rank >= entry.offset && rank < entry.offset + resolved.length) {
return start + rank - entry.offset return start + rank - entry.offset
} }
val fallback = fallbackRank(entry, resolved.length, songId) val fallback = fallbackRank(entry, resolved.length, songId)
@@ -15,6 +15,7 @@ import com.subtracks.data.model.QueueOrigin
import com.subtracks.data.model.Song import com.subtracks.data.model.Song
import com.subtracks.data.model.SongDownload import com.subtracks.data.model.SongDownload
import com.subtracks.data.model.Source import com.subtracks.data.model.Source
import com.subtracks.data.model.UpNextEntry
import kotlinx.coroutines.CoroutineStart import kotlinx.coroutines.CoroutineStart
import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
@@ -441,6 +442,55 @@ class QueueRepositoryTest {
assertEquals(4, repository.snapshot().size) assertEquals(4, repository.snapshot().size)
} }
@Test
fun aLegacyContextFragmentDoesNotRemapIntoTheFollowingOrigin() =
runTest {
seedLibrary()
setPlaylist("p1", "s1", "s2")
db.queueDao().insert(
listOf(
repository.playlistEntry(1, "p1").copy(position = 0, rangeStart = 0, rangeEnd = 0),
repository.songEntry(1, "s4").copy(position = 1),
),
)
repository.setCursor(0)
val point = repository.captureResetPoint()
setPlaylist("p1", "s3", "s1", "s2")
repository.resetChangedOrigins(setOf(QueueOrigin(QueueKind.Playlist, 1, "p1")), point)
val updated = repository.snapshot()
assertNull(repository.remapPosition(updated, point.current!!))
assertEquals(0L, repository.cursor())
assertEquals("s3", repository.itemAt(updated, 0)?.song?.id)
assertEquals("s4", repository.itemAt(updated, 1)?.song?.id)
}
@Test
fun aLegacyUpNextFragmentDoesNotRemapIntoTheFollowingOrigin() =
runTest {
seedLibrary()
setPlaylist("p1", "s1", "s2")
repository.replace(listOf(repository.albumEntry(1, "al1")))
db.queueDao().insertUpNext(
listOf(
UpNextEntry.from(repository.playlistEntry(1, "p1").copy(rangeStart = 0, rangeEnd = 0)).copy(position = 0),
UpNextEntry.from(repository.songEntry(1, "s4")).copy(position = 1),
),
)
repository.setCursor(1)
val point = repository.captureResetPoint()
setPlaylist("p1", "s3", "s1", "s2")
repository.resetChangedOrigins(setOf(QueueOrigin(QueueKind.Playlist, 1, "p1")), point)
val updated = repository.snapshot()
assertNull(repository.remapPosition(updated, point.current!!))
assertEquals(1L, repository.cursor())
assertEquals("s3", repository.itemAt(updated, 1)?.song?.id)
assertEquals("s4", repository.itemAt(updated, 2)?.song?.id)
}
@Test @Test
fun aChangedOriginResetMergesEditsAndRemapsTheCurrentItem() = fun aChangedOriginResetMergesEditsAndRemapsTheCurrentItem() =
runTest { runTest {
@@ -1449,6 +1499,18 @@ class QueueRepositoryTest {
assertTrue(repository.resolvedRowCount <= QUEUE_CHUNK) assertTrue(repository.resolvedRowCount <= QUEUE_CHUNK)
} }
@Test
fun artistRankLookupDoesNotResolveSongRows() = runTest { assertRankLookupDoesNotResolveSongRows(QueueKind.Artist) }
@Test
fun genreRankLookupDoesNotResolveSongRows() = runTest { assertRankLookupDoesNotResolveSongRows(QueueKind.Genre) }
@Test
fun downloadedRankLookupDoesNotResolveSongRows() = runTest { assertRankLookupDoesNotResolveSongRows(QueueKind.Downloaded) }
@Test
fun starredRankLookupDoesNotResolveSongRows() = runTest { assertRankLookupDoesNotResolveSongRows(QueueKind.Starred) }
@Test @Test
fun upNextCombinedIndexOfCountsFromTheAnchorIntoTheBlock() = fun upNextCombinedIndexOfCountsFromTheAnchorIntoTheBlock() =
runTest { runTest {
@@ -1615,6 +1677,45 @@ class QueueRepositoryTest {
assertEquals(QueueModes(shuffle = false, repeat = 0), repository.modes()) assertEquals(QueueModes(shuffle = false, repeat = 0), repository.modes())
} }
private suspend fun assertRankLookupDoesNotResolveSongRows(kind: QueueKind) {
db.sourcesDao().upsertSource(
Source(id = 1, name = "test", address = "http://localhost", isActive = true, createdAt = 0),
)
db.libraryDao().upsertAlbums(listOf(album("al1", "First Album", "Artist One")))
db.libraryDao().upsertSongs(
(1..62).map { index ->
val suffix = index.toString().padStart(2, '0')
song("s$suffix", "al1", index.toLong(), title = "Song $suffix", starred = (100 - index).toLong())
},
)
(1..62).forEach { index ->
val suffix = index.toString().padStart(2, '0')
insertGenre("s$suffix", "Rock")
db.downloadDao().upsert(
SongDownload(1, "s$suffix", DownloadStatus.Completed, downloadedAt = (100 - index).toLong()),
)
}
val entry =
when (kind) {
QueueKind.Artist -> repository.artistEntry(1, "ar1")
QueueKind.Genre -> repository.genreEntry(1, "Rock")
QueueKind.Downloaded -> repository.downloadedEntry(1)
QueueKind.Starred -> repository.starredEntry(1)
else -> error("Unsupported kind $kind")
}
val snapshot = repository.snapshotAfter(listOf(entry))
val resolvedBefore = repository.resolvedRowCount
val seeksBefore = repository.offsetSeeks
val origin = entry.origin
assertEquals(61L, repository.combinedContextIndexOf(snapshot, "s62"))
assertNull(repository.combinedContextIndexOf(snapshot, "missing"))
assertEquals(61L, repository.remapPosition(snapshot, QueueResetItem(origin, "s62", 0, upNext = false)))
assertNull(repository.remapPosition(snapshot, QueueResetItem(origin, "missing", 0, upNext = false)))
assertEquals(resolvedBefore, repository.resolvedRowCount)
assertEquals(seeksBefore, repository.offsetSeeks)
}
private fun playlist( private fun playlist(
id: String, id: String,
name: String, name: String,