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: align artist counts with credited songs

Browse Source 
Artist badges and play rankings only counted album credits, while the artist
page, shuffle queue and download attribution already included track credits.
Derive albumCount and the artist plays from the same four-way credit union,
count lead ids that the server omits from the plural refs, and bump the song's
credited artists on a local scrobble even when the track has no album.

Sort the downloaded artist list by the downloaded album count it displays
instead of the synced total, and backfill the artist totals on upgrade so the
fix applies before the next sync.
This commit is contained in:
austinried
2026-10-18 14:25:07 +00:00
parent 916da72339
commit e0cfba489e
7 changed files with 2199 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/schemas/com.subtracks.data.db.SubtracksDatabase/34.json
Generated
+1900
View File
File diff suppressed because it is too large Load Diff
app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt
+47 -31
View File
@@ -97,6 +97,25 @@ private const val DOWNLOADED_ARTIST_ALBUMS =
internal const val ARTIST_CREDITED =
"(artists.credited = 1 OR COALESCE((SELECT creditsStale FROM sources WHERE id = artists.sourceId), 1) = 1)"
private const val CREDITED_SONG_ROWS =
"SELECT songs.artistId AS artistId, songs.id AS songId, songs.albumId AS albumId, " +
"songs.playCount AS playCount, songs.played AS played " +
"FROM songs WHERE songs.sourceId = :sourceId AND songs.artistId IS NOT NULL " +
"UNION " +
"SELECT song_artists.artistId, songs.id, songs.albumId, songs.playCount, songs.played " +
"FROM song_artists JOIN songs " +
"ON songs.sourceId = song_artists.sourceId AND songs.id = song_artists.songId " +
"WHERE song_artists.sourceId = :sourceId " +
"UNION " +
"SELECT albums.artistId, songs.id, songs.albumId, songs.playCount, songs.played " +
"FROM albums JOIN songs ON songs.sourceId = albums.sourceId AND songs.albumId = albums.id " +
"WHERE albums.sourceId = :sourceId AND albums.artistId IS NOT NULL " +
"UNION " +
"SELECT album_artists.artistId, songs.id, songs.albumId, songs.playCount, songs.played " +
"FROM album_artists JOIN songs " +
"ON songs.sourceId = album_artists.sourceId AND songs.albumId = album_artists.albumId " +
"WHERE album_artists.sourceId = :sourceId"
internal const val ARTISTS_FILTER =
"FROM artists WHERE sourceId = :sourceId " +
"AND (:starredFilter = 0 OR (:starredFilter = 1 AND starred IS NOT NULL) OR (:starredFilter = 2 AND starred IS NULL)) " +
@@ -124,6 +143,11 @@ internal const val ARTIST_ORDER_BY_NAME = "sortName COLLATE NOCASE, id"
internal const val ARTIST_ORDER_BY_NAME_REVERSED = "sortName COLLATE NOCASE DESC, id DESC"
internal const val ARTIST_ORDER_BY_ALBUM_COUNT = "artists.albumCount DESC, sortName COLLATE NOCASE, id"
internal const val ARTIST_ORDER_BY_ALBUM_COUNT_REVERSED = "artists.albumCount ASC, sortName COLLATE NOCASE DESC, id DESC"
// These order on the projected alias so the downloaded list sorts by the number it shows.
internal const val ARTIST_ORDER_BY_DOWNLOADED_ALBUM_COUNT = "albumCount DESC, sortName COLLATE NOCASE, id"
internal const val ARTIST_ORDER_BY_DOWNLOADED_ALBUM_COUNT_REVERSED =
"albumCount ASC, sortName COLLATE NOCASE DESC, id DESC"
internal const val ARTIST_ORDER_BY_STARRED = "starred DESC NULLS LAST, sortName COLLATE NOCASE, id"
internal const val ARTIST_ORDER_BY_STARRED_REVERSED = "starred ASC NULLS LAST, sortName COLLATE NOCASE DESC, id DESC"
internal const val ARTIST_ORDER_BY_FREQUENT = "artists.playCount DESC, sortName COLLATE NOCASE, id"
@@ -211,19 +235,10 @@ interface LibraryDao {
)
suspend fun recomputeAlbumPlayData(sourceId: Long)
// The credited albums are the union of the lead id and the relation table, so a shared album
// counts once per artist rather than twice.
// Every credited song, from both the lead ids and the relation tables, deduplicated so a song
// counts once per artist even when several credit paths point at it.
@Query(
"WITH credited AS (" +
"SELECT albums.artistId AS artistId, albums.id AS albumId, " +
"albums.playCount AS playCount, albums.played AS played " +
"FROM albums WHERE albums.sourceId = :sourceId AND albums.artistId IS NOT NULL " +
"UNION " +
"SELECT album_artists.artistId, albums.id, albums.playCount, albums.played " +
"FROM album_artists JOIN albums " +
"ON albums.sourceId = album_artists.sourceId AND albums.id = album_artists.albumId " +
"WHERE album_artists.sourceId = :sourceId" +
"), totals AS (" +
"WITH credited AS ($CREDITED_SONG_ROWS), totals AS (" +
"SELECT artistId AS id, SUM(playCount) AS playCount, MAX(played) AS played " +
"FROM credited GROUP BY artistId" +
") " +
@@ -236,20 +251,17 @@ interface LibraryDao {
)
suspend fun recomputeArtistPlayData(sourceId: Long)
// Stored so the download badge query can report an artist's total without walking every song.
// Stored so the download badge query can report an artist's total without walking every song,
// and so the artist badges use the same credit set as the artist page and the shuffle queue.
@Query(
"WITH credited AS (" +
"SELECT artistId, songId FROM song_artists WHERE sourceId = :sourceId " +
"UNION " +
"SELECT album_artists.artistId AS artistId, songs.id AS songId " +
"FROM album_artists " +
"JOIN songs ON songs.sourceId = album_artists.sourceId AND songs.albumId = album_artists.albumId " +
"WHERE album_artists.sourceId = :sourceId" +
"), counts AS (" +
"SELECT artistId AS id, COUNT(*) AS songCount FROM credited GROUP BY artistId" +
"WITH credited AS ($CREDITED_SONG_ROWS), counts AS (" +
"SELECT artistId AS id, COUNT(DISTINCT songId) AS songCount, " +
"COUNT(DISTINCT albumId) AS albumCount " +
"FROM credited GROUP BY artistId" +
") " +
"UPDATE artists SET songCount = c.songCount, credited = c.credited " +
"UPDATE artists SET songCount = c.songCount, albumCount = c.albumCount, credited = c.credited " +
"FROM (SELECT artists.id AS id, COALESCE(counts.songCount, 0) AS songCount, " +
"COALESCE(counts.albumCount, 0) AS albumCount, " +
"((NOT EXISTS (SELECT 1 FROM album_artists WHERE sourceId = artists.sourceId) " +
"AND NOT EXISTS (SELECT 1 FROM song_artists WHERE sourceId = artists.sourceId)) " +
"OR EXISTS (SELECT 1 FROM album_artists aa " +
@@ -259,7 +271,8 @@ interface LibraryDao {
"FROM artists LEFT JOIN counts ON counts.id = artists.id " +
"WHERE artists.sourceId = :sourceId) c " +
"WHERE artists.sourceId = :sourceId AND artists.id = c.id " +
"AND (artists.songCount != c.songCount OR artists.credited != c.credited)",
"AND (artists.songCount != c.songCount OR artists.albumCount != c.albumCount " +
"OR artists.credited != c.credited)",
)
suspend fun recomputeArtistSongCounts(sourceId: Long)
@@ -293,11 +306,14 @@ interface LibraryDao {
"UPDATE artists SET playCount = playCount + 1, played = MAX(COALESCE(played, :at), :at) " +
"WHERE sourceId = :sourceId AND (" +
"id = (SELECT artistId FROM albums WHERE sourceId = :sourceId AND id = :albumId) " +
"OR id IN (SELECT artistId FROM album_artists WHERE sourceId = :sourceId AND albumId = :albumId))",
"OR id IN (SELECT artistId FROM album_artists WHERE sourceId = :sourceId AND albumId = :albumId) " +
"OR id = (SELECT artistId FROM songs WHERE sourceId = :sourceId AND id = :songId) " +
"OR id IN (SELECT artistId FROM song_artists WHERE sourceId = :sourceId AND songId = :songId))",
)
suspend fun bumpArtistPlay(
sourceId: Long,
albumId: String,
albumId: String?,
songId: String,
at: Long,
)
@@ -316,9 +332,9 @@ interface LibraryDao {
at: Long,
) {
bumpSongPlay(sourceId, songId, at)
val albumId = albumIdForSong(sourceId, songId) ?: return
bumpAlbumPlay(sourceId, albumId, at)
bumpArtistPlay(sourceId, albumId, at)
val albumId = albumIdForSong(sourceId, songId)
if (albumId != null) bumpAlbumPlay(sourceId, albumId, at)
bumpArtistPlay(sourceId, albumId, songId, at)
}
// Albums, not songs: these flows re-run on every write to their table, and songs are written
@@ -702,7 +718,7 @@ interface LibraryDao {
downloadedFilter: Int = 0,
): PagingSource<Int, Artist>
@Query("$ARTISTS_SELECT $ARTISTS_FILTER ORDER BY $ARTIST_ORDER_BY_ALBUM_COUNT")
@Query("$ARTISTS_SELECT $ARTISTS_FILTER ORDER BY $ARTIST_ORDER_BY_DOWNLOADED_ALBUM_COUNT")
fun artistsByAlbumCount(
sourceId: Long,
starredFilter: Int,
@@ -710,7 +726,7 @@ interface LibraryDao {
downloadedFilter: Int = 0,
): PagingSource<Int, Artist>
@Query("$ARTISTS_SELECT $ARTISTS_FILTER ORDER BY $ARTIST_ORDER_BY_ALBUM_COUNT_REVERSED")
@Query("$ARTISTS_SELECT $ARTISTS_FILTER ORDER BY $ARTIST_ORDER_BY_DOWNLOADED_ALBUM_COUNT_REVERSED")
fun artistsByAlbumCountReversed(
sourceId: Long,
starredFilter: Int,
app/src/main/kotlin/com/subtracks/data/db/Migrations.kt
+50
View File
@@ -579,6 +579,55 @@ val MIGRATION_32_33 =
}
}
// Recomputes the artist totals from the same credit union the sync uses, so badges, the artist
// page and the shuffle queue agree for libraries that predate the change.
val MIGRATION_33_34 =
object : Migration(33, 34) {
override suspend fun migrate(connection: SQLiteConnection) {
connection.execSQL(
"WITH credited AS (" +
"SELECT songs.`sourceId` AS sourceId, songs.`artistId` AS artistId, " +
"songs.`id` AS songId, songs.`albumId` AS albumId, " +
"songs.`playCount` AS playCount, songs.`played` AS played " +
"FROM songs WHERE songs.`artistId` IS NOT NULL " +
"UNION " +
"SELECT song_artists.`sourceId`, song_artists.`artistId`, songs.`id`, songs.`albumId`, " +
"songs.`playCount`, songs.`played` " +
"FROM song_artists JOIN songs " +
"ON songs.`sourceId` = song_artists.`sourceId` AND songs.`id` = song_artists.`songId` " +
"UNION " +
"SELECT albums.`sourceId`, albums.`artistId`, songs.`id`, songs.`albumId`, " +
"songs.`playCount`, songs.`played` " +
"FROM albums JOIN songs " +
"ON songs.`sourceId` = albums.`sourceId` AND songs.`albumId` = albums.`id` " +
"WHERE albums.`artistId` IS NOT NULL " +
"UNION " +
"SELECT album_artists.`sourceId`, album_artists.`artistId`, songs.`id`, songs.`albumId`, " +
"songs.`playCount`, songs.`played` " +
"FROM album_artists JOIN songs " +
"ON songs.`sourceId` = album_artists.`sourceId` AND songs.`albumId` = album_artists.`albumId`" +
"), totals AS (" +
"SELECT sourceId, artistId AS id, COUNT(DISTINCT songId) AS songCount, " +
"COUNT(DISTINCT albumId) AS albumCount, SUM(playCount) AS playCount, MAX(played) AS played " +
"FROM credited GROUP BY sourceId, artistId" +
") " +
"UPDATE artists SET songCount = c.songCount, albumCount = c.albumCount, " +
"playCount = c.playCount, played = c.played " +
"FROM (SELECT artists.`sourceId` AS sourceId, artists.`id` AS id, " +
"COALESCE(totals.songCount, 0) AS songCount, COALESCE(totals.albumCount, 0) AS albumCount, " +
"COALESCE(totals.playCount, 0) AS playCount, totals.played AS played " +
"FROM artists LEFT JOIN totals " +
"ON totals.sourceId = artists.`sourceId` AND totals.id = artists.`id` " +
"WHERE EXISTS (SELECT 1 FROM album_artists WHERE album_artists.`sourceId` = artists.`sourceId`) " +
"AND EXISTS (SELECT 1 FROM song_artists WHERE song_artists.`sourceId` = artists.`sourceId`)" +
") c " +
"WHERE artists.`sourceId` = c.sourceId AND artists.`id` = c.id " +
"AND (artists.`songCount` != c.songCount OR artists.`albumCount` != c.albumCount " +
"OR artists.`playCount` != c.playCount OR artists.`played` IS NOT c.played)",
)
}
}
// Mirrors the FTS5 table and content-sync triggers Room generates for the @Fts5 entities.
private suspend fun createSearchIndex(
connection: SQLiteConnection,
@@ -645,4 +694,5 @@ val MIGRATIONS: Array<Migration> =
MIGRATION_30_31,
MIGRATION_31_32,
MIGRATION_32_33,
MIGRATION_33_34,
)
app/src/main/kotlin/com/subtracks/data/db/SubtracksDatabase.kt
+1 -1
View File
@@ -54,7 +54,7 @@ import com.subtracks.data.model.UpNextEntry
SongDownload::class,
PendingAction::class,
],
version = 33,
version = 34,
exportSchema = true,
)
@ColumnTypeConverters(
app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt
+9 -7
View File
@@ -182,8 +182,10 @@ class LibraryDaoTest {
dao.upsertSongs(
listOf(
song(sourceId, "s1", "One", starred = null).copy(albumId = "al-1", playCount = 3, played = 100),
song(sourceId, "s2", "Two", starred = null).copy(albumId = "al-2", playCount = 4, played = 200),
song(sourceId, "s3", "Three", starred = null).copy(albumId = "al-2", playCount = 5, played = 400),
song(sourceId, "s2", "Two", starred = null)
.copy(albumId = "al-2", artistId = "ar-2", playCount = 4, played = 200),
song(sourceId, "s3", "Three", starred = null)
.copy(albumId = "al-2", artistId = "ar-2", playCount = 5, played = 400),
),
)
@@ -607,7 +609,7 @@ class LibraryDaoTest {
}
@Test
fun downloadedFilterShowsDownloadedAlbumCountsButSortsByTheSyncedCount() =
fun downloadedFilterSortsByTheDownloadedAlbumCountItShows() =
runTest {
val sourceId = source()
val dao = db.libraryDao()
@@ -635,11 +637,11 @@ class LibraryDaoTest {
db.downloadDao().upsert(SongDownload(sourceId, it, DownloadStatus.Completed))
}
// ar-1 has two downloaded albums, ar-2 one. The list still orders by the synced count
// (so the album-count index stays usable) but reports the downloaded count.
// ar-1 has two downloaded albums, ar-2 one, so the downloaded order is ar-1 first even
// though the synced counts would put ar-2 first.
val filtered = dao.artistsByAlbumCount(sourceId, 0, "", downloadedFilter = 1).page()
assertEquals(listOf("ar-2", "ar-1"), filtered.map { it.id })
assertEquals(listOf(1L, 2L), filtered.map { it.albumCount })
assertEquals(listOf("ar-1", "ar-2"), filtered.map { it.id })
assertEquals(listOf(2L, 1L), filtered.map { it.albumCount })
}
@Test
app/src/test/kotlin/com/subtracks/data/db/MigrationsTest.kt
+61
View File
@@ -272,6 +272,67 @@ class MigrationsTest {
}
}
@Test
fun theCreditTotalsBackfillUsesEveryCreditedSong() =
runTest {
driver.open(":memory:").use { migrated ->
applySchema(migrated, 33)
migrated.execSQL(
"INSERT INTO sources (id, name, address, isActive, createdAt) VALUES (1, 's', 'http://x', 1, 0)",
)
migrated.execSQL(
"INSERT INTO sources (id, name, address, isActive, createdAt) VALUES (2, 't', 'http://y', 1, 0)",
)
migrated.execSQL(
"INSERT INTO artists (sourceId, id, name, albumCount, songCount, playCount, played) " +
"VALUES (1, 'a1', 'Album Artist', 1, 0, 0, NULL)",
)
migrated.execSQL(
"INSERT INTO artists (sourceId, id, name, albumCount, songCount, playCount, played) " +
"VALUES (1, 'a2', 'Guest', 0, 0, 0, NULL)",
)
migrated.execSQL(
"INSERT INTO artists (sourceId, id, name, albumCount, songCount, playCount, played) " +
"VALUES (2, 'b1', 'Unsynced', 7, 6, 5, 100)",
)
migrated.execSQL(
"INSERT INTO albums (sourceId, id, artistId, name, albumArtist, created, songCount) " +
"VALUES (1, 'al1', 'a1', 'Album', 'Album Artist', 0, 1)",
)
migrated.execSQL(
"INSERT INTO album_artists (sourceId, albumId, position, artistId, name) " +
"VALUES (1, 'al1', 0, 'a1', 'Album Artist')",
)
migrated.execSQL(
"INSERT INTO songs (sourceId, id, albumId, artistId, title, playCount, played) " +
"VALUES (1, 's1', 'al1', 'a1', 'One', 4, 200)",
)
migrated.execSQL(
"INSERT INTO song_artists (sourceId, songId, position, artistId, name) " +
"VALUES (1, 's1', 0, 'a1', 'Album Artist')",
)
migrated.execSQL(
"INSERT INTO song_artists (sourceId, songId, position, artistId, name) " +
"VALUES (1, 's1', 1, 'a2', 'Guest')",
)
MIGRATION_33_34.migrate(migrated)
assertEquals(1L, long(migrated, "SELECT songCount FROM artists WHERE id = 'a1'"))
assertEquals(1L, long(migrated, "SELECT albumCount FROM artists WHERE id = 'a1'"))
assertEquals(4L, long(migrated, "SELECT playCount FROM artists WHERE id = 'a1'"))
assertEquals(200L, long(migrated, "SELECT played FROM artists WHERE id = 'a1'"))
assertEquals(1L, long(migrated, "SELECT songCount FROM artists WHERE id = 'a2'"))
assertEquals(1L, long(migrated, "SELECT albumCount FROM artists WHERE id = 'a2'"))
assertEquals(4L, long(migrated, "SELECT playCount FROM artists WHERE id = 'a2'"))
assertEquals(200L, long(migrated, "SELECT played FROM artists WHERE id = 'a2'"))
assertEquals(6L, long(migrated, "SELECT songCount FROM artists WHERE id = 'b1'"))
assertEquals(7L, long(migrated, "SELECT albumCount FROM artists WHERE id = 'b1'"))
assertEquals(5L, long(migrated, "SELECT playCount FROM artists WHERE id = 'b1'"))
assertEquals(100L, long(migrated, "SELECT played FROM artists WHERE id = 'b1'"))
}
}
private fun text(
connection: SQLiteConnection,
sql: String,
app/src/test/kotlin/com/subtracks/data/sync/SyncServiceTest.kt
+131
View File
@@ -1465,6 +1465,137 @@ class SyncServiceTest {
assertEquals(500L, db.libraryDao().artistOnce(1, "a2")!!.played)
}
@Test
fun trackOnlyCreditsReachTheArtistBadgeAndPlays() =
runTest {
insertSource()
val source =
FakeMusicSource(
artists = listOf(artist("a1", "Album Artist"), artist("a2", "Guest").copy(albumCount = 7)),
albums =
listOf(
album("al1").copy(
artistId = "a1",
artists = listOf(ArtistRef("a1", "Album Artist")),
),
),
songs =
listOf(
song("s1").copy(
albumId = "al1",
artistId = "a1",
artists = listOf(ArtistRef("a1", "Album Artist"), ArtistRef("a2", "Guest")),
playCount = 3,
played = 100,
),
),
)
SyncService(db, source).sync()
val guest = db.libraryDao().artistOnce(1, "a2")!!
assertEquals(1L, guest.albumCount)
assertEquals(1L, guest.songCount)
assertEquals(3L, guest.playCount)
assertEquals(100L, guest.played)
assertEquals(
listOf("al1"),
db
.libraryDao()
.albumsForArtist(1, "a2")
.first()
.map { it.id },
)
assertEquals(
db
.libraryDao()
.albumsForArtist(1, "a2")
.first()
.size
.toLong(),
db
.libraryDao()
.artistsByName(1, 0, "")
.allRows()
.first { it.id == "a2" }
.albumCount,
)
}
@Test
fun albumlessTrackCreditsCountAndBumpOnPlay() =
runTest {
insertSource()
val source =
FakeMusicSource(
artists = listOf(artist("a1", "Album Artist"), artist("a2", "Guest")),
albums = emptyList(),
songs =
listOf(
song("s1").copy(
albumId = null,
artistId = "a1",
artists = listOf(ArtistRef("a1", "Album Artist"), ArtistRef("a2", "Guest")),
playCount = 2,
played = 50,
),
),
)
SyncService(db, source).sync()
val guest = db.libraryDao().artistOnce(1, "a2")!!
assertEquals(0L, guest.albumCount)
assertEquals(1L, guest.songCount)
assertEquals(2L, guest.playCount)
assertEquals(50L, guest.played)
db.libraryDao().recordPlay(1, "s1", 300)
assertEquals(3L, db.libraryDao().artistOnce(1, "a2")!!.playCount)
assertEquals(300L, db.libraryDao().artistOnce(1, "a2")!!.played)
}
@Test
fun leadIdsOutsideThePluralRefsKeepArtistTotalsAndShufflesAligned() =
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()
val lead = db.libraryDao().artistOnce(1, "a1")!!
assertEquals(1L, lead.songCount)
assertEquals(1L, lead.albumCount)
assertEquals(db.queueDao().artistLength(1, "a1"), lead.songCount)
assertEquals(
lead.songCount,
db
.downloadDao()
.artistStatus(1, "a1")
.first()
.total,
)
}
private suspend fun totalChanges(): Long =
db.useWriterConnection { connection ->
connection.usePrepared("SELECT total_changes()") { statement ->