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.
This commit is contained in:
austinried
2026-10-18 14:47:00 +00:00
parent 65d74faa1d
commit d03dd982b2
2 changed files with 79 additions and 8 deletions
@@ -134,8 +134,8 @@ class SyncService(
} }
db.useWriterConnection { connection -> db.useWriterConnection { connection ->
connection.immediateTransaction { connection.immediateTransaction {
val albumChanged = connection.albumIdsChanged(batch) val songCreditsChanged = connection.songCreditsChanged(batch)
if (albumChanged) creditsChanged = true if (songCreditsChanged) creditsChanged = true
val resolved = connection.resolveSongSortNames(batch) val resolved = connection.resolveSongSortNames(batch)
connection.upsertChanged("songs", songColumns, primaryKey, resolved) { it.values() } connection.upsertChanged("songs", songColumns, primaryKey, resolved) { it.values() }
connection.usePrepared("DELETE FROM song_genres WHERE sourceId = ? AND songId = ? AND position >= ?") { statement -> 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() } connection.upsertChanged("song_artists", songArtistColumns, songArtistKey, artists) { it.values() }
val artistRowsChanged = connection.totalChanges() != artistRowsBefore val artistRowsChanged = connection.totalChanges() != artistRowsBefore
if (albumChanged || artistRowsChanged) { if (songCreditsChanged || artistRowsChanged) {
connection.markCreditsStale(batch.first().sourceId) connection.markCreditsStale(batch.first().sourceId)
creditsChanged = true creditsChanged = true
} }
@@ -196,8 +196,8 @@ class SyncService(
} }
} }
private suspend fun PooledConnection.albumIdsChanged(batch: List<Song>): Boolean { private suspend fun PooledConnection.songCreditsChanged(batch: List<Song>): Boolean {
val expected = batch.associate { it.id to it.albumId }.toMutableMap() val expected = batch.associate { it.id to (it.albumId to it.artistId) }.toMutableMap()
if (expected.isEmpty()) return false if (expected.isEmpty()) return false
var changed = false var changed = false
batch batch
@@ -205,13 +205,36 @@ class SyncService(
.chunked(SQL_VARIABLES) .chunked(SQL_VARIABLES)
.forEach { chunk -> .forEach { chunk ->
val placeholders = chunk.joinToString(",") { "?" } 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) statement.bindLong(1, chunk.first().first)
chunk.forEachIndexed { index, (_, id) -> statement.bindText(index + 2, id) } chunk.forEachIndexed { index, (_, id) -> statement.bindText(index + 2, id) }
while (statement.step()) { while (statement.step()) {
val id = statement.getText(0) val id = statement.getText(0)
val albumId = if (statement.isNull(1)) null else statement.getText(1) 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<Album>): 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 -> db.useWriterConnection { connection ->
connection.immediateTransaction { connection.immediateTransaction {
val leadsChanged = connection.albumLeadsChanged(albums)
val resolved = connection.resolveAlbumSortNames(albums) val resolved = connection.resolveAlbumSortNames(albums)
connection.upsertChanged("albums", albumColumns, primaryKey, resolved) { it.values() } connection.upsertChanged("albums", albumColumns, primaryKey, resolved) { it.values() }
connection.upsertChanged("discs", discColumns, discKey, discs) { 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() } connection.upsertChanged("album_artists", albumArtistColumns, albumArtistKey, artists) { it.values() }
if (connection.totalChanges() != artistRowsBefore) { if (leadsChanged || connection.totalChanges() != artistRowsBefore) {
connection.markCreditsStale(albums.first().sourceId) connection.markCreditsStale(albums.first().sourceId)
creditsChanged = true creditsChanged = true
} }
@@ -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 = private suspend fun totalChanges(): Long =
db.useWriterConnection { connection -> db.useWriterConnection { connection ->
connection.usePrepared("SELECT total_changes()") { statement -> connection.usePrepared("SELECT total_changes()") { statement ->