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: count credited albums from album credits too

Browse Source 
The artist badge now counts albums from album lead ids and album_artists even
when an album has no songs, and only counts song-credited albums that still
exist, matching what albumsForArtist lists. Preserve the derived albumCount
when the server re-upserts an artist row so an identical sync does not rewrite
it twice, relax the migration guard to either relation table, and share one
album-count order between the plain and downloaded artist lists.
This commit is contained in:
austinried
2026-10-18 14:40:16 +00:00
parent e0cfba489e
commit 65d74faa1d
5 changed files with 158 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt
+33 -14
View File
@@ -116,6 +116,25 @@ private const val CREDITED_SONG_ROWS =
"ON songs.sourceId = album_artists.sourceId AND songs.albumId = album_artists.albumId " +
"WHERE album_artists.sourceId = :sourceId"
private const val CREDITED_ALBUM_ROWS =
"SELECT albums.artistId AS artistId, albums.id AS albumId " +
"FROM albums WHERE albums.sourceId = :sourceId AND albums.artistId IS NOT NULL " +
"UNION " +
"SELECT album_artists.artistId, albums.id " +
"FROM album_artists JOIN albums " +
"ON albums.sourceId = album_artists.sourceId AND albums.id = album_artists.albumId " +
"WHERE album_artists.sourceId = :sourceId " +
"UNION " +
"SELECT songs.artistId, songs.albumId " +
"FROM songs JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " +
"WHERE songs.sourceId = :sourceId AND songs.artistId IS NOT NULL " +
"UNION " +
"SELECT song_artists.artistId, songs.albumId " +
"FROM song_artists JOIN songs " +
"ON songs.sourceId = song_artists.sourceId AND songs.id = song_artists.songId " +
"JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " +
"WHERE song_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)) " +
@@ -141,13 +160,8 @@ internal const val ARTISTS_SELECT =
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_ALBUM_COUNT = "albumCount DESC, sortName COLLATE NOCASE, id"
internal const val ARTIST_ORDER_BY_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"
@@ -252,16 +266,20 @@ interface LibraryDao {
suspend fun recomputeArtistPlayData(sourceId: Long)
// 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.
// and so the artist badges use the same counts as the artist page and the shuffle queue.
@Query(
"WITH credited AS ($CREDITED_SONG_ROWS), counts AS (" +
"SELECT artistId AS id, COUNT(DISTINCT songId) AS songCount, " +
"COUNT(DISTINCT albumId) AS albumCount " +
"WITH credited AS ($CREDITED_SONG_ROWS), " +
"credited_albums AS ($CREDITED_ALBUM_ROWS), " +
"counts AS (" +
"SELECT artistId AS id, COUNT(DISTINCT songId) AS songCount " +
"FROM credited GROUP BY artistId" +
"), album_counts AS (" +
"SELECT artistId AS id, COUNT(DISTINCT albumId) AS albumCount " +
"FROM credited_albums GROUP BY artistId" +
") " +
"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, " +
"COALESCE(album_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 " +
@@ -269,6 +287,7 @@ interface LibraryDao {
"OR EXISTS (SELECT 1 FROM song_artists sa " +
"WHERE sa.sourceId = artists.sourceId AND sa.artistId = artists.id)) AS credited " +
"FROM artists LEFT JOIN counts ON counts.id = artists.id " +
"LEFT JOIN album_counts ON album_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.albumCount != c.albumCount " +
@@ -718,7 +737,7 @@ interface LibraryDao {
downloadedFilter: Int = 0,
): PagingSource<Int, Artist>
@Query("$ARTISTS_SELECT $ARTISTS_FILTER ORDER BY $ARTIST_ORDER_BY_DOWNLOADED_ALBUM_COUNT")
@Query("$ARTISTS_SELECT $ARTISTS_FILTER ORDER BY $ARTIST_ORDER_BY_ALBUM_COUNT")
fun artistsByAlbumCount(
sourceId: Long,
starredFilter: Int,
@@ -726,7 +745,7 @@ interface LibraryDao {
downloadedFilter: Int = 0,
): PagingSource<Int, Artist>
@Query("$ARTISTS_SELECT $ARTISTS_FILTER ORDER BY $ARTIST_ORDER_BY_DOWNLOADED_ALBUM_COUNT_REVERSED")
@Query("$ARTISTS_SELECT $ARTISTS_FILTER ORDER BY $ARTIST_ORDER_BY_ALBUM_COUNT_REVERSED")
fun artistsByAlbumCountReversed(
sourceId: Long,
starredFilter: Int,
app/src/main/kotlin/com/subtracks/data/db/Migrations.kt
+34 -11
View File
@@ -579,47 +579,70 @@ val MIGRATION_32_33 =
}
}
// Recomputes the artist totals from the same credit union the sync uses, so badges, the artist
// Recomputes the artist totals from the same credit unions 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 (" +
"WITH credited_songs 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 " +
"songs.`id` AS songId, 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`, " +
"SELECT song_artists.`sourceId`, song_artists.`artistId`, songs.`id`, " +
"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`, " +
"SELECT albums.`sourceId`, albums.`artistId`, songs.`id`, " +
"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`, " +
"SELECT album_artists.`sourceId`, album_artists.`artistId`, songs.`id`, " +
"songs.`playCount`, songs.`played` " +
"FROM album_artists JOIN songs " +
"ON songs.`sourceId` = album_artists.`sourceId` AND songs.`albumId` = album_artists.`albumId`" +
"), credited_albums AS (" +
"SELECT albums.`sourceId` AS sourceId, albums.`artistId` AS artistId, " +
"albums.`id` AS albumId " +
"FROM albums WHERE albums.`artistId` IS NOT NULL " +
"UNION " +
"SELECT album_artists.`sourceId`, album_artists.`artistId`, albums.`id` " +
"FROM album_artists JOIN albums " +
"ON albums.`sourceId` = album_artists.`sourceId` AND albums.`id` = album_artists.`albumId` " +
"UNION " +
"SELECT songs.`sourceId`, songs.`artistId`, songs.`albumId` " +
"FROM songs JOIN albums " +
"ON albums.`sourceId` = songs.`sourceId` AND albums.`id` = songs.`albumId` " +
"WHERE songs.`artistId` IS NOT NULL " +
"UNION " +
"SELECT song_artists.`sourceId`, song_artists.`artistId`, songs.`albumId` " +
"FROM song_artists JOIN songs " +
"ON songs.`sourceId` = song_artists.`sourceId` AND songs.`id` = song_artists.`songId` " +
"JOIN albums ON albums.`sourceId` = songs.`sourceId` AND albums.`id` = songs.`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" +
"SUM(playCount) AS playCount, MAX(played) AS played " +
"FROM credited_songs GROUP BY sourceId, artistId" +
"), album_totals AS (" +
"SELECT sourceId, artistId AS id, COUNT(DISTINCT albumId) AS albumCount " +
"FROM credited_albums 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.songCount, 0) AS songCount, " +
"COALESCE(album_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` " +
"LEFT JOIN album_totals " +
"ON album_totals.sourceId = artists.`sourceId` AND album_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`)" +
"OR 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 " +
app/src/main/kotlin/com/subtracks/data/sync/SyncService.kt
+5 -3
View File
@@ -65,7 +65,7 @@ class SyncService(
val seen = HashedIds()
source.artists().collect { batch ->
if (batch.isEmpty()) return@collect
write("artists", artistColumns, primaryKey, batch) { it.values() }
write("artists", artistColumns, primaryKey, batch, preserved = setOf("albumCount")) { it.values() }
batch.forEach { seen.add(idHash(it.id)) }
artistsSynced += batch.size
}
@@ -314,11 +314,12 @@ class SyncService(
columns: List<String>,
key: List<String>,
rows: List<T>,
preserved: Set<String> = emptySet(),
values: (T) -> List<Any?>,
) {
db.useWriterConnection { connection ->
connection.immediateTransaction {
connection.upsertChanged(table, columns, key, rows, values)
connection.upsertChanged(table, columns, key, rows, preserved, values)
}
}
}
@@ -613,11 +614,12 @@ private suspend fun <T> PooledConnection.upsertChanged(
columns: List<String>,
key: List<String>,
rows: List<T>,
preserved: Set<String> = emptySet(),
values: (T) -> List<Any?>,
) {
if (rows.isEmpty()) return
check(values(rows.first()).size == columns.size) { "column/value mismatch for $table" }
val updates = columns.filterNot { it in key }
val updates = columns.filterNot { it in key || it in preserved }
val sql =
"INSERT INTO $table (${columns.joinToString()}) VALUES (${columns.joinToString { "?" }}) " +
"ON CONFLICT(${key.joinToString()}) DO UPDATE SET " +
app/src/test/kotlin/com/subtracks/data/db/MigrationsTest.kt
+28 -1
View File
@@ -283,6 +283,9 @@ class MigrationsTest {
migrated.execSQL(
"INSERT INTO sources (id, name, address, isActive, createdAt) VALUES (2, 't', 'http://y', 1, 0)",
)
migrated.execSQL(
"INSERT INTO sources (id, name, address, isActive, createdAt) VALUES (3, 'u', 'http://z', 1, 0)",
)
migrated.execSQL(
"INSERT INTO artists (sourceId, id, name, albumCount, songCount, playCount, played) " +
"VALUES (1, 'a1', 'Album Artist', 1, 0, 0, NULL)",
@@ -295,10 +298,18 @@ class MigrationsTest {
"INSERT INTO artists (sourceId, id, name, albumCount, songCount, playCount, played) " +
"VALUES (2, 'b1', 'Unsynced', 7, 6, 5, 100)",
)
migrated.execSQL(
"INSERT INTO artists (sourceId, id, name, albumCount, songCount, playCount, played) " +
"VALUES (3, 'c1', 'Track Only', 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 albums (sourceId, id, artistId, name, albumArtist, created, songCount) " +
"VALUES (1, 'al2', 'a2', 'Empty Album', 'Guest', 0, 0)",
)
migrated.execSQL(
"INSERT INTO album_artists (sourceId, albumId, position, artistId, name) " +
"VALUES (1, 'al1', 0, 'a1', 'Album Artist')",
@@ -315,6 +326,18 @@ class MigrationsTest {
"INSERT INTO song_artists (sourceId, songId, position, artistId, name) " +
"VALUES (1, 's1', 1, 'a2', 'Guest')",
)
migrated.execSQL(
"INSERT INTO songs (sourceId, id, albumId, artistId, title, playCount, played) " +
"VALUES (3, 's3', NULL, 'c1', 'Three', 2, 50)",
)
migrated.execSQL(
"INSERT INTO song_artists (sourceId, songId, position, artistId, name) " +
"VALUES (3, 's3', 0, 'c1', 'Track Only')",
)
migrated.execSQL(
"INSERT INTO songs (sourceId, id, albumId, artistId, title, playCount, played) " +
"VALUES (3, 's4', 'missing', 'c1', 'Four', 3, 60)",
)
MIGRATION_33_34.migrate(migrated)
@@ -323,13 +346,17 @@ class MigrationsTest {
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(2L, 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'"))
assertEquals(2L, long(migrated, "SELECT songCount FROM artists WHERE id = 'c1'"))
assertEquals(0L, long(migrated, "SELECT albumCount FROM artists WHERE id = 'c1'"))
assertEquals(5L, long(migrated, "SELECT playCount FROM artists WHERE id = 'c1'"))
assertEquals(60L, long(migrated, "SELECT played FROM artists WHERE id = 'c1'"))
}
}
app/src/test/kotlin/com/subtracks/data/sync/SyncServiceTest.kt
+58 -18
View File
@@ -1373,6 +1373,11 @@ class SyncServiceTest {
insertSource()
val source =
FakeMusicSource(
artists =
listOf(
artist("a1", "Artist").copy(albumCount = 9),
artist("a2", "Other").copy(albumCount = 9),
),
albums =
listOf(
album("al1").copy(artists = listOf(ArtistRef("a1", "Artist"), ArtistRef("a2", "Other"))),
@@ -1478,6 +1483,10 @@ class SyncServiceTest {
artistId = "a1",
artists = listOf(ArtistRef("a1", "Album Artist")),
),
album("al2").copy(
artistId = "a2",
artists = listOf(ArtistRef("a2", "Guest")),
),
),
songs =
listOf(
@@ -1488,31 +1497,28 @@ class SyncServiceTest {
playCount = 3,
played = 100,
),
song("s2").copy(
albumId = "al1",
artistId = "a2",
artists = listOf(ArtistRef("a2", "Guest")),
playCount = 4,
played = 200,
),
),
)
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(2L, guest.albumCount)
assertEquals(2L, guest.songCount)
assertEquals(7L, guest.playCount)
assertEquals(200L, guest.played)
val albums = db.libraryDao().albumsForArtist(1, "a2").first()
assertEquals(listOf("al1", "al2"), albums.map { it.id }.sorted())
assertEquals(albums.size.toLong(), guest.albumCount)
assertEquals(
listOf("al1"),
db
.libraryDao()
.albumsForArtist(1, "a2")
.first()
.map { it.id },
)
assertEquals(
db
.libraryDao()
.albumsForArtist(1, "a2")
.first()
.size
.toLong(),
albums.size.toLong(),
db
.libraryDao()
.artistsByName(1, 0, "")
@@ -1520,6 +1526,40 @@ class SyncServiceTest {
.first { it.id == "a2" }
.albumCount,
)
assertEquals(guest.songCount, db.queueDao().artistLength(1, "a2"))
}
@Test
fun songsOnMissingAlbumsAreSongsButNotAlbums() =
runTest {
insertSource()
val source =
FakeMusicSource(
artists = listOf(artist("a1", "Artist")),
albums = emptyList(),
songs =
listOf(
song("s1").copy(
albumId = "missing",
artistId = "a1",
artists = listOf(ArtistRef("a1", "Artist")),
),
),
)
SyncService(db, source).sync()
val artist = db.libraryDao().artistOnce(1, "a1")!!
assertEquals(1L, artist.songCount)
assertEquals(0L, artist.albumCount)
assertEquals(
emptyList<String>(),
db
.libraryDao()
.albumsForArtist(1, "a1")
.first()
.map { it.id },
)
}
@Test