address review: dedupe and trim genres, add a starred queue reference, fix the offline source switch

app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt

@@ -801,6 +801,19 @@ interface LibraryDao {
         id: String,
     ): Long
 
+    @Query(
+        "SELECT COUNT(*) FROM songs s " +
+            "WHERE s.sourceId = :sourceId AND s.starred IS NOT NULL " +
+            "AND (s.starred > (SELECT starred FROM songs WHERE sourceId = :sourceId AND id = :id) " +
+            "OR (s.starred = (SELECT starred FROM songs WHERE sourceId = :sourceId AND id = :id) " +
+            "AND (s.title COLLATE NOCASE < (SELECT title FROM songs WHERE sourceId = :sourceId AND id = :id) " +
+            "OR (s.title COLLATE NOCASE = (SELECT title FROM songs WHERE sourceId = :sourceId AND id = :id) AND s.id < :id))))",
+    )
+    suspend fun starredSongOrdinal(
+        sourceId: Long,
+        id: String,
+    ): Long
+
     @Query(
         "SELECT * FROM albums WHERE sourceId = :sourceId AND year >= :start AND year < :end " +
             "ORDER BY year DESC, name COLLATE NOCASE, id",
@@ -859,17 +872,6 @@ interface LibraryDao {
         songId: String,
     ): Flow<List<String>>
 
-    @Query(
-        "SELECT DISTINCT song_genres.genre FROM song_genres " +
-            "JOIN songs ON songs.sourceId = song_genres.sourceId AND songs.id = song_genres.songId " +
-            "WHERE songs.sourceId = :sourceId AND songs.albumId = :albumId " +
-            "ORDER BY song_genres.genre COLLATE NOCASE",
-    )
-    fun albumGenres(
-        sourceId: Long,
-        albumId: String,
-    ): Flow<List<String>>
-
     @Query("SELECT * FROM albums WHERE sourceId = :sourceId AND artistId = :artistId ORDER BY year DESC, name COLLATE NOCASE, id")
     fun albumsForArtist(
         sourceId: Long,

app/src/main/kotlin/com/subtracks/data/db/Migrations.kt

@@ -344,8 +344,8 @@ val MIGRATION_23_24 =
         override suspend fun migrate(connection: SQLiteConnection) {
             connection.execSQL(
                 "INSERT INTO `song_genres` (`sourceId`, `songId`, `position`, `genre`) " +
-                    "SELECT `sourceId`, `id`, 0, `genre` FROM `songs` " +
+                    "SELECT `sourceId`, `id`, 0, trim(`genre`) FROM `songs` " +
-                    "WHERE `genre` IS NOT NULL AND `genre` != '' " +
+                    "WHERE `genre` IS NOT NULL AND trim(`genre`) != '' " +
                     "AND NOT EXISTS (SELECT 1 FROM `song_genres` sg " +
                     "WHERE sg.`sourceId` = `songs`.`sourceId` AND sg.`songId` = `songs`.`id`)",
             )

app/src/main/kotlin/com/subtracks/data/db/QueueDao.kt

@@ -254,6 +254,27 @@ interface QueueDao {
     @Query("SELECT COUNT(*) FROM song_downloads WHERE sourceId = :sourceId AND status = 'Completed'")
     suspend fun downloadedLength(sourceId: Long): Long
 
+    @Query("SELECT COUNT(*) FROM songs WHERE sourceId = :sourceId AND starred IS NOT NULL")
+    suspend fun starredLength(sourceId: Long): Long
+
+    @Query(
+        "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.starred IS NOT NULL " +
+            "ORDER BY songs.starred DESC, songs.title COLLATE NOCASE, songs.id LIMIT :limit OFFSET :offset",
+    )
+    suspend fun starredSongs(
+        sourceId: Long,
+        offset: Long,
+        limit: Int,
+    ): List<AlbumSongItem>
+
+    @Query(
+        "SELECT songs.id FROM songs WHERE sourceId = :sourceId AND starred IS NOT NULL " +
+            "ORDER BY starred DESC, title COLLATE NOCASE, id",
+    )
+    suspend fun starredSongIds(sourceId: Long): List<String>
+
     @Query(
         "SELECT songs.*, albums.coverArt AS coverArt FROM song_downloads d " +
             "JOIN songs ON songs.sourceId = d.sourceId AND songs.id = d.songId " +

app/src/main/kotlin/com/subtracks/data/model/Models.kt

@@ -316,7 +316,7 @@ data class DiscKey(
     val disc: Long,
 )
 
-enum class QueueKind { Playlist, Album, Song, Artist, Genre, Downloaded }
+enum class QueueKind { Playlist, Album, Song, Artist, Genre, Downloaded, Starred }
 
 @Entity(
     tableName = "queue_entries",

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

@@ -88,12 +88,8 @@ class LibraryRepository(
 
     fun starredSongs(sourceId: Long): Flow<PagingData<AlbumSongItem>> = pager(60) { db.libraryDao().starredSongs(sourceId) }
 
-    suspend fun starredSongIds(sourceId: Long): List<String> = db.libraryDao().starredSongIds(sourceId)
-
     fun downloadedSongs(sourceId: Long): Flow<List<AlbumSongItem>> = db.libraryDao().downloadedSongs(sourceId)
 
-    suspend fun downloadedSongIds(sourceId: Long): List<String> = db.libraryDao().downloadedSongIds(sourceId)
-
     fun downloadedSongsPage(sourceId: Long): Flow<PagingData<AlbumSongItem>> = pager(60) { db.libraryDao().homeDownloadedSongs(sourceId) }
 
     fun downloadedAlbumsPage(sourceId: Long): Flow<PagingData<Album>> = pager(40) { db.libraryDao().downloadedAlbums(sourceId) }
@@ -105,11 +101,6 @@ class LibraryRepository(
         genre: String,
     ): Flow<PagingData<AlbumSongItem>> = pager(60) { db.libraryDao().songsByGenre(sourceId, genre) }
 
-    suspend fun genreSongIds(
-        sourceId: Long,
-        genre: String,
-    ): List<String> = db.libraryDao().songsByGenreIds(sourceId, genre)
-
     suspend fun genreSongOrdinal(
         sourceId: Long,
         genre: String,
@@ -122,6 +113,11 @@ class LibraryRepository(
         id: String,
     ): Long = db.libraryDao().downloadedSongOrdinal(sourceId, id)
 
+    suspend fun starredSongOrdinal(
+        sourceId: Long,
+        id: String,
+    ): Long = db.libraryDao().starredSongOrdinal(sourceId, id)
+
     fun albumsByDecade(
         sourceId: Long,
         decade: Long,

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

@@ -262,6 +262,8 @@ class QueueRepository(
 
     fun downloadedEntry(sourceId: Long) = QueueEntry(position = 0, sourceId = sourceId, kind = QueueKind.Downloaded, refId = "")
 
+    fun starredEntry(sourceId: Long) = QueueEntry(position = 0, sourceId = sourceId, kind = QueueKind.Starred, refId = "")
+
     suspend fun sourceName(
         kind: QueueKind,
         sourceId: Long,
@@ -310,6 +312,10 @@ class QueueRepository(
             QueueKind.Downloaded -> {
                 null
             }
+
+            QueueKind.Starred -> {
+                null
+            }
         }
     }
 
@@ -663,6 +669,7 @@ class QueueRepository(
                     QueueKind.Artist -> dao.artistSongIds(ref.sourceId, ref.refId)
                     QueueKind.Genre -> dao.genreSongIds(ref.sourceId, ref.refId)
                     QueueKind.Downloaded -> dao.downloadedSongIds(ref.sourceId)
+                    QueueKind.Starred -> dao.starredSongIds(ref.sourceId)
                 }
             val from = ref.offset.toInt().coerceIn(0, entryIds.size)
             val to = (ref.offset + entry.length).toInt().coerceIn(from, entryIds.size)
@@ -765,6 +772,7 @@ class QueueRepository(
                 QueueKind.Artist -> dao.artistSongs(entry.sourceId, entry.refId, offset, limit)
                 QueueKind.Genre -> dao.genreSongs(entry.sourceId, entry.refId, offset, limit)
                 QueueKind.Downloaded -> dao.downloadedSongs(entry.sourceId, offset, limit)
+                QueueKind.Starred -> dao.starredSongs(entry.sourceId, offset, limit)
                 QueueKind.Song -> emptyList()
             }
         if (fresh.isNotEmpty()) remember(ref, offset, fresh)
@@ -790,6 +798,7 @@ class QueueRepository(
             QueueKind.Artist -> OrderKey(row.song.id)
             QueueKind.Genre -> OrderKey(row.song.id)
             QueueKind.Downloaded -> OrderKey(row.song.id)
+            QueueKind.Starred -> OrderKey(row.song.id)
             QueueKind.Song -> OrderKey(row.song.id)
         }
 
@@ -832,6 +841,7 @@ class QueueRepository(
             QueueKind.Artist -> emptyList()
             QueueKind.Genre -> emptyList()
             QueueKind.Downloaded -> emptyList()
+            QueueKind.Starred -> emptyList()
             QueueKind.Song -> emptyList()
         }
 
@@ -847,10 +857,10 @@ class QueueRepository(
             QueueKind.Artist -> emptyList()
             QueueKind.Genre -> emptyList()
             QueueKind.Downloaded -> emptyList()
+            QueueKind.Starred -> emptyList()
             QueueKind.Song -> emptyList()
         }
 
-    // Artist songs span albums, so there is no single keyset to seek on; artist pages use OFFSET.
     private fun OrderKey.seekable(kind: QueueKind): Boolean =
         when (kind) {
             QueueKind.Playlist -> position != null
@@ -858,6 +868,7 @@ class QueueRepository(
             QueueKind.Artist -> false
             QueueKind.Genre -> false
             QueueKind.Downloaded -> false
+            QueueKind.Starred -> false
             QueueKind.Song -> false
         }
 
@@ -909,6 +920,10 @@ class QueueRepository(
                 QueueKind.Downloaded -> {
                     dao.downloadedLength(sourceId)
                 }
+
+                QueueKind.Starred -> {
+                    dao.starredLength(sourceId)
+                }
             }
         val available = (total - offset).coerceAtLeast(0)
         return count?.coerceAtMost(available) ?: available

app/src/main/kotlin/com/subtracks/data/sync/SyncService.kt

@@ -285,7 +285,7 @@ private val songColumns =
 
 private val playlistColumns = listOf("sourceId", "id", "name", "comment", "coverArt", "songCount", "created", "changed", "duration")
 
-private fun Song.genreNames(): List<String> = genres.ifEmpty { listOfNotNull(genre?.trim()?.takeIf { it.isNotEmpty() }) }
+private fun Song.genreNames(): List<String> = genres.distinct().ifEmpty { listOfNotNull(genre?.trim()?.takeIf { it.isNotEmpty() }) }
 
 private val songGenreColumns = listOf("sourceId", "songId", "position", "genre")
 

app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt

@@ -210,16 +210,6 @@ class PlaybackController(
         songId: String,
     ) = playContext(queueRepository.songEntry(sourceId, songId), 0)
 
-    fun playSongs(
-        sourceId: Long,
-        songIds: List<String>,
-        startIndex: Int,
-    ) {
-        if (songIds.isEmpty()) return
-        val entries = songIds.map { id -> QueueEntry(position = 0, sourceId = sourceId, kind = QueueKind.Song, refId = id) }
-        play(entries, startIndex.toLong().coerceIn(0L, songIds.lastIndex.toLong()))
-    }
-
     fun playPlaylist(
         sourceId: Long,
         playlistId: String,
@@ -237,6 +227,11 @@ class PlaybackController(
         startOrdinal: Long,
     ) = playContext(queueRepository.downloadedEntry(sourceId), startOrdinal)
 
+    fun playStarred(
+        sourceId: Long,
+        startOrdinal: Long,
+    ) = playContext(queueRepository.starredEntry(sourceId), startOrdinal)
+
     fun playAlbumInOrder(
         sourceId: Long,
         albumId: String,
@@ -1250,6 +1245,12 @@ class PlaybackController(
             loadWindow(position, autoplay = player.playWhenReady, startPositionMs = player.currentPositionMs)
             return
         }
+        // A track downloaded while it was streaming still holds a stream URL; fix the current item's
+        // source in place (without resetting the window) so an offline switch can play the local file.
+        val current = ordered[index]
+        val wantsLocal = downloads.localUri(current.item.song.id) != null
+        val isLocal = player.itemUris().getOrNull(player.currentIndex)?.startsWith(FILE_SCHEME) == true
+        if (wantsLocal != isLocal) player.replaceItem(player.currentIndex, current.item.toQueueItem())
         windowPositions = LongArray(ordered.size) { ordered[it].position }
         windowFiltered = offline
         windowStart = ordered.first().position

app/src/main/kotlin/com/subtracks/ui/home/HomeViewModel.kt

@@ -206,8 +206,8 @@ internal fun CoroutineScope.playStarredList(
     song: Song,
 ) {
     launch {
-        val ids = libraryRepository.starredSongIds(song.sourceId)
+        val ordinal = libraryRepository.starredSongOrdinal(song.sourceId, song.id)
-        playbackController.playSongs(song.sourceId, ids, ids.indexOf(song.id).coerceAtLeast(0))
+        playbackController.playStarred(song.sourceId, ordinal)
     }
 }
 

app/src/main/kotlin/com/subtracks/ui/library/LibraryScreen.kt

@@ -406,7 +406,11 @@ fun LibraryScreen(
         if (pagerState.currentPage != selectedTab.ordinal) {
             pagerState.animateScrollToPage(selectedTab.ordinal)
         }
-        if (search.isNotEmpty()) searchActive = true
+        if (selectedTab == LibraryTab.Home) {
+            searchActive = false
+        } else if (search.isNotEmpty()) {
+            searchActive = true
+        }
     }
 
     fun dismissSearch() {

app/src/main/kotlin/com/subtracks/ui/playback/NowPlayingScreen.kt

@@ -472,6 +472,7 @@ private fun QueueKind.label(): String =
         QueueKind.Artist -> "artist"
         QueueKind.Genre -> "genre"
         QueueKind.Downloaded -> "downloads"
+        QueueKind.Starred -> "starred"
     }
 
 private fun formatTime(milliseconds: Long): String {

app/src/test/kotlin/com/subtracks/data/db/LibraryHomeDaoTest.kt

@@ -217,6 +217,27 @@ class LibraryHomeDaoTest {
             assertEquals(page.map { it.song.id }, dao.songsByGenreIds(sourceId, "Rock"))
         }
 
+    @Test
+    fun genreSongOrdinalMatchesTheListOrder() =
+        runTest {
+            val sourceId = source()
+            val dao = db.libraryDao()
+            dao.upsertSongs(
+                listOf(
+                    song(sourceId, "s1", "Alpha"),
+                    song(sourceId, "s2", "Bravo"),
+                    song(sourceId, "s3", "Charlie"),
+                ),
+            )
+            insertGenre(sourceId, "s1", 0, "Rock")
+            insertGenre(sourceId, "s2", 0, "Rock")
+            insertGenre(sourceId, "s3", 0, "Rock")
+
+            assertEquals(0L, dao.genreSongOrdinal(sourceId, "Rock", "Alpha", "s1"))
+            assertEquals(1L, dao.genreSongOrdinal(sourceId, "Rock", "Bravo", "s2"))
+            assertEquals(2L, dao.genreSongOrdinal(sourceId, "Rock", "Charlie", "s3"))
+        }
+
     @Test
     fun downloadedSongsOrderByNewestDownload() =
         runTest {

app/src/test/kotlin/com/subtracks/data/db/MigrationsTest.kt

@@ -134,6 +134,25 @@ class MigrationsTest {
             }
         }
 
+    @Test
+    fun theGenreBackfillTrimsAndSkipsBlankSingularGenres() =
+        runTest {
+            driver.open(":memory:").use { migrated ->
+                applySchema(migrated, versions.first())
+                MIGRATIONS.filter { it.endVersion <= 23 }.sortedBy { it.startVersion }.forEach { it.migrate(migrated) }
+                migrated.execSQL(
+                    "INSERT INTO sources (id, name, address, isActive, createdAt) VALUES (1, 's', 'http://x', 1, 0)",
+                )
+                migrated.execSQL("INSERT INTO songs (sourceId, id, title, genre) VALUES (1, 's1', 'Song', '  Rock ')")
+                migrated.execSQL("INSERT INTO songs (sourceId, id, title, genre) VALUES (1, 's2', 'Blank', '   ')")
+
+                MIGRATIONS.first { it.endVersion == 24 }.migrate(migrated)
+
+                assertEquals("Rock", text(migrated, "SELECT genre FROM song_genres WHERE songId = 's1'"))
+                assertEquals(0L, long(migrated, "SELECT COUNT(*) FROM song_genres WHERE songId = 's2'"))
+            }
+        }
+
     private fun text(
         connection: SQLiteConnection,
         sql: String,

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

@@ -87,6 +87,24 @@ class QueueRepositoryTest {
             assertEquals(listOf("s1", "s3"), resolveAll(snapshot))
         }
 
+    @Test
+    fun starredEntryResolvesStarredTracksNewestFirst() =
+        runTest {
+            seedLibrary()
+            db.libraryDao().upsertSongs(
+                listOf(
+                    song("s1", "al1", track = 1, starred = 100),
+                    song("s2", "al1", track = 2, starred = 300),
+                    song("s3", "al1", track = 3, starred = 200),
+                ),
+            )
+
+            val snapshot = repository.snapshotAfter(listOf(repository.starredEntry(1)))
+
+            assertEquals(3, snapshot.size)
+            assertEquals(listOf("s2", "s3", "s1"), resolveAll(snapshot))
+        }
+
     @Test
     fun rangeLimitsAnEntryToASlice() =
         runTest {

app/src/test/kotlin/com/subtracks/data/sync/SyncServiceTest.kt

@@ -326,7 +326,7 @@ class SyncServiceTest {
         }
 
     @Test
-    fun albumGenresUnionsSongGenresAndFallsBackToTheSingleValue() =
+    fun syncNormalizesSingularGenresIntoTheGenreRows() =
         runTest {
             insertSource()
             val source =
@@ -342,7 +342,24 @@ class SyncServiceTest {
 
             SyncService(db, source).sync()
 
-            assertEquals(listOf("Jazz", "Pop", "Rock"), db.libraryDao().albumGenres(1, "al1").first())
+            assertEquals(listOf("Rock", "Pop"), db.libraryDao().songGenres(1, "s1").first())
+            assertEquals(listOf("Jazz"), db.libraryDao().songGenres(1, "s2").first())
+            assertEquals(listOf("Rock"), db.libraryDao().songGenres(1, "s3").first())
+        }
+
+    @Test
+    fun thePluralGenresWinOverTheSingularAttribute() =
+        runTest {
+            insertSource()
+            val source =
+                FakeMusicSource(
+                    albums = listOf(album("al1")),
+                    songs = listOf(song("s1").copy(genres = listOf("Rock", "Pop"), genre = "Jazz")),
+                )
+
+            SyncService(db, source).sync()
+
+            assertEquals(listOf("Rock", "Pop"), db.libraryDao().songGenres(1, "s1").first())
         }
 
     @Test

app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt

@@ -2537,6 +2537,7 @@ class PlaybackControllerTest {
 
         await { handle.operations.any { it.startsWith("add") } }
         assertFalse(handle.operations.any { it.startsWith("setWindow") })
+        assertFalse(handle.operations.any { it == "prepare" || it == "play" || it == "pause" })
         assertEquals(
             "s5",
             controller.state.value.item
@@ -2804,6 +2805,7 @@ class PlaybackControllerTest {
 
         await { handle.operations.any { it.startsWith("remove") || it.startsWith("add") } }
         assertFalse(handle.operations.any { it.startsWith("setWindow") })
+        assertFalse(handle.operations.any { it == "prepare" || it == "play" || it == "pause" })
         assertEquals(
             "s2",
             controller.state.value.item