diff --git a/app/src/main/kotlin/com/subtracks/data/db/DownloadDao.kt b/app/src/main/kotlin/com/subtracks/data/db/DownloadDao.kt index 7af9fcdf..906269ee 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/DownloadDao.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/DownloadDao.kt @@ -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 " + diff --git a/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt b/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt index 4023944e..8a692813 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt @@ -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> @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 + // Download visibility follows the credit path: see DOWNLOADED_ARTIST_ALBUMS. @Query( "SELECT * FROM (" + "SELECT albums.* FROM albums " + diff --git a/app/src/main/kotlin/com/subtracks/data/db/QueueDao.kt b/app/src/main/kotlin/com/subtracks/data/db/QueueDao.kt index cfedc066..c96cc2b8 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/QueueDao.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/QueueDao.kt @@ -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 " + diff --git a/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt b/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt index 6dd7bc3b..0d1452d2 100644 --- a/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt @@ -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 -> diff --git a/app/src/test/kotlin/com/subtracks/data/sync/SyncServiceTest.kt b/app/src/test/kotlin/com/subtracks/data/sync/SyncServiceTest.kt index aab636cf..349aad6d 100644 --- a/app/src/test/kotlin/com/subtracks/data/sync/SyncServiceTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/sync/SyncServiceTest.kt @@ -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 {