Merge pull request #3082 from element-hq/feature/bma/fixClearCacheImage

Fix image rendering after clear cache

appnav/src/main/kotlin/io/element/android/appnav/root/RootNavStateFlowFactory.kt

@@ -22,6 +22,7 @@ import io.element.android.appnav.di.MatrixClientsHolder
 import io.element.android.features.login.api.LoginUserStory
 import io.element.android.features.preferences.api.CacheService
 import io.element.android.libraries.matrix.api.auth.MatrixAuthenticationService
+import io.element.android.libraries.matrix.ui.media.ImageLoaderHolder
 import io.element.android.libraries.sessionstorage.api.LoggedInState
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.combine
@@ -39,6 +40,7 @@ class RootNavStateFlowFactory @Inject constructor(
     private val authenticationService: MatrixAuthenticationService,
     private val cacheService: CacheService,
     private val matrixClientsHolder: MatrixClientsHolder,
+    private val imageLoaderHolder: ImageLoaderHolder,
     private val loginUserStory: LoginUserStory,
 ) {
     private var currentCacheIndex = 0
@@ -69,6 +71,8 @@ class RootNavStateFlowFactory @Inject constructor(
         return cacheService.clearedCacheEventFlow
             .onEach { sessionId ->
                 matrixClientsHolder.remove(sessionId)
+                // Ensure image loader will be recreated with the new MatrixClient
+                imageLoaderHolder.remove(sessionId)
             }
             .toIndexFlow(initialCacheIndex)
             .onEach { cacheIndex ->

changelog.d/3082.bugfix

@@ -0,0 +1 @@
+Fix image rendering after clear cache

libraries/matrixui/build.gradle.kts

@@ -46,8 +46,11 @@ dependencies {
 
     ksp(libs.showkase.processor)
 
+    testImplementation(libs.coroutines.test)
     testImplementation(libs.test.junit)
-    testImplementation(libs.test.truth)
     testImplementation(libs.test.robolectric)
+    testImplementation(libs.test.truth)
     testImplementation(projects.libraries.matrix.test)
+    testImplementation(projects.libraries.sessionStorage.test)
+    testImplementation(projects.tests.testutils)
 }

libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/ImageLoaderFactories.kt

@@ -22,18 +22,24 @@ import coil.ImageLoader
 import coil.ImageLoaderFactory
 import coil.decode.GifDecoder
 import coil.decode.ImageDecoderDecoder
+import com.squareup.anvil.annotations.ContributesBinding
+import io.element.android.libraries.di.AppScope
 import io.element.android.libraries.di.ApplicationContext
 import io.element.android.libraries.matrix.api.MatrixClient
 import okhttp3.OkHttpClient
 import javax.inject.Inject
 import javax.inject.Provider
 
-class LoggedInImageLoaderFactory(
+interface LoggedInImageLoaderFactory {
-    private val context: Context,
+    fun newImageLoader(matrixClient: MatrixClient): ImageLoader
-    private val matrixClient: MatrixClient,
+}
+
+@ContributesBinding(AppScope::class)
+class DefaultLoggedInImageLoaderFactory @Inject constructor(
+    @ApplicationContext private val context: Context,
     private val okHttpClient: Provider<OkHttpClient>,
-) : ImageLoaderFactory {
+) : LoggedInImageLoaderFactory {
-    override fun newImageLoader(): ImageLoader {
+    override fun newImageLoader(matrixClient: MatrixClient): ImageLoader {
         return ImageLoader
             .Builder(context)
             .okHttpClient { okHttpClient.get() }

libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/ImageLoaderHolder.kt

@@ -16,29 +16,25 @@
 
 package io.element.android.libraries.matrix.ui.media
 
-import android.content.Context
 import coil.ImageLoader
 import com.squareup.anvil.annotations.ContributesBinding
 import io.element.android.libraries.di.AppScope
-import io.element.android.libraries.di.ApplicationContext
 import io.element.android.libraries.di.SingleIn
 import io.element.android.libraries.matrix.api.MatrixClient
 import io.element.android.libraries.matrix.api.core.SessionId
 import io.element.android.libraries.sessionstorage.api.observer.SessionListener
 import io.element.android.libraries.sessionstorage.api.observer.SessionObserver
-import okhttp3.OkHttpClient
 import javax.inject.Inject
-import javax.inject.Provider
 
 interface ImageLoaderHolder {
     fun get(client: MatrixClient): ImageLoader
+    fun remove(sessionId: SessionId)
 }
 
 @ContributesBinding(AppScope::class)
 @SingleIn(AppScope::class)
 class DefaultImageLoaderHolder @Inject constructor(
-    @ApplicationContext private val context: Context,
+    private val loggedInImageLoaderFactory: LoggedInImageLoaderFactory,
-    private val okHttpClient: Provider<OkHttpClient>,
     private val sessionObserver: SessionObserver,
 ) : ImageLoaderHolder {
     private val map = mutableMapOf<SessionId, ImageLoader>()
@@ -52,7 +48,7 @@ class DefaultImageLoaderHolder @Inject constructor(
             override suspend fun onSessionCreated(userId: String) = Unit
 
             override suspend fun onSessionDeleted(userId: String) {
-                map.remove(SessionId(userId))
+                remove(SessionId(userId))
             }
         })
     }
@@ -60,12 +56,15 @@ class DefaultImageLoaderHolder @Inject constructor(
     override fun get(client: MatrixClient): ImageLoader {
         return synchronized(map) {
             map.getOrPut(client.sessionId) {
-                LoggedInImageLoaderFactory(
+                loggedInImageLoaderFactory
-                    context = context,
+                    .newImageLoader(client)
-                    matrixClient = client,
-                    okHttpClient = okHttpClient,
-                ).newImageLoader()
             }
         }
     }
+
+    override fun remove(sessionId: SessionId) {
+        synchronized(map) {
+            map.remove(sessionId)
+        }
+    }
 }

libraries/matrixui/src/test/kotlin/io/element/android/libraries/matrix/ui/media/DefaultImageLoaderHolderTest.kt

@@ -0,0 +1,86 @@
+/*
+ * Copyright (c) 2024 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.element.android.libraries.matrix.ui.media
+
+import androidx.test.platform.app.InstrumentationRegistry
+import coil.ImageLoader
+import com.google.common.truth.Truth.assertThat
+import io.element.android.libraries.matrix.api.MatrixClient
+import io.element.android.libraries.matrix.test.A_SESSION_ID
+import io.element.android.libraries.matrix.test.FakeMatrixClient
+import io.element.android.libraries.sessionstorage.test.observer.FakeSessionObserver
+import io.element.android.libraries.sessionstorage.test.observer.NoOpSessionObserver
+import io.element.android.tests.testutils.lambda.lambdaRecorder
+import io.element.android.tests.testutils.lambda.value
+import kotlinx.coroutines.test.runTest
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+
+@RunWith(RobolectricTestRunner::class)
+class DefaultImageLoaderHolderTest {
+    @Test
+    fun `get - returns the same ImageLoader for the same client`() {
+        val context = InstrumentationRegistry.getInstrumentation().context
+        val lambda = lambdaRecorder<MatrixClient, ImageLoader> { ImageLoader.Builder(context).build() }
+
+        val holder = DefaultImageLoaderHolder(
+            loggedInImageLoaderFactory = FakeLoggedInImageLoaderFactory(lambda),
+            sessionObserver = NoOpSessionObserver()
+        )
+        val client = FakeMatrixClient()
+        val imageLoader1 = holder.get(client)
+        val imageLoader2 = holder.get(client)
+        assert(imageLoader1 === imageLoader2)
+        lambda.assertions()
+            .isCalledOnce()
+            .with(value(client))
+    }
+
+    @Test
+    fun `when session is deleted, the image loader is deleted`() = runTest {
+        val context = InstrumentationRegistry.getInstrumentation().context
+        val lambda =
+            lambdaRecorder<MatrixClient, ImageLoader> { ImageLoader.Builder(context).build() }
+        val sessionObserver = FakeSessionObserver()
+        val holder = DefaultImageLoaderHolder(
+            loggedInImageLoaderFactory = FakeLoggedInImageLoaderFactory(lambda),
+            sessionObserver = sessionObserver
+        )
+        assertThat(sessionObserver.listeners.size).isEqualTo(1)
+        val client = FakeMatrixClient()
+        holder.get(client)
+        sessionObserver.onSessionDeleted(client.sessionId.value)
+        holder.get(client)
+        lambda.assertions()
+            .isCalledExactly(2)
+    }
+
+    @Test
+    fun `when session is created, nothing happen`() = runTest {
+        val context = InstrumentationRegistry.getInstrumentation().context
+        val lambda =
+            lambdaRecorder<MatrixClient, ImageLoader> { ImageLoader.Builder(context).build() }
+        val sessionObserver = FakeSessionObserver()
+        DefaultImageLoaderHolder(
+            loggedInImageLoaderFactory = FakeLoggedInImageLoaderFactory(lambda),
+            sessionObserver = sessionObserver
+        )
+        assertThat(sessionObserver.listeners.size).isEqualTo(1)
+        sessionObserver.onSessionCreated(A_SESSION_ID.value)
+    }
+}

libraries/matrixui/src/test/kotlin/io/element/android/libraries/matrix/ui/media/FakeLoggedInImageLoaderFactory.kt

@@ -0,0 +1,28 @@
+/*
+ * Copyright (c) 2024 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.element.android.libraries.matrix.ui.media
+
+import coil.ImageLoader
+import io.element.android.libraries.matrix.api.MatrixClient
+
+class FakeLoggedInImageLoaderFactory(
+    private val newImageLoaderLambda: (MatrixClient) -> ImageLoader
+) : LoggedInImageLoaderFactory {
+    override fun newImageLoader(matrixClient: MatrixClient): ImageLoader {
+        return newImageLoaderLambda(matrixClient)
+    }
+}

libraries/matrixui/src/test/kotlin/io/element/android/libraries/matrix/ui/messages/ToHtmlDocumentTest.kt

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-package io.element.android.libraries.matrixui.messages
+package io.element.android.libraries.matrix.ui.messages
 
 import android.net.Uri
 import com.google.common.truth.Truth.assertThat
@@ -24,7 +24,6 @@ import io.element.android.libraries.matrix.api.permalink.PermalinkParser
 import io.element.android.libraries.matrix.api.timeline.item.event.FormattedBody
 import io.element.android.libraries.matrix.api.timeline.item.event.MessageFormat
 import io.element.android.libraries.matrix.test.permalink.FakePermalinkParser
-import io.element.android.libraries.matrix.ui.messages.toHtmlDocument
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.robolectric.RobolectricTestRunner

libraries/matrixui/src/test/kotlin/io/element/android/libraries/matrix/ui/messages/ToPlainTextTest.kt

@@ -14,14 +14,13 @@
  * limitations under the License.
  */
 
-package io.element.android.libraries.matrixui.messages
+package io.element.android.libraries.matrix.ui.messages
 
 import com.google.common.truth.Truth.assertThat
 import io.element.android.libraries.matrix.api.timeline.item.event.FormattedBody
 import io.element.android.libraries.matrix.api.timeline.item.event.MessageFormat
 import io.element.android.libraries.matrix.api.timeline.item.event.TextMessageType
 import io.element.android.libraries.matrix.test.permalink.FakePermalinkParser
-import io.element.android.libraries.matrix.ui.messages.toPlainText
 import org.jsoup.Jsoup
 import org.junit.Test
 import org.junit.runner.RunWith

libraries/push/test/src/main/kotlin/io/element/android/libraries/push/test/notifications/FakeImageLoaderHolder.kt

@@ -18,6 +18,7 @@ package io.element.android.libraries.push.test.notifications
 
 import coil.ImageLoader
 import io.element.android.libraries.matrix.api.MatrixClient
+import io.element.android.libraries.matrix.api.core.SessionId
 import io.element.android.libraries.matrix.ui.media.ImageLoaderHolder
 
 class FakeImageLoaderHolder : ImageLoaderHolder {
@@ -25,4 +26,8 @@ class FakeImageLoaderHolder : ImageLoaderHolder {
     override fun get(client: MatrixClient): ImageLoader {
         return fakeImageLoader.getImageLoader()
     }
+
+    override fun remove(sessionId: SessionId) {
+        // No-op
+    }
 }

libraries/session-storage/test/src/main/kotlin/io/element/android/libraries/sessionstorage/test/observer/FakeSessionObserver.kt

@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2024 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.element.android.libraries.sessionstorage.test.observer
+
+import io.element.android.libraries.sessionstorage.api.observer.SessionListener
+import io.element.android.libraries.sessionstorage.api.observer.SessionObserver
+
+class FakeSessionObserver : SessionObserver {
+    private val _listeners = mutableListOf<SessionListener>()
+
+    val listeners: List<SessionListener>
+        get() = _listeners
+
+    override fun addListener(listener: SessionListener) {
+        _listeners.add(listener)
+    }
+
+    override fun removeListener(listener: SessionListener) {
+        _listeners.remove(listener)
+    }
+
+    suspend fun onSessionCreated(userId: String) {
+        listeners.forEach { it.onSessionCreated(userId) }
+    }
+
+    suspend fun onSessionDeleted(userId: String) {
+        listeners.forEach { it.onSessionDeleted(userId) }
+    }
+}