Initial support for room heroes

internal/roomname.go

@@ -155,19 +155,21 @@ func (m *RoomMetadata) IsSpace() bool {
 }
 
 type Hero struct {
-	ID     string
-	Name   string
-	Avatar string
+	ID     string `json:"user_id"`
+	Name   string `json:"displayname,omitempty"`
+	Avatar string `json:"avatar_url,omitempty"`
 }
 
-func CalculateRoomName(heroInfo *RoomMetadata, maxNumNamesPerRoom int) string {
+// CalculateRoomName calculates the room name. Returns the name and if the name was actually calculated
+// based on room heroes.
+func CalculateRoomName(heroInfo *RoomMetadata, maxNumNamesPerRoom int) (name string, calculated bool) {
 	// If the room has an m.room.name state event with a non-empty name field, use the name given by that field.
 	if heroInfo.NameEvent != "" {
-		return heroInfo.NameEvent
+		return heroInfo.NameEvent, false
 	}
 	// If the room has an m.room.canonical_alias state event with a valid alias field, use the alias given by that field as the name.
 	if heroInfo.CanonicalAlias != "" {
-		return heroInfo.CanonicalAlias
+		return heroInfo.CanonicalAlias, false
 	}
 	// If none of the above conditions are met, a name should be composed based on the members of the room.
 	disambiguatedNames := disambiguate(heroInfo.Heroes)
@@ -178,21 +180,21 @@ func CalculateRoomName(heroInfo *RoomMetadata, maxNumNamesPerRoom int) string {
 	// the client should use the rules BELOW to indicate that the room was empty. For example, "Empty Room (was Alice)",
 	// "Empty Room (was Alice and 1234 others)", or "Empty Room" if there are no heroes.
 	if len(heroInfo.Heroes) == 0 && isAlone {
-		return "Empty Room"
+		return "Empty Room", false
 	}
 
 	// If the number of m.heroes for the room are greater or equal to m.joined_member_count + m.invited_member_count - 1,
 	// then use the membership events for the heroes to calculate display names for the users (disambiguating them if required)
 	// and concatenating them.
 	if len(heroInfo.Heroes) >= totalNumOtherUsers {
 		if len(disambiguatedNames) == 1 {
-			return disambiguatedNames[0]
+			return disambiguatedNames[0], true
 		}
 		calculatedRoomName := strings.Join(disambiguatedNames[:len(disambiguatedNames)-1], ", ") + " and " + disambiguatedNames[len(disambiguatedNames)-1]
 		if isAlone {
-			return fmt.Sprintf("Empty Room (was %s)", calculatedRoomName)
+			return fmt.Sprintf("Empty Room (was %s)", calculatedRoomName), true
 		}
-		return calculatedRoomName
+		return calculatedRoomName, true
 	}
 
 	// if we're here then len(heroes) < (joinedCount + invitedCount - 1)
@@ -208,13 +210,13 @@ func CalculateRoomName(heroInfo *RoomMetadata, maxNumNamesPerRoom int) string {
 	// and m.joined_member_count + m.invited_member_count is greater than 1, the client should use the heroes to calculate
 	// display names for the users (disambiguating them if required) and concatenating them alongside a count of the remaining users.
 	if (heroInfo.JoinCount + heroInfo.InviteCount) > 1 {
-		return calculatedRoomName
+		return calculatedRoomName, true
 	}
 
 	// If m.joined_member_count + m.invited_member_count is less than or equal to 1 (indicating the member is alone),
 	// the client should use the rules above to indicate that the room was empty. For example, "Empty Room (was Alice)",
 	// "Empty Room (was Alice and 1234 others)", or "Empty Room" if there are no heroes.
-	return fmt.Sprintf("Empty Room (was %s)", calculatedRoomName)
+	return fmt.Sprintf("Empty Room (was %s)", calculatedRoomName), true
 }
 
 func disambiguate(heroes []Hero) []string {

internal/roomname_test.go

@@ -11,7 +11,8 @@ func TestCalculateRoomName(t *testing.T) {
 		invitedCount       int
 		maxNumNamesPerRoom int
 
-		wantRoomName string
+		wantRoomName   string
+		wantCalculated bool
 	}{
 		// Room name takes precedence
 		{
@@ -65,7 +66,8 @@ func TestCalculateRoomName(t *testing.T) {
 					Name: "Bob",
 				},
 			},
-			wantRoomName: "Alice, Bob and 3 others",
+			wantRoomName:   "Alice, Bob and 3 others",
+			wantCalculated: true,
 		},
 		// Small group chat
 		{
@@ -86,7 +88,8 @@ func TestCalculateRoomName(t *testing.T) {
 					Name: "Charlie",
 				},
 			},
-			wantRoomName: "Alice, Bob and Charlie",
+			wantRoomName:   "Alice, Bob and Charlie",
+			wantCalculated: true,
 		},
 		// DM room
 		{
@@ -99,7 +102,8 @@ func TestCalculateRoomName(t *testing.T) {
 					Name: "Alice",
 				},
 			},
-			wantRoomName: "Alice",
+			wantRoomName:   "Alice",
+			wantCalculated: true,
 		},
 		// 3-way room
 		{
@@ -116,7 +120,8 @@ func TestCalculateRoomName(t *testing.T) {
 					Name: "Bob",
 				},
 			},
-			wantRoomName: "Alice and Bob",
+			wantRoomName:   "Alice and Bob",
+			wantCalculated: true,
 		},
 		// 3-way room, one person invited with no display name
 		{
@@ -132,7 +137,8 @@ func TestCalculateRoomName(t *testing.T) {
 					ID: "@bob:localhost",
 				},
 			},
-			wantRoomName: "Alice and @bob:localhost",
+			wantRoomName:   "Alice and @bob:localhost",
+			wantCalculated: true,
 		},
 		// 3-way room, no display names
 		{
@@ -147,7 +153,8 @@ func TestCalculateRoomName(t *testing.T) {
 					ID: "@bob:localhost",
 				},
 			},
