mirror of
https://github.com/austinried/subtracks.git
synced 2026-11-10 15:11:58 +01:00
library: share the artist credit membership queries
The artist page, shuffle, download attribution and artist badge each spelled out the four credit paths. Reuse the shared credited song and album rows, and keep only the download-aware album queries hand-written, where the per-credit download visibility genuinely differs. The shared song and album rows use UNION ALL so the artist list filter can push its artist predicate into every branch instead of materialising a deduplicated union; callers that need a set use DISTINCT or COUNT(DISTINCT).
This commit is contained in:
@@ -15,31 +15,13 @@ 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 " +
|
||||
"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 " +
|
||||
"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 " +
|
||||
"FROM songs " +
|
||||
"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 " +
|
||||
"FROM songs " +
|
||||
"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"
|
||||
"SELECT credits.artistId AS id, credits.songId AS songId, song_downloads.status AS status " +
|
||||
"FROM (" + CREDITED_SONG_ROWS + ") credits " +
|
||||
"LEFT JOIN song_downloads ON song_downloads.sourceId = :sourceId " +
|
||||
"AND song_downloads.songId = credits.songId"
|
||||
|
||||
// Starts from downloads so lists and filters over partially downloaded libraries do not scan songs.
|
||||
// This is the one hand-written copy of the credit paths, kept for that query plan.
|
||||
internal const val DOWNLOADED_ARTIST_ROWS =
|
||||
"SELECT albums.artistId AS id, song_downloads.songId AS songId, " +
|
||||
"song_downloads.downloadedAt AS downloadedAt, song_downloads.status AS status " +
|
||||
|
||||
@@ -69,6 +69,51 @@ internal const val ALBUM_ORDER_BY_FREQUENT_REVERSED = "playCount ASC, sortName C
|
||||
internal const val ALBUM_ORDER_BY_RECENT = "played DESC NULLS LAST, sortName COLLATE NOCASE, id"
|
||||
internal const val ALBUM_ORDER_BY_RECENT_REVERSED = "played ASC NULLS LAST, sortName COLLATE NOCASE DESC, id DESC"
|
||||
|
||||
internal const val ARTIST_CREDITED =
|
||||
"(artists.credited = 1 OR COALESCE((SELECT creditsStale FROM sources WHERE id = artists.sourceId), 1) = 1)"
|
||||
|
||||
// May repeat a (artistId, songId) pair when several credit paths reach it; consumers dedupe.
|
||||
internal 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 ALL " +
|
||||
"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 ALL " +
|
||||
"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 ALL " +
|
||||
"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"
|
||||
|
||||
// May repeat an (artistId, albumId) pair when several credit paths reach it; consumers dedupe.
|
||||
internal 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 ALL " +
|
||||
"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 ALL " +
|
||||
"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 ALL " +
|
||||
"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"
|
||||
|
||||
// Download visibility follows the credit path: an album credit includes the album when any song is
|
||||
// downloaded, while a track credit includes it only when the credited track is downloaded.
|
||||
private const val DOWNLOADED_ARTIST_ALBUMS =
|
||||
"SELECT albums.id FROM albums " +
|
||||
"WHERE albums.sourceId = :sourceId AND albums.artistId = artists.id " +
|
||||
@@ -94,47 +139,6 @@ private const val DOWNLOADED_ARTIST_ALBUMS =
|
||||
"WHERE song_artists.sourceId = :sourceId AND song_artists.artistId = artists.id " +
|
||||
"AND sd.status = 'Completed' AND songs.albumId IS NOT NULL"
|
||||
|
||||
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"
|
||||
|
||||
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)) " +
|
||||
@@ -252,7 +256,7 @@ interface LibraryDao {
|
||||
// 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 ($CREDITED_SONG_ROWS), totals AS (" +
|
||||
"WITH credited AS (SELECT DISTINCT * FROM ($CREDITED_SONG_ROWS)), totals AS (" +
|
||||
"SELECT artistId AS id, SUM(playCount) AS playCount, MAX(played) AS played " +
|
||||
"FROM credited GROUP BY artistId" +
|
||||
") " +
|
||||
@@ -268,7 +272,7 @@ interface LibraryDao {
|
||||
// Stored so the download badge query can report an artist's total without walking every song,
|
||||
// and so the artist badges use the same counts as the artist page and the shuffle queue.
|
||||
@Query(
|
||||
"WITH credited AS ($CREDITED_SONG_ROWS), " +
|
||||
"WITH credited AS (SELECT DISTINCT * FROM ($CREDITED_SONG_ROWS)), " +
|
||||
"credited_albums AS ($CREDITED_ALBUM_ROWS), " +
|
||||
"counts AS (" +
|
||||
"SELECT artistId AS id, COUNT(DISTINCT songId) AS songCount " +
|
||||
@@ -1061,22 +1065,9 @@ interface LibraryDao {
|
||||
): Flow<List<String>>
|
||||
|
||||
@Query(
|
||||
"SELECT * FROM (" +
|
||||
"SELECT albums.* FROM albums WHERE albums.sourceId = :sourceId AND albums.artistId = :artistId " +
|
||||
"UNION " +
|
||||
"SELECT albums.* FROM album_artists " +
|
||||
"JOIN albums ON albums.sourceId = album_artists.sourceId AND albums.id = album_artists.albumId " +
|
||||
"WHERE album_artists.sourceId = :sourceId AND album_artists.artistId = :artistId " +
|
||||
"UNION " +
|
||||
"SELECT albums.* 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 AND song_artists.artistId = :artistId " +
|
||||
"UNION " +
|
||||
"SELECT albums.* FROM songs " +
|
||||
"JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " +
|
||||
"WHERE songs.sourceId = :sourceId AND songs.artistId = :artistId" +
|
||||
") ORDER BY year DESC, sortName COLLATE NOCASE, id",
|
||||
"SELECT albums.* FROM (SELECT DISTINCT albumId FROM ($CREDITED_ALBUM_ROWS) WHERE artistId = :artistId) credits " +
|
||||
"JOIN albums ON albums.sourceId = :sourceId AND albums.id = credits.albumId " +
|
||||
"ORDER BY year DESC, sortName COLLATE NOCASE, id",
|
||||
)
|
||||
fun albumsForArtist(
|
||||
sourceId: Long,
|
||||
@@ -1118,6 +1109,7 @@ interface LibraryDao {
|
||||
playlistId: String,
|
||||
): PagingSource<Int, PlaylistSongItem>
|
||||
|
||||
// Download visibility follows the credit path: see DOWNLOADED_ARTIST_ALBUMS.
|
||||
@Query(
|
||||
"SELECT * FROM (" +
|
||||
"SELECT albums.* FROM albums " +
|
||||
|
||||
@@ -20,17 +20,7 @@ private const val ALBUM_SONGS_ORDER = " ORDER BY songs.disc, songs.track, songs.
|
||||
private const val ALBUM_SONGS_SQL = ALBUM_SONGS_SELECT + ALBUM_SONGS_ORDER
|
||||
|
||||
private const val ARTIST_SONG_IDS =
|
||||
"(SELECT id FROM songs WHERE sourceId = :sourceId AND artistId = :artistId " +
|
||||
"UNION " +
|
||||
"SELECT songId FROM song_artists WHERE sourceId = :sourceId AND artistId = :artistId " +
|
||||
"UNION " +
|
||||
"SELECT songs.id FROM albums " +
|
||||
"JOIN songs ON songs.sourceId = albums.sourceId AND songs.albumId = albums.id " +
|
||||
"WHERE albums.sourceId = :sourceId AND albums.artistId = :artistId " +
|
||||
"UNION " +
|
||||
"SELECT songs.id FROM album_artists " +
|
||||
"JOIN songs ON songs.sourceId = album_artists.sourceId AND songs.albumId = album_artists.albumId " +
|
||||
"WHERE album_artists.sourceId = :sourceId AND album_artists.artistId = :artistId)"
|
||||
"(SELECT songId FROM ($CREDITED_SONG_ROWS) WHERE artistId = :artistId)"
|
||||
|
||||
private const val ARTIST_SONGS_SELECT =
|
||||
"SELECT songs.*, albums.coverArt AS coverArt FROM songs " +
|
||||
|
||||
@@ -857,6 +857,25 @@ class LibraryDaoTest {
|
||||
assertTrue(dao.searchSongs(sourceId, "ga", 50).first().isEmpty())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun theArtistPageAlbumQueryFindsArtistCreditsThroughArtistIndexes() =
|
||||
runTest {
|
||||
val plan =
|
||||
plan(
|
||||
"SELECT albums.* FROM (SELECT DISTINCT albumId FROM ($CREDITED_ALBUM_ROWS) " +
|
||||
"WHERE artistId = :artistId) credits " +
|
||||
"JOIN albums ON albums.sourceId = :sourceId AND albums.id = credits.albumId " +
|
||||
"ORDER BY year DESC, sortName COLLATE NOCASE, id",
|
||||
)
|
||||
|
||||
assertTrue(plan, plan.contains("index_albums_sourceId_artistId"))
|
||||
assertTrue(plan, plan.contains("index_album_artists_sourceId_artistId"))
|
||||
assertTrue(plan, plan.contains("index_songs_sourceId_artistId"))
|
||||
assertTrue(plan, plan.contains("index_song_artists_sourceId_artistId"))
|
||||
assertFalse(plan, plan.contains("SCAN albums"))
|
||||
assertFalse(plan, plan.contains("SCAN songs"))
|
||||
}
|
||||
|
||||
private suspend fun plan(sql: String): String =
|
||||
db.useReaderConnection { connection ->
|
||||
connection.usePrepared("EXPLAIN QUERY PLAN $sql") { statement ->
|
||||
|
||||
@@ -1636,6 +1636,68 @@ class SyncServiceTest {
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun everyArtistConsumerAgreesOnTheCreditedSet() =
|
||||
runTest {
|
||||
insertSource()
|
||||
val source =
|
||||
FakeMusicSource(
|
||||
artists = listOf(artist("a1", "Album Artist"), artist("a2", "Guest"), artist("a3", "Track Only")),
|
||||
albums =
|
||||
listOf(
|
||||
album("al1").copy(artistId = "a1", artists = listOf(ArtistRef("a1", "Album Artist"))),
|
||||
album("al2").copy(artistId = "a2", artists = listOf(ArtistRef("a2", "Guest"))),
|
||||
),
|
||||
songs =
|
||||
listOf(
|
||||
song("s1").copy(
|
||||
albumId = "al1",
|
||||
artistId = "a1",
|
||||
artists = listOf(ArtistRef("a1", "Album Artist"), ArtistRef("a2", "Guest")),
|
||||
playCount = 1,
|
||||
played = 100,
|
||||
),
|
||||
song("s2").copy(
|
||||
albumId = "al1",
|
||||
artistId = "a2",
|
||||
artists = listOf(ArtistRef("a2", "Guest"), ArtistRef("a3", "Track Only")),
|
||||
),
|
||||
song("s3").copy(
|
||||
albumId = null,
|
||||
artistId = "a3",
|
||||
artists = listOf(ArtistRef("a3", "Track Only")),
|
||||
),
|
||||
song("s4").copy(
|
||||
albumId = "missing",
|
||||
artistId = "a2",
|
||||
artists = listOf(ArtistRef("a2", "Guest")),
|
||||
),
|
||||
),
|
||||
)
|
||||
|
||||
SyncService(db, source).sync()
|
||||
db.downloadDao().upsert(SongDownload(1, "s1", DownloadStatus.Completed))
|
||||
db.downloadDao().upsert(SongDownload(1, "s3", DownloadStatus.Completed))
|
||||
|
||||
for (artistId in listOf("a1", "a2", "a3")) {
|
||||
val artist = db.libraryDao().artistOnce(1, artistId)!!
|
||||
val songIds = db.queueDao().artistSongIds(1, artistId)
|
||||
val albums = db.libraryDao().albumsForArtist(1, artistId).first()
|
||||
val status = db.downloadDao().artistStatus(1, artistId).first()
|
||||
val downloaded =
|
||||
db
|
||||
.downloadDao()
|
||||
.all()
|
||||
.count { it.status == DownloadStatus.Completed && it.songId in songIds }
|
||||
|
||||
assertEquals("$artistId albums", albums.size.toLong(), artist.albumCount)
|
||||
assertEquals("$artistId songs", songIds.size.toLong(), artist.songCount)
|
||||
assertEquals("$artistId length", songIds.size.toLong(), db.queueDao().artistLength(1, artistId))
|
||||
assertEquals("$artistId total", songIds.size.toLong(), status.total)
|
||||
assertEquals("$artistId downloaded", downloaded.toLong(), status.downloaded)
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun changingOnlyTheLeadArtistRecomputesTheCreditedCounts() =
|
||||
runTest {
|
||||
|
||||
Reference in New Issue
Block a user