feat/bugfix: Add invited|joined_count to room response JSON

This is so clients can accurately calculate the push rule:
```
{"kind":"room_member_count","is":"2"}
```
Also fixed a bug in the global room metadata for the joined/invited
counts where it could be wrong because of Synapse sending duplicate
join events as we were tracking +-1 deltas. We now calculate these
counts based on the set of user IDs in a specific membership state.

internal/roomname.go

@@ -32,6 +32,14 @@ func (m *RoomMetadata) SameRoomName(other *RoomMetadata) bool {
 		sameHeroes(m.Heroes, other.Heroes))
 }
 
+func (m *RoomMetadata) SameJoinCount(other *RoomMetadata) bool {
+	return m.JoinCount == other.JoinCount
+}
+
+func (m *RoomMetadata) SameInviteCount(other *RoomMetadata) bool {
+	return m.InviteCount == other.InviteCount
+}
+
 func sameHeroes(a, b []Hero) bool {
 	if len(a) != len(b) {
 		return false

sync3/caches/global.go

@@ -25,6 +25,12 @@ type EventData struct {
 	Content   gjson.Result
 	Timestamp uint64
 
+	// the number of joined users in this room. Use this value and don't try to work it out as you
+	// may get it wrong due to Synapse sending duplicate join events(!) This value has them de-duped
+	// correctly.
+	JoinCount   int
+	InviteCount int
+
 	// the absolute latest position for this event data. The NID for this event is guaranteed to
 	// be <= this value. See PosAlwaysProcess and PosDoNotProcess for things outside the event timeline
 	// e.g invites
@@ -225,24 +231,18 @@ func (c *GlobalCache) OnNewEvent(
 			membership := ed.Content.Get("membership").Str
 			eventJSON := gjson.ParseBytes(ed.Event)
 			if internal.IsMembershipChange(eventJSON) {
-				if membership == "invite" {
-					metadata.InviteCount += 1
-				} else if membership == "join" {
-					metadata.JoinCount += 1
-				} else if membership == "leave" || membership == "ban" {
-					metadata.JoinCount -= 1
+				metadata.JoinCount = ed.JoinCount
+				metadata.InviteCount = ed.InviteCount
+				if membership == "leave" || membership == "ban" {
 					// remove this user as a hero
 					metadata.RemoveHero(*ed.StateKey)
 				}
 
-				if eventJSON.Get("unsigned.prev_content.membership").Str == "invite" {
-					metadata.InviteCount -= 1
-					if membership == "join" {
-						// invite -> join, retire any outstanding invites
-						err := c.store.InvitesTable.RemoveInvite(*ed.StateKey, ed.RoomID)
-						if err != nil {
-							logger.Err(err).Str("user", *ed.StateKey).Str("room", ed.RoomID).Msg("failed to remove accepted invite")
-						}
+				if membership == "join" && eventJSON.Get("unsigned.prev_content.membership").Str == "invite" {
+					// invite -> join, retire any outstanding invites
+					err := c.store.InvitesTable.RemoveInvite(*ed.StateKey, ed.RoomID)
+					if err != nil {
+						logger.Err(err).Str("user", *ed.StateKey).Str("room", ed.RoomID).Msg("failed to remove accepted invite")
 					}
 				}
 			}

sync3/dispatcher.go

@@ -112,9 +112,11 @@ func (d *Dispatcher) onNewEvent(
 	shouldForceInitial := false
 	if ed.EventType == "m.room.member" && ed.StateKey != nil {
 		targetUser = *ed.StateKey
-		// TODO: de-dupe joins in jrt else profile changes will results in 2x room IDs
 		membership = ed.Content.Get("membership").Str
 		switch membership {
+		case "invite":
+			// we only do this to track invite counts correctly.
+			d.jrt.UserInvitedToRoom(targetUser, ed.RoomID)
 		case "join":
 			if d.jrt.UserJoinedRoom(targetUser, ed.RoomID) {
 				shouldForceInitial = true
@@ -124,10 +126,12 @@ func (d *Dispatcher) onNewEvent(
 		case "leave":
 			d.jrt.UserLeftRoom(targetUser, ed.RoomID)
 		}
+		ed.InviteCount = d.jrt.NumInvitedUsersForRoom(ed.RoomID)
 	}
 
 	// notify all people in this room
 	userIDs := d.jrt.JoinedUsersForRoom(ed.RoomID)
+	ed.JoinCount = len(userIDs)
 
 	// invoke listeners
 	d.userToReceiverMu.RLock()

sync3/handler/connstate.go

@@ -367,6 +367,8 @@ func (s *ConnState) getInitialRoomData(ctx context.Context, roomSub sync3.RoomSu
 			InviteState:       inviteState,
 			Initial:           true,
 			IsDM:              userRoomData.IsDM,
+			JoinedCount:       metadata.JoinCount,
+			InvitedCount:      metadata.InviteCount,
 			PrevBatch:         prevBatch,
 		}
 	}

sync3/handler/connstate_live.go

@@ -148,15 +148,24 @@ func (s *connStateLive) processLiveUpdate(ctx context.Context, up caches.Update,
 	for roomID, room := range rooms {
 		response.Rooms[roomID] = room
 	}
-	if delta.RoomNameChanged {
-		// try to find this room in the response. If it's there, then we need to inject a new room name.
+
+	if roomUpdate != nil {
+		// try to find this room in the response. If it's there, then we may need to update some fields.
 		// there's no guarantees that the room will be in the response if say the event caused it to move
 		// off a list.
-		room, exists := response.Rooms[roomUpdate.RoomID()]
+		thisRoom, exists := response.Rooms[roomUpdate.RoomID()]
 		if exists {
-			room.Name = internal.CalculateRoomName(roomUpdate.GlobalRoomMetadata(), 5) // TODO: customisable?
+			if delta.RoomNameChanged {
+				thisRoom.Name = internal.CalculateRoomName(roomUpdate.GlobalRoomMetadata(), 5) // TODO: customisable?
+			}
+			if delta.InviteCountChanged {
+				thisRoom.InvitedCount = roomUpdate.GlobalRoomMetadata().InviteCount
+			}
+			if delta.JoinCountChanged {
+				thisRoom.JoinedCount = roomUpdate.GlobalRoomMetadata().JoinCount
+			}
+			response.Rooms[roomUpdate.RoomID()] = thisRoom
 		}
-		response.Rooms[roomUpdate.RoomID()] = room
 	}
 	return hasUpdates
 }

sync3/lists.go

@@ -31,8 +31,10 @@ type RoomListDelta struct {
 }
 
 type RoomDelta struct {
-	RoomNameChanged bool
-	Lists           []RoomListDelta
+	RoomNameChanged    bool
+	JoinCountChanged   bool
+	InviteCountChanged bool
+	Lists              []RoomListDelta
 }
 
 // InternalRequestLists is a list of lists which matches each index position in the request
@@ -52,6 +54,8 @@ func NewInternalRequestLists() *InternalRequestLists {
 func (s *InternalRequestLists) SetRoom(r RoomConnMetadata) (delta RoomDelta) {
 	existing, exists := s.allRooms[r.RoomID]
 	if exists {
+		delta.InviteCountChanged = !existing.SameInviteCount(&r.RoomMetadata)
+		delta.JoinCountChanged = !existing.SameJoinCount(&r.RoomMetadata)
 		delta.RoomNameChanged = !existing.SameRoomName(&r.RoomMetadata)
 		if delta.RoomNameChanged {
 			// update the canonical name to allow room name sorting to continue to work

sync3/room.go

@@ -16,6 +16,8 @@ type Room struct {
 	HighlightCount    int64             `json:"highlight_count"`
 	Initial           bool              `json:"initial,omitempty"`
 	IsDM              bool              `json:"is_dm,omitempty"`
+	JoinedCount       int               `json:"joined_count,omitempty"`
+	InvitedCount      int               `json:"invited_count,omitempty"`
 	PrevBatch         string            `json:"prev_batch,omitempty"`
 }
 

sync3/tracker.go

@@ -15,14 +15,18 @@ type JoinedRoomsTracker struct {
 	// map of room_id to joined user IDs.
 	roomIDToJoinedUsers map[string]set
 	userIDToJoinedRooms map[string]set
-	mu                  *sync.RWMutex
+	// not for security, just to track invite counts correctly as Synapse can send dupe invite->join events
+	// so increment +-1 counts don't work.
+	roomIDToInvitedUsers map[string]set
+	mu                   *sync.RWMutex
 }
 
 func NewJoinedRoomsTracker() *JoinedRoomsTracker {
 	return &JoinedRoomsTracker{
-		roomIDToJoinedUsers: make(map[string]set),
-		userIDToJoinedRooms: make(map[string]set),
-		mu:                  &sync.RWMutex{},
+		roomIDToJoinedUsers:  make(map[string]set),
+		userIDToJoinedRooms:  make(map[string]set),
+		roomIDToInvitedUsers: make(map[string]set),
+		mu:                   &sync.RWMutex{},
 	}
 }
 
@@ -57,10 +61,13 @@ func (t *JoinedRoomsTracker) UserJoinedRoom(userID, roomID string) bool {
 	if joinedUsers == nil {
 		joinedUsers = make(set)
 	}
+	invitedUsers := t.roomIDToInvitedUsers[roomID]
+	delete(invitedUsers, userID)
 	joinedRooms[roomID] = struct{}{}
 	joinedUsers[userID] = struct{}{}
 	t.userIDToJoinedRooms[userID] = joinedRooms
 	t.roomIDToJoinedUsers[roomID] = joinedUsers
+	t.roomIDToInvitedUsers[roomID] = invitedUsers
 	return !wasJoined
 }
 
@@ -71,8 +78,11 @@ func (t *JoinedRoomsTracker) UserLeftRoom(userID, roomID string) {
 	delete(joinedRooms, roomID)
 	joinedUsers := t.roomIDToJoinedUsers[roomID]
 	delete(joinedUsers, userID)
+	invitedUsers := t.roomIDToInvitedUsers[roomID]
+	delete(invitedUsers, userID)
 	t.userIDToJoinedRooms[userID] = joinedRooms
 	t.roomIDToJoinedUsers[roomID] = joinedUsers
+	t.roomIDToInvitedUsers[roomID] = invitedUsers
 }
 func (t *JoinedRoomsTracker) JoinedRoomsForUser(userID string) []string {
 	t.mu.RLock()
@@ -106,3 +116,21 @@ func (t *JoinedRoomsTracker) JoinedUsersForRoom(roomID string) []string {
 	}
 	return result
 }
+
+// Returns the new invite count
+func (t *JoinedRoomsTracker) UserInvitedToRoom(userID, roomID string) {
+	t.mu.Lock()
+	defer t.mu.Unlock()
+	users := t.roomIDToInvitedUsers[roomID]
+	if users == nil {
+		users = make(set)
+	}
+	users[userID] = struct{}{}
+	t.roomIDToInvitedUsers[roomID] = users
+}
+
+func (t *JoinedRoomsTracker) NumInvitedUsersForRoom(roomID string) int {
+	t.mu.RLock()
+	defer t.mu.RUnlock()
+	return len(t.roomIDToInvitedUsers[roomID])
+}

sync3/tracker_test.go

@@ -26,6 +26,26 @@ func TestTracker(t *testing.T) {
 	jrt.UserLeftRoom("alice", "unknown")
 	jrt.UserLeftRoom("unknown", "unknown2")
 	assertEqualSlices(t, jrt.JoinedRoomsForUser("alice"), []string{"room2"})
+
+	jrt.UserInvitedToRoom("alice", "room4")
+	assertNumEquals(t, jrt.NumInvitedUsersForRoom("room4"), 1)
+	jrt.UserJoinedRoom("alice", "room4")
+	assertNumEquals(t, jrt.NumInvitedUsersForRoom("room4"), 0)
+	jrt.UserJoinedRoom("alice", "room4") // dupe joins don't bother it
+	assertNumEquals(t, jrt.NumInvitedUsersForRoom("room4"), 0)
+	jrt.UserInvitedToRoom("bob", "room4")
+	assertNumEquals(t, jrt.NumInvitedUsersForRoom("room4"), 1)
+	jrt.UserInvitedToRoom("bob", "room4") // dupe invites don't bother it
+	assertNumEquals(t, jrt.NumInvitedUsersForRoom("room4"), 1)
+	jrt.UserLeftRoom("bob", "room4")
+	assertNumEquals(t, jrt.NumInvitedUsersForRoom("room4"), 0)
+}
+
+func assertNumEquals(t *testing.T, got, want int) {
+	t.Helper()
+	if got != want {
+		t.Errorf("wrong number: got %v want %v", got, want)
+	}
 }
 
 func assertEqualSlices(t *testing.T, got, want []string) {

tests-e2e/membership_transitions_test.go

@@ -132,6 +132,8 @@ func TestInviteRejection(t *testing.T) {
 	)), m.MatchRoomSubscriptionsStrict(map[string][]m.RoomMatcher{
 		firstInviteRoomID: {
 			m.MatchRoomInitial(true),
+			m.MatchInviteCount(1),
+			m.MatchJoinCount(1),
 			MatchRoomInviteState([]Event{
 				{
 					Type:     "m.room.name",
@@ -236,6 +238,8 @@ func TestInviteAcceptance(t *testing.T) {
 	)), m.MatchRoomSubscriptionsStrict(map[string][]m.RoomMatcher{
 		firstInviteRoomID: {
 			m.MatchRoomInitial(true),
+			m.MatchInviteCount(1),
+			m.MatchJoinCount(1),
 			MatchRoomInviteState([]Event{
 				{
 					Type:     "m.room.name",
@@ -268,6 +272,8 @@ func TestInviteAcceptance(t *testing.T) {
 	)), m.MatchRoomSubscriptionsStrict(map[string][]m.RoomMatcher{
 		secondInviteRoomID: {
 			m.MatchRoomInitial(true),
+			m.MatchInviteCount(1),
+			m.MatchJoinCount(1),
 			MatchRoomInviteState([]Event{
 				{
 					Type:     "m.room.name",
@@ -314,3 +320,123 @@ func TestInviteAcceptance(t *testing.T) {
 	})
 	m.MatchResponse(t, res, m.MatchNoV3Ops(), m.MatchRoomSubscriptionsStrict(nil), m.MatchList(0, m.MatchV3Count(0)))
 }
+
+// test invite/join counts update and are accurate
+func TestMemberCounts(t *testing.T) {
+	alice := registerNewUser(t)
+	bob := registerNewUser(t)
+	charlie := registerNewUser(t)
+
+	firstRoomID := alice.CreateRoom(t, map[string]interface{}{"preset": "public_chat", "name": "First"})
+	alice.InviteRoom(t, firstRoomID, bob.UserID)
+	secondRoomID := alice.CreateRoom(t, map[string]interface{}{"preset": "public_chat", "name": "Second"})
+	alice.InviteRoom(t, secondRoomID, bob.UserID)
+	charlie.JoinRoom(t, secondRoomID, nil)
+	t.Logf("first %s second %s", firstRoomID, secondRoomID)
+
+	// sync as bob, we should see 2 invited rooms with the same join counts so as not to leak join counts
+	res := bob.SlidingSync(t, sync3.Request{
+		Lists: []sync3.RequestList{
+			{
+				Ranges: sync3.SliceRanges{{0, 20}},
+			},
+		},
+	})
+	m.MatchResponse(t, res, m.MatchList(0, m.MatchV3Count(2), m.MatchV3Ops(
+		m.MatchV3SyncOp(0, 20, []string{firstRoomID, secondRoomID}, true),
+	)), m.MatchRoomSubscriptionsStrict(map[string][]m.RoomMatcher{
+		firstRoomID: {
+			m.MatchRoomInitial(true),
+			m.MatchInviteCount(1),
+			m.MatchJoinCount(1),
+			MatchRoomInviteState([]Event{
+				{
+					Type:     "m.room.name",
+					StateKey: ptr(""),
+					Content: map[string]interface{}{
+						"name": "First",
+					},
+				},
+			}, true),
+		},
+		secondRoomID: {
+			m.MatchRoomInitial(true),
+			m.MatchInviteCount(1),
+			m.MatchJoinCount(1),
+			MatchRoomInviteState([]Event{
+				{
+					Type:     "m.room.name",
+					StateKey: ptr(""),
+					Content: map[string]interface{}{
+						"name": "Second",
+					},
+				},
+			}, true),
+		},
+	}))
+
+	// join both rooms, the counts should now reflect reality
+	bob.JoinRoom(t, firstRoomID, nil)
+	bob.JoinRoom(t, secondRoomID, nil)
+	bob.MustSyncUntil(t, SyncReq{}, SyncJoinedTo(bob.UserID, firstRoomID), SyncJoinedTo(bob.UserID, secondRoomID))
+	time.Sleep(100 * time.Millisecond) // let the proxy process the joins
+
+	res = bob.SlidingSync(t, sync3.Request{
+		Lists: []sync3.RequestList{
+			{
+				Ranges: sync3.SliceRanges{{0, 20}},
+			},
+		},
+	}, WithPos(res.Pos))
+	m.MatchResponse(t, res, m.MatchRoomSubscriptionsStrict(map[string][]m.RoomMatcher{
+		firstRoomID: {
+			m.MatchRoomInitial(true),
+			m.MatchInviteCount(0),
+			m.MatchJoinCount(2),
+		},
+		secondRoomID: {
+			m.MatchRoomInitial(true),
+			m.MatchInviteCount(0),
+			m.MatchJoinCount(3),
+		},
+	}))
+
+	// sending a message shouldn't update the count as it wastes bandwidth
+	charlie.SendEventSynced(t, secondRoomID, Event{
+		Type:    "m.room.message",
+		Content: map[string]interface{}{"body": "ping", "msgtype": "m.text"},
+	})
+	res = bob.SlidingSync(t, sync3.Request{
+		Lists: []sync3.RequestList{
+			{
+				Ranges: sync3.SliceRanges{{0, 20}},
+			},
+		},
+	}, WithPos(res.Pos))
+	m.MatchResponse(t, res, m.MatchRoomSubscriptionsStrict(map[string][]m.RoomMatcher{
+		secondRoomID: {
+			m.MatchRoomInitial(false),
+			m.MatchInviteCount(0),
+			m.MatchJoinCount(0), // omitempty
+		},
+	}))
+
+	// leaving a room should update the count
+	charlie.LeaveRoom(t, secondRoomID)
+	bob.MustSyncUntil(t, SyncReq{}, SyncLeftFrom(charlie.UserID, secondRoomID))
+
+	res = bob.SlidingSync(t, sync3.Request{
+		Lists: []sync3.RequestList{
+			{
+				Ranges: sync3.SliceRanges{{0, 20}},
+			},
+		},
+	}, WithPos(res.Pos))
+	m.MatchResponse(t, res, m.MatchRoomSubscriptionsStrict(map[string][]m.RoomMatcher{
+		secondRoomID: {
+			m.MatchRoomInitial(false),
+			m.MatchInviteCount(0),
+			m.MatchJoinCount(2),
+		},
+	}))
+}

testutils/m/match.go

@@ -27,6 +27,25 @@ func MatchRoomName(name string) RoomMatcher {
 		return nil
 	}
 }
+
+func MatchJoinCount(count int) RoomMatcher {
+	return func(r sync3.Room) error {
+		if r.JoinedCount != count {
+			return fmt.Errorf("MatchJoinCount: got %v want %v", r.JoinedCount, count)
+		}
+		return nil
+	}
+}
+
+func MatchInviteCount(count int) RoomMatcher {
+	return func(r sync3.Room) error {
+		if r.InvitedCount != count {
+			return fmt.Errorf("MatchInviteCount: got %v want %v", r.InvitedCount, count)
+		}
+		return nil
+	}
+}
+
 func MatchRoomRequiredState(events []json.RawMessage) RoomMatcher {
 	return func(r sync3.Room) error {
 		if len(r.RequiredState) != len(events) {