Location avatar

changelog.d/8749.bugfix

@@ -0,0 +1 @@
+Fix issues about location Event avatar rendering.

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/SessionExtensions.kt

@@ -19,6 +19,7 @@ package org.matrix.android.sdk.api.session
 import org.matrix.android.sdk.api.session.room.Room
 import org.matrix.android.sdk.api.session.room.model.RoomSummary
 import org.matrix.android.sdk.api.session.user.model.User
+import timber.log.Timber
 
 /**
  * Get a room using the RoomService of a Session.
@@ -41,4 +42,5 @@ fun Session.getUser(userId: String): User? = userService().getUser(userId)
 /**
  * Similar to [getUser], but fallback to a User without details if the User is not known by the SDK, or if Session is null.
  */
-fun Session?.getUserOrDefault(userId: String): User = this?.userService()?.getUser(userId) ?: User(userId)
+fun Session?.getUserOrDefault(userId: String): User = this?.userService()?.getUser(userId)
+        ?: User(userId).also { Timber.w("User $userId not found in local cache, fallback to default") }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/model/livelocation/LiveLocationShareAggregatedSummary.kt

@@ -22,6 +22,7 @@ import org.matrix.android.sdk.api.session.room.model.message.MessageBeaconLocati
  * Aggregation info concerning a live location share.
  */
 data class LiveLocationShareAggregatedSummary(
+        val roomId: String?,
         val userId: String?,
         /**
          * Indicate whether the live is currently running.

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/mapper/LiveLocationShareAggregatedSummaryMapper.kt

@@ -28,6 +28,7 @@ internal class LiveLocationShareAggregatedSummaryMapper @Inject constructor() :
 
     override fun map(entity: LiveLocationShareAggregatedSummaryEntity): LiveLocationShareAggregatedSummary {
         return LiveLocationShareAggregatedSummary(
+                roomId = entity.roomId,
                 userId = entity.userId,
                 isActive = entity.isActive,
                 endOfLiveTimestampMillis = entity.endOfLiveTimestampMillis,

matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/database/mapper/LiveLocationShareAggregatedSummaryMapperTest.kt

@@ -24,6 +24,7 @@ import org.matrix.android.sdk.api.session.room.model.message.LocationInfo
 import org.matrix.android.sdk.api.session.room.model.message.MessageBeaconLocationDataContent
 import org.matrix.android.sdk.internal.database.model.livelocation.LiveLocationShareAggregatedSummaryEntity
 
+private const val ANY_ROOM_ID = "a-room-id"
 private const val ANY_USER_ID = "a-user-id"
 private const val ANY_ACTIVE_STATE = true
 private const val ANY_TIMEOUT = 123L
@@ -40,6 +41,7 @@ class LiveLocationShareAggregatedSummaryMapperTest {
         val summary = mapper.map(entity)
 
         summary shouldBeEqualTo LiveLocationShareAggregatedSummary(
+                roomId = ANY_ROOM_ID,
                 userId = ANY_USER_ID,
                 isActive = ANY_ACTIVE_STATE,
                 endOfLiveTimestampMillis = ANY_TIMEOUT,
@@ -48,6 +50,7 @@ class LiveLocationShareAggregatedSummaryMapperTest {
     }
 
     private fun anEntity(content: MessageBeaconLocationDataContent) = LiveLocationShareAggregatedSummaryEntity(
+            roomId = ANY_ROOM_ID,
             userId = ANY_USER_ID,
             isActive = ANY_ACTIVE_STATE,
             endOfLiveTimestampMillis = ANY_TIMEOUT,

matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/room/location/DefaultLocationSharingServiceTest.kt

@@ -229,6 +229,7 @@ internal class DefaultLocationSharingServiceTest {
     fun `livedata of live summaries is correctly computed`() {
         val entity = LiveLocationShareAggregatedSummaryEntity()
         val summary = LiveLocationShareAggregatedSummary(
+                roomId = A_ROOM_ID,
                 userId = "",
                 isActive = true,
                 endOfLiveTimestampMillis = 123,
@@ -255,6 +256,7 @@ internal class DefaultLocationSharingServiceTest {
     fun `given an event id when getting livedata on corresponding live summary then it is correctly computed`() {
         val entity = LiveLocationShareAggregatedSummaryEntity()
         val summary = LiveLocationShareAggregatedSummary(
+                roomId = A_ROOM_ID,
                 userId = "",
                 isActive = true,
                 endOfLiveTimestampMillis = 123,

vector/src/main/java/im/vector/app/core/epoxy/bottomsheet/BottomSheetMessagePreviewItem.kt

@@ -103,10 +103,10 @@ abstract class BottomSheetMessagePreviewItem : VectorEpoxyModel<BottomSheetMessa
                     .apply(RequestOptions.centerCropTransform())
                     .into(holder.staticMapImageView)
 
-            safeLocationUiData.locationPinProvider.create(safeLocationUiData.locationOwnerId) { pinDrawable ->
-                GlideApp.with(holder.staticMapPinImageView)
-                        .load(pinDrawable)
-                        .into(holder.staticMapPinImageView)
+            val pinMatrixItem = matrixItem.takeIf { safeLocationUiData.locationOwnerId != null }
+            safeLocationUiData.locationPinProvider.create(pinMatrixItem) { pinDrawable ->
+                // we are not using Glide since it does not display it correctly when there is no user photo
+                holder.staticMapPinImageView.setImageDrawable(pinDrawable)
             }
         }
     }

vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsEpoxyController.kt

@@ -238,7 +238,7 @@ class MessageActionsEpoxyController @Inject constructor(
         val locationUrl = locationContent.toLocationData()
                 ?.let { urlMapProvider.buildStaticMapUrl(it, INITIAL_MAP_ZOOM_IN_TIMELINE, 1200, 800) }
                 ?: return null
-        val locationOwnerId = if (locationContent.isSelfLocation()) state.informationData.matrixItem.id else null
+        val locationOwnerId = if (locationContent.isSelfLocation()) state.informationData.senderId else null
 
         return LocationUiData(
                 locationUrl = locationUrl,

vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/LiveLocationShareMessageItemFactory.kt

@@ -114,7 +114,7 @@ class LiveLocationShareMessageItemFactory @Inject constructor(
                 .locationUrl(locationUrl)
                 .mapWidth(width)
                 .mapHeight(height)
-                .locationUserId(attributes.informationData.senderId)
+                .pinMatrixItem(attributes.informationData.matrixItem)
                 .locationPinProvider(locationPinProvider)
                 .highlighted(highlight)
                 .leftGuideline(avatarSizeProvider.leftGuideline)

vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/MessageItemFactory.kt

@@ -233,14 +233,14 @@ class MessageItemFactory @Inject constructor(
             urlMapProvider.buildStaticMapUrl(it, INITIAL_MAP_ZOOM_IN_TIMELINE, width, height)
         }
 
-        val locationUserId = if (locationContent.isSelfLocation()) informationData.senderId else null
+        val pinMatrixItem = if (locationContent.isSelfLocation()) informationData.matrixItem else null
 
         return MessageLocationItem_()
                 .attributes(attributes)
                 .locationUrl(locationUrl)
                 .mapWidth(width)
                 .mapHeight(height)
-                .locationUserId(locationUserId)
+                .pinMatrixItem(pinMatrixItem)
                 .locationPinProvider(locationPinProvider)
                 .highlighted(highlight)
                 .leftGuideline(avatarSizeProvider.leftGuideline)

vector/src/main/java/im/vector/app/features/home/room/detail/timeline/helper/LocationPinProvider.kt

@@ -19,85 +19,85 @@ package im.vector.app.features.home.room.detail.timeline.helper
 import android.content.Context
 import android.graphics.drawable.Drawable
 import android.graphics.drawable.LayerDrawable
-import androidx.annotation.ColorInt
+import android.util.LruCache
 import androidx.core.content.ContextCompat
 import androidx.core.graphics.drawable.DrawableCompat
 import com.bumptech.glide.request.target.CustomTarget
 import com.bumptech.glide.request.transition.Transition
 import im.vector.app.R
-import im.vector.app.core.di.ActiveSessionHolder
 import im.vector.app.core.glide.GlideApp
 import im.vector.app.core.utils.DimensionConverter
 import im.vector.app.features.home.AvatarRenderer
-import org.matrix.android.sdk.api.session.getUserOrDefault
-import org.matrix.android.sdk.api.util.toMatrixItem
+import org.matrix.android.sdk.api.util.MatrixItem
 import timber.log.Timber
 import javax.inject.Inject
 import javax.inject.Singleton
 
+private data class CachedDrawable(
+        val drawable: Drawable,
+        val isError: Boolean,
+)
+
 @Singleton
 class LocationPinProvider @Inject constructor(
         private val context: Context,
-        private val activeSessionHolder: ActiveSessionHolder,
         private val dimensionConverter: DimensionConverter,
         private val avatarRenderer: AvatarRenderer,
         private val matrixItemColorProvider: MatrixItemColorProvider
 ) {
-    private val cache = mutableMapOf<String, Drawable>()
+    private val cache = LruCache<MatrixItem, CachedDrawable>(32)
 
     private val glideRequests by lazy {
         GlideApp.with(context)
     }
 
     /**
      * Creates a pin drawable. If userId is null then a generic pin drawable will be created.
-     * @param userId userId that will be used to retrieve user avatar
+     * @param matrixUser user that will be used to retrieve user avatar
      * @param callback Pin drawable will be sent through the callback
      */
-    fun create(userId: String?, callback: (Drawable) -> Unit) {
-        if (userId == null) {
+    fun create(matrixUser: MatrixItem?, callback: (Drawable) -> Unit) {
+        if (matrixUser == null) {
             callback(ContextCompat.getDrawable(context, R.drawable.ic_location_pin)!!)
             return
         }
+        val size = dimensionConverter.dpToPx(44)
+        avatarRenderer.render(glideRequests, matrixUser, object : CustomTarget<Drawable>(size, size) {
+            override fun onResourceReady(resource: Drawable, transition: Transition<in Drawable>?) {
+                Timber.d("## Location: onResourceReady")
+                val pinDrawable = createPinDrawable(matrixUser, resource, isError = false)
+                callback(pinDrawable)
+            }
 
-        if (cache.contains(userId)) {
-            callback(cache[userId]!!)
-            return
-        }
-
-        activeSessionHolder
-                .getActiveSession()
-                .getUserOrDefault(userId)
-                .toMatrixItem()
-                .let { userItem ->
-                    val size = dimensionConverter.dpToPx(44)
-                    val bgTintColor = matrixItemColorProvider.getColor(userItem)
-                    avatarRenderer.render(glideRequests, userItem, object : CustomTarget<Drawable>(size, size) {
-                        override fun onResourceReady(resource: Drawable, transition: Transition<in Drawable>?) {
-                            Timber.d("## Location: onResourceReady")
-                            val pinDrawable = createPinDrawable(resource, bgTintColor)
-                            cache[userId] = pinDrawable
-                            callback(pinDrawable)
-                        }
+            override fun onLoadCleared(placeholder: Drawable?) {
+                // Is it possible? Put placeholder instead?
+                // FIXME The doc says it has to be implemented and should free resources
+                Timber.d("## Location: onLoadCleared")
+            }
 
-                        override fun onLoadCleared(placeholder: Drawable?) {
-                            // Is it possible? Put placeholder instead?
-                            // FIXME The doc says it has to be implemented and should free resources
-                            Timber.d("## Location: onLoadCleared")
-                        }
-
-                        override fun onLoadFailed(errorDrawable: Drawable?) {
-                            Timber.w("## Location: onLoadFailed")
-                            errorDrawable ?: return
-                            val pinDrawable = createPinDrawable(errorDrawable, bgTintColor)
-                            cache[userId] = pinDrawable
-                            callback(pinDrawable)
-                        }
-                    })
-                }
+            override fun onLoadFailed(errorDrawable: Drawable?) {
+                // Note: `onLoadFailed` is also called when the user has no avatarUrl
+                // and the errorDrawable is actually the placeholder.
+                Timber.w("## Location: onLoadFailed")
+                errorDrawable ?: return
+                val pinDrawable = createPinDrawable(matrixUser, errorDrawable, isError = true)
+                callback(pinDrawable)
+            }
+        })
     }
 
-    private fun createPinDrawable(drawable: Drawable, @ColorInt bgTintColor: Int): Drawable {
+    private fun createPinDrawable(
+            userItem: MatrixItem,
+            drawable: Drawable,
+            isError: Boolean,
+    ): Drawable {
+        val fromCache = cache.get(userItem)
+        // Return the cached drawable only if it is valid, or the new drawable is again an error
+        if (fromCache != null && (!fromCache.isError || isError)) {
+            return fromCache.drawable
+        }
+
+        val bgTintColor = matrixItemColorProvider.getColor(userItem)
         val bgUserPin = ContextCompat.getDrawable(context, R.drawable.bg_map_user_pin)!!
         // use mutate on drawable to avoid sharing the color when we have multiple different user pins
         DrawableCompat.setTint(bgUserPin.mutate(), bgTintColor)
@@ -106,6 +106,7 @@ class LocationPinProvider @Inject constructor(
         val topInset = dimensionConverter.dpToPx(4)
         val bottomInset = dimensionConverter.dpToPx(8)
         layerDrawable.setLayerInset(1, horizontalInset, topInset, horizontalInset, bottomInset)
+        cache.put(userItem, CachedDrawable(layerDrawable, isError))
         return layerDrawable
     }
 }

vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/AbsMessageLocationItem.kt

@@ -38,6 +38,7 @@ import im.vector.app.features.home.room.detail.timeline.style.TimelineMessageLay
 import im.vector.app.features.home.room.detail.timeline.style.granularRoundedCorners
 import im.vector.app.features.location.MapLoadingErrorView
 import im.vector.app.features.location.MapLoadingErrorViewState
+import org.matrix.android.sdk.api.util.MatrixItem
 
 abstract class AbsMessageLocationItem<H : AbsMessageLocationItem.Holder>(
         @LayoutRes layoutId: Int = R.layout.item_timeline_event_base
@@ -47,7 +48,7 @@ abstract class AbsMessageLocationItem<H : AbsMessageLocationItem.Holder>(
     var locationUrl: String? = null
 
     @EpoxyAttribute
-    var locationUserId: String? = null
+    var pinMatrixItem: MatrixItem? = null
 
     @EpoxyAttribute
     var mapWidth: Int = 0
@@ -103,7 +104,7 @@ abstract class AbsMessageLocationItem<H : AbsMessageLocationItem.Holder>(
                             dataSource: DataSource?,
                             isFirstResource: Boolean
                     ): Boolean {
-                        locationPinProvider?.create(locationUserId) { pinDrawable ->
+                        locationPinProvider?.create(pinMatrixItem) { pinDrawable ->
                             // we are not using Glide since it does not display it correctly when there is no user photo
                             holder.staticMapPinImageView.setImageDrawable(pinDrawable)
                         }

vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageLiveLocationItem.kt

@@ -49,7 +49,7 @@ abstract class MessageLiveLocationItem : AbsMessageLocationItem<MessageLiveLocat
 
     private fun bindLiveLocationBanner(holder: Holder) {
         // TODO in a future PR add check on device id to confirm that is the one that sent the beacon
-        val isEmitter = currentUserId != null && currentUserId == locationUserId
+        val isEmitter = currentUserId != null && currentUserId == pinMatrixItem?.id
         val messageLayout = attributes.informationData.messageLayout
         val viewState = buildViewState(holder, messageLayout, isEmitter)
         holder.liveLocationRunningBanner.isVisible = true

vector/src/main/java/im/vector/app/features/location/LocationSharingViewModel.kt

@@ -106,11 +106,13 @@ class LocationSharingViewModel @AssistedInject constructor(
 
     private fun updatePin(isUserPin: Boolean? = true) {
         if (isUserPin.orFalse()) {
-            locationPinProvider.create(userId = session.myUserId) {
+            val matrixItem = room.membershipService().getRoomMember(session.myUserId)?.toMatrixItem()
+                    ?: session.getUserOrDefault(session.myUserId).toMatrixItem()
+            locationPinProvider.create(matrixItem) {
                 updatePinDrawableInState(it)
             }
         } else {
-            locationPinProvider.create(userId = null) {
+            locationPinProvider.create(null) {
                 updatePinDrawableInState(it)
             }
         }

vector/src/main/java/im/vector/app/features/location/live/map/UserLiveLocationViewStateMapper.kt

@@ -20,8 +20,10 @@ import im.vector.app.core.di.ActiveSessionHolder
 import im.vector.app.features.home.room.detail.timeline.helper.LocationPinProvider
 import im.vector.app.features.location.toLocationData
 import kotlinx.coroutines.suspendCancellableCoroutine
+import org.matrix.android.sdk.api.session.getRoom
 import org.matrix.android.sdk.api.session.getUserOrDefault
 import org.matrix.android.sdk.api.session.room.model.livelocation.LiveLocationShareAggregatedSummary
+import org.matrix.android.sdk.api.util.MatrixItem
 import org.matrix.android.sdk.api.util.toMatrixItem
 import javax.inject.Inject
 
@@ -43,11 +45,21 @@ class UserLiveLocationViewStateMapper @Inject constructor(
                         // do nothing on cancellation
                     }
                     else -> {
-                        locationPinProvider.create(userId) { pinDrawable ->
-                            val session = activeSessionHolder.getActiveSession()
+                        val session = activeSessionHolder.getActiveSession()
+                        val roomId = liveLocationShareAggregatedSummary.roomId
+                        val matrixItem = if (roomId != null) {
+                            session.getRoom(roomId)
+                                    ?.membershipService()
+                                    ?.getRoomMember(userId)
+                                    ?.toMatrixItem()
+                                    ?: MatrixItem.UserItem(userId)
+                        } else {
+                            session.getUserOrDefault(userId).toMatrixItem()
+                        }
+                        locationPinProvider.create(matrixItem) { pinDrawable ->
                             val locationTimestampMillis = liveLocationShareAggregatedSummary.lastLocationDataContent?.getBestTimestampMillis()
                             val viewState = UserLiveLocationViewState(
-                                    matrixItem = session.getUserOrDefault(userId).toMatrixItem(),
+                                    matrixItem = matrixItem,
                                     pinDrawable = pinDrawable,
                                     locationData = locationData,
                                     endOfLiveTimestampMillis = liveLocationShareAggregatedSummary.endOfLiveTimestampMillis,

vector/src/main/java/im/vector/app/features/location/preview/LocationPreviewViewModel.kt

@@ -30,6 +30,8 @@ import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.launch
 import org.matrix.android.sdk.api.session.Session
+import org.matrix.android.sdk.api.util.MatrixItem
+import org.matrix.android.sdk.api.util.toMatrixItem
 
 class LocationPreviewViewModel @AssistedInject constructor(
         @Assisted private val initialState: LocationPreviewViewState,
@@ -46,12 +48,23 @@ class LocationPreviewViewModel @AssistedInject constructor(
     companion object : MavericksViewModelFactory<LocationPreviewViewModel, LocationPreviewViewState> by hiltMavericksViewModelFactory()
 
     init {
-        initPin(initialState.pinUserId)
+        val matrixItem = if (initialState.roomId != null && initialState.pinUserId != null) {
+            session
+                    .roomService()
+                    .getRoom(initialState.roomId)
+                    ?.membershipService()
+                    ?.getRoomMember(initialState.pinUserId)
+                    ?.toMatrixItem()
+                    ?: MatrixItem.UserItem(initialState.pinUserId)
+        } else {
+            null
+        }
+        initPin(matrixItem)
         initLocationTracking()
     }
 
-    private fun initPin(userId: String?) {
-        locationPinProvider.create(userId) { pinDrawable ->
+    private fun initPin(matrixItem: MatrixItem?) {
+        locationPinProvider.create(matrixItem) { pinDrawable ->
             setState { copy(pinDrawable = pinDrawable) }
         }
     }

vector/src/main/java/im/vector/app/features/location/preview/LocationPreviewViewState.kt

@@ -23,6 +23,7 @@ import im.vector.app.features.location.LocationSharingArgs
 
 data class LocationPreviewViewState(
         val pinLocationData: LocationData? = null,
+        val roomId: String? = null,
         val pinUserId: String? = null,
         val pinDrawable: Drawable? = null,
         val loadingMapHasFailed: Boolean = false,
@@ -32,6 +33,7 @@ data class LocationPreviewViewState(
 
     constructor(args: LocationSharingArgs) : this(
             pinLocationData = args.initialLocationData,
+            roomId = args.roomId,
             pinUserId = args.locationOwnerId,
     )
 }