diff --git a/app/src/main/kotlin/com/subtracks/data/repo/DownloadRepository.kt b/app/src/main/kotlin/com/subtracks/data/repo/DownloadRepository.kt index d2bc8ca8..5e0e39db 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/DownloadRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/DownloadRepository.kt @@ -432,18 +432,19 @@ class DownloadRepository( artistId: String, ) { if (db.libraryDao().artistBioOnce(sourceId, artistId) != null) return - if (!attemptedBios.add(sourceId to artistId)) return - val biography = - try { - sourceRepository.artistBiography(sourceId, artistId) - } catch (e: CancellationException) { - throw e - } catch (_: Exception) { - return - } - db.libraryDao().upsertArtistBio( - ArtistBio(sourceId, artistId, biography?.trim()?.takeIf { it.isNotEmpty() }), - ) + val key = sourceId to artistId + if (!attemptedBios.add(key)) return + try { + val biography = sourceRepository.artistBiography(sourceId, artistId) + db.libraryDao().upsertArtistBio( + ArtistBio(sourceId, artistId, normalizeBiography(biography)), + ) + } catch (e: CancellationException) { + throw e + } catch (_: Exception) { + } finally { + attemptedBios.remove(key) + } } private suspend fun fetchArtistBiosSafely( @@ -451,13 +452,8 @@ class DownloadRepository( artistIds: Set, ) { if (artistIds.isEmpty()) return - try { - withTimeoutOrNull(BIO_TIMEOUT_MS) { - artistIds.forEach { fetchArtistBio(sourceId, it) } - } - } catch (e: CancellationException) { - throw e - } catch (_: Exception) { + withTimeoutOrNull(BIO_TIMEOUT_MS) { + artistIds.forEach { fetchArtistBio(sourceId, it) } } } diff --git a/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt b/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt index ce8fff92..303b6ded 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt @@ -365,22 +365,20 @@ class LibraryRepository( suspend fun refreshArtistBio( sourceId: Long, artistId: String, - ): Result { - val biography = - try { - sourceRepository.artistBiography(sourceId, artistId)?.trim()?.takeIf { it.isNotEmpty() } - } catch (cancellation: CancellationException) { - throw cancellation - } catch (failure: Exception) { - Log.w("artist", "failed to refresh bio for $artistId", failure) - return Result.failure(failure) + ): Result = + try { + val biography = normalizeBiography(sourceRepository.artistBiography(sourceId, artistId)) + val current = db.libraryDao().artistBioOnce(sourceId, artistId) + if (current == null || current.biography != biography) { + db.libraryDao().upsertArtistBio(ArtistBio(sourceId, artistId, biography)) } - val current = db.libraryDao().artistBioOnce(sourceId, artistId) - if (current == null || current.biography != biography) { - db.libraryDao().upsertArtistBio(ArtistBio(sourceId, artistId, biography)) + Result.success(Unit) + } catch (cancellation: CancellationException) { + throw cancellation + } catch (failure: Exception) { + Log.w("artist", "failed to refresh bio for $artistId", failure) + Result.failure(failure) } - return Result.success(Unit) - } fun song( sourceId: Long, @@ -555,4 +553,6 @@ const val REDISCOVER_WINDOW_MS = 60L * 24 * 60 * 60 * 1000 const val SEARCH_RESULT_LIMIT = 50 +internal fun normalizeBiography(value: String?): String? = value?.trim()?.takeIf { it.isNotEmpty() } + fun rediscoverCutoff(): Long = System.currentTimeMillis() - REDISCOVER_WINDOW_MS 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 1dd94730..7f7bde08 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt @@ -116,6 +116,7 @@ import com.subtracks.ui.theme.heroBarColor import com.subtracks.ui.theme.rememberArtworkColors import com.subtracks.ui.theme.rememberOverlaidNameBusy import kotlinx.coroutines.flow.emptyFlow +import kotlinx.coroutines.flow.map import kotlinx.coroutines.launch import org.koin.compose.koinInject import org.koin.compose.viewmodel.koinViewModel @@ -155,6 +156,12 @@ private const val SCRIM_FADE_MS = 180 internal const val ARTIST_NAME_SCRIM_TAG = "artistNameScrim" internal const val ARTIST_BIO_MORE_TAG = "artistBioMore" +private data class BioSnapshot( + val known: Boolean, + val present: Boolean, + val biography: String?, +) + @Composable fun ArtistDetailRoute( artistId: String, @@ -170,8 +177,16 @@ fun ArtistDetailRoute( val artist by viewModel.artist.collectAsStateWithLifecycle(initialValue = null) val artistName by viewModel.artistName.collectAsStateWithLifecycle(initialValue = null) val albums by viewModel.albums.collectAsStateWithLifecycle(initialValue = null) - val artistBio by viewModel.artistBio.collectAsStateWithLifecycle(initialValue = null) + val bioSnapshots = + remember(viewModel) { + viewModel.artistBio.map { BioSnapshot(known = true, present = it != null, biography = it?.biography) } + } + val bioSnapshot by + bioSnapshots.collectAsStateWithLifecycle( + initialValue = BioSnapshot(known = false, present = false, biography = null), + ) val bioLoading by viewModel.bioLoading.collectAsStateWithLifecycle() + val bioUnavailable by viewModel.bioUnavailable.collectAsStateWithLifecycle() val bioExpanded by viewModel.bioExpanded.collectAsStateWithLifecycle() LaunchedEffect(artistId) { viewModel.refreshBio() } val art by viewModel.art.collectAsStateWithLifecycle() @@ -208,8 +223,10 @@ fun ArtistDetailRoute( artistName = artistName.orEmpty(), albums = albums ?: ArtistAlbums(), albumsLoaded = albums != null, - bio = artistBio?.biography, - bioLoading = bioLoading && artistBio == null, + bio = bioSnapshot.biography, + bioLoading = bioLoading && !bioSnapshot.present, + bioUnavailable = bioUnavailable && !bioSnapshot.present, + bioPending = !bioSnapshot.known, bioExpanded = bioExpanded, art = art, artThumbnail = artThumbnail, @@ -246,6 +263,8 @@ fun ArtistDetailScreen( albumsLoaded: Boolean = true, bio: String? = null, bioLoading: Boolean = false, + bioUnavailable: Boolean = false, + bioPending: Boolean = false, bioExpanded: Boolean = false, art: CoverArtRef?, artThumbnail: CoverArtRef? = null, @@ -284,8 +303,10 @@ fun ArtistDetailScreen( val bioCollapsed = hasBio && !bioExpanded val bioText = when { + bioPending -> "" hasBio -> bio.orEmpty() bioLoading -> stringResource(R.string.artist_bio_loading) + bioUnavailable -> stringResource(R.string.artist_bio_unavailable) else -> stringResource(R.string.artist_bio_empty) } var titleHeightPx by remember { mutableFloatStateOf(0f) } @@ -654,6 +675,9 @@ internal fun splitBioCollapsed( if (measurer.measure(AnnotatedString(candidate), style, maxLines = 1).size.width <= available) { first = candidate index++ + } else if (first.isEmpty()) { + first = words[index] + index++ } else { break } @@ -661,19 +685,9 @@ internal fun splitBioCollapsed( if (index >= words.size) return BioLines(listOf(AnnotatedString(first)), more = false) val secondStart = index - var second = "" - var secondIndex = secondStart - while (secondIndex < words.size) { - val candidate = if (second.isEmpty()) words[secondIndex] else "$second ${words[secondIndex]}" - if (measurer.measure(AnnotatedString(candidate), style, maxLines = 1).size.width <= available) { - second = candidate - secondIndex++ - } else { - break - } - } - if (secondIndex >= words.size) { - return BioLines(listOf(AnnotatedString(first), AnnotatedString(second)), more = false) + val remaining = words.drop(secondStart).joinToString(" ") + if (measurer.measure(AnnotatedString(remaining), style, maxLines = 1).size.width <= available) { + return BioLines(listOf(AnnotatedString(first), AnnotatedString(remaining)), more = false) } val more = diff --git a/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailViewModel.kt b/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailViewModel.kt index 636db0a3..adbb0bc2 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailViewModel.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailViewModel.kt @@ -29,6 +29,9 @@ import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch +import kotlinx.coroutines.withTimeoutOrNull + +private const val BIO_REFRESH_TIMEOUT_MS = 15_000L @OptIn(ExperimentalCoroutinesApi::class) class ArtistDetailViewModel( @@ -73,6 +76,9 @@ class ArtistDetailViewModel( private val _bioLoading = MutableStateFlow(true) val bioLoading: StateFlow = _bioLoading + private val _bioUnavailable = MutableStateFlow(false) + val bioUnavailable: StateFlow = _bioUnavailable + private val _bioExpanded = MutableStateFlow(false) val bioExpanded: StateFlow = _bioExpanded @@ -82,18 +88,22 @@ class ArtistDetailViewModel( if (bioRefresh?.isActive == true) return bioRefresh = viewModelScope.launch { + _bioUnavailable.value = false if (sourceRepository.offline.value) { _bioLoading.value = false + _bioUnavailable.value = true return@launch } val id = sourceRepository.activeSourceIdOnce() if (id == null) { _bioLoading.value = false + _bioUnavailable.value = true return@launch } _bioLoading.value = true try { - libraryRepository.refreshArtistBio(id, artistId) + val result = withTimeoutOrNull(BIO_REFRESH_TIMEOUT_MS) { libraryRepository.refreshArtistBio(id, artistId) } + _bioUnavailable.value = result == null || result.isFailure } finally { _bioLoading.value = false } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 06d4ce00..cfbfee3f 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -39,6 +39,7 @@ Appears on No artist bio available. Loading artist bio… + Artist bio unavailable. More %1$s · %2$s Genre diff --git a/app/src/test/kotlin/com/subtracks/ui/library/ArtistDetailScreenTest.kt b/app/src/test/kotlin/com/subtracks/ui/library/ArtistDetailScreenTest.kt index 14d72ecd..4dd488d2 100644 --- a/app/src/test/kotlin/com/subtracks/ui/library/ArtistDetailScreenTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/library/ArtistDetailScreenTest.kt @@ -44,6 +44,8 @@ class ArtistDetailScreenTest { albumsLoaded: Boolean = true, bio: String? = null, bioLoading: Boolean = false, + bioUnavailable: Boolean = false, + bioPending: Boolean = false, ) { composeRule.setContent { var expanded by remember { mutableStateOf(false) } @@ -55,6 +57,8 @@ class ArtistDetailScreenTest { albumsLoaded = albumsLoaded, bio = bio, bioLoading = bioLoading, + bioUnavailable = bioUnavailable, + bioPending = bioPending, bioExpanded = expanded, art = null, coverArt = { _, _ -> null }, @@ -151,6 +155,32 @@ class ArtistDetailScreenTest { composeRule.onNodeWithText("No artist bio available.").assertDoesNotExist() } + @Test + fun theBioAreaWaitsForTheStoredValueBeforeChoosingAText() { + render( + artist = Artist(sourceId = 1, id = "ar-1", name = "Radiohead", albumCount = 1, starred = null), + artistName = "Radiohead", + bioLoading = true, + bioPending = true, + ) + + composeRule.onNodeWithText("Loading artist bio…").assertDoesNotExist() + composeRule.onNodeWithText("No artist bio available.").assertDoesNotExist() + composeRule.onNodeWithText("Artist bio unavailable.").assertDoesNotExist() + } + + @Test + fun aFailedRefreshIsNotMistakenForAnArtistWithoutABio() { + render( + artist = Artist(sourceId = 1, id = "ar-1", name = "Radiohead", albumCount = 1, starred = null), + artistName = "Radiohead", + bioUnavailable = true, + ) + + composeRule.onNodeWithText("Artist bio unavailable.").assertExists() + composeRule.onNodeWithText("No artist bio available.").assertDoesNotExist() + } + @Test fun aMissingBioShowsTheEmptyStateAfterTheRefresh() { render( @@ -218,6 +248,33 @@ class ArtistDetailScreenTest { ) } + @Test + fun anOversizedFirstWordIsStillShown() { + var result: BioLines? = null + composeRule.setContent { + val measurer = rememberTextMeasurer() + result = + splitBioCollapsed( + measurer, + "Supercalifragilisticexpialidocious and the rest", + TextStyle(fontSize = 14.sp), + 20, + 0, + "More", + ) + } + composeRule.waitForIdle() + + val lines = requireNotNull(result) + assertTrue(lines.more) + assertTrue( + lines.lines + .first() + .text + .isNotBlank(), + ) + } + private fun album( id: String, name: String,