From 87dc4499ca1e88654b50562655a2c29a1e1a9353 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Sun, 27 Sep 2026 08:46:44 +0000 Subject: [PATCH] split the song projections; the queue carries the playlist key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `SongListItem` carried a nullable `position` that only playlist rows have, so the album/song queries projected a fake `NULL AS position` just to map the shared type — a leak of the playlist concept into unrelated reads. Split it into `AlbumSongItem` (song, coverArt) and `PlaylistSongItem` (song, coverArt, position), plus a `SongItem` interface so the queue stays generic over kinds. The queue's `QueueWindowItem` now carries the optional `playlistPosition` for the keyset cursor, so no projection fabricates it. Playlist detail and its paging use `PlaylistSongItem`. --- .../com/subtracks/data/db/LibraryDao.kt | 4 +-- .../kotlin/com/subtracks/data/db/QueueDao.kt | 25 +++++++------- .../kotlin/com/subtracks/data/model/Models.kt | 23 ++++++++++--- .../subtracks/data/repo/LibraryRepository.kt | 4 +-- .../subtracks/data/repo/QueueRepository.kt | 34 ++++++++++++------- .../com/subtracks/playback/QueueItem.kt | 4 +-- .../ui/library/PlaylistDetailScreen.kt | 4 +-- .../ui/library/PlaylistDetailViewModel.kt | 4 +-- .../com/subtracks/ui/playback/QueueScreen.kt | 4 +-- .../subtracks/data/repo/QueueKeysetTest.kt | 4 +-- .../com/subtracks/ui/AppScreenshotTest.kt | 10 ++++-- .../ui/playback/QueueViewModelTest.kt | 4 +-- 12 files changed, 75 insertions(+), 49 deletions(-) diff --git a/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt b/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt index 02a483d2..658dd693 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt @@ -12,8 +12,8 @@ import com.subtracks.data.model.Disc import com.subtracks.data.model.DiscKey import com.subtracks.data.model.Playlist import com.subtracks.data.model.PlaylistSong +import com.subtracks.data.model.PlaylistSongItem import com.subtracks.data.model.Song -import com.subtracks.data.model.SongListItem import kotlinx.coroutines.flow.Flow internal const val PLAYLIST_SONGS_SELECT = @@ -352,7 +352,7 @@ interface LibraryDao { fun playlistSongs( sourceId: Long, playlistId: String, - ): PagingSource + ): PagingSource @Query("SELECT * FROM albums WHERE sourceId = :sourceId AND id = :albumId") fun album( diff --git a/app/src/main/kotlin/com/subtracks/data/db/QueueDao.kt b/app/src/main/kotlin/com/subtracks/data/db/QueueDao.kt index 916ee019..601b5342 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/QueueDao.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/QueueDao.kt @@ -4,13 +4,14 @@ import androidx.room3.Dao import androidx.room3.Insert import androidx.room3.Query import androidx.room3.Upsert +import com.subtracks.data.model.AlbumSongItem import com.subtracks.data.model.PlaybackCursor +import com.subtracks.data.model.PlaylistSongItem import com.subtracks.data.model.QueueEntry -import com.subtracks.data.model.SongListItem import com.subtracks.data.model.UpNextEntry private const val ALBUM_SONGS_SELECT = - "SELECT songs.*, albums.coverArt AS coverArt, NULL AS position FROM songs " + + "SELECT songs.*, albums.coverArt AS coverArt FROM songs " + "LEFT JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " + "WHERE songs.sourceId = :sourceId AND songs.albumId = :albumId" @@ -19,7 +20,7 @@ private const val ALBUM_SONGS_ORDER = " ORDER BY songs.disc, songs.track, songs. private const val ALBUM_SONGS_SQL = ALBUM_SONGS_SELECT + ALBUM_SONGS_ORDER private const val SONG_SQL = - "SELECT songs.*, albums.coverArt AS coverArt, NULL AS position FROM songs " + + "SELECT songs.*, albums.coverArt AS coverArt FROM songs " + "LEFT JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " + "WHERE songs.sourceId = :sourceId AND songs.id = :songId" @@ -77,7 +78,7 @@ interface QueueDao { playlistId: String, offset: Long, limit: Int, - ): List + ): List @Query( "$PLAYLIST_SONGS_SELECT AND playlist_songs.position >= :position$PLAYLIST_SONGS_ORDER " + @@ -89,7 +90,7 @@ interface QueueDao { position: Long, skip: Long, limit: Int, - ): List + ): List @Query( "$PLAYLIST_SONGS_SELECT AND playlist_songs.position < :position " + @@ -101,7 +102,7 @@ interface QueueDao { position: Long, skip: Long, limit: Int, - ): List + ): List @Query("$ALBUM_SONGS_SQL LIMIT :limit OFFSET :offset") suspend fun albumSongs( @@ -109,7 +110,7 @@ interface QueueDao { albumId: String, offset: Long, limit: Int, - ): List + ): List @Query( "$ALBUM_SONGS_SELECT AND (songs.disc, songs.track, songs.id) >= (:disc, :track, :id)" + @@ -123,7 +124,7 @@ interface QueueDao { id: String, skip: Long, limit: Int, - ): List + ): List // Row-value `<` is null-intolerant, and NULL disc/track sort first, so a plain // `(disc, track, id) < (...)` would drop exactly the untagged tracks a backward window needs. @@ -142,7 +143,7 @@ interface QueueDao { id: String, skip: Long, limit: Int, - ): List + ): List @Query("$SONG_SQL LIMIT :limit OFFSET :offset") suspend fun song( @@ -150,7 +151,7 @@ interface QueueDao { songId: String, offset: Long, limit: Int, - ): List + ): List @Query( "SELECT songs.id FROM songs WHERE songs.sourceId = :sourceId AND songs.albumId = :albumId " + @@ -173,12 +174,12 @@ interface QueueDao { ): List @Query( - "SELECT songs.*, albums.coverArt AS coverArt, NULL AS position FROM songs " + + "SELECT songs.*, albums.coverArt AS coverArt FROM songs " + "LEFT JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " + "WHERE songs.sourceId = :sourceId AND songs.id IN (:ids)", ) suspend fun songsByIds( sourceId: Long, ids: List, - ): List + ): List } diff --git a/app/src/main/kotlin/com/subtracks/data/model/Models.kt b/app/src/main/kotlin/com/subtracks/data/model/Models.kt index e1dfac05..75ef74ac 100644 --- a/app/src/main/kotlin/com/subtracks/data/model/Models.kt +++ b/app/src/main/kotlin/com/subtracks/data/model/Models.kt @@ -233,11 +233,24 @@ data class Song( @ColumnInfo(defaultValue = "0") val created: Long = 0, ) -data class SongListItem( - @Embedded val song: Song, - val coverArt: String?, - val position: Long? = null, -) +sealed interface SongItem { + val song: Song + val coverArt: String? + val playlistPosition: Long? get() = null +} + +data class AlbumSongItem( + @Embedded override val song: Song, + override val coverArt: String?, +) : SongItem + +data class PlaylistSongItem( + @Embedded override val song: Song, + override val coverArt: String?, + val position: Long, +) : SongItem { + override val playlistPosition: Long get() = position +} data class DiscKey( val albumId: String, diff --git a/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt b/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt index c1823fee..e96b2748 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt @@ -9,8 +9,8 @@ import com.subtracks.data.model.Album import com.subtracks.data.model.Artist import com.subtracks.data.model.Disc import com.subtracks.data.model.Playlist +import com.subtracks.data.model.PlaylistSongItem import com.subtracks.data.model.Song -import com.subtracks.data.model.SongListItem import com.subtracks.data.prefs.AlbumSort import com.subtracks.data.prefs.ArtistSort import com.subtracks.data.prefs.PlaylistSort @@ -192,7 +192,7 @@ class LibraryRepository( fun playlistSongs( sourceId: Long, playlistId: String, - ): Flow> = pager(60) { db.libraryDao().playlistSongs(sourceId, playlistId) } + ): Flow> = pager(60) { db.libraryDao().playlistSongs(sourceId, playlistId) } private val starLock = Mutex() diff --git a/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt b/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt index f9f90719..bd3b8a12 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt @@ -8,7 +8,7 @@ import com.subtracks.data.db.SubtracksDatabase import com.subtracks.data.model.PlaybackCursor import com.subtracks.data.model.QueueEntry import com.subtracks.data.model.QueueKind -import com.subtracks.data.model.SongListItem +import com.subtracks.data.model.SongItem import com.subtracks.data.model.UpNextEntry import kotlinx.coroutines.flow.first import kotlinx.coroutines.sync.Mutex @@ -197,7 +197,8 @@ data class QueueModes( data class QueueWindowItem( val position: Long, - val item: SongListItem, + val item: SongItem, + val playlistPosition: Long? = null, val upNext: Boolean = false, ) @@ -363,7 +364,7 @@ class QueueRepository( suspend fun itemAt( snapshot: QueueSnapshot, position: Long, - ): SongListItem? { + ): SongItem? { val (entry, offset) = snapshot.locate(position) ?: return null return rows(entry, entry.offset + offset, 1).firstOrNull() } @@ -673,8 +674,9 @@ class QueueRepository( val from = maxOf(first, entryStart) val to = minOf(last, end) val offset = resolved.entry.offset + (from - entryStart) - rows(resolved.entry, offset, (to - from + 1).toInt()) - .forEachIndexed { index, item -> items += QueueWindowItem(snapshot.combined(from + index), item) } + rows(resolved.entry, offset, (to - from + 1).toInt()).forEachIndexed { index, item -> + items += QueueWindowItem(snapshot.combined(from + index), item, playlistPosition = item.playlistPosition) + } } return items } @@ -696,7 +698,13 @@ class QueueRepository( val to = minOf(last, end) val offset = resolved.entry.offset + (from - entryStart) rows(resolved.entry, offset, (to - from + 1).toInt()).forEachIndexed { index, item -> - items += QueueWindowItem(snapshot.anchorPlay + 1 + from + index, item, upNext = true) + items += + QueueWindowItem( + snapshot.anchorPlay + 1 + from + index, + item, + playlistPosition = item.playlistPosition, + upNext = true, + ) } } return items @@ -706,7 +714,7 @@ class QueueRepository( entry: QueueEntry, offset: Long, limit: Int, - ): List { + ): List { if (entry.kind == QueueKind.Song) { offsetSeeks++ return dao.song(entry.sourceId, entry.refId, offset, limit) @@ -732,7 +740,7 @@ class QueueRepository( private fun remember( ref: QueueRef, offset: Long, - resolved: List, + resolved: List, ) { if (keyCache.size >= LENGTH_CACHE_LIMIT) keyCache.clear() keyCache[ref] = KeyCursor(offset + resolved.size - 1, orderKey(ref.kind, resolved.last())) @@ -740,11 +748,11 @@ class QueueRepository( private fun orderKey( kind: QueueKind, - row: SongListItem, + row: SongItem, ): OrderKey = when (kind) { QueueKind.Album -> OrderKey(row.song.id, disc = row.song.disc, track = row.song.track) - QueueKind.Playlist -> OrderKey(row.song.id, position = row.position) + QueueKind.Playlist -> OrderKey(row.song.id, position = row.playlistPosition) QueueKind.Song -> OrderKey(row.song.id) } @@ -753,7 +761,7 @@ class QueueRepository( offset: Long, limit: Int, cursor: KeyCursor, - ): List? { + ): List? { if (!cursor.key.seekable(entry.kind)) return null val from = cursor.ordinal return if (offset >= from) { @@ -775,7 +783,7 @@ class QueueRepository( key: OrderKey, skip: Long, limit: Int, - ): List = + ): List = when (entry.kind) { QueueKind.Playlist -> dao.playlistSongsFrom(entry.sourceId, entry.refId, key.position!!, skip, limit) QueueKind.Album -> dao.albumSongsFrom(entry.sourceId, entry.refId, key.disc!!, key.track!!, key.id, skip, limit) @@ -787,7 +795,7 @@ class QueueRepository( key: OrderKey, skip: Long, limit: Int, - ): List = + ): List = when (entry.kind) { QueueKind.Playlist -> dao.playlistSongsBefore(entry.sourceId, entry.refId, key.position!!, skip, limit) QueueKind.Album -> dao.albumSongsBefore(entry.sourceId, entry.refId, key.disc!!, key.track!!, key.id, skip, limit) diff --git a/app/src/main/kotlin/com/subtracks/playback/QueueItem.kt b/app/src/main/kotlin/com/subtracks/playback/QueueItem.kt index a17c2ba7..4691f83c 100644 --- a/app/src/main/kotlin/com/subtracks/playback/QueueItem.kt +++ b/app/src/main/kotlin/com/subtracks/playback/QueueItem.kt @@ -1,8 +1,8 @@ package com.subtracks.playback -import com.subtracks.data.model.SongListItem +import com.subtracks.data.model.SongItem -fun SongListItem.toQueueItem() = +fun SongItem.toQueueItem() = QueueItem( id = song.id, title = song.title, diff --git a/app/src/main/kotlin/com/subtracks/ui/library/PlaylistDetailScreen.kt b/app/src/main/kotlin/com/subtracks/ui/library/PlaylistDetailScreen.kt index dd2c05fe..780e56e2 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/PlaylistDetailScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/PlaylistDetailScreen.kt @@ -13,9 +13,9 @@ import androidx.paging.compose.collectAsLazyPagingItems import androidx.paging.compose.itemKey import com.subtracks.data.model.CoverArtRef import com.subtracks.data.model.Playlist +import com.subtracks.data.model.PlaylistSongItem import com.subtracks.data.model.QueueKind import com.subtracks.data.model.SongDownload -import com.subtracks.data.model.SongListItem import com.subtracks.data.source.StarType import com.subtracks.playback.PlaybackController import com.subtracks.ui.components.ContextMenuHost @@ -79,7 +79,7 @@ fun PlaylistDetailRoute( @Composable fun PlaylistDetailScreen( playlist: Playlist?, - songs: LazyPagingItems, + songs: LazyPagingItems, coverArt: (String?, Boolean) -> CoverArtRef?, artwork: ArtworkColors?, downloads: Map = emptyMap(), diff --git a/app/src/main/kotlin/com/subtracks/ui/library/PlaylistDetailViewModel.kt b/app/src/main/kotlin/com/subtracks/ui/library/PlaylistDetailViewModel.kt index 0da0253f..982cda93 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/PlaylistDetailViewModel.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/PlaylistDetailViewModel.kt @@ -6,9 +6,9 @@ import androidx.paging.PagingData import androidx.paging.cachedIn import com.subtracks.data.model.CoverArtRef import com.subtracks.data.model.Playlist +import com.subtracks.data.model.PlaylistSongItem import com.subtracks.data.model.Song import com.subtracks.data.model.SongDownload -import com.subtracks.data.model.SongListItem import com.subtracks.data.repo.DownloadRepository import com.subtracks.data.repo.LibraryRepository import com.subtracks.data.repo.SourceRepository @@ -33,7 +33,7 @@ class PlaylistDetailViewModel( val playlist: Flow = sourceId.flatMapLatest { libraryRepository.playlist(it, playlistId) } - val songs: Flow> = + val songs: Flow> = sourceId .flatMapLatest { libraryRepository.playlistSongs(it, playlistId) } .cachedIn(viewModelScope) 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 d5e5f6c6..da95f72c 100644 --- a/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt @@ -48,7 +48,7 @@ import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.lifecycle.viewModelScope import com.subtracks.data.model.CoverArtRef import com.subtracks.data.model.SongDownload -import com.subtracks.data.model.SongListItem +import com.subtracks.data.model.SongItem import com.subtracks.data.repo.DownloadRepository import com.subtracks.data.repo.QUEUE_CHUNK import com.subtracks.data.repo.QueueRepository @@ -78,7 +78,7 @@ internal const val QUEUE_WINDOW_ROWS = QUEUE_CHUNK * 3 data class QueueRow( val id: Long, val position: Long, - val song: SongListItem, + val song: SongItem, val upNext: Boolean = false, ) diff --git a/app/src/test/kotlin/com/subtracks/data/repo/QueueKeysetTest.kt b/app/src/test/kotlin/com/subtracks/data/repo/QueueKeysetTest.kt index 723d2cf8..17665b25 100644 --- a/app/src/test/kotlin/com/subtracks/data/repo/QueueKeysetTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/repo/QueueKeysetTest.kt @@ -146,7 +146,7 @@ class QueueKeysetTest { val actual = db .queueDao() - .playlistSongsFrom(1, "pl1", anchor.position!!, skip = 0, limit = 5) + .playlistSongsFrom(1, "pl1", anchor.position, skip = 0, limit = 5) .map { it.song.id } assertEquals(expected, actual) } @@ -234,7 +234,7 @@ class QueueKeysetTest { seedLibrary() val plan = queryPlan( - "SELECT songs.*, albums.coverArt AS coverArt, NULL AS position FROM songs " + + "SELECT songs.*, albums.coverArt AS coverArt FROM songs " + "LEFT JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " + "WHERE songs.sourceId = 1 AND songs.albumId = 'big' " + "AND (songs.disc, songs.track, songs.id) >= (1, 1, 'b0') " + diff --git a/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt b/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt index e46f52e7..d60e5b1a 100644 --- a/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt @@ -33,8 +33,8 @@ import com.subtracks.data.model.Album import com.subtracks.data.model.Artist import com.subtracks.data.model.CoverArtRef import com.subtracks.data.model.Playlist +import com.subtracks.data.model.PlaylistSongItem import com.subtracks.data.model.Song -import com.subtracks.data.model.SongListItem import com.subtracks.data.model.Source import com.subtracks.data.prefs.StreamQuality import com.subtracks.playback.PlaybackState @@ -530,8 +530,12 @@ private object Fixtures { ) val songItems = - songs.map { song -> - SongListItem(song = song, coverArt = albums.firstOrNull { it.id == song.albumId }?.coverArt) + songs.mapIndexed { index, song -> + PlaylistSongItem( + song = song, + coverArt = albums.firstOrNull { it.id == song.albumId }?.coverArt, + position = index.toLong(), + ) } val playlists = 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 f0ae448d..f28fba8c 100644 --- a/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt @@ -8,9 +8,9 @@ import androidx.test.ext.junit.runners.AndroidJUnit4 import com.subtracks.data.db.SubtracksDatabase import com.subtracks.data.download.ArtworkStore import com.subtracks.data.download.FakeDownloadEngine +import com.subtracks.data.model.AlbumSongItem import com.subtracks.data.model.QueueKind import com.subtracks.data.model.Song -import com.subtracks.data.model.SongListItem import com.subtracks.data.model.Source import com.subtracks.data.prefs.fakeUserPreferences import com.subtracks.data.repo.DownloadRepository @@ -408,7 +408,7 @@ class QueueViewModelTest { QueueRow( position.toLong(), position.toLong(), - SongListItem( + AlbumSongItem( song = Song( sourceId = 1,