From d03dd982b2f120562162c52d4af46d8c3e2d25ea Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Sun, 18 Oct 2026 14:47:00 +0000 Subject: [PATCH] library: recompute credits when only a lead artist changes The credit recompute was keyed on song-to-album moves and relation-table writes, so a server that changed songs.artistId or albums.artistId while the plural refs stayed the same left the counts attributed to the old lead. Fold the lead ids into the change check for both songs and albums. --- .../com/subtracks/data/sync/SyncService.kt | 40 ++++++++++++---- .../subtracks/data/sync/SyncServiceTest.kt | 47 +++++++++++++++++++ 2 files changed, 79 insertions(+), 8 deletions(-) diff --git a/app/src/main/kotlin/com/subtracks/data/sync/SyncService.kt b/app/src/main/kotlin/com/subtracks/data/sync/SyncService.kt index 9fd34505..ad9049dd 100644 --- a/app/src/main/kotlin/com/subtracks/data/sync/SyncService.kt +++ b/app/src/main/kotlin/com/subtracks/data/sync/SyncService.kt @@ -134,8 +134,8 @@ class SyncService( } db.useWriterConnection { connection -> connection.immediateTransaction { - val albumChanged = connection.albumIdsChanged(batch) - if (albumChanged) creditsChanged = true + val songCreditsChanged = connection.songCreditsChanged(batch) + if (songCreditsChanged) creditsChanged = true val resolved = connection.resolveSongSortNames(batch) connection.upsertChanged("songs", songColumns, primaryKey, resolved) { it.values() } connection.usePrepared("DELETE FROM song_genres WHERE sourceId = ? AND songId = ? AND position >= ?") { statement -> @@ -162,7 +162,7 @@ class SyncService( } connection.upsertChanged("song_artists", songArtistColumns, songArtistKey, artists) { it.values() } val artistRowsChanged = connection.totalChanges() != artistRowsBefore - if (albumChanged || artistRowsChanged) { + if (songCreditsChanged || artistRowsChanged) { connection.markCreditsStale(batch.first().sourceId) creditsChanged = true } @@ -196,8 +196,8 @@ class SyncService( } } - private suspend fun PooledConnection.albumIdsChanged(batch: List): Boolean { - val expected = batch.associate { it.id to it.albumId }.toMutableMap() + private suspend fun PooledConnection.songCreditsChanged(batch: List): Boolean { + val expected = batch.associate { it.id to (it.albumId to it.artistId) }.toMutableMap() if (expected.isEmpty()) return false var changed = false batch @@ -205,13 +205,36 @@ class SyncService( .chunked(SQL_VARIABLES) .forEach { chunk -> val placeholders = chunk.joinToString(",") { "?" } - usePrepared("SELECT id, albumId FROM songs WHERE sourceId = ? AND id IN ($placeholders)") { statement -> + usePrepared("SELECT id, albumId, artistId FROM songs WHERE sourceId = ? AND id IN ($placeholders)") { statement -> statement.bindLong(1, chunk.first().first) chunk.forEachIndexed { index, (_, id) -> statement.bindText(index + 2, id) } while (statement.step()) { val id = statement.getText(0) val albumId = if (statement.isNull(1)) null else statement.getText(1) - if (expected.remove(id) != albumId) changed = true + val artistId = if (statement.isNull(2)) null else statement.getText(2) + if (expected.remove(id) != (albumId to artistId)) changed = true + } + } + } + return changed || expected.isNotEmpty() + } + + private suspend fun PooledConnection.albumLeadsChanged(batch: List): Boolean { + val expected = batch.associate { it.id to it.artistId }.toMutableMap() + if (expected.isEmpty()) return false + var changed = false + batch + .map { it.sourceId to it.id } + .chunked(SQL_VARIABLES) + .forEach { chunk -> + val placeholders = chunk.joinToString(",") { "?" } + usePrepared("SELECT id, artistId FROM albums WHERE sourceId = ? AND id IN ($placeholders)") { statement -> + statement.bindLong(1, chunk.first().first) + chunk.forEachIndexed { index, (_, id) -> statement.bindText(index + 2, id) } + while (statement.step()) { + val id = statement.getText(0) + val artistId = if (statement.isNull(1)) null else statement.getText(1) + if (expected.remove(id) != artistId) changed = true } } } @@ -334,6 +357,7 @@ class SyncService( } db.useWriterConnection { connection -> connection.immediateTransaction { + val leadsChanged = connection.albumLeadsChanged(albums) val resolved = connection.resolveAlbumSortNames(albums) connection.upsertChanged("albums", albumColumns, primaryKey, resolved) { it.values() } connection.upsertChanged("discs", discColumns, discKey, discs) { it.values() } @@ -349,7 +373,7 @@ class SyncService( } } connection.upsertChanged("album_artists", albumArtistColumns, albumArtistKey, artists) { it.values() } - if (connection.totalChanges() != artistRowsBefore) { + if (leadsChanged || connection.totalChanges() != artistRowsBefore) { connection.markCreditsStale(albums.first().sourceId) creditsChanged = true } diff --git a/app/src/test/kotlin/com/subtracks/data/sync/SyncServiceTest.kt b/app/src/test/kotlin/com/subtracks/data/sync/SyncServiceTest.kt index 35496dbe..aab636cf 100644 --- a/app/src/test/kotlin/com/subtracks/data/sync/SyncServiceTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/sync/SyncServiceTest.kt @@ -1636,6 +1636,53 @@ class SyncServiceTest { ) } + @Test + fun changingOnlyTheLeadArtistRecomputesTheCreditedCounts() = + runTest { + insertSource() + val source = + FakeMusicSource( + artists = listOf(artist("a1", "Lead"), artist("a2", "Ref")), + albums = + listOf( + album("al1").copy( + artistId = "a1", + artists = listOf(ArtistRef("a2", "Ref")), + ), + ), + songs = + listOf( + song("s1").copy( + albumId = "al1", + artistId = "a1", + artists = listOf(ArtistRef("a2", "Ref")), + ), + ), + ) + + SyncService(db, source).sync() + + assertEquals(1L, db.libraryDao().artistOnce(1, "a1")!!.songCount) + assertEquals(1L, db.libraryDao().artistOnce(1, "a1")!!.albumCount) + + source.albums = listOf(album("al1").copy(artistId = "a2", artists = listOf(ArtistRef("a2", "Ref")))) + source.songs = + listOf( + song("s1").copy( + albumId = "al1", + artistId = "a2", + artists = listOf(ArtistRef("a2", "Ref")), + ), + ) + + SyncService(db, source).sync() + + assertEquals(0L, db.libraryDao().artistOnce(1, "a1")!!.songCount) + assertEquals(0L, db.libraryDao().artistOnce(1, "a1")!!.albumCount) + assertEquals(1L, db.libraryDao().artistOnce(1, "a2")!!.songCount) + assertEquals(1L, db.libraryDao().artistOnce(1, "a2")!!.albumCount) + } + private suspend fun totalChanges(): Long = db.useWriterConnection { connection -> connection.usePrepared("SELECT total_changes()") { statement ->