Merge pull request #3105 from vector-im/feature/bma/displayname_fallback

Displayname and avatar fallback for DM, especially when other has left

CHANGES.md

@@ -14,6 +14,7 @@ Improvements 🙌:
  - Update reactions to Unicode 13.1 (#2998)
  - Be more robust when parsing some enums
  - Improve timeline filtering (dissociate membership and profile events, display hidden events when highlighted, fix hidden item/read receipts behavior)
+ - Add better support for empty room name fallback (#3106)
  - Room list improvements (paging)
 
 Bugfix 🐛:

matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/common/TestRoomDisplayNameFallbackProvider.kt

@@ -18,12 +18,12 @@ package org.matrix.android.sdk.common
 
 import org.matrix.android.sdk.api.RoomDisplayNameFallbackProvider
 
-class TestRoomDisplayNameFallbackProvider() : RoomDisplayNameFallbackProvider {
+class TestRoomDisplayNameFallbackProvider : RoomDisplayNameFallbackProvider {
 
     override fun getNameForRoomInvite() =
             "Room invite"
 
-    override fun getNameForEmptyRoom() =
+    override fun getNameForEmptyRoom(isDirect: Boolean, leftMemberNames: List<String>) =
             "Empty room"
 
     override fun getNameFor2members(name1: String?, name2: String?) =

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

@@ -18,7 +18,7 @@ package org.matrix.android.sdk.api
 
 interface RoomDisplayNameFallbackProvider {
     fun getNameForRoomInvite(): String
-    fun getNameForEmptyRoom(): String
+    fun getNameForEmptyRoom(isDirect: Boolean, leftMemberNames: List<String>): String
     fun getNameFor2members(name1: String?, name2: String?): String
     fun getNameFor3members(name1: String?, name2: String?, name3: String?): String
     fun getNameFor4members(name1: String?, name2: String?, name3: String?, name4: String?): String

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/model/RoomMemberSummaryEntity.kt

@@ -39,5 +39,7 @@ internal open class RoomMemberSummaryEntity(@PrimaryKey var primaryKey: String =
             membershipStr = value.name
         }
 
+    fun getBestName() = displayName?.takeIf { it.isNotBlank() } ?: userId
+
     companion object
 }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/events/EventExt.kt

@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2021 The Matrix.org Foundation C.I.C.
+ *
+ * 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 org.matrix.android.sdk.internal.session.events
+
+import org.matrix.android.sdk.api.session.events.model.Event
+import org.matrix.android.sdk.api.session.events.model.toModel
+import org.matrix.android.sdk.api.session.room.model.RoomMemberContent
+
+internal fun Event.getFixedRoomMemberContent(): RoomMemberContent? {
+    val content = content.toModel<RoomMemberContent>()
+    // if user is leaving, we should grab his last name and avatar from prevContent
+    return if (content?.membership?.isLeft() == true) {
+        val prevContent = resolvedPrevContent().toModel<RoomMemberContent>()
+        content.copy(
+                displayName = prevContent?.displayName,
+                avatarUrl = prevContent?.avatarUrl
+        )
+    } else {
+        content
+    }
+}

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/RoomAvatarResolver.kt

@@ -17,10 +17,11 @@
 package org.matrix.android.sdk.internal.session.room
 
 import io.realm.Realm
+import org.matrix.android.sdk.api.extensions.orFalse
 import org.matrix.android.sdk.api.session.events.model.EventType
 import org.matrix.android.sdk.api.session.events.model.toModel
 import org.matrix.android.sdk.api.session.room.model.RoomAvatarContent
-import org.matrix.android.sdk.internal.database.mapper.ContentMapper
+import org.matrix.android.sdk.internal.database.mapper.asDomain
 import org.matrix.android.sdk.internal.database.model.CurrentStateEventEntity
 import org.matrix.android.sdk.internal.database.model.RoomMemberSummaryEntityFields
 import org.matrix.android.sdk.internal.database.model.RoomSummaryEntity
@@ -39,24 +40,35 @@ internal class RoomAvatarResolver @Inject constructor(@UserId private val userId
      * @return the room avatar url, can be a fallback to a room member avatar or null
      */
     fun resolve(realm: Realm, roomId: String): String? {
-        var res: String?
-        val roomName = CurrentStateEventEntity.getOrNull(realm, roomId, type = EventType.STATE_ROOM_AVATAR, stateKey = "")?.root
-        res = ContentMapper.map(roomName?.content).toModel<RoomAvatarContent>()?.avatarUrl
-        if (!res.isNullOrEmpty()) {
-            return res
+        val roomName = CurrentStateEventEntity.getOrNull(realm, roomId, type = EventType.STATE_ROOM_AVATAR, stateKey = "")
+                ?.root
+                ?.asDomain()
+                ?.content
+                ?.toModel<RoomAvatarContent>()
+                ?.avatarUrl
+        if (!roomName.isNullOrEmpty()) {
+            return roomName
         }
         val roomMembers = RoomMemberHelper(realm, roomId)
         val members = roomMembers.queryActiveRoomMembersEvent().findAll()
         // detect if it is a room with no more than 2 members (i.e. an alone or a 1:1 chat)
-        val isDirectRoom = RoomSummaryEntity.where(realm, roomId).findFirst()?.isDirect ?: false
+        val isDirectRoom = RoomSummaryEntity.where(realm, roomId).findFirst()?.isDirect.orFalse()
+
         if (isDirectRoom) {
             if (members.size == 1) {
-                res = members.firstOrNull()?.avatarUrl
+                // Use avatar of a left user
+                val firstLeftAvatarUrl = roomMembers.queryLeftRoomMembersEvent()
+                        .findAll()
+                        .firstOrNull { !it.avatarUrl.isNullOrEmpty() }
+                        ?.avatarUrl
+
+                return firstLeftAvatarUrl ?: members.firstOrNull()?.avatarUrl
             } else if (members.size == 2) {
                 val firstOtherMember = members.where().notEqualTo(RoomMemberSummaryEntityFields.USER_ID, userId).findFirst()
-                res = firstOtherMember?.avatarUrl
+                return firstOtherMember?.avatarUrl
             }
         }
-        return res
+
+        return null
     }
 }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/membership/RoomDisplayNameResolver.kt

@@ -18,6 +18,7 @@ package org.matrix.android.sdk.internal.session.room.membership
 
 import io.realm.Realm
 import org.matrix.android.sdk.api.MatrixConfiguration
+import org.matrix.android.sdk.api.extensions.orFalse
 import org.matrix.android.sdk.api.session.events.model.EventType
 import org.matrix.android.sdk.api.session.events.model.toModel
 import org.matrix.android.sdk.api.session.room.model.Membership
@@ -51,14 +52,14 @@ internal class RoomDisplayNameResolver @Inject constructor(
      * @param roomId: the roomId to resolve the name of.
      * @return the room display name
      */
-    fun resolve(realm: Realm, roomId: String): CharSequence {
+    fun resolve(realm: Realm, roomId: String): String {
         // this algorithm is the one defined in
         // https://github.com/matrix-org/matrix-js-sdk/blob/develop/lib/models/room.js#L617
         // calculateRoomName(room, userId)
 
         // For Lazy Loaded room, see algorithm here:
         // https://docs.google.com/document/d/11i14UI1cUz-OJ0knD5BFu7fmT6Fo327zvMYqfSAR7xs/edit#heading=h.qif6pkqyjgzn
-        var name: CharSequence?
+        var name: String?
         val roomEntity = RoomEntity.where(realm, roomId = roomId).findFirst()
         val roomName = CurrentStateEventEntity.getOrNull(realm, roomId, type = EventType.STATE_ROOM_NAME, stateKey = "")?.root
         name = ContentMapper.map(roomName?.content).toModel<RoomNameContent>()?.name
@@ -77,14 +78,14 @@ internal class RoomDisplayNameResolver @Inject constructor(
         if (roomEntity?.membership == Membership.INVITE) {
             val inviteMeEvent = roomMembers.getLastStateEvent(userId)
             val inviterId = inviteMeEvent?.sender
-            name = if (inviterId != null) {
-                activeMembers.where()
-                        .equalTo(RoomMemberSummaryEntityFields.USER_ID, inviterId)
-                        .findFirst()
-                        ?.displayName
-            } else {
-                roomDisplayNameFallbackProvider.getNameForRoomInvite()
-            }
+            name = inviterId
+                    ?.let {
+                        activeMembers.where()
+                                .equalTo(RoomMemberSummaryEntityFields.USER_ID, it)
+                                .findFirst()
+                                ?.getBestName()
+                    }
+                    ?: roomDisplayNameFallbackProvider.getNameForRoomInvite()
         } else if (roomEntity?.membership == Membership.JOIN) {
             val roomSummary = RoomSummaryEntity.where(realm, roomId).findFirst()
             val invitedCount = roomSummary?.invitedMembersCount ?: 0
@@ -105,8 +106,11 @@ internal class RoomDisplayNameResolver @Inject constructor(
             val otherMembersCount = otherMembersSubset.count()
             name = when (otherMembersCount) {
                 0    -> {
-                    roomDisplayNameFallbackProvider.getNameForEmptyRoom()
-                    // TODO (was xx and yyy) ...
+                    // Get left members if any
+                    val leftMembersNames = roomMembers.queryLeftRoomMembersEvent()
+                            .findAll()
+                            .map { it.getBestName() }
+                    roomDisplayNameFallbackProvider.getNameForEmptyRoom(roomSummary?.isDirect.orFalse(), leftMembersNames)
                 }
                 1    -> resolveRoomMemberName(otherMembersSubset[0], roomMembers)
                 2    -> {
@@ -150,7 +154,7 @@ internal class RoomDisplayNameResolver @Inject constructor(
         if (roomMemberSummary == null) return null
         val isUnique = roomMemberHelper.isUniqueDisplayName(roomMemberSummary.displayName)
         return if (isUnique) {
-            roomMemberSummary.displayName
+            roomMemberSummary.getBestName()
         } else {
             "${roomMemberSummary.displayName} (${roomMemberSummary.userId})"
         }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/membership/RoomMemberEventHandler.kt

@@ -16,12 +16,12 @@
 
 package org.matrix.android.sdk.internal.session.room.membership
 
+import io.realm.Realm
 import org.matrix.android.sdk.api.session.events.model.Event
 import org.matrix.android.sdk.api.session.events.model.EventType
-import org.matrix.android.sdk.api.session.events.model.toModel
 import org.matrix.android.sdk.api.session.room.model.RoomMemberContent
+import org.matrix.android.sdk.internal.session.events.getFixedRoomMemberContent
 import org.matrix.android.sdk.internal.session.user.UserEntityFactory
-import io.realm.Realm
 import javax.inject.Inject
 
 internal class RoomMemberEventHandler @Inject constructor() {
@@ -31,7 +31,7 @@ internal class RoomMemberEventHandler @Inject constructor() {
             return false
         }
         val userId = event.stateKey ?: return false
-        val roomMember = event.content.toModel<RoomMemberContent>()
+        val roomMember = event.getFixedRoomMemberContent()
         return handle(realm, roomId, userId, roomMember)
     }
 

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/membership/RoomMemberHelper.kt

@@ -75,6 +75,11 @@ internal class RoomMemberHelper(private val realm: Realm,
                 .equalTo(RoomMemberSummaryEntityFields.MEMBERSHIP_STR, Membership.INVITE.name)
     }
 
+    fun queryLeftRoomMembersEvent(): RealmQuery<RoomMemberSummaryEntity> {
+        return queryRoomMembersEvent()
+                .equalTo(RoomMemberSummaryEntityFields.MEMBERSHIP_STR, Membership.LEAVE.name)
+    }
+
     fun queryActiveRoomMembersEvent(): RealmQuery<RoomMemberSummaryEntity> {
         return queryRoomMembersEvent()
                 .beginGroup()

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryUpdater.kt

@@ -107,7 +107,7 @@ internal class RoomSummaryUpdater @Inject constructor(
                 // avoid this call if we are sure there are unread events
                 || !isEventRead(realm.configuration, userId, roomId, latestPreviewableEvent?.eventId)
 
-        roomSummaryEntity.displayName = roomDisplayNameResolver.resolve(realm, roomId).toString()
+        roomSummaryEntity.displayName = roomDisplayNameResolver.resolve(realm, roomId)
         roomSummaryEntity.avatarUrl = roomAvatarResolver.resolve(realm, roomId)
         roomSummaryEntity.name = ContentMapper.map(lastNameEvent?.content).toModel<RoomNameContent>()?.name
         roomSummaryEntity.topic = ContentMapper.map(lastTopicEvent?.content).toModel<RoomTopicContent>()?.topic

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/RoomSyncHandler.kt

@@ -49,6 +49,7 @@ import org.matrix.android.sdk.internal.database.query.where
 import org.matrix.android.sdk.internal.di.MoshiProvider
 import org.matrix.android.sdk.internal.di.UserId
 import org.matrix.android.sdk.internal.extensions.clearWith
+import org.matrix.android.sdk.internal.session.events.getFixedRoomMemberContent
 import org.matrix.android.sdk.internal.session.initsync.ProgressReporter
 import org.matrix.android.sdk.internal.session.initsync.mapWithProgress
 import org.matrix.android.sdk.internal.session.initsync.reportSubtask
@@ -464,18 +465,4 @@ internal class RoomSyncHandler @Inject constructor(private val readReceiptHandle
             }
         }
     }
-
-    private fun Event.getFixedRoomMemberContent(): RoomMemberContent? {
-        val content = content.toModel<RoomMemberContent>()
-        // if user is leaving, we should grab his last name and avatar from prevContent
-        return if (content?.membership?.isLeft() == true) {
-            val prevContent = resolvedPrevContent().toModel<RoomMemberContent>()
-            content.copy(
-                    displayName = prevContent?.displayName,
-                    avatarUrl = prevContent?.avatarUrl
-            )
-        } else {
-            content
-        }
-    }
 }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/UserAccountDataSyncHandler.kt

@@ -46,6 +46,7 @@ import org.matrix.android.sdk.internal.database.query.where
 import org.matrix.android.sdk.internal.di.SessionDatabase
 import org.matrix.android.sdk.internal.di.UserId
 import org.matrix.android.sdk.internal.session.room.RoomAvatarResolver
+import org.matrix.android.sdk.internal.session.room.membership.RoomDisplayNameResolver
 import org.matrix.android.sdk.internal.session.room.membership.RoomMemberHelper
 import org.matrix.android.sdk.internal.session.sync.model.InvitedRoomSync
 import org.matrix.android.sdk.internal.session.sync.model.accountdata.BreadcrumbsContent
@@ -62,7 +63,8 @@ internal class UserAccountDataSyncHandler @Inject constructor(
         @UserId private val userId: String,
         private val directChatsHelper: DirectChatsHelper,
         private val updateUserAccountDataTask: UpdateUserAccountDataTask,
-        private val roomAvatarResolver: RoomAvatarResolver
+        private val roomAvatarResolver: RoomAvatarResolver,
+        private val roomDisplayNameResolver: RoomDisplayNameResolver
 ) {
 
     fun handle(realm: Realm, accountData: UserAccountDataSync?) {
@@ -161,8 +163,9 @@ internal class UserAccountDataSyncHandler @Inject constructor(
                 if (roomSummaryEntity != null) {
                     roomSummaryEntity.isDirect = true
                     roomSummaryEntity.directUserId = userId
-                    // Also update the avatar, there is a specific treatment for DMs
+                    // Also update the avatar and displayname, there is a specific treatment for DMs
                     roomSummaryEntity.avatarUrl = roomAvatarResolver.resolve(realm, roomId)
+                    roomSummaryEntity.displayName = roomDisplayNameResolver.resolve(realm, roomId)
                 }
             }
         }
@@ -172,8 +175,9 @@ internal class UserAccountDataSyncHandler @Inject constructor(
                 .forEach {
                     it.isDirect = false
                     it.directUserId = null
-                    // Also update the avatar, there was a specific treatment for DMs
+                    // Also update the avatar and displayname, there was a specific treatment for DMs
                     it.avatarUrl = roomAvatarResolver.resolve(realm, it.roomId)
+                    it.displayName = roomDisplayNameResolver.resolve(realm, it.roomId)
                 }
     }
 

vector/src/main/java/im/vector/app/features/room/VectorRoomDisplayNameFallbackProvider.kt

@@ -28,8 +28,12 @@ class VectorRoomDisplayNameFallbackProvider(
         return context.getString(R.string.room_displayname_room_invite)
     }
 
-    override fun getNameForEmptyRoom(): String {
-        return context.getString(R.string.room_displayname_empty_room)
+    override fun getNameForEmptyRoom(isDirect: Boolean, leftMemberNames: List<String>): String {
+        return if (leftMemberNames.isEmpty()) {
+            context.getString(R.string.room_displayname_empty_room)
+        } else {
+            context.getString(R.string.room_displayname_empty_room_was, leftMemberNames.joinToString())
+        }
     }
 
     override fun getNameFor2members(name1: String?, name2: String?): String {