Simplify AvatarData and avoid carrying ByteArray

2023-02-01 15:57:00 +01:00 · 2023-02-01 15:57:00 +01:00 · b8860a6658
commit b8860a6658
parent d43d433d38
17 changed files with 99 additions and 115 deletions
--- a/features/messages/src/main/kotlin/io/element/android/features/messages/MessagesPresenter.kt
+++ b/features/messages/src/main/kotlin/io/element/android/features/messages/MessagesPresenter.kt
@ -24,9 +24,6 @@ import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
 import androidx.compose.runtime.rememberCoroutineScope
 import androidx.compose.runtime.saveable.rememberSaveable
-import io.element.android.libraries.architecture.Presenter
-import io.element.android.libraries.designsystem.components.avatar.AvatarData
-import io.element.android.libraries.designsystem.components.avatar.AvatarSize
 import io.element.android.features.messages.actionlist.ActionListPresenter
 import io.element.android.features.messages.actionlist.model.TimelineItemAction
 import io.element.android.features.messages.textcomposer.MessageComposerEvents
@ -36,9 +33,10 @@ import io.element.android.features.messages.timeline.TimelineEvents
 import io.element.android.features.messages.timeline.TimelinePresenter
 import io.element.android.features.messages.timeline.model.TimelineItem
 import io.element.android.features.messages.timeline.model.content.TimelineItemTextBasedContent
-import io.element.android.libraries.matrix.MatrixClient
+import io.element.android.libraries.architecture.Presenter
+import io.element.android.libraries.designsystem.components.avatar.AvatarData
+import io.element.android.libraries.designsystem.components.avatar.AvatarSize
 import io.element.android.libraries.matrix.room.MatrixRoom