-			wantRoomName: "@alice:localhost and @bob:localhost",
+			wantRoomName:   "@alice:localhost and @bob:localhost",
+			wantCalculated: true,
 		},
 		// disambiguation all
 		{
@@ -168,7 +175,8 @@ func TestCalculateRoomName(t *testing.T) {
 					Name: "Alice",
 				},
 			},
-			wantRoomName: "Alice (@alice:localhost), Alice (@bob:localhost), Alice (@charlie:localhost) and 6 others",
+			wantRoomName:   "Alice (@alice:localhost), Alice (@bob:localhost), Alice (@charlie:localhost) and 6 others",
+			wantCalculated: true,
 		},
 		// disambiguation some
 		{
@@ -189,7 +197,8 @@ func TestCalculateRoomName(t *testing.T) {
 					Name: "Alice",
 				},
 			},
-			wantRoomName: "Alice (@alice:localhost), Bob, Alice (@charlie:localhost) and 6 others",
+			wantRoomName:   "Alice (@alice:localhost), Bob, Alice (@charlie:localhost) and 6 others",
+			wantCalculated: true,
 		},
 		// disambiguation, faking user IDs as display names
 		{
@@ -205,7 +214,8 @@ func TestCalculateRoomName(t *testing.T) {
 					ID: "@alice:localhost",
 				},
 			},
-			wantRoomName: "@alice:localhost (@evil:localhost) and @alice:localhost (@alice:localhost)",
+			wantRoomName:   "@alice:localhost (@evil:localhost) and @alice:localhost (@alice:localhost)",
+			wantCalculated: true,
 		},
 		// left room
 		{
@@ -222,7 +232,8 @@ func TestCalculateRoomName(t *testing.T) {
 					Name: "Bob",
 				},
 			},
