mirror of
https://github.com/austinried/subtracks.git
synced 2026-11-10 15:11:58 +01:00
store multiple OpenSubsonic song genres
OpenSubsonic servers return a per-song <genres> array that was dropped, leaving every song with only its first (or joined) legacy genre. Parse it and store the ordered list in a song_genres table, keeping the single genre column as the fallback for base Subsonic servers. Album genres are derived by unioning the album's song genres, falling back to the single song genre, so album-level genre never trusts the per-server album genres[] behaviour. Schema 21 -> 22, with parser, sync, unit and integration coverage.
This commit is contained in:
+1504
File diff suppressed because it is too large
Load Diff
+8
-1
@@ -63,7 +63,14 @@ class SubsonicSourceIntegrationTest(
|
||||
@Test
|
||||
fun songs() =
|
||||
runBlocking {
|
||||
assertEquals(20, source.songs().collectAll().size)
|
||||
val songs = source.songs().collectAll()
|
||||
|
||||
assertEquals(20, songs.size)
|
||||
assertTrue(songs.all { song -> song.genre == null || song.genres.isEmpty() || song.genres.contains(song.genre) })
|
||||
// Nextcloud Music does not populate the OpenSubsonic genres array.
|
||||
if (server.name != "nextcloud") {
|
||||
assertTrue(songs.any { it.genres.isNotEmpty() })
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
@@ -627,6 +627,29 @@ interface LibraryDao {
|
||||
songId: String,
|
||||
): Song?
|
||||
|
||||
@Query("SELECT genre FROM song_genres WHERE sourceId = :sourceId AND songId = :songId ORDER BY position")
|
||||
fun songGenres(
|
||||
sourceId: Long,
|
||||
songId: String,
|
||||
): Flow<List<String>>
|
||||
|
||||
@Query(
|
||||
"SELECT genre FROM (" +
|
||||
"SELECT song_genres.genre AS 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 " +
|
||||
"UNION " +
|
||||
"SELECT songs.genre AS genre FROM songs " +
|
||||
"WHERE songs.sourceId = :sourceId AND songs.albumId = :albumId AND songs.genre IS NOT NULL " +
|
||||
"AND NOT EXISTS (SELECT 1 FROM song_genres " +
|
||||
"WHERE song_genres.sourceId = songs.sourceId AND song_genres.songId = songs.id)" +
|
||||
") ORDER BY 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,
|
||||
|
||||
@@ -308,6 +308,22 @@ val MIGRATION_20_21 =
|
||||
}
|
||||
}
|
||||
|
||||
val MIGRATION_21_22 =
|
||||
object : Migration(21, 22) {
|
||||
override suspend fun migrate(connection: SQLiteConnection) {
|
||||
connection.execSQL(
|
||||
"CREATE TABLE IF NOT EXISTS `song_genres` (`sourceId` INTEGER NOT NULL, `songId` TEXT NOT NULL, " +
|
||||
"`position` INTEGER NOT NULL, `genre` TEXT NOT NULL, " +
|
||||
"PRIMARY KEY(`sourceId`, `songId`, `position`), " +
|
||||
"FOREIGN KEY(`sourceId`, `songId`) REFERENCES `songs`(`sourceId`, `id`) " +
|
||||
"ON UPDATE NO ACTION ON DELETE CASCADE )",
|
||||
)
|
||||
connection.execSQL(
|
||||
"CREATE INDEX IF NOT EXISTS `index_song_genres_sourceId_genre` ON `song_genres` (`sourceId`, `genre`)",
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// Mirrors the FTS5 table and content-sync triggers Room generates for the @Fts5 entities.
|
||||
private suspend fun createSearchIndex(
|
||||
connection: SQLiteConnection,
|
||||
@@ -362,4 +378,5 @@ val MIGRATIONS: Array<Migration> =
|
||||
MIGRATION_18_19,
|
||||
MIGRATION_19_20,
|
||||
MIGRATION_20_21,
|
||||
MIGRATION_21_22,
|
||||
)
|
||||
|
||||
@@ -18,6 +18,7 @@ import com.subtracks.data.model.QueueEntry
|
||||
import com.subtracks.data.model.QueueKindConverter
|
||||
import com.subtracks.data.model.Song
|
||||
import com.subtracks.data.model.SongDownload
|
||||
import com.subtracks.data.model.SongGenre
|
||||
import com.subtracks.data.model.Source
|
||||
import com.subtracks.data.model.SubsonicSource
|
||||
import com.subtracks.data.model.UpNextEntry
|
||||
@@ -35,13 +36,14 @@ import com.subtracks.data.model.UpNextEntry
|
||||
PlaylistSearch::class,
|
||||
PlaylistSong::class,
|
||||
Song::class,
|
||||
SongGenre::class,
|
||||
QueueEntry::class,
|
||||
UpNextEntry::class,
|
||||
PlaybackCursor::class,
|
||||
ArtworkSeed::class,
|
||||
SongDownload::class,
|
||||
],
|
||||
version = 21,
|
||||
version = 22,
|
||||
exportSchema = true,
|
||||
)
|
||||
@ColumnTypeConverters(QueueKindConverter::class, DownloadStatusConverter::class)
|
||||
|
||||
@@ -259,6 +259,27 @@ data class Song(
|
||||
@ColumnInfo(defaultValue = "0") val created: Long = 0,
|
||||
@ColumnInfo(defaultValue = "0") val playCount: Long = 0,
|
||||
val played: Long? = null,
|
||||
@Ignore val genres: List<String> = emptyList(),
|
||||
)
|
||||
|
||||
@Entity(
|
||||
tableName = "song_genres",
|
||||
primaryKeys = ["sourceId", "songId", "position"],
|
||||
foreignKeys = [
|
||||
ForeignKey(
|
||||
entity = Song::class,
|
||||
parentColumns = ["sourceId", "id"],
|
||||
childColumns = ["sourceId", "songId"],
|
||||
onDelete = ForeignKey.CASCADE,
|
||||
),
|
||||
],
|
||||
indices = [Index("sourceId", "genre")],
|
||||
)
|
||||
data class SongGenre(
|
||||
val sourceId: Long,
|
||||
val songId: String,
|
||||
val position: Long,
|
||||
val genre: String,
|
||||
)
|
||||
|
||||
sealed interface SongItem {
|
||||
|
||||
@@ -202,10 +202,11 @@ class SubsonicSource(
|
||||
"albumCount" to "0",
|
||||
),
|
||||
tag = "song",
|
||||
create = { SubsonicXml.song(id, it) },
|
||||
create = { SubsonicXml.songDraft(id, it) },
|
||||
onChild = { song, name, attrs -> if (name == "genres") song.addGenre(attrs) },
|
||||
).collect { batch ->
|
||||
raw += batch.size
|
||||
val accepted = batch.filter { it.id.isNotEmpty() }
|
||||
val accepted = batch.map { it.toSong() }.filter { it.id.isNotEmpty() }
|
||||
if (accepted.isNotEmpty()) emit(accepted)
|
||||
}
|
||||
if (raw < PAGE_SIZE) {
|
||||
@@ -244,9 +245,10 @@ class SubsonicSource(
|
||||
method = "getAlbum",
|
||||
params = mapOf("id" to albumId),
|
||||
tag = "song",
|
||||
create = { SubsonicXml.song(id, it) },
|
||||
create = { SubsonicXml.songDraft(id, it) },
|
||||
onChild = { song, name, attrs -> if (name == "genres") song.addGenre(attrs) },
|
||||
).collect { batch ->
|
||||
val accepted = batch.filter { it.id.isNotEmpty() }
|
||||
val accepted = batch.map { it.toSong() }.filter { it.id.isNotEmpty() }
|
||||
if (accepted.isNotEmpty()) send(accepted)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -52,8 +52,13 @@ object SubsonicXml {
|
||||
internal fun song(
|
||||
sourceId: Long,
|
||||
attrs: Attributes,
|
||||
): Song =
|
||||
Song(
|
||||
): Song = songDraft(sourceId, attrs).toSong()
|
||||
|
||||
internal fun songDraft(
|
||||
sourceId: Long,
|
||||
attrs: Attributes,
|
||||
): SongDraft =
|
||||
SongDraft(
|
||||
sourceId = sourceId,
|
||||
id = attrs.attr("id"),
|
||||
albumId = attrs.attr("albumId").ifEmpty { null },
|
||||
@@ -161,6 +166,53 @@ internal class AlbumDraft(
|
||||
)
|
||||
}
|
||||
|
||||
internal class SongDraft(
|
||||
private val sourceId: Long,
|
||||
private val id: String = "",
|
||||
private val albumId: String? = null,
|
||||
private val artistId: String? = null,
|
||||
private val title: String = "",
|
||||
private val album: String? = null,
|
||||
private val artist: String? = null,
|
||||
private val duration: Long? = null,
|
||||
private val track: Long? = null,
|
||||
private val disc: Long? = null,
|
||||
private val starred: Long? = null,
|
||||
private val genre: String? = null,
|
||||
private val created: Long = 0L,
|
||||
private val playCount: Long = 0L,
|
||||
private val played: Long? = null,
|
||||
private val genres: MutableList<String> = mutableListOf(),
|
||||
) {
|
||||
fun addGenre(attrs: Attributes) {
|
||||
attrs
|
||||
.attr("name")
|
||||
.trim()
|
||||
.takeIf { it.isNotEmpty() }
|
||||
?.let { genres += it }
|
||||
}
|
||||
|
||||
fun toSong(): Song =
|
||||
Song(
|
||||
sourceId = sourceId,
|
||||
id = id,
|
||||
albumId = albumId,
|
||||
artistId = artistId,
|
||||
title = title,
|
||||
album = album,
|
||||
artist = artist,
|
||||
duration = duration,
|
||||
track = track,
|
||||
disc = disc,
|
||||
starred = starred,
|
||||
genre = genre,
|
||||
created = created,
|
||||
playCount = playCount,
|
||||
played = played,
|
||||
genres = genres.toList(),
|
||||
)
|
||||
}
|
||||
|
||||
internal fun Attributes.attr(name: String): String = getValue(name) ?: ""
|
||||
|
||||
internal fun Attributes.longAttr(name: String): Long? = attr(name).takeIf { it.isNotEmpty() }?.toLongOrNull()
|
||||
|
||||
@@ -12,6 +12,7 @@ import com.subtracks.data.model.DiscKey
|
||||
import com.subtracks.data.model.Playlist
|
||||
import com.subtracks.data.model.PlaylistSong
|
||||
import com.subtracks.data.model.Song
|
||||
import com.subtracks.data.model.SongGenre
|
||||
import com.subtracks.data.source.MusicSource
|
||||
import java.util.Arrays
|
||||
|
||||
@@ -64,12 +65,35 @@ class SyncService(
|
||||
val seen = HashedIds()
|
||||
source.songs().collect { batch ->
|
||||
if (batch.isEmpty()) return@collect
|
||||
write("songs", songColumns, primaryKey, batch) { it.values() }
|
||||
writeSongs(batch)
|
||||
batch.forEach { seen.add(idHash(it.id)) }
|
||||
}
|
||||
pruneStaleIds(seen, { library.songIdsAfter(source.id, it, PRUNE_PAGE) }, { library.deleteSongs(source.id, it) })
|
||||
}
|
||||
|
||||
private suspend fun writeSongs(batch: List<Song>) {
|
||||
val genres =
|
||||
batch.flatMap { song ->
|
||||
song.genres.mapIndexed { index, genre -> SongGenre(song.sourceId, song.id, index.toLong(), genre) }
|
||||
}
|
||||
db.useWriterConnection { connection ->
|
||||
connection.immediateTransaction {
|
||||
connection.upsertChanged("songs", songColumns, primaryKey, batch) { it.values() }
|
||||
connection.usePrepared("DELETE FROM song_genres WHERE sourceId = ? AND songId = ? AND position >= ?") { statement ->
|
||||
batch.forEach { song ->
|
||||
statement.bind(1, song.sourceId)
|
||||
statement.bind(2, song.id)
|
||||
statement.bind(3, song.genres.size.toLong())
|
||||
statement.step()
|
||||
statement.reset()
|
||||
statement.clearBindings()
|
||||
}
|
||||
}
|
||||
connection.upsertChanged("song_genres", songGenreColumns, songGenreKey, genres) { it.values() }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private suspend fun aggregatePlayData() {
|
||||
db.libraryDao().recomputePlayData(source.id)
|
||||
}
|
||||
@@ -260,6 +284,10 @@ private val songColumns =
|
||||
|
||||
private val playlistColumns = listOf("sourceId", "id", "name", "comment", "coverArt", "songCount", "created", "changed", "duration")
|
||||
|
||||
private val songGenreColumns = listOf("sourceId", "songId", "position", "genre")
|
||||
|
||||
private val songGenreKey = listOf("sourceId", "songId", "position")
|
||||
|
||||
private val playlistSongColumns = listOf("sourceId", "playlistId", "songId", "position")
|
||||
|
||||
private val playlistSongKey = listOf("sourceId", "playlistId", "position")
|
||||
@@ -289,6 +317,8 @@ private fun Song.values() =
|
||||
played,
|
||||
)
|
||||
|
||||
private fun SongGenre.values() = listOf<Any?>(sourceId, songId, position, genre)
|
||||
|
||||
private fun Playlist.values() = listOf<Any?>(sourceId, id, name, comment, coverArt, songCount, created, changed, duration)
|
||||
|
||||
private fun PlaylistSong.values() = listOf<Any?>(sourceId, playlistId, songId, position)
|
||||
|
||||
@@ -193,6 +193,69 @@ class SubsonicSourceTest {
|
||||
assertEquals(listOf(0, 500, 1000), firstOffsets)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun songsKeepTheOpenSubsonicGenresArray() =
|
||||
runBlocking {
|
||||
server.dispatcher =
|
||||
object : Dispatcher() {
|
||||
override fun dispatch(request: RecordedRequest): MockResponse {
|
||||
val url = request.requestUrl!!
|
||||
if (url.encodedPath != "/rest/search3.view") return MockResponse().setResponseCode(404)
|
||||
return MockResponse().setBody(
|
||||
"<subsonic-response status=\"ok\"><searchResult3>" +
|
||||
"<song id=\"s1\" title=\"Multi\" genre=\"Rock\">" +
|
||||
"<genres name=\"Rock\"/><genres name=\"Electronic\"/>" +
|
||||
"</song></searchResult3></subsonic-response>",
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
val songs = SubsonicSource(1, client()).songs().toList().flatten()
|
||||
|
||||
assertEquals(listOf("Rock", "Electronic"), songs.single().genres)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun albumSongsKeepTheOpenSubsonicGenresArray() =
|
||||
runBlocking {
|
||||
server.dispatcher =
|
||||
object : Dispatcher() {
|
||||
override fun dispatch(request: RecordedRequest): MockResponse {
|
||||
val url = request.requestUrl!!
|
||||
return when (url.encodedPath) {
|
||||
"/rest/search3.view" -> {
|
||||
MockResponse().setBody(failed(10, "Required parameter is missing"))
|
||||
}
|
||||
|
||||
"/rest/getAlbumList2.view" -> {
|
||||
MockResponse().setBody(
|
||||
"<subsonic-response status=\"ok\"><albumList2>" +
|
||||
"<album id=\"al1\" name=\"Album\" artist=\"Artist\" artistId=\"ar1\" songCount=\"1\"/>" +
|
||||
"</albumList2></subsonic-response>",
|
||||
)
|
||||
}
|
||||
|
||||
"/rest/getAlbum.view" -> {
|
||||
MockResponse().setBody(
|
||||
"<subsonic-response status=\"ok\"><album id=\"al1\">" +
|
||||
"<song id=\"s1\" title=\"Multi\" genre=\"Rock\">" +
|
||||
"<genres name=\"Rock\"/><genres name=\"Electronic\"/>" +
|
||||
"</song></album></subsonic-response>",
|
||||
)
|
||||
}
|
||||
|
||||
else -> {
|
||||
MockResponse().setResponseCode(404)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
val songs = SubsonicSource(1, client()).songs().toList().flatten()
|
||||
|
||||
assertEquals(listOf("Rock", "Electronic"), songs.single().genres)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun entitiesWithEmptyIdsAreDropped() =
|
||||
runBlocking {
|
||||
|
||||
@@ -154,6 +154,57 @@ class SubsonicXmlTest {
|
||||
assertEquals(null, songs[1].played)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun mapsSongGenresAndIgnoresEmptyOnes() {
|
||||
val xml =
|
||||
"""
|
||||
<subsonic-response status="ok">
|
||||
<searchResult3>
|
||||
<song id="s1" title="Multi">
|
||||
<genres name="Rock"/>
|
||||
<genres name="Electronic"/>
|
||||
<genres name=""/>
|
||||
<genres name=" "/>
|
||||
</song>
|
||||
</searchResult3>
|
||||
</subsonic-response>
|
||||
""".trimIndent()
|
||||
|
||||
val songs =
|
||||
parse(
|
||||
xml,
|
||||
tag = "song",
|
||||
create = { SubsonicXml.songDraft(1, it) },
|
||||
onChild = { draft, name, attrs -> if (name == "genres") draft.addGenre(attrs) },
|
||||
).map { it.toSong() }
|
||||
|
||||
assertEquals(listOf("Rock", "Electronic"), songs.single().genres)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun ignoresSongGenresThatAreNotDirectChildren() {
|
||||
val xml =
|
||||
"""
|
||||
<subsonic-response status="ok">
|
||||
<searchResult3>
|
||||
<song id="s1" title="Nested">
|
||||
<wrapper><genres name="Rock"/></wrapper>
|
||||
</song>
|
||||
</searchResult3>
|
||||
</subsonic-response>
|
||||
""".trimIndent()
|
||||
|
||||
val songs =
|
||||
parse(
|
||||
xml,
|
||||
tag = "song",
|
||||
create = { SubsonicXml.songDraft(1, it) },
|
||||
onChild = { draft, name, attrs -> if (name == "genres") draft.addGenre(attrs) },
|
||||
).map { it.toSong() }
|
||||
|
||||
assertEquals(emptyList<String>(), songs.single().genres)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun statusOnlyReadRejectsAnHtmlLoginPage() {
|
||||
val failure =
|
||||
|
||||
@@ -248,6 +248,103 @@ class SyncServiceTest {
|
||||
assertEquals(5678L, stored?.starred)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun syncStoresSongGenresInOrder() =
|
||||
runTest {
|
||||
insertSource()
|
||||
val source =
|
||||
FakeMusicSource(
|
||||
albums = listOf(album("al1")),
|
||||
songs = listOf(song("s1").copy(genres = listOf("Rock", "Electronic"))),
|
||||
)
|
||||
|
||||
SyncService(db, source).sync()
|
||||
|
||||
assertEquals(listOf("Rock", "Electronic"), db.libraryDao().songGenres(1, "s1").first())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun genresShrinkAndClearWithTheSong() =
|
||||
runTest {
|
||||
insertSource()
|
||||
val source =
|
||||
FakeMusicSource(
|
||||
albums = listOf(album("al1")),
|
||||
songs = listOf(song("s1").copy(genres = listOf("Rock", "Electronic", "Pop"))),
|
||||
)
|
||||
|
||||
SyncService(db, source).sync()
|
||||
assertEquals(listOf("Rock", "Electronic", "Pop"), db.libraryDao().songGenres(1, "s1").first())
|
||||
|
||||
source.songs = listOf(song("s1").copy(genres = listOf("Rock")))
|
||||
|
||||
SyncService(db, source).sync()
|
||||
assertEquals(listOf("Rock"), db.libraryDao().songGenres(1, "s1").first())
|
||||
|
||||
source.songs = listOf(song("s1"))
|
||||
|
||||
SyncService(db, source).sync()
|
||||
assertEquals(emptyList<String>(), db.libraryDao().songGenres(1, "s1").first())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun anIdenticalSyncDoesNotRewriteGenreRows() =
|
||||
runTest {
|
||||
insertSource()
|
||||
val source =
|
||||
FakeMusicSource(
|
||||
albums = listOf(album("al1")),
|
||||
songs = listOf(song("s1").copy(genres = listOf("Rock", "Electronic"))),
|
||||
)
|
||||
|
||||
SyncService(db, source).sync()
|
||||
val before = totalChanges()
|
||||
|
||||
SyncService(db, source).sync()
|
||||
|
||||
assertEquals(before, totalChanges())
|
||||
assertEquals(listOf("Rock", "Electronic"), db.libraryDao().songGenres(1, "s1").first())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun removingASongRemovesItsGenres() =
|
||||
runTest {
|
||||
insertSource()
|
||||
val source =
|
||||
FakeMusicSource(
|
||||
albums = listOf(album("al1")),
|
||||
songs = listOf(song("s1").copy(genres = listOf("Rock"))),
|
||||
)
|
||||
|
||||
SyncService(db, source).sync()
|
||||
assertEquals(listOf("Rock"), db.libraryDao().songGenres(1, "s1").first())
|
||||
|
||||
source.songs = emptyList()
|
||||
|
||||
SyncService(db, source).sync()
|
||||
assertEquals(emptyList<String>(), db.libraryDao().songGenres(1, "s1").first())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun albumGenresUnionsSongGenresAndFallsBackToTheSingleValue() =
|
||||
runTest {
|
||||
insertSource()
|
||||
val source =
|
||||
FakeMusicSource(
|
||||
albums = listOf(album("al1")),
|
||||
songs =
|
||||
listOf(
|
||||
song("s1").copy(genres = listOf("Rock", "Pop")),
|
||||
song("s2").copy(genre = "Jazz"),
|
||||
song("s3").copy(genre = "Rock"),
|
||||
),
|
||||
)
|
||||
|
||||
SyncService(db, source).sync()
|
||||
|
||||
assertEquals(listOf("Jazz", "Pop", "Rock"), db.libraryDao().albumGenres(1, "al1").first())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun syncStoresAlbumDiscTitles() =
|
||||
runTest {
|
||||
|
||||
Reference in New Issue
Block a user