Makussu/subtracks
1
0
Fork 0
mirror of https://github.com/austinried/subtracks.git synced 2026-11-10 15:11:58 +01:00
Code Issues Packages Projects Releases Wiki Activity

library: unify artist credit totals and backfill song counts

Browse Source 
Use songCount as the total for both artist status queries and stop excluding
albumless tracks from download attribution, so the Artists badge and the
artist page agree. Backfill song counts in the 29->30 migration for offline
upgrades, and recompute only when a song moves albums or its credits change
rather than on every play-metadata sync.
This commit is contained in:
austinried
2026-10-18 04:14:29 +00:00
parent ba77b31dc4
commit 891406eab9
7 changed files with 179 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/src/main/kotlin/com/subtracks/data/db/DownloadDao.kt
+11 -14
View File
@@ -15,32 +15,26 @@ import kotlinx.coroutines.flow.Flow
// downloads are visible even though they lead no albums of their own.
// Starts from songs so status totals include every song the artist is credited on.
internal const val ARTIST_DOWNLOAD_ROWS =
"SELECT albums.artistId AS id, songs.id AS songId, song_downloads.status AS status, " +
"song_downloads.downloadedAt AS downloadedAt " +
"SELECT albums.artistId AS id, songs.id AS songId, song_downloads.status AS status " +
"FROM songs " +
"JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " +
"LEFT JOIN song_downloads ON song_downloads.sourceId = songs.sourceId AND song_downloads.songId = songs.id " +
"WHERE songs.sourceId = :sourceId AND albums.artistId IS NOT NULL " +
"UNION ALL " +
"SELECT album_artists.artistId AS id, songs.id AS songId, song_downloads.status AS status, " +
"song_downloads.downloadedAt AS downloadedAt " +
"SELECT album_artists.artistId AS id, songs.id AS songId, song_downloads.status AS status " +
"FROM songs " +
"JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " +
"JOIN album_artists ON album_artists.sourceId = albums.sourceId AND album_artists.albumId = albums.id " +
"LEFT JOIN song_downloads ON song_downloads.sourceId = songs.sourceId AND song_downloads.songId = songs.id " +
"WHERE songs.sourceId = :sourceId " +
"UNION ALL " +
"SELECT songs.artistId AS id, songs.id AS songId, song_downloads.status AS status, " +
"song_downloads.downloadedAt AS downloadedAt " +
"SELECT songs.artistId AS id, songs.id AS songId, song_downloads.status AS status " +
"FROM songs " +
"JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " +
"LEFT JOIN song_downloads ON song_downloads.sourceId = songs.sourceId AND song_downloads.songId = songs.id " +
"WHERE songs.sourceId = :sourceId AND songs.artistId IS NOT NULL " +
"UNION ALL " +
"SELECT song_artists.artistId AS id, songs.id AS songId, song_downloads.status AS status, " +
"song_downloads.downloadedAt AS downloadedAt " +
"SELECT song_artists.artistId AS id, songs.id AS songId, song_downloads.status AS status " +
"FROM songs " +
"JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " +
"JOIN song_artists ON song_artists.sourceId = songs.sourceId AND song_artists.songId = songs.id " +
"LEFT JOIN song_downloads ON song_downloads.sourceId = songs.sourceId AND song_downloads.songId = songs.id " +
"WHERE songs.sourceId = :sourceId"
@@ -66,14 +60,12 @@ internal const val DOWNLOADED_ARTIST_ROWS =
"song_downloads.downloadedAt AS downloadedAt, song_downloads.status AS status " +
"FROM song_downloads " +
"JOIN songs ON songs.sourceId = song_downloads.sourceId AND songs.id = song_downloads.songId " +
"JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " +
"WHERE song_downloads.sourceId = :sourceId AND songs.artistId IS NOT NULL " +
"UNION ALL " +
"SELECT song_artists.artistId AS id, song_downloads.songId AS songId, " +
"song_downloads.downloadedAt AS downloadedAt, song_downloads.status AS status " +
"FROM song_downloads " +
"JOIN songs ON songs.sourceId = song_downloads.sourceId AND songs.id = song_downloads.songId " +
"JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " +
"JOIN song_artists ON song_artists.sourceId = songs.sourceId AND song_artists.songId = songs.id " +
"WHERE song_downloads.sourceId = :sourceId"
@@ -89,10 +81,15 @@ interface DownloadDao {
suspend fun all(): List<SongDownload>
@Query(
"SELECT COUNT(DISTINCT songId) AS total, " +
"SELECT COALESCE((SELECT songCount FROM artists " +
"WHERE sourceId = :sourceId AND id = :artistId), 0) AS total, " +
"COALESCE(statuses.downloaded, 0) AS downloaded, " +
"COALESCE(statuses.downloading, 0) AS downloading " +
"FROM (SELECT " +
"COUNT(DISTINCT CASE WHEN status = 'Completed' THEN songId END) AS downloaded, " +
"COUNT(DISTINCT CASE WHEN status IN ('Queued', 'Running') THEN songId END) AS downloading " +
"FROM (" + ARTIST_DOWNLOAD_ROWS + ") WHERE id = :artistId",
"FROM (" + DOWNLOADED_ARTIST_ROWS + ") " +
"WHERE id = :artistId AND status IN ('Completed', 'Queued', 'Running')) statuses",
)
fun artistStatus(
sourceId: Long,
app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt
+1 -1
View File
@@ -108,7 +108,7 @@ internal const val ARTISTS_SELECT =
"CASE WHEN :downloadedFilter = 1 THEN (" +
"SELECT COUNT(*) FROM ($DOWNLOADED_ARTIST_ALBUMS)" +
") ELSE artists.albumCount END AS albumCount, " +
"artists.starred, artists.coverArt, artists.playCount, artists.played "
"artists.starred, artists.coverArt, artists.playCount, artists.played, artists.songCount "
internal const val ARTIST_ORDER_BY_NAME = "name COLLATE NOCASE, id"
internal const val ARTIST_ORDER_BY_NAME_REVERSED = "name COLLATE NOCASE DESC, id DESC"
app/src/main/kotlin/com/subtracks/data/db/Migrations.kt
+24
View File
@@ -426,6 +426,30 @@ val MIGRATION_29_30 =
object : Migration(29, 30) {
override suspend fun migrate(connection: SQLiteConnection) {
connection.execSQL("ALTER TABLE `artists` ADD COLUMN `songCount` INTEGER NOT NULL DEFAULT 0")
connection.execSQL(
"WITH credited AS (" +
"SELECT sourceId, artistId, songId FROM song_artists " +
"UNION " +
"SELECT sourceId, artistId, id AS songId FROM songs WHERE artistId IS NOT NULL " +
"UNION " +
"SELECT album_artists.sourceId, album_artists.artistId, songs.id " +
"FROM album_artists JOIN songs " +
"ON songs.sourceId = album_artists.sourceId AND songs.albumId = album_artists.albumId " +
"UNION " +
"SELECT albums.sourceId, albums.artistId, songs.id " +
"FROM albums JOIN songs ON songs.sourceId = albums.sourceId AND songs.albumId = albums.id " +
"WHERE albums.artistId IS NOT NULL" +
"), counts AS (" +
"SELECT sourceId, artistId AS id, COUNT(*) AS songCount FROM credited GROUP BY sourceId, artistId" +
") " +
"UPDATE artists SET songCount = c.songCount " +
"FROM (SELECT artists.sourceId AS sourceId, artists.id AS id, " +
"COALESCE(counts.songCount, 0) AS songCount " +
"FROM artists LEFT JOIN counts " +
"ON counts.sourceId = artists.sourceId AND counts.id = artists.id) c " +
"WHERE artists.sourceId = c.sourceId AND artists.id = c.id " +
"AND artists.songCount != c.songCount",
)
}
}
app/src/main/kotlin/com/subtracks/data/sync/SyncService.kt
+43 -9
View File
@@ -31,14 +31,15 @@ class SyncService(
private var playlistSongsSynced = 0
private var pruned = 0
private var creditsChanged = false
suspend fun sync(): SyncSummary {
creditsChanged = false
syncArtists()
val libraryBefore = totalChanges()
syncAlbums()
syncSongs()
val libraryChanged = totalChanges() != libraryBefore
optimize()
if (libraryChanged) recomputeArtistSongCounts()
if (creditsChanged) recomputeArtistSongCounts()
aggregatePlayData()
syncPlaylists()
syncPlaylistSongs()
@@ -79,7 +80,9 @@ class SyncService(
discs.forEach { seenDiscs.add(discHash(it.albumId, it.disc)) }
albumsSynced += batch.size
}
val prunedBefore = pruned
pruned += pruneStaleIds(seenAlbums, { library.albumIdsAfter(source.id, it, PRUNE_PAGE) }, { library.deleteAlbums(source.id, it) })
if (pruned > prunedBefore) creditsChanged = true
pruned +=
pruneStaleDiscs(seenDiscs, { albumId, disc -> library.discKeysAfter(source.id, albumId, disc, PRUNE_PAGE) }) {
deleteStaleDiscs(it)
@@ -95,7 +98,9 @@ class SyncService(
batch.forEach { seen.add(idHash(it.id)) }
songsSynced += batch.size
}
val prunedBefore = pruned
pruned += pruneStaleIds(seen, { library.songIdsAfter(source.id, it, PRUNE_PAGE) }, { library.deleteSongs(source.id, it) })
if (pruned > prunedBefore) creditsChanged = true
}
private suspend fun writeSongs(batch: List<Song>) {
@@ -110,6 +115,7 @@ class SyncService(
}
db.useWriterConnection { connection ->
connection.immediateTransaction {
if (connection.albumIdsChanged(batch)) creditsChanged = true
connection.upsertChanged("songs", songColumns, primaryKey, batch) { it.values() }
connection.usePrepared("DELETE FROM song_genres WHERE sourceId = ? AND songId = ? AND position >= ?") { statement ->
batch.forEach { song ->
@@ -122,6 +128,7 @@ class SyncService(
}
}
connection.upsertChanged("song_genres", songGenreColumns, songGenreKey, genres) { it.values() }
val artistRowsBefore = connection.totalChanges()
connection.usePrepared("DELETE FROM song_artists WHERE sourceId = ? AND songId = ? AND position >= ?") { statement ->
batch.forEach { song ->
statement.bind(1, song.sourceId)
@@ -133,6 +140,7 @@ class SyncService(
}
}
connection.upsertChanged("song_artists", songArtistColumns, songArtistKey, artists) { it.values() }
if (connection.totalChanges() != artistRowsBefore) creditsChanged = true
}
}
}
@@ -145,13 +153,36 @@ class SyncService(
db.libraryDao().recomputeArtistSongCounts(source.id)
}
private suspend fun totalChanges(): Long =
db.useWriterConnection { connection ->
connection.usePrepared("SELECT total_changes()") { statement ->
statement.step()
statement.getLong(0)
}
private suspend fun PooledConnection.totalChanges(): Long {
var value = 0L
usePrepared("SELECT total_changes()") { statement ->
statement.step()
value = statement.getLong(0)
}
return value
}
private suspend fun PooledConnection.albumIdsChanged(batch: List<Song>): Boolean {
val expected = batch.associate { it.id to it.albumId }.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, albumId 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
}
}
}
return changed || expected.isNotEmpty()
}
// Without sqlite_stat1 the artist and relation queries can fall back to whole-table scans.
private suspend fun optimize() {
@@ -213,6 +244,7 @@ class SyncService(
connection.immediateTransaction {
connection.upsertChanged("albums", albumColumns, primaryKey, albums) { it.values() }
connection.upsertChanged("discs", discColumns, discKey, discs) { it.values() }
val artistRowsBefore = connection.totalChanges()
connection.usePrepared("DELETE FROM album_artists WHERE sourceId = ? AND albumId = ? AND position >= ?") { statement ->
albums.forEach { album ->
statement.bind(1, album.sourceId)
@@ -224,6 +256,7 @@ class SyncService(
}
}
connection.upsertChanged("album_artists", albumArtistColumns, albumArtistKey, artists) { it.values() }
if (connection.totalChanges() != artistRowsBefore) creditsChanged = true
}
}
}
@@ -298,6 +331,7 @@ data class SyncSummary(
)
private const val DELETE_CHUNK = 500
private const val SQL_VARIABLES = 500
private const val PRUNE_PAGE = 500
private const val INITIAL_HASH_CAPACITY = 256
private const val FNV_OFFSET = -3750763034362895579L
app/src/test/kotlin/com/subtracks/data/db/MigrationsTest.kt
+24
View File
@@ -153,6 +153,30 @@ class MigrationsTest {
}
}
@Test
fun theArtistSongCountBackfillCountsLegacyCredits() =
runTest {
driver.open(":memory:").use { migrated ->
applySchema(migrated, 29)
migrated.execSQL(
"INSERT INTO sources (id, name, address, isActive, createdAt) VALUES (1, 's', 'http://x', 1, 0)",
)
migrated.execSQL("INSERT INTO artists (sourceId, id, name, albumCount) VALUES (1, 'ar1', 'Album Artist', 1)")
migrated.execSQL("INSERT INTO artists (sourceId, id, name, albumCount) VALUES (1, 'ar2', 'Guest', 0)")
migrated.execSQL(
"INSERT INTO albums (sourceId, id, artistId, name, albumArtist, created, songCount) " +
"VALUES (1, 'al1', 'ar1', 'Album', 'Album Artist', 0, 1)",
)
migrated.execSQL("INSERT INTO songs (sourceId, id, albumId, artistId, title) VALUES (1, 's1', 'al1', 'ar1', 'One')")
migrated.execSQL("INSERT INTO songs (sourceId, id, albumId, artistId, title) VALUES (1, 's2', NULL, 'ar2', 'Two')")
MIGRATION_29_30.migrate(migrated)
assertEquals(1L, long(migrated, "SELECT songCount FROM artists WHERE id = 'ar1'"))
assertEquals(1L, long(migrated, "SELECT songCount FROM artists WHERE id = 'ar2'"))
}
}
private fun text(
connection: SQLiteConnection,
sql: String,
app/src/test/kotlin/com/subtracks/data/repo/DownloadRepositoryTest.kt
+11 -1
View File
@@ -647,7 +647,17 @@ class DownloadRepositoryTest {
),
)
db.libraryDao().upsertArtists(
listOf(Artist(sourceId = 1, id = "ar-guest", name = "Guest", albumCount = 0, starred = null, coverArt = null)),
listOf(
Artist(
sourceId = 1,
id = "ar-guest",
name = "Guest",
albumCount = 0,
starred = null,
coverArt = null,
songCount = 1,
),
),
)
}
app/src/test/kotlin/com/subtracks/data/sync/SyncServiceTest.kt
+65
View File
@@ -999,6 +999,71 @@ class SyncServiceTest {
assertEquals(0L, db.libraryDao().artistOnce(1, "a2")!!.songCount)
}
@Test
fun artistSongCountsAndDownloadsIncludeAlbumlessTracks() =
runTest {
insertSource()
val source =
FakeMusicSource(
artists = listOf(artist("a1", "A")),
songs =
listOf(
song("s1").copy(albumId = null),
song("s2").copy(albumId = null),
),
)
SyncService(db, source).sync()
assertEquals(2L, db.libraryDao().artistOnce(1, "a1")!!.songCount)
db.downloadDao().upsert(SongDownload(1, "s1", DownloadStatus.Completed))
db.downloadDao().upsert(SongDownload(1, "s2", DownloadStatus.Completed))
assertEquals(
ListDownloadStatus(total = 2, downloaded = 2),
db.downloadDao().artistStatus(1, "a1").first(),
)
assertEquals(
2L,
db
.downloadDao()
.artistStatuses(1)
.first()
.single { it.id == "a1" }
.downloaded,
)
}
@Test
fun movingASongBetweenAlbumsUpdatesAlbumArtistCounts() =
runTest {
insertSource()
val source =
FakeMusicSource(
artists = listOf(artist("a1", "A"), artist("a2", "B"), artist("a3", "Track")),
albums =
listOf(
album("al1").copy(artistId = "a1"),
album("al2").copy(artistId = "a2"),
),
songs = listOf(song("s1").copy(artistId = "a3", albumId = "al1")),
)
SyncService(db, source).sync()
assertEquals(1L, db.libraryDao().artistOnce(1, "a1")!!.songCount)
assertEquals(0L, db.libraryDao().artistOnce(1, "a2")!!.songCount)
assertEquals(1L, db.libraryDao().artistOnce(1, "a3")!!.songCount)
source.songs = listOf(song("s1").copy(artistId = "a3", albumId = "al2"))
SyncService(db, source).sync()
assertEquals(0L, db.libraryDao().artistOnce(1, "a1")!!.songCount)
assertEquals(1L, db.libraryDao().artistOnce(1, "a2")!!.songCount)
assertEquals(1L, db.libraryDao().artistOnce(1, "a3")!!.songCount)
}
@Test
fun anIdenticalSyncDoesNotRewriteArtistRows() =
runTest {