mirror of
https://github.com/austinried/subtracks.git
synced 2026-11-11 07:29:14 +01:00
library: rank artist cover fallback like the materialized list
Reuse the materialized ranking CTE chain for the per-artist fallback so duplicate covers keep the same album, exercise the four-row cap, and test every credit path plus source isolation. Resolve the art input as a pure function and key the artist and covers off the same source.
This commit is contained in:
@@ -204,8 +204,8 @@ private const val ARTIST_COVER_CREDITS =
|
|||||||
"JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " +
|
"JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " +
|
||||||
"WHERE song_artists.sourceId = :sourceId AND albums.coverArt IS NOT NULL"
|
"WHERE song_artists.sourceId = :sourceId AND albums.coverArt IS NOT NULL"
|
||||||
|
|
||||||
internal const val ARTIST_ART_UPDATE_SQL =
|
private const val ARTIST_COVER_RANKS =
|
||||||
"WITH credits AS (SELECT artistId, albumId, " +
|
"credits AS (SELECT artistId, albumId, " +
|
||||||
"MIN(CASE creditType WHEN 'album' THEN 0 ELSE 1 END) AS creditRank " +
|
"MIN(CASE creditType WHEN 'album' THEN 0 ELSE 1 END) AS creditRank " +
|
||||||
"FROM ($ARTIST_COVER_CREDITS) GROUP BY artistId, albumId), " +
|
"FROM ($ARTIST_COVER_CREDITS) GROUP BY artistId, albumId), " +
|
||||||
"ranked AS (SELECT credits.artistId AS artistId, albums.coverArt AS coverArt, " +
|
"ranked AS (SELECT credits.artistId AS artistId, albums.coverArt AS coverArt, " +
|
||||||
@@ -215,7 +215,10 @@ internal const val ARTIST_ART_UPDATE_SQL =
|
|||||||
"WHERE albums.coverArt IS NOT NULL), " +
|
"WHERE albums.coverArt IS NOT NULL), " +
|
||||||
"unique_ranked AS (SELECT artistId, coverArt, MIN(coverRank) AS coverRank FROM ranked GROUP BY artistId, coverArt), " +
|
"unique_ranked AS (SELECT artistId, coverArt, MIN(coverRank) AS coverRank FROM ranked GROUP BY artistId, coverArt), " +
|
||||||
"deduped AS (SELECT artistId, coverArt, " +
|
"deduped AS (SELECT artistId, coverArt, " +
|
||||||
"ROW_NUMBER() OVER (PARTITION BY artistId ORDER BY coverRank) AS coverRank FROM unique_ranked), " +
|
"ROW_NUMBER() OVER (PARTITION BY artistId ORDER BY coverRank) AS coverRank FROM unique_ranked)"
|
||||||
|
|
||||||
|
internal const val ARTIST_ART_UPDATE_SQL =
|
||||||
|
"WITH $ARTIST_COVER_RANKS, " +
|
||||||
"covers AS (SELECT artistId, group_concat(coverArt, char(31)) AS coverArts FROM (" +
|
"covers AS (SELECT artistId, group_concat(coverArt, char(31)) AS coverArts FROM (" +
|
||||||
"SELECT artistId, coverArt, coverRank FROM deduped WHERE coverRank <= 4 ORDER BY artistId, coverRank" +
|
"SELECT artistId, coverArt, coverRank FROM deduped WHERE coverRank <= 4 ORDER BY artistId, coverRank" +
|
||||||
") GROUP BY artistId), " +
|
") GROUP BY artistId), " +
|
||||||
@@ -241,11 +244,8 @@ internal const val ARTIST_ART_UPDATE_SQL =
|
|||||||
"(artists.coverArts != desired.coverArts OR artists.downloadedCoverArts != desired.downloadedCoverArts)"
|
"(artists.coverArts != desired.coverArts OR artists.downloadedCoverArts != desired.downloadedCoverArts)"
|
||||||
|
|
||||||
internal const val ARTIST_COVER_ARTS_SQL =
|
internal const val ARTIST_COVER_ARTS_SQL =
|
||||||
"SELECT DISTINCT albums.coverArt FROM ($ARTIST_COVER_CREDITS) credits " +
|
"WITH $ARTIST_COVER_RANKS " +
|
||||||
"JOIN albums ON albums.sourceId = :sourceId AND albums.id = credits.albumId " +
|
"SELECT coverArt FROM deduped WHERE artistId = :artistId AND coverRank <= 4 ORDER BY coverRank"
|
||||||
"WHERE credits.artistId = :artistId AND albums.coverArt IS NOT NULL " +
|
|
||||||
"ORDER BY CASE credits.creditType WHEN 'album' THEN 0 ELSE 1 END, albums.year DESC, " +
|
|
||||||
"albums.sortName COLLATE NOCASE, albums.id LIMIT 4"
|
|
||||||
|
|
||||||
internal const val DOWNLOADED_ARTIST_ART_UPDATE_SQL =
|
internal const val DOWNLOADED_ARTIST_ART_UPDATE_SQL =
|
||||||
"WITH downloaded_albums AS (SELECT DISTINCT artistId, albumId FROM ($DOWNLOADED_COVER_ALBUM_ROWS) " +
|
"WITH downloaded_albums AS (SELECT DISTINCT artistId, albumId FROM ($DOWNLOADED_COVER_ALBUM_ROWS) " +
|
||||||
@@ -433,13 +433,7 @@ interface LibraryDao {
|
|||||||
suspend fun recomputeArtistArt(sourceId: Long)
|
suspend fun recomputeArtistArt(sourceId: Long)
|
||||||
|
|
||||||
@Query(ARTIST_COVER_ARTS_SQL)
|
@Query(ARTIST_COVER_ARTS_SQL)
|
||||||
suspend fun artistCoverArts(
|
fun artistCoverArts(
|
||||||
sourceId: Long,
|
|
||||||
artistId: String,
|
|
||||||
): List<String>
|
|
||||||
|
|
||||||
@Query(ARTIST_COVER_ARTS_SQL)
|
|
||||||
fun artistCoverArtsFlow(
|
|
||||||
sourceId: Long,
|
sourceId: Long,
|
||||||
artistId: String,
|
artistId: String,
|
||||||
): Flow<List<String>>
|
): Flow<List<String>>
|
||||||
|
|||||||
@@ -445,7 +445,7 @@ class LibraryRepository(
|
|||||||
fun artistCoverArts(
|
fun artistCoverArts(
|
||||||
sourceId: Long,
|
sourceId: Long,
|
||||||
artistId: String,
|
artistId: String,
|
||||||
): Flow<List<String>> = db.libraryDao().artistCoverArtsFlow(sourceId, artistId)
|
): Flow<List<String>> = db.libraryDao().artistCoverArts(sourceId, artistId)
|
||||||
|
|
||||||
fun albumDiscs(
|
fun albumDiscs(
|
||||||
sourceId: Long,
|
sourceId: Long,
|
||||||
|
|||||||
@@ -145,50 +145,16 @@ class ArtistDetailViewModel(
|
|||||||
|
|
||||||
init {
|
init {
|
||||||
viewModelScope.launch {
|
viewModelScope.launch {
|
||||||
combine(
|
sourceRepository.activeSource
|
||||||
artist,
|
.filterNotNull()
|
||||||
sourceRepository.offline,
|
.flatMapLatest { sourceId ->
|
||||||
sourceRepository.activeSource,
|
combine(
|
||||||
sourceRepository.activeSource.filterNotNull().flatMapLatest {
|
libraryRepository.artist(sourceId, artistId),
|
||||||
libraryRepository.artistCoverArts(it, artistId)
|
libraryRepository.artistCoverArts(sourceId, artistId),
|
||||||
},
|
sourceRepository.offline,
|
||||||
) { loaded, offline, activeSource, covers -> ArtistArtInput(loaded, offline, activeSource, covers) }
|
) { loaded, covers, offline -> ArtistArtInput(loaded, artistId, offline, sourceId, covers) }
|
||||||
.collectLatest { input ->
|
}.collectLatest { input ->
|
||||||
if (input.sourceId == null) return@collectLatest
|
val resolved = resolveArtistArtInput(input)
|
||||||
val resolved =
|
|
||||||
if (input.covers.isEmpty()) {
|
|
||||||
input.artist
|
|
||||||
} else {
|
|
||||||
when {
|
|
||||||
input.artist == null -> {
|
|
||||||
Artist(
|
|
||||||
sourceId = input.sourceId,
|
|
||||||
id = artistId,
|
|
||||||
name = "",
|
|
||||||
albumCount = 0,
|
|
||||||
starred = null,
|
|
||||||
coverArts = input.covers,
|
|
||||||
downloadedCoverArts = if (input.offline) input.covers else emptyList(),
|
|
||||||
)
|
|
||||||
}
|
|
||||||
|
|
||||||
input.offline -> {
|
|
||||||
if (input.artist.downloadedCoverArts.isNullOrEmpty()) {
|
|
||||||
input.artist.copy(downloadedCoverArts = input.covers)
|
|
||||||
} else {
|
|
||||||
input.artist
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
input.artist.coverArts == input.covers -> {
|
|
||||||
input.artist
|
|
||||||
}
|
|
||||||
|
|
||||||
else -> {
|
|
||||||
input.artist.copy(coverArts = input.covers)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
if (resolved == null) {
|
if (resolved == null) {
|
||||||
_art.value = null
|
_art.value = null
|
||||||
_artThumbnail.value = null
|
_artThumbnail.value = null
|
||||||
@@ -216,9 +182,34 @@ class ArtistDetailViewModel(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private data class ArtistArtInput(
|
internal data class ArtistArtInput(
|
||||||
val artist: Artist?,
|
val artist: Artist?,
|
||||||
|
val artistId: String,
|
||||||
val offline: Boolean,
|
val offline: Boolean,
|
||||||
val sourceId: Long?,
|
val sourceId: Long,
|
||||||
val covers: List<String>,
|
val covers: List<String>,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
internal fun resolveArtistArtInput(input: ArtistArtInput): Artist? {
|
||||||
|
val artist = input.artist
|
||||||
|
if (input.covers.isEmpty()) return artist
|
||||||
|
if (artist == null) {
|
||||||
|
return Artist(
|
||||||
|
sourceId = input.sourceId,
|
||||||
|
id = input.artistId,
|
||||||
|
name = "",
|
||||||
|
albumCount = 0,
|
||||||
|
starred = null,
|
||||||
|
coverArts = input.covers,
|
||||||
|
downloadedCoverArts = if (input.offline) input.covers else emptyList(),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
return if (input.offline) {
|
||||||
|
artist.copy(
|
||||||
|
coverArts = input.covers,
|
||||||
|
downloadedCoverArts = artist.downloadedCoverArts.ifEmpty { input.covers },
|
||||||
|
)
|
||||||
|
} else {
|
||||||
|
artist.copy(coverArts = input.covers)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -656,7 +656,7 @@ class LibraryDaoTest {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun coverArtFallbackFindsTrackCreditsWithoutAnArtistRow() =
|
fun coverArtFallbackFindsEveryCreditPath() =
|
||||||
runTest {
|
runTest {
|
||||||
val sourceId = source()
|
val sourceId = source()
|
||||||
val dao = db.libraryDao()
|
val dao = db.libraryDao()
|
||||||
@@ -696,9 +696,44 @@ class LibraryDaoTest {
|
|||||||
assertNull(dao.artistOnce(sourceId, "ar-missing"))
|
assertNull(dao.artistOnce(sourceId, "ar-missing"))
|
||||||
assertNull(dao.artistOnce(sourceId, "ar-featured"))
|
assertNull(dao.artistOnce(sourceId, "ar-featured"))
|
||||||
assertNull(dao.artistOnce(sourceId, "ar-guest"))
|
assertNull(dao.artistOnce(sourceId, "ar-guest"))
|
||||||
assertEquals(listOf("track-art"), dao.artistCoverArts(sourceId, "ar-missing"))
|
assertEquals(listOf("track-art"), dao.artistCoverArts(sourceId, "ar-missing").first())
|
||||||
assertEquals(listOf("featured-art"), dao.artistCoverArts(sourceId, "ar-featured"))
|
assertEquals(listOf("featured-art", "track-art"), dao.artistCoverArts(sourceId, "ar-other").first())
|
||||||
assertEquals(listOf("featured-art"), dao.artistCoverArts(sourceId, "ar-guest"))
|
assertEquals(listOf("featured-art"), dao.artistCoverArts(sourceId, "ar-featured").first())
|
||||||
|
assertEquals(listOf("featured-art"), dao.artistCoverArts(sourceId, "ar-guest").first())
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun artistCoverArtsRanksDeduplicatesCapsAndStaysInSource() =
|
||||||
|
runTest {
|
||||||
|
val sourceId = source()
|
||||||
|
val otherSourceId = source()
|
||||||
|
val dao = db.libraryDao()
|
||||||
|
dao.upsertArtists(listOf(artist(sourceId, "ar-rank", "Rank", albumCount = 0)))
|
||||||
|
dao.upsertAlbums(
|
||||||
|
listOf(
|
||||||
|
album(sourceId, "a-new", "New", year = 2020, starred = null)
|
||||||
|
.copy(artistId = "ar-rank", coverArt = "shared"),
|
||||||
|
album(sourceId, "a-mid", "Mid", year = 2015, starred = null)
|
||||||
|
.copy(artistId = "ar-rank", coverArt = "other"),
|
||||||
|
album(sourceId, "a-old", "Old", year = 2010, starred = null)
|
||||||
|
.copy(artistId = "ar-rank", coverArt = "shared"),
|
||||||
|
album(sourceId, "a-4", "Four", year = 2012, starred = null)
|
||||||
|
.copy(artistId = "ar-rank", coverArt = "c4"),
|
||||||
|
album(sourceId, "a-5", "Five", year = 2011, starred = null)
|
||||||
|
.copy(artistId = "ar-rank", coverArt = "c5"),
|
||||||
|
album(sourceId, "a-6", "Six", year = 2009, starred = null)
|
||||||
|
.copy(artistId = "ar-rank", coverArt = "c6"),
|
||||||
|
album(otherSourceId, "a-leak", "Leak", year = 2025, starred = null)
|
||||||
|
.copy(artistId = "ar-rank", coverArt = "leak"),
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
val expected = listOf("shared", "other", "c4", "c5")
|
||||||
|
assertEquals(expected, dao.artistCoverArts(sourceId, "ar-rank").first())
|
||||||
|
|
||||||
|
dao.recomputeArtistArt(sourceId)
|
||||||
|
|
||||||
|
assertEquals(expected, dao.artistOnce(sourceId, "ar-rank")!!.coverArts)
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
|||||||
@@ -0,0 +1,105 @@
|
|||||||
|
package com.subtracks.ui.library
|
||||||
|
|
||||||
|
import com.subtracks.data.model.Artist
|
||||||
|
import org.junit.Assert.assertEquals
|
||||||
|
import org.junit.Assert.assertNull
|
||||||
|
import org.junit.Test
|
||||||
|
|
||||||
|
class ArtistDetailArtTest {
|
||||||
|
@Test
|
||||||
|
fun aCreditedArtistWithoutARowUsesTheCreditCovers() {
|
||||||
|
val resolved =
|
||||||
|
resolveArtistArtInput(
|
||||||
|
ArtistArtInput(
|
||||||
|
artist = null,
|
||||||
|
artistId = "ar1",
|
||||||
|
offline = false,
|
||||||
|
sourceId = 7,
|
||||||
|
covers = listOf("a", "b"),
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
assertEquals(7L, resolved?.sourceId)
|
||||||
|
assertEquals("ar1", resolved?.id)
|
||||||
|
assertEquals(listOf("a", "b"), resolved?.coverArts)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun anOfflineArtistKeepsItsDownloadedCovers() {
|
||||||
|
val resolved =
|
||||||
|
resolveArtistArtInput(
|
||||||
|
ArtistArtInput(
|
||||||
|
artist = artist(coverArts = listOf("online"), downloadedCoverArts = listOf("downloaded")),
|
||||||
|
artistId = "ar1",
|
||||||
|
offline = true,
|
||||||
|
sourceId = 7,
|
||||||
|
covers = listOf("online", "other"),
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
assertEquals(listOf("online", "other"), resolved?.coverArts)
|
||||||
|
assertEquals(listOf("downloaded"), resolved?.downloadedCoverArts)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun anOfflineArtistWithoutDownloadedCoversFallsBackToTheCreditCovers() {
|
||||||
|
val resolved =
|
||||||
|
resolveArtistArtInput(
|
||||||
|
ArtistArtInput(
|
||||||
|
artist = artist(downloadedCoverArts = emptyList()),
|
||||||
|
artistId = "ar1",
|
||||||
|
offline = true,
|
||||||
|
sourceId = 7,
|
||||||
|
covers = listOf("a", "b"),
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
assertEquals(listOf("a", "b"), resolved?.downloadedCoverArts)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun anArtistWithMaterializedCoversIsKeptWhenTheCreditQueryIsEmpty() {
|
||||||
|
val original = artist(coverArts = listOf("materialized"))
|
||||||
|
|
||||||
|
val resolved =
|
||||||
|
resolveArtistArtInput(
|
||||||
|
ArtistArtInput(
|
||||||
|
artist = original,
|
||||||
|
artistId = "ar1",
|
||||||
|
offline = false,
|
||||||
|
sourceId = 7,
|
||||||
|
covers = emptyList(),
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
assertEquals(original, resolved)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun noArtistAndNoCoversClearsTheArt() {
|
||||||
|
assertNull(
|
||||||
|
resolveArtistArtInput(
|
||||||
|
ArtistArtInput(
|
||||||
|
artist = null,
|
||||||
|
artistId = "ar1",
|
||||||
|
offline = false,
|
||||||
|
sourceId = 7,
|
||||||
|
covers = emptyList(),
|
||||||
|
),
|
||||||
|
),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
private fun artist(
|
||||||
|
coverArts: List<String> = emptyList(),
|
||||||
|
downloadedCoverArts: List<String> = emptyList(),
|
||||||
|
) = Artist(
|
||||||
|
sourceId = 7,
|
||||||
|
id = "ar1",
|
||||||
|
name = "Artist",
|
||||||
|
albumCount = 1,
|
||||||
|
starred = null,
|
||||||
|
coverArts = coverArts,
|
||||||
|
downloadedCoverArts = downloadedCoverArts,
|
||||||
|
)
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user