-			wantRoomName: "Empty Room (was Alice and Bob)",
+			wantRoomName:   "Empty Room (was Alice and Bob)",
+			wantCalculated: true,
 		},
 		// empty room
 		{
@@ -235,7 +246,7 @@ func TestCalculateRoomName(t *testing.T) {
 	}
 
 	for _, tc := range testCases {
-		gotName := CalculateRoomName(&RoomMetadata{
+		gotName, gotCalculated := CalculateRoomName(&RoomMetadata{
 			NameEvent:      tc.roomName,
 			CanonicalAlias: tc.canonicalAlias,
 			Heroes:         tc.heroes,
@@ -245,6 +256,9 @@ func TestCalculateRoomName(t *testing.T) {
 		if gotName != tc.wantRoomName {
 			t.Errorf("got %s want %s for test case: %+v", gotName, tc.wantRoomName, tc)
 		}
+		if gotCalculated != tc.wantCalculated {
+			t.Errorf("got %v want %v for test case: %+v", gotCalculated, tc.wantCalculated, tc)
+		}
 	}
 }
 

sync3/handler/connstate.go

@@ -664,8 +664,9 @@ func (s *ConnState) getInitialRoomData(ctx context.Context, roomSub sync3.RoomSu
 			}
 		}
 
-		rooms[roomID] = sync3.Room{
-			Name:              internal.CalculateRoomName(metadata, 5), // TODO: customisable?
+		roomName, calculated := internal.CalculateRoomName(metadata, 5) // TODO: customisable?
+		room := sync3.Room{
+			Name:              roomName,
 			AvatarChange:      sync3.NewAvatarChange(internal.CalculateAvatar(metadata)),
 			NotificationCount: int64(userRoomData.NotificationCount),
 			HighlightCount:    int64(userRoomData.HighlightCount),
@@ -679,6 +680,10 @@ func (s *ConnState) getInitialRoomData(ctx context.Context, roomSub sync3.RoomSu
 			PrevBatch:         userRoomData.RequestedLatestEvents.PrevBatch,
 			Timestamp:         maxTs,
 		}
+		if calculated {
+			room.Heroes = metadata.Heroes
+		}
+		rooms[roomID] = room
 	}
 
 	if rsm.IsLazyLoading() {

sync3/handler/connstate_live.go

@@ -259,7 +259,12 @@ func (s *connStateLive) processLiveUpdate(ctx context.Context, up caches.Update,
 			if delta.RoomNameChanged {
 				metadata := roomUpdate.GlobalRoomMetadata()
 				metadata.RemoveHero(s.userID)
-				thisRoom.Name = internal.CalculateRoomName(metadata, 5) // TODO: customisable?
+				roomName, calculated := internal.CalculateRoomName(metadata, 5) // TODO: customisable?
+
+				thisRoom.Name = roomName
+				if calculated {
+					thisRoom.Heroes = metadata.Heroes
+				}
 			}
 			if delta.RoomAvatarChanged {
 				metadata := roomUpdate.GlobalRoomMetadata()

sync3/lists.go

@@ -70,8 +70,9 @@ func (s *InternalRequestLists) SetRoom(r RoomConnMetadata) (delta RoomDelta) {
 		delta.RoomNameChanged = !existing.SameRoomName(&r.RoomMetadata)
 		if delta.RoomNameChanged {
 			// update the canonical name to allow room name sorting to continue to work
+			roomName, _ := internal.CalculateRoomName(&r.RoomMetadata, 5)
 			r.CanonicalisedName = strings.ToLower(
-				strings.Trim(internal.CalculateRoomName(&r.RoomMetadata, 5), "#!():_@"),
+				strings.Trim(roomName, "#!():_@"),
 			)
 		} else {
 			// XXX: during TestConnectionTimeoutNotReset there is some situation where
@@ -109,8 +110,9 @@ func (s *InternalRequestLists) SetRoom(r RoomConnMetadata) (delta RoomDelta) {
 		}
 	} else {
 		// set the canonical name to allow room name sorting to work
+		roomName, _ := internal.CalculateRoomName(&r.RoomMetadata, 5)
 		r.CanonicalisedName = strings.ToLower(
-			strings.Trim(internal.CalculateRoomName(&r.RoomMetadata, 5), "#!():_@"),
+			strings.Trim(roomName, "#!():_@"),
 		)
 		r.ResolvedAvatarURL = internal.CalculateAvatar(&r.RoomMetadata)
 		// We'll automatically use the LastInterestedEventTimestamps provided by the

sync3/request.go

@@ -57,6 +57,7 @@ type RequestList struct {
 	SlowGetAllRooms *bool           `json:"slow_get_all_rooms,omitempty"`
 	Deleted         bool            `json:"deleted,omitempty"`
 	BumpEventTypes  []string        `json:"bump_event_types"`
+	Heroes          bool            `json:"heroes"`
 }
 
 func (rl *RequestList) ShouldGetAllRooms() bool {
@@ -517,7 +518,8 @@ func (rf *RequestFilters) Include(r *RoomConnMetadata, finder RoomFinder) bool {
 	if rf.IsInvite != nil && *rf.IsInvite != r.IsInvite {
 		return false
 	}
-	if rf.RoomNameFilter != "" && !strings.Contains(strings.ToLower(internal.CalculateRoomName(&r.RoomMetadata, 5)), strings.ToLower(rf.RoomNameFilter)) {
+	roomName, _ := internal.CalculateRoomName(&r.RoomMetadata, 5)
+	if rf.RoomNameFilter != "" && !strings.Contains(strings.ToLower(roomName), strings.ToLower(rf.RoomNameFilter)) {
 		return false
 	}
 	if len(rf.NotTags) > 0 {

sync3/room.go

@@ -11,6 +11,7 @@ import (
 type Room struct {
 	Name              string            `json:"name,omitempty"`
 	AvatarChange      AvatarChange      `json:"avatar,omitempty"`
+	Heroes            []internal.Hero   `json:"heroes,omitempty"`
 	RequiredState     []json.RawMessage `json:"required_state,omitempty"`
 	Timeline          []json.RawMessage `json:"timeline,omitempty"`
 	InviteState       []json.RawMessage `json:"invite_state,omitempty"`

tests-e2e/membership_transitions_test.go

@@ -532,6 +532,96 @@ func TestRejectingInviteReturnsOneEvent(t *testing.T) {
 	}
 }
 
+// Test to check that room heroes are returned if the membership changes
+func TestHeroesOnMembershipChanges(t *testing.T) {
+	alice := registerNewUser(t)
+	bob := registerNewUser(t)
+	charlie := registerNewUser(t)
+
+	t.Run("nameless room uses heroes to calculate roomname", func(t *testing.T) {
+		// create a room without a name, to ensure we calculate the room name based on
+		// room heroes
+		roomID := alice.CreateRoom(t, map[string]interface{}{"preset": "public_chat"})
+
+		bob.JoinRoom(t, roomID, []string{})
+
+		res := alice.SlidingSyncUntilMembership(t, "", roomID, bob, "join")
+		// we expect to see Bob as a hero
+		if c := len(res.Rooms[roomID].Heroes); c > 1 {
+			t.Errorf("expected 1 room hero, got %d", c)
+		}
+		if gotUserID := res.Rooms[roomID].Heroes[0].ID; gotUserID != bob.UserID {
+			t.Errorf("expected userID %q, got %q", gotUserID, bob.UserID)
+		}
+
+		// Now join with Charlie, the heroes and the room name should change
+		charlie.JoinRoom(t, roomID, []string{})
+		res = alice.SlidingSyncUntilMembership(t, res.Pos, roomID, charlie, "join")
+
+		// we expect to see Bob as a hero
+		if c := len(res.Rooms[roomID].Heroes); c > 2 {
+			t.Errorf("expected 2 room hero, got %d", c)
+		}
+		if gotUserID := res.Rooms[roomID].Heroes[0].ID; gotUserID != bob.UserID {
+			t.Errorf("expected userID %q, got %q", gotUserID, bob.UserID)
+		}
+		if gotUserID := res.Rooms[roomID].Heroes[1].ID; gotUserID != charlie.UserID {
+			t.Errorf("expected userID %q, got %q", gotUserID, charlie.UserID)
+		}
+
+		// Send a message, the heroes shouldn't change
+		msgEv := bob.SendEventSynced(t, roomID, Event{
+			Type:    "m.room.roomID",
+			Content: map[string]interface{}{"body": "Hello world", "msgtype": "m.text"},
+		})
+
+		res = alice.SlidingSyncUntilEventID(t, res.Pos, roomID, msgEv)
+		if len(res.Rooms[roomID].Heroes) > 0 {
+			t.Errorf("expected no change to room heros")
+		}
+
+		// Now leave with Charlie, only Bob should be in the heroes list
+		charlie.LeaveRoom(t, roomID)
+		res = alice.SlidingSyncUntilMembership(t, res.Pos, roomID, charlie, "leave")
+		if c := len(res.Rooms[roomID].Heroes); c > 1 {
+			t.Errorf("expected 1 room hero, got %d", c)
+		}
+		if gotUserID := res.Rooms[roomID].Heroes[0].ID; gotUserID != bob.UserID {
+			t.Errorf("expected userID %q, got %q", gotUserID, bob.UserID)
+		}
+	})
+
+	t.Run("named rooms don't have heroes", func(t *testing.T) {
+		namedRoomID := alice.CreateRoom(t, map[string]interface{}{"preset": "public_chat", "name": "my room without heroes"})
+		// this makes sure that even if bob is joined, we don't return any heroes
+		bob.JoinRoom(t, namedRoomID, []string{})
+
+		res := alice.SlidingSyncUntilMembership(t, "", namedRoomID, bob, "join")
+		if len(res.Rooms[namedRoomID].Heroes) > 0 {
+			t.Errorf("expected no heroes, got %#v", res.Rooms[namedRoomID].Heroes)
+		}
+	})
+
+	t.Run("rooms with aliases don't have heroes", func(t *testing.T) {
+		aliasRoomID := alice.CreateRoom(t, map[string]interface{}{"preset": "public_chat"})
+
+		alias := fmt.Sprintf("#%s-%d:%s", t.Name(), time.Now().Unix(), alice.Domain)
+		alice.MustDoFunc(t, "PUT", []string{"_matrix", "client", "v3", "directory", "room", alias},
+			WithJSONBody(t, map[string]any{"room_id": aliasRoomID}),
+		)
+		alice.SetState(t, aliasRoomID, "m.room.canonical_alias", "", map[string]any{
+			"alias": alias,
+		})
+
+		bob.JoinRoom(t, aliasRoomID, []string{})
+
+		res := alice.SlidingSyncUntilMembership(t, "", aliasRoomID, bob, "join")
+		if len(res.Rooms[aliasRoomID].Heroes) > 0 {
+			t.Errorf("expected no heroes, got %#v", res.Rooms[aliasRoomID].Heroes)
+		}
+	})
+}
+
 // test invite/join counts update and are accurate
 func TestMemberCounts(t *testing.T) {
 	alice := registerNewUser(t)