-import io.element.android.libraries.matrix.ui.MatrixItemHelper
 import io.element.android.libraries.textcomposer.MessageComposerMode
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.launch
@ -46,15 +44,12 @@ import timber.log.Timber
 import javax.inject.Inject

 class MessagesPresenter @Inject constructor(
-    private val matrixClient: MatrixClient,
    private val room: MatrixRoom,
    private val composerPresenter: MessageComposerPresenter,
    private val timelinePresenter: TimelinePresenter,
    private val actionListPresenter: ActionListPresenter,
 ) : Presenter<MessagesState> {

-    private val matrixItemHelper = MatrixItemHelper(matrixClient)
-
    @Composable
    override fun present(): MessagesState {
        val localCoroutineScope = rememberCoroutineScope()
@ -71,8 +66,9 @@ class MessagesPresenter @Inject constructor(
        }
        LaunchedEffect(syncUpdateFlow) {
            roomAvatar.value =
-                matrixItemHelper.loadAvatarData(
-                    room = room,
+                AvatarData(
+                    name = room.bestName,
+                    url = room.avatarUrl,
                    size = AvatarSize.SMALL
                )
            roomName.value = room.name
--- a/features/messages/src/main/kotlin/io/element/android/features/messages/timeline/TimelineItemsFactory.kt
+++ b/features/messages/src/main/kotlin/io/element/android/features/messages/timeline/TimelineItemsFactory.kt
@ -17,7 +17,6 @@
 package io.element.android.features.messages.timeline

 import androidx.recyclerview.widget.DiffUtil
-import io.element.android.libraries.designsystem.components.avatar.AvatarSize
 import io.element.android.features.messages.timeline.diff.CacheInvalidator
 import io.element.android.features.messages.timeline.diff.MatrixTimelineItemsDiffCallback
 import io.element.android.features.messages.timeline.model.AggregatedReaction
@ -33,6 +32,8 @@ import io.element.android.features.messages.timeline.model.content.TimelineItemR
 import io.element.android.features.messages.timeline.model.content.TimelineItemTextContent
 import io.element.android.features.messages.timeline.model.content.TimelineItemUnknownContent
 import io.element.android.features.messages.timeline.util.invalidateLast
+import io.element.android.libraries.designsystem.components.avatar.AvatarData
+import io.element.android.libraries.designsystem.components.avatar.AvatarSize
 import io.element.android.libraries.matrix.core.EventId
 import io.element.android.libraries.matrix.media.MediaResolver
 import io.element.android.libraries.matrix.room.MatrixRoom
@ -154,12 +155,11 @@ class TimelineItemsFactory @Inject constructor(
            computeGroupPosition(currentTimelineItem, timelineItems, index)
        val senderDisplayName = room.userDisplayName(currentSender).getOrNull()
        val senderAvatarUrl = room.userAvatarUrl(currentSender).getOrNull()
-        val senderAvatarData =
-            matrixItemHelper.loadAvatarData(
-                name = senderDisplayName ?: currentSender,
-                url = senderAvatarUrl,
-                size = AvatarSize.SMALL
-            )
+        val senderAvatarData = AvatarData(
+            name = senderDisplayName ?: currentSender,
+            url = senderAvatarUrl,
+            size = AvatarSize.SMALL
+        )
        return TimelineItem.MessageEvent(
            id = EventId(currentTimelineItem.uniqueId),
            senderId = currentSender,
--- a/features/roomlist/src/main/kotlin/io/element/android/features/roomlist/RoomListPresenter.kt
+++ b/features/roomlist/src/main/kotlin/io/element/android/features/roomlist/RoomListPresenter.kt
@ -25,16 +25,15 @@ import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
 import androidx.compose.runtime.saveable.rememberSaveable
 import androidx.compose.runtime.setValue
-import io.element.android.libraries.architecture.Presenter
-import io.element.android.libraries.core.coroutine.parallelMap
-import io.element.android.libraries.designsystem.components.avatar.AvatarData
-import io.element.android.libraries.designsystem.components.avatar.AvatarSize
 import io.element.android.features.roomlist.model.RoomListEvents
 import io.element.android.features.roomlist.model.RoomListRoomSummary
 import io.element.android.features.roomlist.model.RoomListRoomSummaryPlaceholders
 import io.element.android.features.roomlist.model.RoomListState
+import io.element.android.libraries.architecture.Presenter
+import io.element.android.libraries.core.coroutine.parallelMap
+import io.element.android.libraries.designsystem.components.avatar.AvatarData
+import io.element.android.libraries.designsystem.components.avatar.AvatarSize
 import io.element.android.libraries.matrix.MatrixClient
-import io.element.android.libraries.matrix.media.MediaResolver
 import io.element.android.libraries.matrix.room.RoomSummary
 import io.element.android.libraries.matrix.ui.model.MatrixUser
 import kotlinx.collections.immutable.ImmutableList
@ -42,6 +41,7 @@ import kotlinx.collections.immutable.persistentListOf
 import kotlinx.collections.immutable.toImmutableList
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.launch
+import timber.log.Timber
 import javax.inject.Inject

 private const val extendedRangeSize = 40
@ -61,7 +61,9 @@ class RoomListPresenter @Inject constructor(
        val roomSummaries by client
            .roomSummaryDataSource()
            .roomSummaries()
-            .collectAsState(initial = null)
+            .collectAsState()
+
+        Timber.v("RoomSummaries size = ${roomSummaries.size}")

        val filteredRoomSummaries: MutableState<ImmutableList<RoomListRoomSummary>> = remember {
            mutableStateOf(persistentListOf())
@ -105,15 +107,14 @@ class RoomListPresenter @Inject constructor(
        val userAvatarUrl = client.loadUserAvatarURLString().getOrNull()
        val userDisplayName = client.loadUserDisplayName().getOrNull()
        val avatarData =
-            loadAvatarData(
-                userDisplayName ?: client.userId().value,
-                userAvatarUrl,
-                AvatarSize.SMALL
+            AvatarData(
+                name = userDisplayName ?: client.userId().value,
+                url = userAvatarUrl,
+                size = AvatarSize.SMALL
            )
        matrixUser.value = MatrixUser(
            id = client.userId(),
            username = userDisplayName ?: client.userId().value,
-            avatarUrl = userAvatarUrl,
            avatarData = avatarData,
        )
    }
@ -135,9 +136,9 @@ class RoomListPresenter @Inject constructor(
            when (roomSummary) {
                is RoomSummary.Empty -> RoomListRoomSummaryPlaceholders.create(roomSummary.identifier)
                is RoomSummary.Filled -> {
-                    val avatarData = loadAvatarData(
-                        roomSummary.details.name,
-                        roomSummary.details.avatarURLString
+                    val avatarData = AvatarData(
+                        name = roomSummary.details.name,
+                        url = roomSummary.details.avatarURLString
                    )
                    RoomListRoomSummary(
                        id = roomSummary.identifier(),
@ -151,14 +152,4 @@ class RoomListPresenter @Inject constructor(
            }
        }
    }
-
-    private suspend fun loadAvatarData(
-        name: String,
-        url: String?,
-        size: AvatarSize = AvatarSize.MEDIUM
-    ): AvatarData {
-        val model = client.mediaResolver()
-            .resolve(url, kind = MediaResolver.Kind.Thumbnail(size.value))
-        return AvatarData(name, model, size)
-    }
 }
--- a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/avatar/Avatar.kt
+++ b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/avatar/Avatar.kt
@ -41,7 +41,7 @@ fun Avatar(avatarData: AvatarData, modifier: Modifier = Modifier) {
    val commonModifier = modifier
        .size(avatarData.size.dp)
        .clip(CircleShape)
-    if (avatarData.model == null) {
+    if (avatarData.url == null) {
        InitialsAvatar(
            avatarData = avatarData,
            modifier = commonModifier,
@ -60,7 +60,7 @@ private fun ImageAvatar(
    modifier: Modifier = Modifier,
 ) {
    AsyncImage(
-        model = avatarData.model,
+        model = avatarData,
        onError = {
            Timber.e("TAG", "Error $it\n${it.result}", it.result.throwable)
        },
--- a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/avatar/AvatarData.kt
+++ b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/avatar/AvatarData.kt
@ -21,29 +21,6 @@ import androidx.compose.runtime.Immutable
@Immutable
 data class AvatarData(
    val name: String = "",
-    val model: ByteArray? = null,
+    val url: String? = null,
    val size: AvatarSize = AvatarSize.MEDIUM
-) {
-    override fun equals(other: Any?): Boolean {
-        if (this === other) return true
-        if (javaClass != other?.javaClass) return false
-
-        other as AvatarData
-
-        if (name != other.name) return false
-        if (model != null) {
-            if (other.model == null) return false
-            if (!model.contentEquals(other.model)) return false
-        } else if (other.model != null) return false
-        if (size != other.size) return false
-
-        return true
-    }
-
-    override fun hashCode(): Int {
-        var result = name.hashCode()
-        result = 31 * result + (model?.contentHashCode() ?: 0)
-        result = 31 * result + size.value
-        return result
-    }
-}
+)
--- a/libraries/matrix/src/main/kotlin/io/element/android/libraries/matrix/media/MediaResolver.kt
+++ b/libraries/matrix/src/main/kotlin/io/element/android/libraries/matrix/media/MediaResolver.kt
@ -29,7 +29,7 @@ interface MediaResolver {
    }

    data class Meta(
-        val source: MediaSource,
+        val source: MediaSource?,
        val kind: Kind
    )

--- a/libraries/matrix/src/main/kotlin/io/element/android/libraries/matrix/media/RustMediaResolver.kt
+++ b/libraries/matrix/src/main/kotlin/io/element/android/libraries/matrix/media/RustMediaResolver.kt
@ -28,6 +28,7 @@ internal class RustMediaResolver(private val client: MatrixClient) : MediaResolv
    }

    override suspend fun resolve(meta: MediaResolver.Meta): ByteArray? {
+        if (meta.source == null) return null
        return when (meta.kind) {
            is MediaResolver.Kind.Content -> client.loadMediaContentForSource(meta.source)
            is MediaResolver.Kind.Thumbnail -> client.loadMediaThumbnailForSource(
--- a/libraries/matrix/src/main/kotlin/io/element/android/libraries/matrix/room/RoomSummaryDataSource.kt
+++ b/libraries/matrix/src/main/kotlin/io/element/android/libraries/matrix/room/RoomSummaryDataSource.kt
@ -29,6 +29,7 @@ import kotlinx.coroutines.cancel
 import kotlinx.coroutines.cancelChildren
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.flow.sample
@ -43,7 +44,7 @@ import org.matrix.rustcomponents.sdk.UpdateSummary
 import timber.log.Timber

 interface RoomSummaryDataSource {
-    fun roomSummaries(): Flow<List<RoomSummary>>
+    fun roomSummaries(): StateFlow<List<RoomSummary>>
    fun setSlidingSyncRange(range: IntRange)
 }

@ -98,9 +99,9 @@ internal class RustRoomSummaryDataSource(
        coroutineScope.cancel()
    }

-    @OptIn(FlowPreview::class)
-    override fun roomSummaries(): Flow<List<RoomSummary>> {
-        return roomSummaries.sample(50)
+    //@OptIn(FlowPreview::class)
+    override fun roomSummaries(): StateFlow<List<RoomSummary>> {
+        return roomSummaries
    }

    override fun setSlidingSyncRange(range: IntRange) {
--- a/libraries/matrixtest/src/main/kotlin/io/element/android/libraries/matrixtest/room/InMemoryRoomSummaryDataSource.kt
+++ b/libraries/matrixtest/src/main/kotlin/io/element/android/libraries/matrixtest/room/InMemoryRoomSummaryDataSource.kt
@ -18,13 +18,13 @@ package io.element.android.libraries.matrixtest.room

 import io.element.android.libraries.matrix.room.RoomSummary
 import io.element.android.libraries.matrix.room.RoomSummaryDataSource
-import kotlinx.coroutines.flow.Flow
-import kotlinx.coroutines.flow.emptyFlow
+import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.flow.StateFlow

 class InMemoryRoomSummaryDataSource : RoomSummaryDataSource {

-    override fun roomSummaries(): Flow<List<RoomSummary>> {
-        return emptyFlow()
+    override fun roomSummaries(): StateFlow<List<RoomSummary>> {
+        return MutableStateFlow(emptyList())
    }

    override fun setSlidingSyncRange(range: IntRange) = Unit
--- a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/MatrixItemHelper.kt
+++ b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/MatrixItemHelper.kt
@ -19,9 +19,6 @@ package io.element.android.libraries.matrix.ui
 import io.element.android.libraries.designsystem.components.avatar.AvatarData
 import io.element.android.libraries.designsystem.components.avatar.AvatarSize
 import io.element.android.libraries.matrix.MatrixClient
-import io.element.android.libraries.matrix.media.MediaResolver
-import io.element.android.libraries.matrix.room.MatrixRoom
-import io.element.android.libraries.matrix.room.RoomSummary
 import io.element.android.libraries.matrix.ui.model.MatrixUser
 import kotlinx.coroutines.FlowPreview
 import kotlinx.coroutines.flow.Flow
@ -40,7 +37,7 @@ class MatrixItemHelper @Inject constructor(
            val userAvatarUrl = client.loadUserAvatarURLString().getOrNull()
            val userDisplayName = client.loadUserDisplayName().getOrNull()
            val avatarData =
-                loadAvatarData(
+                AvatarData(
                    userDisplayName ?: client.userId().value,
                    userAvatarUrl,
                    avatarSize
@ -48,35 +45,8 @@ class MatrixItemHelper @Inject constructor(
            MatrixUser(
                id = client.userId(),
                username = userDisplayName,
-                avatarUrl = userAvatarUrl,
                avatarData = avatarData,
            )
        }.asFlow()
    }
-
-    suspend fun loadAvatarData(room: MatrixRoom, size: AvatarSize): AvatarData {
-        return loadAvatarData(
-            name = room.bestName,
-            url = room.avatarUrl,
-            size = size
-        )
-    }
-
-    suspend fun loadAvatarData(roomSummary: RoomSummary.Filled, size: AvatarSize): AvatarData {
-        return loadAvatarData(
-            name = roomSummary.details.name,
-            url = roomSummary.details.avatarURLString,
-            size = size
-        )
-    }
-
-    suspend fun loadAvatarData(
-        name: String,
-        url: String?,
-        size: AvatarSize
-    ): AvatarData {
-        val model = client.mediaResolver()
-            .resolve(url, kind = MediaResolver.Kind.Thumbnail(size.value))
-        return AvatarData(name, model, size)
-    }
 }
--- a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/components/MatrixUserHeader.kt
+++ b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/components/MatrixUserHeader.kt
@ -87,7 +87,6 @@ fun MatrixUserHeaderPreview() {
        MatrixUser(
            id = UserId("@alice:server.org"),
            username = "Alice",
-            avatarUrl = null,
            avatarData = AvatarData("Alice")
        )
    )
@ -100,7 +99,6 @@ fun MatrixUserHeaderNoUsernamePreview() {
        MatrixUser(
            id = UserId("@alice:server.org"),
            username = null,
-            avatarUrl = null,
            avatarData = AvatarData("Alice")
        )
    )
--- a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/components/MatrixUserRow.kt
+++ b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/components/MatrixUserRow.kt
@ -91,7 +91,6 @@ fun MatrixUserRowPreview() {
        MatrixUser(
            id = UserId("@alice:server.org"),
            username = "Alice",
-            avatarUrl = null,
            avatarData = AvatarData("Alice")
        )
    )
--- a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/AvatarDataExt.kt
+++ b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/AvatarDataExt.kt
@ -0,0 +1,26 @@
+/*
+ * Copyright (c) 2023 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 io.element.android.libraries.designsystem.components.avatar.AvatarData
+import io.element.android.libraries.matrix.media.MediaResolver
+import org.matrix.rustcomponents.sdk.mediaSourceFromUrl
+
+fun AvatarData.toMetadata(): MediaResolver.Meta {
+    val mediaSource = url?.let { mediaSourceFromUrl(it) }
+    return MediaResolver.Meta(source = mediaSource, kind = MediaResolver.Kind.Thumbnail(size.value))
+}
--- 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
@ -31,8 +31,10 @@ class LoggedInImageLoaderFactory @Inject constructor(
        return ImageLoader
            .Builder(context)
            .components {
+                add(AvatarKeyer())
                add(MediaKeyer())
-                add(MediaFetcher.Factory(matrixClient))
+                add(MediaFetcher.AvatarFactory(matrixClient))
+                add(MediaFetcher.MetaFactory(matrixClient))
            }
            .build()
    }
--- a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/MediaFetchers.kt
+++ b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/MediaFetchers.kt
@ -20,6 +20,7 @@ import coil.ImageLoader
 import coil.fetch.FetchResult
 import coil.fetch.Fetcher
 import coil.request.Options
+import io.element.android.libraries.designsystem.components.avatar.AvatarData
 import io.element.android.libraries.matrix.MatrixClient
 import io.element.android.libraries.matrix.media.MediaResolver
 import java.nio.ByteBuffer
@ -37,7 +38,7 @@ internal class MediaFetcher(
        return imageLoader.components.newFetcher(byteBuffer, options, imageLoader)?.first?.fetch()
    }

-    class Factory(private val client: MatrixClient) :
+    class MetaFactory(private val client: MatrixClient) :
        Fetcher.Factory<MediaResolver.Meta> {
        override fun create(
            data: MediaResolver.Meta,
@ -52,4 +53,20 @@ internal class MediaFetcher(
            )
        }
    }
+
+    class AvatarFactory(private val client: MatrixClient) :
+        Fetcher.Factory<AvatarData> {
+        override fun create(
+            data: AvatarData,
+            options: Options,
+            imageLoader: ImageLoader
+        ): Fetcher {
+            return MediaFetcher(
+                mediaResolver = client.mediaResolver(),
+                meta = data.toMetadata(),
+                options = options,
+                imageLoader = imageLoader
+            )
+        }
+    }
 }
--- a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/MediaKeyer.kt
+++ b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/MediaKeyer.kt
@ -18,10 +18,17 @@ package io.element.android.libraries.matrix.ui.media

 import coil.key.Keyer
 import coil.request.Options
+import io.element.android.libraries.designsystem.components.avatar.AvatarData
 import io.element.android.libraries.matrix.media.MediaResolver

+internal class AvatarKeyer : Keyer<AvatarData> {
+    override fun key(data: AvatarData, options: Options): String? {
+        return MediaKeyer().key(data.toMetadata(), options)
+    }
+}
+
 internal class MediaKeyer : Keyer<MediaResolver.Meta> {
    override fun key(data: MediaResolver.Meta, options: Options): String? {
-        return "${data.source.url()}_${data.kind}"
+        return "${data.source?.url()}_${data.kind}"
    }
 }
--- a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/model/MatrixUser.kt
+++ b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/model/MatrixUser.kt
@ -24,7 +24,6 @@ import io.element.android.libraries.matrix.core.UserId
 data class MatrixUser(
    val id: UserId,
    val username: String? = null,
-    val avatarUrl: String? = null,
    val avatarData: AvatarData = AvatarData(),
 )