fix automatic DM avatar with functional members

spec/unit/room.spec.ts

@@ -2204,7 +2204,7 @@ describe("Room", function () {
     });
 
     describe("getAvatarFallbackMember", () => {
-        it("should should return undefined if the room isn't a 1:1", () => {
+        it("should return undefined if the room isn't a 1:1", () => {
             const room = new Room(roomId, null!, userA);
             room.currentState.setJoinedMemberCount(2);
             room.currentState.setInvitedMemberCount(1);
@@ -2231,6 +2231,78 @@ describe("Room", function () {
             });
             expect(room.getAvatarFallbackMember()?.userId).toBe(userD);
         });
+
+        it("should return undefined if the room is a 1:1 plus functional member", async function () {
+            const room = new Room(roomId, null!, userA);
+            await room.currentState.setStateEvents([
+                utils.mkMembership({
+                    user: userA,
+                    mship: "join",
+                    room: roomId,
+                    event: true,
+                    name: "User A",
+                }),
+                utils.mkMembership({
+                    user: userB,
+                    mship: "join",
+                    room: roomId,
+                    event: true,
+                    name: "User B",
+                }),
+                utils.mkEvent({
+                    type: UNSTABLE_ELEMENT_FUNCTIONAL_USERS.name,
+                    skey: "",
+                    room: roomId,
+                    event: true,
+                    content: {
+                        service_members: [userB],
+                    },
+                }),
+            ]);
+            expect(room.getAvatarFallbackMember()).toBeUndefined();
+        });
+
+        it("should pick nonfunctional member from summary heroes if room is a 1:1 plus functional member", async function () {
+            const room = new Room(roomId, null!, userA);
+            await room.currentState.setStateEvents([
+                utils.mkMembership({
+                    user: userA,
+                    mship: "join",
+                    room: roomId,
+                    event: true,
+                    name: "User A",
+                }),
+                utils.mkMembership({
+                    user: userB,
+                    mship: "join",
+                    room: roomId,
+                    event: true,
+                    name: "User B",
+                }),
+                utils.mkMembership({
+                    user: userD,
+                    mship: "join",
+                    room: roomId,
+                    event: true,
+                    name: "User D",
+                }),
+                utils.mkEvent({
+                    type: UNSTABLE_ELEMENT_FUNCTIONAL_USERS.name,
+                    skey: "",
+                    room: roomId,
+                    event: true,
+                    content: {
+                        service_members: [userB],
+                    },
+                }),
+            ]);
+            room.setSummary({
+                "m.heroes": [userA, userD, userB],
+                "m.joined_member_count": 2,
+                "m.invited_member_count": 1,
+            });
+            expect(room.getAvatarFallbackMember()?.userId).toBe(userD);
+        });
     });
 
     describe("maySendMessage", function () {

src/models/room.ts

@@ -914,37 +914,79 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
         return this.myUserId;
     }
 
+    /**
+     * Gets the "functional members" in this room.
+     *
+     * Returns the list of userIDs from the `io.element.functional_members` event. Does not consider the
+     * current membership states of those users.
+     *
+     * @see https://github.com/element-hq/element-meta/blob/develop/spec/functional_members.md.
+     */
+    private getFunctionalMembers(): string[] {
+        const mFunctionalMembers = this.currentState.getStateEvents(UNSTABLE_ELEMENT_FUNCTIONAL_USERS.name, "");
+        if (Array.isArray(mFunctionalMembers?.getContent().service_members)) {
+            return mFunctionalMembers!.getContent().service_members;
+        }
+        return [];
+    }
+
     public getAvatarFallbackMember(): RoomMember | undefined {
-        const memberCount = this.getInvitedAndJoinedMemberCount();
-        if (memberCount > 2) {
+        const functionalMembers = this.getFunctionalMembers();
+        // Optimizing for performance, we iterate over all members outermost, as is it the longest of the 3 lists we are comparing.
+        const nonFunctionalMemberCount = this.getMembers()!.reduce((count, m) => {
+            if (
+                m.membership &&
+                // We care most about big rooms where also possibly lots of users may have left,
+                // so we first do the O(2) membership check, as it is definitely cheap and might remove candidates before the next step.
+                ["join", "invite"].includes(m.membership) &&
+                // Then we do the O(functionalMembers.length) functionalMember check, which is probably usually still cheap.
+                !functionalMembers.includes(m.userId)
+            ) {
+                return count + 1;
+            }
+            return count;
+        }, 0);
+
+        // Only generate a fallback avatar if the conversation is with a single specific other user (a "DM").
+        if (nonFunctionalMemberCount > 2) {
             return;
         }
-        const hasHeroes = Array.isArray(this.summaryHeroes) && this.summaryHeroes.length;
+
+        // Prefer the list of heroes, if present. It should only include the single other user in the DM.
+        const nonFunctionalHeroes = this.summaryHeroes?.filter((h) => !functionalMembers.includes(h));
+        const hasHeroes = Array.isArray(nonFunctionalHeroes) && nonFunctionalHeroes.length;
         if (hasHeroes) {
-            const availableMember = this.summaryHeroes!.map((userId) => {
-                return this.getMember(userId);
-            }).find((member) => !!member);
+            const availableMember = nonFunctionalHeroes
+                .map((userId) => {
+                    return this.getMember(userId);
+                })
+                .find((member) => !!member);
             if (availableMember) {
                 return availableMember;
             }
         }
-        const members = this.currentState.getMembers();
-        // could be different than memberCount
-        // as this includes left members
-        if (members.length <= 2) {
-            const availableMember = members.find((m) => {
+
+        // Consider *all*, including previous, members, to generate the avatar for DMs where the other user left.
+        // Needed to generate a matching avatar for rooms named "Empty Room (was Alice)".
+        const members = this.getMembers();
+        const nonFunctionalMembers = members?.filter((m) => !functionalMembers.includes(m.userId));
+        if (nonFunctionalMembers.length <= 2) {
+            const availableMember = nonFunctionalMembers.find((m) => {
                 return m.userId !== this.myUserId;
             });
             if (availableMember) {
                 return availableMember;
             }
         }
-        // if all else fails, try falling back to a user,
-        // and create a one-off member for it
+
+        // If all else failed, but the homeserver gave us heroes that previously could not be found in the room members,
+        // trust and try falling back to a hero, creating a one-off member for it
         if (hasHeroes) {
-            const availableUser = this.summaryHeroes!.map((userId) => {
-                return this.client.getUser(userId);
-            }).find((user) => !!user);
+            const availableUser = nonFunctionalHeroes
+                .map((userId) => {
+                    return this.client.getUser(userId);
+                })
+                .find((user) => !!user);
             if (availableUser) {
                 const member = new RoomMember(this.roomId, availableUser.userId);
                 member.user = availableUser;
@@ -3351,11 +3393,7 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
         let inviteJoinCount = joinedMemberCount + invitedMemberCount - 1;
 
         // get service members (e.g. helper bots) for exclusion
-        let excludedUserIds: string[] = [];
-        const mFunctionalMembers = this.currentState.getStateEvents(UNSTABLE_ELEMENT_FUNCTIONAL_USERS.name, "");
-        if (Array.isArray(mFunctionalMembers?.getContent().service_members)) {
-            excludedUserIds = mFunctionalMembers!.getContent().service_members;
-        }
+        const excludedUserIds = this.getFunctionalMembers();
 
         // get members that are NOT ourselves and are actually in the room.
         let otherNames: string[] = [];