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 df70478b..b3b4e1fb 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/DownloadDao.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/DownloadDao.kt @@ -11,6 +11,21 @@ import com.subtracks.data.model.SongDownload import com.subtracks.data.model.SourceCoverArt import kotlinx.coroutines.flow.Flow +// A song is attributed to its album artist and to its credited track artist, so a guest artist's +// downloads are visible even though they lead no albums of their own. +private 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 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" + @Dao interface DownloadDao { @Query("SELECT * FROM song_downloads WHERE sourceId = :sourceId") @@ -23,13 +38,10 @@ interface DownloadDao { suspend fun all(): List @Query( - "SELECT COUNT(songs.id) AS total, " + - "COALESCE(SUM(CASE WHEN song_downloads.status = 'Completed' THEN 1 ELSE 0 END), 0) AS downloaded, " + - "COALESCE(SUM(CASE WHEN song_downloads.status IN ('Queued', 'Running') THEN 1 ELSE 0 END), 0) AS downloading " + - "FROM songs " + - "LEFT JOIN song_downloads ON song_downloads.sourceId = songs.sourceId AND song_downloads.songId = songs.id " + - "WHERE songs.sourceId = :sourceId " + - "AND songs.albumId IN (SELECT id FROM albums WHERE sourceId = :sourceId AND artistId = :artistId)", + "SELECT COUNT(DISTINCT songId) AS total, " + + "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", ) fun artistStatus( sourceId: Long, @@ -77,15 +89,11 @@ interface DownloadDao { fun albumStatuses(sourceId: Long): Flow> @Query( - "SELECT albums.artistId AS id, " + - "COUNT(songs.id) AS total, " + - "COALESCE(SUM(CASE WHEN song_downloads.status = 'Completed' THEN 1 ELSE 0 END), 0) AS downloaded, " + - "COALESCE(SUM(CASE WHEN song_downloads.status IN ('Queued', 'Running') THEN 1 ELSE 0 END), 0) AS downloading " + - "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 " + - "GROUP BY albums.artistId " + + "SELECT id, " + + "COUNT(DISTINCT songId) AS total, " + + "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 + ") GROUP BY id " + "HAVING downloaded > 0 OR downloading > 0", ) fun artistStatuses(sourceId: Long): Flow> 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 0c408c11..1b7a33e8 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt @@ -68,10 +68,15 @@ internal const val ALBUM_ORDER_BY_RECENT_REVERSED = "played ASC NULLS LAST, name 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)) " + - "AND (:downloadedFilter = 0 OR EXISTS (SELECT 1 FROM songs " + + "AND (:downloadedFilter = 0 OR " + + "EXISTS (SELECT 1 FROM songs " + "JOIN albums dl ON dl.sourceId = songs.sourceId AND dl.id = songs.albumId " + "JOIN song_downloads sd ON sd.sourceId = songs.sourceId AND sd.songId = songs.id " + - "WHERE songs.sourceId = artists.sourceId AND dl.artistId = artists.id AND sd.status = 'Completed')) " + + "WHERE songs.sourceId = artists.sourceId AND dl.artistId = artists.id AND sd.status = 'Completed') " + + "OR EXISTS (SELECT 1 FROM songs " + + "JOIN albums dl ON dl.sourceId = songs.sourceId AND dl.id = songs.albumId " + + "JOIN song_downloads sd ON sd.sourceId = songs.sourceId AND sd.songId = songs.id " + + "WHERE songs.sourceId = artists.sourceId AND songs.artistId = artists.id AND sd.status = 'Completed')) " + "AND (:search = '' " + "OR (length(:search) >= 3 AND rowid IN (SELECT rowid FROM artist_search " + "WHERE artist_search MATCH '\"' || replace(:search, '\"', '\"\"') || '\"')) " + @@ -82,11 +87,17 @@ internal const val ARTISTS_FILTER = internal const val ARTISTS_SELECT = "SELECT artists.sourceId, artists.id, artists.name, " + "CASE WHEN :downloadedFilter = 1 THEN (" + - "SELECT COUNT(DISTINCT dl.id) FROM songs " + + "SELECT COUNT(*) FROM (" + + "SELECT dl.id FROM songs " + "JOIN albums dl ON dl.sourceId = songs.sourceId AND dl.id = songs.albumId " + "JOIN song_downloads sd ON sd.sourceId = songs.sourceId AND sd.songId = songs.id " + "WHERE songs.sourceId = artists.sourceId AND dl.artistId = artists.id AND sd.status = 'Completed' " + - ") ELSE artists.albumCount END AS albumCount, " + + "UNION " + + "SELECT dl.id FROM songs " + + "JOIN albums dl ON dl.sourceId = songs.sourceId AND dl.id = songs.albumId " + + "JOIN song_downloads sd ON sd.sourceId = songs.sourceId AND sd.songId = songs.id " + + "WHERE songs.sourceId = artists.sourceId AND songs.artistId = artists.id AND sd.status = 'Completed'" + + ")) ELSE artists.albumCount END AS albumCount, " + "artists.starred, artists.coverArt, artists.playCount, artists.played " internal const val ARTIST_ORDER_BY_NAME = "name COLLATE NOCASE, id" @@ -809,12 +820,22 @@ interface LibraryDao { @Query( "SELECT artists.* FROM artists " + - "JOIN albums ON albums.sourceId = artists.sourceId AND albums.artistId = artists.id " + - "JOIN songs ON songs.sourceId = albums.sourceId AND songs.albumId = albums.id " + + "JOIN (" + + "SELECT albums.artistId AS artistId, d.downloadedAt AS downloadedAt " + + "FROM songs " + + "JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " + "JOIN song_downloads d ON d.sourceId = songs.sourceId AND d.songId = songs.id AND d.status = 'Completed' " + + "WHERE songs.sourceId = :sourceId AND albums.artistId IS NOT NULL " + + "UNION ALL " + + "SELECT songs.artistId AS artistId, d.downloadedAt AS downloadedAt " + + "FROM songs " + + "JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " + + "JOIN song_downloads d ON d.sourceId = songs.sourceId AND d.songId = songs.id AND d.status = 'Completed' " + + "WHERE songs.sourceId = :sourceId AND songs.artistId IS NOT NULL" + + ") dl ON dl.artistId = artists.id " + "WHERE artists.sourceId = :sourceId " + "GROUP BY artists.sourceId, artists.id " + - "ORDER BY MAX(d.downloadedAt) DESC, artists.name COLLATE NOCASE, artists.id", + "ORDER BY MAX(dl.downloadedAt) DESC, artists.name COLLATE NOCASE, artists.id", ) fun downloadedArtists(sourceId: Long): PagingSource @@ -939,7 +960,11 @@ interface LibraryDao { songId: String, ): Flow> - @Query("SELECT * FROM albums WHERE sourceId = :sourceId AND artistId = :artistId ORDER BY year DESC, name COLLATE NOCASE, id") + @Query( + "SELECT * FROM albums WHERE sourceId = :sourceId AND (artistId = :artistId " + + "OR id IN (SELECT albumId FROM songs WHERE sourceId = :sourceId AND artistId = :artistId)) " + + "ORDER BY year DESC, name COLLATE NOCASE, id", + ) fun albumsForArtist( sourceId: Long, artistId: String, @@ -981,9 +1006,13 @@ interface LibraryDao { ): PagingSource @Query( - "SELECT * FROM albums WHERE sourceId = :sourceId AND artistId = :artistId " + - "AND EXISTS (SELECT 1 FROM songs JOIN song_downloads sd ON sd.sourceId = songs.sourceId AND sd.songId = songs.id " + - "WHERE songs.sourceId = albums.sourceId AND songs.albumId = albums.id AND sd.status = 'Completed') " + + "SELECT * FROM albums WHERE sourceId = :sourceId AND (" + + "(artistId = :artistId AND EXISTS (SELECT 1 FROM songs " + + "JOIN song_downloads sd ON sd.sourceId = songs.sourceId AND sd.songId = songs.id " + + "WHERE songs.sourceId = albums.sourceId AND songs.albumId = albums.id AND sd.status = 'Completed')) " + + "OR id IN (SELECT songs.albumId FROM songs " + + "JOIN song_downloads sd ON sd.sourceId = songs.sourceId AND sd.songId = songs.id " + + "WHERE songs.sourceId = :sourceId AND songs.artistId = :artistId AND sd.status = 'Completed')) " + "ORDER BY year DESC, name COLLATE NOCASE, id", ) fun albumsForArtistDownloaded( 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 423236a2..8954f4b4 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/QueueDao.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/QueueDao.kt @@ -22,7 +22,7 @@ private const val ALBUM_SONGS_SQL = ALBUM_SONGS_SELECT + ALBUM_SONGS_ORDER private const val ARTIST_SONGS_SELECT = "SELECT songs.*, albums.coverArt AS coverArt FROM songs " + "JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " + - "WHERE songs.sourceId = :sourceId AND albums.artistId = :artistId" + "WHERE songs.sourceId = :sourceId AND (albums.artistId = :artistId OR songs.artistId = :artistId)" private const val ARTIST_SONGS_ORDER = " ORDER BY albums.year DESC, albums.name COLLATE NOCASE, albums.id, songs.disc, songs.track, songs.id" @@ -83,7 +83,7 @@ interface QueueDao { @Query( "SELECT COUNT(*) FROM songs " + "JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " + - "WHERE songs.sourceId = :sourceId AND albums.artistId = :artistId", + "WHERE songs.sourceId = :sourceId AND (albums.artistId = :artistId OR songs.artistId = :artistId)", ) suspend fun artistLength( sourceId: Long, @@ -191,7 +191,7 @@ interface QueueDao { @Query( "SELECT songs.id FROM songs " + "JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " + - "WHERE songs.sourceId = :sourceId AND albums.artistId = :artistId" + + "WHERE songs.sourceId = :sourceId AND (albums.artistId = :artistId OR songs.artistId = :artistId)" + ARTIST_SONGS_ORDER, ) suspend fun artistSongIds( diff --git a/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt b/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt index b4faac06..e88aa820 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt @@ -197,7 +197,6 @@ fun ArtistDetailRoute( starred = artist?.starred != null, onToggleStar = artist?.let { a -> { setStar(StarType.Artist, a.id, a.starred == null) } }, onShuffle = viewModel::shuffle, - offline = offline, ) pendingDelete?.let { pending -> @@ -227,7 +226,6 @@ fun ArtistDetailScreen( starred: Boolean = false, onToggleStar: (() -> Unit)? = null, onShuffle: () -> Unit = {}, - offline: Boolean = false, modifier: Modifier = Modifier, ) { val listState = rememberLazyGridState() @@ -241,7 +239,7 @@ fun ArtistDetailScreen( val statusBarTop = with(density) { WindowInsets.statusBars.getTop(this).toDp() } val navBarBottom = with(density) { WindowInsets.navigationBars.getBottom(this).toDp() } val barHeight = statusBarTop + TopAppBarDefaults.TopAppBarExpandedHeight - val albumCount = if (offline) albums.size.toLong() else artist?.albumCount ?: 0L + val albumCount = albums.size.toLong() val imageTitleTop = ART_HEIGHT - TITLE_INSET - lineHeight val fadeStartPx = with(density) { (imageTitleTop - barHeight - FADE_LEAD).toPx() } val fadeEndPx = with(density) { (imageTitleTop + lineHeight / 2 - barHeight).toPx() } 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 db9c5a22..1a31140f 100644 --- a/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt @@ -491,6 +491,73 @@ class LibraryDaoTest { ) } + @Test + fun anArtistPageListsAlbumsWhereTheArtistIsOnlyACreditedTrack() = + runTest { + val sourceId = source() + val dao = db.libraryDao() + dao.upsertAlbums(listOf(album(sourceId, "al-1", "Compilation", year = null, starred = null))) + dao.upsertSongs( + listOf( + song(sourceId, "s1", "One", starred = null), + song(sourceId, "s2", "Two", starred = null).copy(artistId = "ar-guest"), + ), + ) + + assertEquals(listOf("al-1"), dao.albumsForArtist(sourceId, "ar-1").first().map { it.id }) + assertEquals(listOf("al-1"), dao.albumsForArtist(sourceId, "ar-guest").first().map { it.id }) + + db.downloadDao().upsert(SongDownload(sourceId, "s1", DownloadStatus.Completed)) + assertEquals(emptyList(), dao.albumsForArtistDownloaded(sourceId, "ar-guest").first().map { it.id }) + + db.downloadDao().upsert(SongDownload(sourceId, "s2", DownloadStatus.Completed)) + assertEquals(listOf("al-1"), dao.albumsForArtistDownloaded(sourceId, "ar-guest").first().map { it.id }) + } + + @Test + fun downloadedArtistsIncludeOneCreditedOnlyOnADownloadedTrack() = + runTest { + val sourceId = source() + val dao = db.libraryDao() + dao.upsertArtists( + listOf( + artist(sourceId, "ar-1", "Album Artist", albumCount = 1), + artist(sourceId, "ar-guest", "Guest", albumCount = 0), + ), + ) + dao.upsertAlbums(listOf(album(sourceId, "al-1", "Compilation", year = null, starred = null))) + dao.upsertSongs( + listOf( + song(sourceId, "s1", "One", starred = null), + song(sourceId, "s2", "Two", starred = null).copy(artistId = "ar-guest"), + ), + ) + db.downloadDao().upsert(SongDownload(sourceId, "s1", DownloadStatus.Completed)) + assertEquals( + listOf("Album Artist"), + dao.artistsByName(sourceId, 0, "", downloadedFilter = 1).page().map { it.name }, + ) + assertEquals( + listOf("Album Artist"), + dao.downloadedArtists(sourceId).page().map { it.name }, + ) + + db.downloadDao().upsert(SongDownload(sourceId, "s2", DownloadStatus.Completed)) + + val filtered = dao.artistsByName(sourceId, 0, "", downloadedFilter = 1).page() + assertEquals(listOf("Album Artist", "Guest"), filtered.map { it.name }) + assertEquals(listOf(1L, 1L), filtered.map { it.albumCount }) + + assertEquals( + setOf("Album Artist", "Guest"), + dao + .downloadedArtists(sourceId) + .page() + .map { it.name } + .toSet(), + ) + } + @Test fun downloadedFilterReportsTheDownloadedAlbumCount() = runTest { diff --git a/app/src/test/kotlin/com/subtracks/data/repo/DownloadRepositoryTest.kt b/app/src/test/kotlin/com/subtracks/data/repo/DownloadRepositoryTest.kt index 2ceb220f..5b37d3e8 100644 --- a/app/src/test/kotlin/com/subtracks/data/repo/DownloadRepositoryTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/repo/DownloadRepositoryTest.kt @@ -623,6 +623,76 @@ class DownloadRepositoryTest { assertEquals(listOf("s1", "s2"), requestedSongIds()) } + @Test + fun aGuestArtistCountsAndDownloadsOnlyTheTracksCreditedToThem() = + runTest { + seedLibrary() + runBlocking { + db.libraryDao().upsertSongs( + listOf( + Song( + sourceId = 1, + id = "s2", + albumId = "al1", + artistId = "ar-guest", + title = "Song 2", + album = "Album", + artist = "Guest", + duration = 100, + track = 2, + disc = 1, + starred = null, + genre = null, + ), + ), + ) + db.libraryDao().upsertArtists( + listOf(Artist(sourceId = 1, id = "ar-guest", name = "Guest", albumCount = 0, starred = null, coverArt = null)), + ) + } + + assertEquals(ListDownloadStatus(total = 1, downloaded = 0, downloading = 0), statusOf(DownloadList.Artist, "ar-guest")) + + runBlocking { repository.downloadAll(1, DownloadList.Artist, "ar-guest") } + + assertEquals(listOf("s2"), requestedSongIds()) + } + + @Test + fun theArtistListBadgesCountADownloadedGuestTrackForTheGuest() = + runTest { + seedLibrary() + runBlocking { + db.libraryDao().upsertSongs( + listOf( + Song( + sourceId = 1, + id = "s2", + albumId = "al1", + artistId = "ar-guest", + title = "Song 2", + album = "Album", + artist = "Guest", + duration = 100, + track = 2, + disc = 1, + starred = null, + genre = null, + ), + ), + ) + db.libraryDao().upsertArtists( + listOf(Artist(sourceId = 1, id = "ar-guest", name = "Guest", albumCount = 0, starred = null, coverArt = null)), + ) + db.downloadDao().upsert(SongDownload(1, "s2", DownloadStatus.Completed)) + } + + val artists = runBlocking { repository.statuses(1, DownloadList.Artist).first() } + + assertEquals(ListDownloadStatus(total = 2, downloaded = 1), artists["ar1"]) + assertEquals(ListDownloadStatus(total = 1, downloaded = 1), artists["ar-guest"]) + } + @Test fun cancellingWhileAListIsStillQueuingRemovesTheWholeList() = runTest {