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

Fix image rendering after clear cache
2024-06-24 16:20:11 +02:00
parent 2d112163d2 8dc0a10a6c
commit 8a233061cb
11 changed files with 195 additions and 22 deletions
--- a/appnav/src/main/kotlin/io/element/android/appnav/root/RootNavStateFlowFactory.kt
+++ b/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 ->
--- a/changelog.d/3082.bugfix
+++ b/changelog.d/3082.bugfix
@@ -0,0 +1 @@
+Fix image rendering after clear cache
--- a/libraries/matrixui/build.gradle.kts
+++ b/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)
 }
--- a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/ImageLoaderFactories.kt
+++ b/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(
-    private val context: Context,
-    private val matrixClient: MatrixClient,
+interface LoggedInImageLoaderFactory {
+    fun newImageLoader(matrixClient: MatrixClient): ImageLoader
+}
+
+@ContributesBinding(AppScope::class)
+class DefaultLoggedInImageLoaderFactory @Inject constructor(
+    @ApplicationContext private val context: Context,
    private val okHttpClient: Provider<OkHttpClient>,
-) : ImageLoaderFactory {
-    override fun newImageLoader(): ImageLoader {
+) : LoggedInImageLoaderFactory {
+    override fun newImageLoader(matrixClient: MatrixClient): ImageLoader {
        return ImageLoader
            .Builder(context)
            .okHttpClient { okHttpClient.get() }
--- a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/ImageLoaderHolder.kt
+++ b/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 okHttpClient: Provider<OkHttpClient>,
+    private val loggedInImageLoaderFactory: LoggedInImageLoaderFactory,
    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(
-                    context = context,
-                    matrixClient = client,
-                    okHttpClient = okHttpClient,
-                ).newImageLoader()
+                loggedInImageLoaderFactory
+                    .newImageLoader(client)
            }
        }
    }
+
+    override fun remove(sessionId: SessionId) {
+        synchronized(map) {
+            map.remove(sessionId)
+        }
+    }
 }
--- a/libraries/matrixui/src/test/kotlin/io/element/android/libraries/matrix/ui/media/DefaultImageLoaderHolderTest.kt
+++ b/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)
+    }
+}
--- a/libraries/matrixui/src/test/kotlin/io/element/android/libraries/matrix/ui/media/FakeLoggedInImageLoaderFactory.kt
+++ b/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)
+    }
+}
--- a/libraries/matrixui/src/test/kotlin/io/element/android/libraries/matrix/ui/messages/ToHtmlDocumentTest.kt
+++ b/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
--- a/libraries/matrixui/src/test/kotlin/io/element/android/libraries/matrix/ui/messages/ToPlainTextTest.kt
+++ b/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
--- a/libraries/push/test/src/main/kotlin/io/element/android/libraries/push/test/notifications/FakeImageLoaderHolder.kt
+++ b/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
+    }
 }
--- a/libraries/session-storage/test/src/main/kotlin/io/element/android/libraries/sessionstorage/test/observer/FakeSessionObserver.kt
+++ b/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) }
+    }
+}