From b92d9da41dd14a20b6189b06af076a7423d982e2 Mon Sep 17 00:00:00 2001 From: David Ly <22302001+lydavid@users.noreply.github.com> Date: Wed, 11 Sep 2024 21:15:05 -0400 Subject: [PATCH] feat: support refreshing release cover arts fixes #1082 --- .../coverart/ReleaseImageRepositoryImpl.kt | 38 ++++++++++++++----- .../data/spotify/ArtistImageRepositoryImpl.kt | 6 --- .../domain/artist/ArtistImageRepository.kt | 7 ++++ .../domain/release/ReleaseImageRepository.kt | 3 +- shared/feature/details/config/baseline.xml | 1 + .../details/release/ReleasePresenter.kt | 26 ++++++------- .../release/ReleasesByEntityPresenter.kt | 3 +- 7 files changed, 53 insertions(+), 31 deletions(-) diff --git a/data/coverart/src/commonMain/kotlin/ly/david/musicsearch/data/coverart/ReleaseImageRepositoryImpl.kt b/data/coverart/src/commonMain/kotlin/ly/david/musicsearch/data/coverart/ReleaseImageRepositoryImpl.kt index 1e9cf3a68..b1649186e 100644 --- a/data/coverart/src/commonMain/kotlin/ly/david/musicsearch/data/coverart/ReleaseImageRepositoryImpl.kt +++ b/data/coverart/src/commonMain/kotlin/ly/david/musicsearch/data/coverart/ReleaseImageRepositoryImpl.kt @@ -3,10 +3,10 @@ package ly.david.musicsearch.data.coverart import io.ktor.client.plugins.ClientRequestException import io.ktor.http.HttpStatusCode import ly.david.musicsearch.core.logging.Logger -import ly.david.musicsearch.shared.domain.image.ImageUrlDao -import ly.david.musicsearch.shared.domain.image.ImageUrls import ly.david.musicsearch.data.coverart.api.CoverArtArchiveApi import ly.david.musicsearch.data.coverart.api.toImageUrlsList +import ly.david.musicsearch.shared.domain.image.ImageUrlDao +import ly.david.musicsearch.shared.domain.image.ImageUrls import ly.david.musicsearch.shared.domain.release.ReleaseImageRepository internal class ReleaseImageRepositoryImpl( @@ -14,31 +14,49 @@ internal class ReleaseImageRepositoryImpl( private val imageUrlDao: ImageUrlDao, private val logger: Logger, ) : ReleaseImageRepository { - override suspend fun getReleaseCoverArtUrlsFromNetworkAndSave( + + override suspend fun getReleaseImageUrl( + releaseId: String, + thumbnail: Boolean, + forceRefresh: Boolean, + ): String { + if (forceRefresh) { + imageUrlDao.deleteAllUrlsById(releaseId) + } + + val cachedImageUrls = imageUrlDao.getAllUrls(releaseId) + return if (cachedImageUrls.isNotEmpty()) { + val frontCoverArt = cachedImageUrls.first() + return if (thumbnail) frontCoverArt.thumbnailUrl else frontCoverArt.largeUrl + } else { + getReleaseImageUrlFromNetwork(releaseId, thumbnail) + } + } + + private suspend fun getReleaseImageUrlFromNetwork( releaseId: String, thumbnail: Boolean, ): String { return try { val coverArts = coverArtArchiveApi.getReleaseCoverArts(releaseId) val imageUrls: MutableList = coverArts.toImageUrlsList().toMutableList() + + // We use an empty ImageUrls to represent that we've searched but failed to find any images. if (imageUrls.isEmpty()) { imageUrls.add(ImageUrls()) } + imageUrlDao.saveUrls( mbid = releaseId, imageUrls = imageUrls, ) - return if (thumbnail) imageUrls.first().thumbnailUrl else imageUrls.first().largeUrl + val frontCoverArt = imageUrls.first() + return if (thumbnail) frontCoverArt.thumbnailUrl else frontCoverArt.largeUrl } catch (ex: ClientRequestException) { if (ex.response.status == HttpStatusCode.NotFound) { imageUrlDao.saveUrls( mbid = releaseId, - imageUrls = listOf( - ImageUrls( - thumbnailUrl = "", - largeUrl = "", - ), - ), + imageUrls = listOf(ImageUrls()), ) } else { logger.e(ex) diff --git a/data/spotify/src/commonMain/kotlin/ly/david/musicsearch/data/spotify/ArtistImageRepositoryImpl.kt b/data/spotify/src/commonMain/kotlin/ly/david/musicsearch/data/spotify/ArtistImageRepositoryImpl.kt index 11b11bd16..e2f31d28c 100644 --- a/data/spotify/src/commonMain/kotlin/ly/david/musicsearch/data/spotify/ArtistImageRepositoryImpl.kt +++ b/data/spotify/src/commonMain/kotlin/ly/david/musicsearch/data/spotify/ArtistImageRepositoryImpl.kt @@ -17,12 +17,6 @@ class ArtistImageRepositoryImpl( private val logger: Logger, ) : ArtistImageRepository { - /** - * Returns a url to the artist image. - * Empty if none found. - * - * Also saves it to db. - */ override suspend fun getArtistImageUrl( artistDetailsModel: ArtistDetailsModel, forceRefresh: Boolean, diff --git a/shared/domain/src/commonMain/kotlin/ly/david/musicsearch/shared/domain/artist/ArtistImageRepository.kt b/shared/domain/src/commonMain/kotlin/ly/david/musicsearch/shared/domain/artist/ArtistImageRepository.kt index dbafe7f33..02f86a304 100644 --- a/shared/domain/src/commonMain/kotlin/ly/david/musicsearch/shared/domain/artist/ArtistImageRepository.kt +++ b/shared/domain/src/commonMain/kotlin/ly/david/musicsearch/shared/domain/artist/ArtistImageRepository.kt @@ -1,6 +1,13 @@ package ly.david.musicsearch.shared.domain.artist interface ArtistImageRepository { + + /** + * Returns a url to the artist image. + * Empty if none found. + * + * Also saves it to db. + */ suspend fun getArtistImageUrl( artistDetailsModel: ArtistDetailsModel, forceRefresh: Boolean, diff --git a/shared/domain/src/commonMain/kotlin/ly/david/musicsearch/shared/domain/release/ReleaseImageRepository.kt b/shared/domain/src/commonMain/kotlin/ly/david/musicsearch/shared/domain/release/ReleaseImageRepository.kt index b0dc0bbde..dc4a50a15 100644 --- a/shared/domain/src/commonMain/kotlin/ly/david/musicsearch/shared/domain/release/ReleaseImageRepository.kt +++ b/shared/domain/src/commonMain/kotlin/ly/david/musicsearch/shared/domain/release/ReleaseImageRepository.kt @@ -14,9 +14,10 @@ interface ReleaseImageRepository { * * Make sure to handle non-404 errors at call site. */ - suspend fun getReleaseCoverArtUrlsFromNetworkAndSave( + suspend fun getReleaseImageUrl( releaseId: String, thumbnail: Boolean, + forceRefresh: Boolean, ): String fun getAllUrls(mbid: String): List diff --git a/shared/feature/details/config/baseline.xml b/shared/feature/details/config/baseline.xml index c0cb4081d..a23c2b74b 100644 --- a/shared/feature/details/config/baseline.xml +++ b/shared/feature/details/config/baseline.xml @@ -4,5 +4,6 @@ CyclomaticComplexMethod:AreaPresenter.kt$AreaPresenter$@Composable override fun present(): AreaUiState CyclomaticComplexMethod:ArtistPresenter.kt$ArtistPresenter$@Composable override fun present(): ArtistUiState + CyclomaticComplexMethod:ReleasePresenter.kt$ReleasePresenter$@Composable override fun present(): ReleaseUiState diff --git a/shared/feature/details/src/commonMain/kotlin/ly/david/musicsearch/shared/feature/details/release/ReleasePresenter.kt b/shared/feature/details/src/commonMain/kotlin/ly/david/musicsearch/shared/feature/details/release/ReleasePresenter.kt index b27e34a00..2e1fab62d 100644 --- a/shared/feature/details/src/commonMain/kotlin/ly/david/musicsearch/shared/feature/details/release/ReleasePresenter.kt +++ b/shared/feature/details/src/commonMain/kotlin/ly/david/musicsearch/shared/feature/details/release/ReleasePresenter.kt @@ -19,13 +19,13 @@ import com.slack.circuit.runtime.presenter.Presenter import kotlinx.collections.immutable.ImmutableList import kotlinx.collections.immutable.toPersistentList import ly.david.musicsearch.core.logging.Logger +import ly.david.musicsearch.data.common.network.RecoverableNetworkException import ly.david.musicsearch.shared.domain.artist.getDisplayNames import ly.david.musicsearch.shared.domain.getNameWithDisambiguation import ly.david.musicsearch.shared.domain.history.LookupHistory +import ly.david.musicsearch.shared.domain.history.usecase.IncrementLookupHistory import ly.david.musicsearch.shared.domain.network.MusicBrainzEntity import ly.david.musicsearch.shared.domain.release.ReleaseDetailsModel -import ly.david.musicsearch.data.common.network.RecoverableNetworkException -import ly.david.musicsearch.shared.domain.history.usecase.IncrementLookupHistory import ly.david.musicsearch.shared.domain.release.ReleaseImageRepository import ly.david.musicsearch.shared.domain.release.ReleaseRepository import ly.david.musicsearch.ui.common.artist.ArtistsByEntityPresenter @@ -87,7 +87,6 @@ internal class ReleasePresenter( title = releaseDetailsModel.getNameWithDisambiguation() subtitle = "Release by ${releaseDetailsModel.artistCredits.getDisplayNames()}" release = releaseDetailsModel - imageUrl = fetchReleaseImage(releaseDetailsModel) isError = false } catch (ex: RecoverableNetworkException) { logger.e(ex) @@ -105,6 +104,16 @@ internal class ReleasePresenter( } } + LaunchedEffect(forceRefreshDetails, release) { + release?.let { release -> + imageUrl = releaseImageRepository.getReleaseImageUrl( + releaseId = release.id, + thumbnail = false, + forceRefresh = forceRefreshDetails, + ) + } + } + LaunchedEffect( key1 = query, key2 = selectedTab, @@ -203,16 +212,6 @@ internal class ReleasePresenter( eventSink = ::eventSink, ) } - - private suspend fun fetchReleaseImage( - releaseDetailsModel: ReleaseDetailsModel, - ): String { - val imageUrl = releaseDetailsModel.imageUrl - return imageUrl ?: releaseImageRepository.getReleaseCoverArtUrlsFromNetworkAndSave( - releaseId = releaseDetailsModel.id, - thumbnail = false, - ) - } } @Stable @@ -241,5 +240,6 @@ internal sealed interface ReleaseUiEvent : CircuitUiEvent { val id: String, val title: String?, ) : ReleaseUiEvent + data object ClickImage : ReleaseUiEvent } diff --git a/ui/common/src/commonMain/kotlin/ly/david/musicsearch/ui/common/release/ReleasesByEntityPresenter.kt b/ui/common/src/commonMain/kotlin/ly/david/musicsearch/ui/common/release/ReleasesByEntityPresenter.kt index a9ea4452c..d1654fbe1 100644 --- a/ui/common/src/commonMain/kotlin/ly/david/musicsearch/ui/common/release/ReleasesByEntityPresenter.kt +++ b/ui/common/src/commonMain/kotlin/ly/david/musicsearch/ui/common/release/ReleasesByEntityPresenter.kt @@ -69,9 +69,10 @@ class ReleasesByEntityPresenter( is ReleasesByEntityUiEvent.RequestForMissingCoverArtUrl -> { scope.launch { try { - releaseImageRepository.getReleaseCoverArtUrlsFromNetworkAndSave( + releaseImageRepository.getReleaseImageUrl( releaseId = event.entityId, thumbnail = true, + forceRefresh = false, ) } catch (ex: Exception) { logger.e(ex)