library: sync every artist credited on an album or song

OpenSubsonic servers send plural artists/albumArtists refs and can omit the singular artistId when an album or song has more than one artist. Parse and store the ordered refs on the item and in indexed album_artists/song_artists tables, so artist pages, shuffles, downloads and the artist filters include every credited artist. Album and now playing artist taps open a chooser when several artists are credited; context menus get one artist row per artist.
2026-11-15 00:42:54 +01:00 · 2026-10-18 01:50:34 +00:00
parent fcfc5561ae
commit daf305f680
21 changed files with 2628 additions and 58 deletions
@@ -1,5 +1,6 @@
 package com.subtracks.data.source.subsonic

+import com.subtracks.data.model.ArtistRef
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertTrue
 import org.junit.Test
@@ -12,7 +13,7 @@ class SubsonicXmlTest {
        xml: String,
        tag: String,
        create: (Attributes) -> T,
-        onChild: (T, String, Attributes) -> Unit = { _, _, _ -> },
+        onChild: (T, String, Attributes, Int) -> Unit = { _, _, _, _ -> },
    ): List<T> {
        val result = mutableListOf<T>()
        SubsonicXml.readEntities(
@@ -30,7 +31,7 @@ class SubsonicXmlTest {
            xml,
            tag = "album",
            create = { SubsonicXml.albumDraft(1, it) },
-            onChild = { draft, name, attrs -> if (name == "discTitles") draft.addDiscTitle(attrs) },
+            onChild = SubsonicXml::albumChild,
        ).map { it.toAlbum() }

    private fun songs(xml: String) =
@@ -107,6 +108,114 @@ class SubsonicXmlTest {
        assertEquals(mapOf(1L to "The Calm", 2L to "The Storm"), albums.single().discTitles)
    }

+    @Test
+    fun mapsAlbumArtistsFromRepeatedOpenSubsonicRefs() {
+        val albums =
+            albums(
+                """
+                <subsonic-response status="ok">
+                  <albumList2>
+                    <album id="al1" name="Split" artist="A • B" songCount="1">
+                      <artists id="ar1" name="A"/>
+                      <artists id="ar2" name="B"/>
+                    </album>
+                  </albumList2>
+                </subsonic-response>
+                """.trimIndent(),
+            )
+
+        assertEquals(
+            listOf(ArtistRef("ar1", "A"), ArtistRef("ar2", "B")),
+            albums.single().artists,
+        )
+    }
+
+    @Test
+    fun mapsNestedAlbumArtistRefs() {
+        val albums =
+            albums(
+                """
+                <subsonic-response status="ok">
+                  <albumList2>
+                    <album id="al1" name="Split" songCount="1">
+                      <artists>
+                        <artist id="ar1" name="A"/>
+                        <artist id="ar2" name="B"/>
+                      </artists>
+                    </album>
+                  </albumList2>
+                </subsonic-response>
+                """.trimIndent(),
+            )
+
+        assertEquals(
+            listOf(ArtistRef("ar1", "A"), ArtistRef("ar2", "B")),
+            albums.single().artists,
+        )
+    }
+
+    @Test
+    fun fallsBackToTheSingularAlbumArtist() {
+        val albums =
+            albums(
+                """
+                <subsonic-response status="ok">
+                  <albumList2>
+                    <album id="al1" name="Kid A" artist="Radiohead" artistId="ar1" songCount="10"/>
+                  </albumList2>
+                </subsonic-response>
+                """.trimIndent(),
+            )
+
+        assertEquals(listOf(ArtistRef("ar1", "Radiohead")), albums.single().artists)
+    }
+
+    @Test
+    fun mapsSongArtistsAndIgnoresAlbumArtists() {
+        val songs =
+            songs(
+                """
+                <subsonic-response status="ok">
+                  <song id="s1" title="Collab" artist="A • B" albumId="al1">
+                    <artists id="ar1" name="A"/>
+                    <artists id="ar2" name="B"/>
+                    <albumArtists id="ar3" name="Album Artist"/>
+                  </song>
+                </subsonic-response>
+                """.trimIndent(),
+            )
+
+        assertEquals(
+            listOf(ArtistRef("ar1", "A"), ArtistRef("ar2", "B")),
+            songs.single().artists,
+        )
+    }
+
+    @Test
+    fun mapsNestedSongArtistRefs() {
+        val songs =
+            songs(
+                """
+                <subsonic-response status="ok">
+                  <song id="s1" title="Collab" albumId="al1">
+                    <artists>
+                      <artist id="ar1" name="A"/>
+                      <artist id="ar2" name="B"/>
+                    </artists>
+                    <albumArtists>
+                      <artist id="ar3" name="Album Artist"/>
+                    </albumArtists>
+                  </song>
+                </subsonic-response>
+                """.trimIndent(),
+            )
+
+        assertEquals(
+            listOf(ArtistRef("ar1", "A"), ArtistRef("ar2", "B")),
+            songs.single().artists,
+        )
+    }
+
    @Test
    fun parsesTimestampsWithFractionsAndOffsets() {
        assertEquals(1789906657L, IsoDate.parse("2026-09-20T12:17:37.370827413Z"))
@@ -138,6 +247,7 @@ class SubsonicXmlTest {
        assertEquals(1, songs.size)
        assertEquals("Idioteque", songs[0].title)
        assertEquals(300L, songs[0].duration)
+        assertEquals(listOf(ArtistRef("ar1", "Radiohead")), songs[0].artists)
        assertEquals(0L, entries[0].position)
        assertEquals("sg1", entries[0].songId)
    }
@@ -10,10 +10,14 @@ import androidx.test.ext.junit.runners.AndroidJUnit4
 import com.subtracks.data.db.SubtracksDatabase
 import com.subtracks.data.model.Album
 import com.subtracks.data.model.Artist
+import com.subtracks.data.model.ArtistRef
 import com.subtracks.data.model.DiscKey
+import com.subtracks.data.model.DownloadStatus
+import com.subtracks.data.model.ListDownloadStatus
 import com.subtracks.data.model.Playlist
 import com.subtracks.data.model.PlaylistSong
 import com.subtracks.data.model.Song
+import com.subtracks.data.model.SongDownload
 import com.subtracks.data.model.Source
 import com.subtracks.data.source.MusicSource
 import com.subtracks.data.source.StarType
@@ -928,6 +932,121 @@ class SyncServiceTest {
            assertEquals(null, cleared.albumPeak)
        }

+    @Test
+    fun everyAlbumArtistAndTrackArtistReachesTheSharedAlbum() =
+        runTest {
+            insertSource()
+            val source =
+                FakeMusicSource(
+                    artists = listOf(artist("a1", "A"), artist("a2", "B")),
+                    albums =
+                        listOf(
+                            album("al1").copy(
+                                artistId = "a1",
+                                albumArtist = "A • B",
+                                artists = listOf(ArtistRef("a1", "A"), ArtistRef("a2", "B")),
+                            ),
+                        ),
+                    songs =
+                        listOf(
+                            song("s1").copy(
+                                artistId = "a1",
+                                artist = "A • B",
+                                artists = listOf(ArtistRef("a1", "A"), ArtistRef("a2", "B")),
+                            ),
+                        ),
+                )
+
+            SyncService(db, source).sync()
+
+            assertEquals(
+                listOf("al1"),
+                db
+                    .libraryDao()
+                    .albumsForArtist(1, "a1")
+                    .first()
+                    .map { it.id },
+            )
+            assertEquals(
+                listOf("al1"),
+                db
+                    .libraryDao()
+                    .albumsForArtist(1, "a2")
+                    .first()
+                    .map { it.id },
+            )
+            assertEquals(listOf("s1"), db.queueDao().artistSongIds(1, "a1"))
+            assertEquals(listOf("s1"), db.queueDao().artistSongIds(1, "a2"))
+            assertEquals(1L, db.queueDao().artistLength(1, "a2"))
+            assertEquals("B", db.libraryDao().artistName(1, "a2").first())
+
+            source.albums = listOf(album("al1").copy(artistId = "a1", artists = listOf(ArtistRef("a1", "A"))))
+            source.songs = listOf(song("s1").copy(artistId = "a1", artists = listOf(ArtistRef("a1", "A"))))
+
+            SyncService(db, source).sync()
+
+            assertEquals(
+                emptyList<String>(),
+                db
+                    .libraryDao()
+                    .albumsForArtist(1, "a2")
+                    .first()
+                    .map { it.id },
+            )
+            assertEquals(emptyList<String>(), db.queueDao().artistSongIds(1, "a2"))
+        }
+
+    @Test
+    fun anIdenticalSyncDoesNotRewriteArtistRows() =
+        runTest {
+            insertSource()
+            val source =
+                FakeMusicSource(
+                    albums =
+                        listOf(
+                            album("al1").copy(artists = listOf(ArtistRef("a1", "Artist"), ArtistRef("a2", "Other"))),
+                        ),
+                    songs =
+                        listOf(
+                            song("s1").copy(artists = listOf(ArtistRef("a1", "Artist"), ArtistRef("a2", "Other"))),
+                        ),
+                )
+
+            SyncService(db, source).sync()
+            val before = totalChanges()
+
+            SyncService(db, source).sync()
+
+            assertEquals(before, totalChanges())
+        }
+
+    @Test
+    fun theDownloadedArtistFilterIncludesEveryAlbumArtist() =
+        runTest {
+            insertSource()
+            val source =
+                FakeMusicSource(
+                    artists = listOf(artist("a1", "A"), artist("a2", "B")),
+                    albums =
+                        listOf(
+                            album("al1").copy(
+                                artistId = "a1",
+                                albumArtist = "A • B",
+                                artists = listOf(ArtistRef("a1", "A"), ArtistRef("a2", "B")),
+                            ),
+                        ),
+                    songs = listOf(song("s1").copy(artistId = "a1")),
+                )
+
+            SyncService(db, source).sync()
+            db.downloadDao().upsert(SongDownload(1, "s1", DownloadStatus.Completed))
+
+            val filtered = db.libraryDao().artistsByName(1, 0, "", downloadedFilter = 1).allRows()
+            assertEquals(listOf("A", "B"), filtered.map { it.name })
+            assertEquals(listOf(1L, 1L), filtered.map { it.albumCount })
+            assertEquals(ListDownloadStatus(total = 1, downloaded = 1), db.downloadDao().artistStatus(1, "a2").first())
+        }
+
    private suspend fun totalChanges(): Long =
        db.useWriterConnection { connection ->
            connection.usePrepared("SELECT total_changes()") { statement ->
@@ -177,7 +177,7 @@ class DemoScreenshotTest {
                            ArtistDetailScreen(
                                artist = demo.artist,
                                artistName = demo.artist.name,
-                                isAlbumArtist = demo.artistAlbums.any { it.artistId == demo.artist.id },
+                                isAlbumArtist = demo.artistAlbums.any { album -> album.artistRefs.any { it.id == demo.artist.id } },
                                albums = demo.artistAlbums,
                                art = demo.coverArt(demo.artist.coverArt, false),
                                artThumbnail = demo.coverArt(demo.artist.coverArt, true),
@@ -0,0 +1,46 @@
+package com.subtracks.ui.components
+
+import androidx.activity.ComponentActivity
+import androidx.compose.ui.test.assertIsDisplayed
+import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule
+import androidx.compose.ui.test.onNodeWithText
+import androidx.compose.ui.test.performClick
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import com.subtracks.data.model.ArtistRef
+import com.subtracks.ui.theme.SubtracksTheme
+import org.junit.Assert.assertEquals
+import org.junit.Rule
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.annotation.GraphicsMode
+
+@RunWith(AndroidJUnit4::class)
+@GraphicsMode(GraphicsMode.Mode.NATIVE)
+class ArtistChooserSheetTest {
+    @get:Rule
+    val composeRule = createAndroidComposeRule<ComponentActivity>()
+
+    @Test
+    fun listsEveryArtistAndOpensTheChosenOne() {
+        var opened: ArtistRef? = null
+        composeRule.setContent {
+            SubtracksTheme {
+                ArtistChooserSheet(
+                    artists = listOf(ArtistRef("ar1", "A"), ArtistRef("ar2", "B")),
+                    onArtistClick = { opened = it },
+                    onDismiss = {},
+                )
+            }
+        }
+
+        composeRule.onNodeWithText("Artists").assertIsDisplayed()
+        composeRule.onNodeWithText("A").assertIsDisplayed()
+        composeRule.onNodeWithText("B").assertIsDisplayed()
+
+        composeRule.mainClock.autoAdvance = false
+        composeRule.onNodeWithText("B").performClick()
+        composeRule.mainClock.advanceTimeBy(1_000)
+
+        assertEquals(ArtistRef("ar2", "B"), opened)
+    }
+}
@@ -0,0 +1,70 @@
+package com.subtracks.ui.components
+
+import androidx.activity.ComponentActivity
+import androidx.compose.runtime.getValue
+import androidx.compose.runtime.mutableStateOf
+import androidx.compose.runtime.remember
+import androidx.compose.runtime.setValue
+import androidx.compose.ui.test.assertIsDisplayed
+import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule
+import androidx.compose.ui.test.onNodeWithText
+import androidx.compose.ui.test.performClick
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import com.subtracks.data.model.ArtistRef
+import com.subtracks.data.model.Song
+import com.subtracks.ui.theme.SubtracksTheme
+import org.junit.Assert.assertEquals
+import org.junit.Rule
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.annotation.GraphicsMode
+
+@RunWith(AndroidJUnit4::class)
+@GraphicsMode(GraphicsMode.Mode.NATIVE)
+class ItemContextMenuArtistTest {
+    @get:Rule
+    val composeRule = createAndroidComposeRule<ComponentActivity>()
+
+    private val song =
+        Song(
+            sourceId = 1,
+            id = "s1",
+            albumId = "al1",
+            artistId = "ar1",
+            title = "Song",
+            album = "Album",
+            artist = "A • B",
+            duration = 100,
+            track = 1,
+            disc = 1,
+            starred = null,
+            genre = null,
+            artists = listOf(ArtistRef("ar1", "A"), ArtistRef("ar2", "B")),
+        )
+
+    @Test
+    fun showsOneEntryPerArtistAndOpensTheChosenOne() {
+        var opened: String? = null
+        composeRule.setContent {
+            SubtracksTheme {
+                var show by remember { mutableStateOf(true) }
+                if (show) {
+                    ItemContextMenu(
+                        target = MenuTarget.Song(song),
+                        actions = ItemActions(viewArtist = { opened = it }),
+                        onDismiss = { show = false },
+                    )
+                }
+            }
+        }
+
+        composeRule.onNodeWithText("A").assertIsDisplayed()
+        composeRule.onNodeWithText("B").assertIsDisplayed()
+
+        composeRule.mainClock.autoAdvance = false
+        composeRule.onNodeWithText("B").performClick()
+        composeRule.mainClock.advanceTimeBy(1_000)
+
+        assertEquals("ar2", opened)
+    }
+}