mirror of
https://github.com/austinried/subtracks.git
synced 2026-11-10 15:11:58 +01:00
split the song projections; the queue carries the playlist key
`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`.
This commit is contained in:
@@ -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<Int, SongListItem>
|
||||
): PagingSource<Int, PlaylistSongItem>
|
||||
|
||||
@Query("SELECT * FROM albums WHERE sourceId = :sourceId AND id = :albumId")
|
||||
fun album(
|
||||
|
||||
@@ -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<SongListItem>
|
||||
): List<PlaylistSongItem>
|
||||
|
||||
@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<SongListItem>
|
||||
): List<PlaylistSongItem>
|
||||
|
||||
@Query(
|
||||
"$PLAYLIST_SONGS_SELECT AND playlist_songs.position < :position " +
|
||||
@@ -101,7 +102,7 @@ interface QueueDao {
|
||||
position: Long,
|
||||
skip: Long,
|
||||
limit: Int,
|
||||
): List<SongListItem>
|
||||
): List<PlaylistSongItem>
|
||||
|
||||
@Query("$ALBUM_SONGS_SQL LIMIT :limit OFFSET :offset")
|
||||
suspend fun albumSongs(
|
||||
@@ -109,7 +110,7 @@ interface QueueDao {
|
||||
albumId: String,
|
||||
offset: Long,
|
||||
limit: Int,
|
||||
): List<SongListItem>
|
||||
): List<AlbumSongItem>
|
||||
|
||||
@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<SongListItem>
|
||||
): List<AlbumSongItem>
|
||||
|
||||
// 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<SongListItem>
|
||||
): List<AlbumSongItem>
|
||||
|
||||
@Query("$SONG_SQL LIMIT :limit OFFSET :offset")
|
||||
suspend fun song(
|
||||
@@ -150,7 +151,7 @@ interface QueueDao {
|
||||
songId: String,
|
||||
offset: Long,
|
||||
limit: Int,
|
||||
): List<SongListItem>
|
||||
): List<AlbumSongItem>
|
||||
|
||||
@Query(
|
||||
"SELECT songs.id FROM songs WHERE songs.sourceId = :sourceId AND songs.albumId = :albumId " +
|
||||
@@ -173,12 +174,12 @@ interface QueueDao {
|
||||
): List<String>
|
||||
|
||||
@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<String>,
|
||||
): List<SongListItem>
|
||||
): List<AlbumSongItem>
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<PagingData<SongListItem>> = pager(60) { db.libraryDao().playlistSongs(sourceId, playlistId) }
|
||||
): Flow<PagingData<PlaylistSongItem>> = pager(60) { db.libraryDao().playlistSongs(sourceId, playlistId) }
|
||||
|
||||
private val starLock = Mutex()
|
||||
|
||||
|
||||
@@ -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<SongListItem> {
|
||||
): List<SongItem> {
|
||||
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<SongListItem>,
|
||||
resolved: List<SongItem>,
|
||||
) {
|
||||
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<SongListItem>? {
|
||||
): List<SongItem>? {
|
||||
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<SongListItem> =
|
||||
): List<SongItem> =
|
||||
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<SongListItem> =
|
||||
): List<SongItem> =
|
||||
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)
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<SongListItem>,
|
||||
songs: LazyPagingItems<PlaylistSongItem>,
|
||||
coverArt: (String?, Boolean) -> CoverArtRef?,
|
||||
artwork: ArtworkColors?,
|
||||
downloads: Map<String, SongDownload> = emptyMap(),
|
||||
|
||||
@@ -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<Playlist?> = sourceId.flatMapLatest { libraryRepository.playlist(it, playlistId) }
|
||||
|
||||
val songs: Flow<PagingData<SongListItem>> =
|
||||
val songs: Flow<PagingData<PlaylistSongItem>> =
|
||||
sourceId
|
||||
.flatMapLatest { libraryRepository.playlistSongs(it, playlistId) }
|
||||
.cachedIn(viewModelScope)
|
||||
|
||||
@@ -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,
|
||||
)
|
||||
|
||||
|
||||
@@ -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') " +
|
||||
|
||||
@@ -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 =
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user