fix: avoid refetching broken assets [WPB-2826]

app/src/main/kotlin/com/wire/android/di/AppModule.kt

@@ -30,6 +30,8 @@ import com.wire.android.mapper.MessageResourceProvider
 import com.wire.android.navigation.NavigationManager
 import com.wire.android.util.dispatchers.DefaultDispatcherProvider
 import com.wire.android.util.dispatchers.DispatcherProvider
+import com.wire.kalium.logic.network.NetworkStateObserver
+import com.wire.kalium.logic.network.NetworkStateObserverImpl
 import dagger.Module
 import dagger.Provides
 import dagger.hilt.InstallIn
@@ -69,6 +71,10 @@ object AppModule {
     fun provideNotificationManagerCompat(appContext: Context): NotificationManagerCompat =
         NotificationManagerCompat.from(appContext)
 
+    @Provides
+    fun provideNetworkStateObserver(appContext: Context): NetworkStateObserver =
+        NetworkStateObserverImpl(appContext)
+
     @Provides
     fun provideNotificationManager(appContext: Context): NotificationManager =
         appContext.getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager

app/src/main/kotlin/com/wire/android/di/ImageLoadingModule.kt

@@ -25,6 +25,7 @@ import com.wire.android.util.ui.WireSessionImageLoader
 import com.wire.kalium.logic.feature.asset.DeleteAssetUseCase
 import com.wire.kalium.logic.feature.asset.GetAvatarAssetUseCase
 import com.wire.kalium.logic.feature.asset.GetMessageAssetUseCase
+import com.wire.kalium.logic.network.NetworkStateObserver
 import dagger.Module
 import dagger.Provides
 import dagger.hilt.InstallIn
@@ -45,10 +46,12 @@ class ImageLoadingModule {
         getAvatarAsset: GetAvatarAssetUseCase,
         deleteAsset: DeleteAssetUseCase,
         getMessageAsset: GetMessageAssetUseCase,
+        networkStateObserver: NetworkStateObserver,
     ): WireSessionImageLoader.Factory = WireSessionImageLoader.Factory(
         context = context,
         getAvatarAsset = getAvatarAsset,
         deleteAsset = deleteAsset,
+        networkStateObserver = networkStateObserver,
         getPrivateAsset = getMessageAsset
     )
 

app/src/main/kotlin/com/wire/android/model/ImageAsset.kt

@@ -74,7 +74,7 @@ sealed class ImageAsset(private val imageLoader: WireSessionImageLoader) {
     }
 
     @Composable
-    fun paint(fallbackData: Any? = null) = imageLoader.paint(asset = this, fallbackData)
+    fun paint(fallbackData: Any? = null, errorDrawable: Int? = null) = imageLoader.paint(asset = this, fallbackData, errorDrawable)
 }
 
 fun String.parseIntoPrivateImageAsset(

app/src/main/kotlin/com/wire/android/ui/common/UserProfileAvatar.kt

@@ -91,14 +91,16 @@ fun UserProfileAvatar(
  * @see [painter] https://developer.android.com/jetpack/compose/tooling
  */
 @Composable
-private fun painter(data: UserAvatarData): Painter =
-    if (data.connectionState == ConnectionState.BLOCKED) {
-        painterResource(id = R.drawable.ic_blocked_user_avatar)
-    } else if (LocalInspectionMode.current || data.asset == null) {
-        getDefaultAvatar(membership = data.membership)
-    } else {
-        data.asset.paint(getUriFromDrawable(LocalContext.current, R.drawable.ic_default_user_avatar))
-    }
+private fun painter(data: UserAvatarData): Painter = if (data.connectionState == ConnectionState.BLOCKED) {
+    painterResource(id = R.drawable.ic_blocked_user_avatar)
+} else if (LocalInspectionMode.current || data.asset == null) {
+    getDefaultAvatar(membership = data.membership)
+} else {
+    data.asset.paint(
+        getUriFromDrawable(LocalContext.current, R.drawable.ic_default_user_avatar),
+        R.drawable.ic_default_user_avatar
+    )
+}
 
 @Composable
 private fun getDefaultAvatar(membership: Membership): Painter =

app/src/main/kotlin/com/wire/android/ui/home/conversations/mock/Mock.kt

@@ -49,6 +49,10 @@ import com.wire.kalium.logic.data.message.Message
 import com.wire.kalium.logic.data.user.ConnectionState
 import com.wire.kalium.logic.data.user.UserAssetId
 import com.wire.kalium.logic.data.user.UserAvailabilityStatus
+import com.wire.kalium.logic.network.NetworkState
+import com.wire.kalium.logic.network.NetworkStateObserver
+import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.flow.StateFlow
 
 val mockFooter = MessageFooter("", mapOf("👍" to 1), setOf("👍"))
 val mockEmptyFooter = MessageFooter("", emptyMap(), emptySet())
@@ -97,7 +101,11 @@ val mockImageLoader = WireSessionImageLoader(object : ImageLoader {
     override suspend fun execute(request: ImageRequest): ImageResult = TODO("Not yet implemented")
     override fun newBuilder(): ImageLoader.Builder = TODO("Not yet implemented")
     override fun shutdown() = TODO("Not yet implemented")
-})
+},
+    object : NetworkStateObserver {
+        override fun observeNetworkState(): StateFlow<NetworkState> = MutableStateFlow(NetworkState.ConnectedWithInternet)
+    }
+    )
 
 fun mockAssetMessage(uploadStatus: Message.UploadStatus = Message.UploadStatus.UPLOADED) = UIMessage.Regular(
     userAvatarData = UserAvatarData(

app/src/main/kotlin/com/wire/android/util/ui/AssetImageFetcher.kt

@@ -61,7 +61,7 @@ internal class AssetImageFetcher(
                     }
 
                     when (val result = getPublicAsset(data.userAssetId)) {
-                        is PublicAssetResult.Failure -> null
+                        is PublicAssetResult.Failure -> throw AssetImageException(result.isRetryNeeded)
                         is PublicAssetResult.Success -> {
                             drawableResultWrapper.toFetchResult(result.assetPath)
                         }
@@ -70,7 +70,7 @@ internal class AssetImageFetcher(
 
                 is ImageAsset.PrivateAsset -> {
                     when (val result = getPrivateAsset(data.conversationId, data.messageId).await()) {
-                        is MessageAssetResult.Failure -> null
+                        is MessageAssetResult.Failure -> throw AssetImageException(result.isRetryNeeded)
                         is MessageAssetResult.Success -> {
                             drawableResultWrapper.toFetchResult(result.decodedAssetPath)
                         }
@@ -90,6 +90,7 @@ internal class AssetImageFetcher(
         }
     }
 
+
     class Factory(
         private val getPublicAssetUseCase: GetAvatarAssetUseCase,
         private val getPrivateAssetUseCase: GetMessageAssetUseCase,
@@ -116,3 +117,5 @@ data class AssetFetcherParameters(
     val data: ImageAsset,
     val options: Options
 )
+
+data class AssetImageException(val isRetryNeeded: Boolean) : Exception("Load asset image exception")

app/src/main/kotlin/com/wire/android/util/ui/WireSessionImageLoader.kt

@@ -22,37 +22,43 @@ package com.wire.android.util.ui
 
 import android.content.Context
 import android.os.Build.VERSION.SDK_INT
+import androidx.annotation.DrawableRes
 import androidx.compose.runtime.Composable
+import androidx.compose.runtime.LaunchedEffect
 import androidx.compose.runtime.Stable
 import androidx.compose.runtime.getValue
 import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
 import androidx.compose.runtime.setValue
 import androidx.compose.ui.graphics.painter.Painter
 import androidx.compose.ui.platform.LocalContext
+import androidx.compose.ui.res.painterResource
 import coil.Coil
 import coil.ImageLoader
 import coil.compose.AsyncImagePainter
 import coil.compose.rememberAsyncImagePainter
 import coil.decode.GifDecoder
 import coil.decode.ImageDecoderDecoder
-import coil.network.HttpException
 import coil.request.ImageRequest
 import com.wire.android.model.ImageAsset
 import com.wire.kalium.logic.feature.asset.DeleteAssetUseCase
 import com.wire.kalium.logic.feature.asset.GetAvatarAssetUseCase
 import com.wire.kalium.logic.feature.asset.GetMessageAssetUseCase
+import com.wire.kalium.logic.network.NetworkState
+import com.wire.kalium.logic.network.NetworkStateObserver
 
 /**
  * An ImageLoader that is able to load AssetIds supplied by Kalium.
  * As it uses Kalium's [GetAvatarAssetUseCase], a loader created for one session may be unable to load images from another session.
  * It wraps Coil, so it becomes easier to refactor in the future if we ever switch from Coil to something else.
  */
 @Stable
-class WireSessionImageLoader(private val coilImageLoader: ImageLoader) {
+class WireSessionImageLoader(
+    private val coilImageLoader: ImageLoader,
+    private val networkStateObserver: NetworkStateObserver
+) {
     private companion object {
         const val RETRY_INCREMENT_ATTEMPT_PER_STEP = 1
-        const val ASSET_NOT_FOUND_ERROR_CODE = 404
     }
 
     /**
@@ -65,10 +71,10 @@ class WireSessionImageLoader(private val coilImageLoader: ImageLoader) {
     @Composable
     fun paint(
         asset: ImageAsset?,
-        fallbackData: Any? = null
+        fallbackData: Any? = null,
+        @DrawableRes errorDrawable: Int?
     ): Painter {
         var retryHash by remember { mutableStateOf(0) }
-
         val painter = rememberAsyncImagePainter(
             model = ImageRequest.Builder(LocalContext.current)
                 .memoryCacheKey(asset?.uniqueKey)
@@ -79,14 +85,20 @@ class WireSessionImageLoader(private val coilImageLoader: ImageLoader) {
                     memoryCacheKey = null
                 )
                 .build(),
+            error = errorDrawable?.let { painterResource(id = it) },
             imageLoader = coilImageLoader
         )
 
-        if (painter.state is AsyncImagePainter.State.Error) {
-            val errorCode = ((painter.state as AsyncImagePainter.State.Error).result.throwable as? HttpException)?.response?.code
+        LaunchedEffect(key1 = asset) {
+            networkStateObserver.observeNetworkState().collect { state ->
+                if (state == NetworkState.ConnectedWithInternet && painter.state is AsyncImagePainter.State.Error) {
+                    val isRetryNeeded = ((painter.state as AsyncImagePainter.State.Error).result.throwable as? AssetImageException)
+                        ?.isRetryNeeded ?: false
 
-            if (errorCode != ASSET_NOT_FOUND_ERROR_CODE) {
-                retryHash += RETRY_INCREMENT_ATTEMPT_PER_STEP
+                    if (isRetryNeeded) {
+                        retryHash += RETRY_INCREMENT_ATTEMPT_PER_STEP
+                    }
+                }
             }
         }
 
@@ -98,6 +110,7 @@ class WireSessionImageLoader(private val coilImageLoader: ImageLoader) {
         private val getAvatarAsset: GetAvatarAssetUseCase,
         private val deleteAsset: DeleteAssetUseCase,
         private val getPrivateAsset: GetMessageAssetUseCase,
+        private val networkStateObserver: NetworkStateObserver,
     ) {
         private val defaultImageLoader = Coil.imageLoader(context)
         private val resources = context.resources
@@ -120,7 +133,8 @@ class WireSessionImageLoader(private val coilImageLoader: ImageLoader) {
                         } else {
                             add(GifDecoder.Factory())
                         }
-                    }.build()
+                    }.build(),
+                networkStateObserver
             )
     }
 }

app/src/test/kotlin/com/wire/android/ui/home/gallery/MediaGalleryViewModelTest.kt

@@ -274,7 +274,12 @@ class MediaGalleryViewModelTest {
                     any(),
                     any()
                 )
-            } returns CompletableDeferred(MessageAssetResult.Failure(CoreFailure.Unknown(java.lang.RuntimeException())))
+            } returns CompletableDeferred(
+                MessageAssetResult.Failure(
+                    CoreFailure.Unknown(java.lang.RuntimeException()),
+                    isRetryNeeded = true
+                )
+            )
             return this
         }
 

app/src/test/kotlin/com/wire/android/util/ui/AssetImageFetcherTest.kt

@@ -44,6 +44,7 @@ import kotlinx.coroutines.test.runTest
 import okio.Path
 import okio.buffer
 import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.assertThrows
 
 @OptIn(ExperimentalCoroutinesApi::class)
 internal class AssetImageFetcherTest {
@@ -122,7 +123,7 @@ internal class AssetImageFetcherTest {
         val (arrangement, assetImageFetcher) = Arrangement().withErrorResponse(data).arrange()
 
         // When
-        assetImageFetcher.fetch()
+        assertThrows<AssetImageException> { assetImageFetcher.fetch() }
 
         // Then
         coVerify(inverse = true) { arrangement.drawableResultWrapper.toFetchResult(any()) }
@@ -137,7 +138,7 @@ internal class AssetImageFetcherTest {
         val (arrangement, assetImageFetcher) = Arrangement().withErrorResponse(data).arrange()
 
         // When
-        assetImageFetcher.fetch()
+        assertThrows<AssetImageException> { assetImageFetcher.fetch() }
 
         // Then
         coVerify(inverse = true) { arrangement.drawableResultWrapper.toFetchResult(any()) }
@@ -197,15 +198,27 @@ internal class AssetImageFetcherTest {
             return this
         }
 
-        fun withErrorResponse(data: ImageAsset): Arrangement {
+        fun withErrorResponse(data: ImageAsset, isRetryNeeded: Boolean = false): Arrangement {
             imageData = data
-            coEvery { getPublicAsset.invoke((any())) }.returns(PublicAssetResult.Failure(CoreFailure.Unknown(null)))
+            coEvery { getPublicAsset.invoke((any())) }.returns(
+                PublicAssetResult.Failure(
+                    CoreFailure.Unknown(null),
+                    isRetryNeeded
+                )
+            )
             coEvery {
                 getPrivateAsset.invoke(
                     any(),
                     any()
                 )
-            }.returns(CompletableDeferred(MessageAssetResult.Failure(CoreFailure.Unknown(null))))
+            }.returns(
+                CompletableDeferred(
+                    MessageAssetResult.Failure(
+                        CoreFailure.Unknown(null),
+                        isRetryNeeded
+                    )
+                )
+            )
 
             return this
         }

buildSrc/src/main/kotlin/Dependencies.kt

@@ -101,7 +101,7 @@ object Libraries {
         const val paging3 = "3.1.1"
         const val paging3Compose = "1.0.0-alpha18"
         const val splashscreen = "1.0.0"
-        const val coil = "2.3.0"
+        const val coil = "2.4.0"
         const val exif = "1.3.6"
         const val firebaseBOM = "31.4.0"
         const val dataDog = "1.18.1"