From 985ac26ca8f88f032833b97d5a13e16a148af3a2 Mon Sep 17 00:00:00 2001
From: Till Faelligen <2353100+S7evinK@users.noreply.github.com>
Date: Wed, 3 Aug 2022 13:49:45 +0200
Subject: [PATCH] Fix query issue, only add "changed" users if we actually
 share a room

---
 syncapi/internal/keychange.go                 | 30 +++++++++++--------
 .../postgres/current_room_state_table.go      |  2 +-
 2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/syncapi/internal/keychange.go b/syncapi/internal/keychange.go
index 03df9285c..8553537e1 100644
--- a/syncapi/internal/keychange.go
+++ b/syncapi/internal/keychange.go
@@ -25,6 +25,7 @@ import (
 	"github.com/matrix-org/dendrite/syncapi/types"
 	"github.com/matrix-org/gomatrixserverlib"
 	"github.com/matrix-org/util"
+	"github.com/sirupsen/logrus"
 )
 
 const DeviceListLogName = "dl"
@@ -93,18 +94,15 @@ func DeviceListCatchup(
 	queryRes.UserIDs = append(queryRes.UserIDs, joinUserIDs...)
 	queryRes.UserIDs = append(queryRes.UserIDs, leaveUserIDs...)
 	queryRes.UserIDs = util.UniqueStrings(queryRes.UserIDs)
-	var sharedUsersMap map[string]int
-	sharedUsersMap, queryRes.UserIDs = filterSharedUsers(ctx, db, userID, queryRes.UserIDs)
-	util.GetLogger(ctx).Debugf(
-		"QueryKeyChanges request off=%d,to=%d response off=%d uids=%v",
-		offset, toOffset, queryRes.Offset, queryRes.UserIDs,
-	)
+	sharedUsersMap := filterSharedUsers(ctx, db, userID, queryRes.UserIDs)
 	userSet := make(map[string]bool)
 	for _, userID := range res.DeviceLists.Changed {
-		userSet[userID] = true
+		if sharedUsersMap[userID] > 0 {
+			userSet[userID] = true
+		}
 	}
 	for _, userID := range queryRes.UserIDs {
-		if !userSet[userID] {
+		if !userSet[userID] && sharedUsersMap[userID] > 0 {
 			res.DeviceLists.Changed = append(res.DeviceLists.Changed, userID)
 			hasNew = true
 			userSet[userID] = true
@@ -113,7 +111,7 @@ func DeviceListCatchup(
 	// Finally, add in users who have joined or left.
 	// TODO: This is sub-optimal because we will add users to `changed` even if we already shared a room with them.
 	for _, userID := range joinUserIDs {
-		if !userSet[userID] {
+		if !userSet[userID] && sharedUsersMap[userID] > 0 {
 			res.DeviceLists.Changed = append(res.DeviceLists.Changed, userID)
 			hasNew = true
 			userSet[userID] = true
@@ -126,6 +124,13 @@ func DeviceListCatchup(
 		}
 	}
 
+	util.GetLogger(ctx).WithFields(logrus.Fields{
+		"user_id":         userID,
+		"from":            offset,
+		"to":              toOffset,
+		"response_offset": queryRes.Offset,
+	}).Debugf("QueryKeyChanges request result: %+v", res.DeviceLists)
+
 	return types.StreamPosition(queryRes.Offset), hasNew, nil
 }
 
@@ -220,15 +225,16 @@ func TrackChangedUsers(
 // it down to include only users who the requesting user shares a room with.
 func filterSharedUsers(
 	ctx context.Context, db storage.SharedUsers, userID string, usersWithChangedKeys []string,
-) (map[string]int, []string) {
+) map[string]int {
 	sharedUsersMap := make(map[string]int, len(usersWithChangedKeys))
 	for _, userID := range usersWithChangedKeys {
 		sharedUsersMap[userID] = 0
 	}
 	sharedUsers, err := db.SharedUsers(ctx, userID, usersWithChangedKeys)
 	if err != nil {
+		util.GetLogger(ctx).WithError(err).Errorf("db.SharedUsers failed: %s", err)
 		// default to all users so we do needless queries rather than miss some important device update
-		return nil, usersWithChangedKeys
+		return nil
 	}
 	for _, userID := range sharedUsers {
 		sharedUsersMap[userID]++
@@ -237,7 +243,7 @@ func filterSharedUsers(
 	// and if we are in 0 rooms then we don't technically share any room with ourselves so we wouldn't
 	// be notified about key changes.
 	sharedUsersMap[userID] = 1
-	return sharedUsersMap, sharedUsers
+	return sharedUsersMap
 }
 
 func joinedRooms(res *types.Response, userID string) []string {
diff --git a/syncapi/storage/postgres/current_room_state_table.go b/syncapi/storage/postgres/current_room_state_table.go
index d13b7be41..678652031 100644
--- a/syncapi/storage/postgres/current_room_state_table.go
+++ b/syncapi/storage/postgres/current_room_state_table.go
@@ -407,7 +407,7 @@ func (s *currentRoomStateStatements) SelectSharedUsers(
 	ctx context.Context, txn *sql.Tx, userID string, otherUserIDs []string,
 ) ([]string, error) {
 	stmt := sqlutil.TxStmt(txn, s.selectSharedUsersStmt)
-	rows, err := stmt.QueryContext(ctx, userID, otherUserIDs)
+	rows, err := stmt.QueryContext(ctx, userID, pq.Array(otherUserIDs))
 	if err != nil {
 		return nil, err
 	}