From def03e976ae718a39f324ef9ca4ebaf1aa27df6a Mon Sep 17 00:00:00 2001
From: Till Faelligen <2353100+S7evinK@users.noreply.github.com>
Date: Wed, 7 Dec 2022 08:14:41 +0100
Subject: [PATCH] Review comments

---
 syncapi/storage/interface.go               | 2 +-
 syncapi/storage/postgres/presence_table.go | 7 +++++--
 syncapi/storage/sqlite3/presence_table.go  | 6 ++++--
 syncapi/streams/stream_presence.go         | 5 +++--
 4 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/syncapi/storage/interface.go b/syncapi/storage/interface.go
index eceb5b942..267c9628c 100644
--- a/syncapi/storage/interface.go
+++ b/syncapi/storage/interface.go
@@ -186,7 +186,7 @@ type Database interface {
 }
 
 type Presence interface {
-	GetPresences(ctx context.Context, userID []string) ([]*types.PresenceInternal, error)
+	GetPresences(ctx context.Context, userIDs []string) ([]*types.PresenceInternal, error)
 	UpdatePresence(ctx context.Context, userID string, presence types.Presence, statusMsg *string, lastActiveTS gomatrixserverlib.Timestamp, fromSync bool) (types.StreamPosition, error)
 }
 
diff --git a/syncapi/storage/postgres/presence_table.go b/syncapi/storage/postgres/presence_table.go
index cdebf9151..a3f7c5213 100644
--- a/syncapi/storage/postgres/presence_table.go
+++ b/syncapi/storage/postgres/presence_table.go
@@ -20,10 +20,11 @@ import (
 	"time"
 
 	"github.com/lib/pq"
+	"github.com/matrix-org/gomatrixserverlib"
+
 	"github.com/matrix-org/dendrite/internal"
 	"github.com/matrix-org/dendrite/internal/sqlutil"
 	"github.com/matrix-org/dendrite/syncapi/types"
-	"github.com/matrix-org/gomatrixserverlib"
 )
 
 const presenceSchema = `
@@ -120,7 +121,8 @@ func (p *presenceStatements) UpsertPresence(
 	return
 }
 
-// GetPresenceForUsers returns the current presence of a user.
+// GetPresenceForUsers returns the current presence for a list of users.
+// If the user doesn't have a presence status yet, it is omitted from the response.
 func (p *presenceStatements) GetPresenceForUsers(
 	ctx context.Context, txn *sql.Tx,
 	userIDs []string,
@@ -131,6 +133,7 @@ func (p *presenceStatements) GetPresenceForUsers(
 	if err != nil {
 		return nil, err
 	}
+	defer internal.CloseAndLogIfError(ctx, rows, "GetPresenceForUsers: rows.close() failed")
 
 	for rows.Next() {
 		presence := &types.PresenceInternal{}
diff --git a/syncapi/storage/sqlite3/presence_table.go b/syncapi/storage/sqlite3/presence_table.go
index c4fc24f76..7641de92f 100644
--- a/syncapi/storage/sqlite3/presence_table.go
+++ b/syncapi/storage/sqlite3/presence_table.go
@@ -20,10 +20,11 @@ import (
 	"strings"
 	"time"
 
+	"github.com/matrix-org/gomatrixserverlib"
+
 	"github.com/matrix-org/dendrite/internal"
 	"github.com/matrix-org/dendrite/internal/sqlutil"
 	"github.com/matrix-org/dendrite/syncapi/types"
-	"github.com/matrix-org/gomatrixserverlib"
 )
 
 const presenceSchema = `
@@ -135,7 +136,8 @@ func (p *presenceStatements) UpsertPresence(
 	return
 }
 
-// GetPresenceForUsers returns the current presence of a user.
+// GetPresenceForUsers returns the current presence for a list of users.
+// If the user doesn't have a presence status yet, it is omitted from the response.
 func (p *presenceStatements) GetPresenceForUsers(
 	ctx context.Context, txn *sql.Tx,
 	userIDs []string,
diff --git a/syncapi/streams/stream_presence.go b/syncapi/streams/stream_presence.go
index 39d88eaa8..445e46b3a 100644
--- a/syncapi/streams/stream_presence.go
+++ b/syncapi/streams/stream_presence.go
@@ -17,6 +17,7 @@ package streams
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"sync"
 
 	"github.com/matrix-org/gomatrixserverlib"
@@ -72,6 +73,7 @@ func (p *PresenceStreamProvider) IncrementalSync(
 
 	getPresenceForUsers, err := p.getNeededUsersFromRequest(ctx, req, presences)
 	if err != nil {
+		req.Log.WithError(err).Error("getNeededUsersFromRequest failed")
 		return from
 	}
 
@@ -167,8 +169,7 @@ func (p *PresenceStreamProvider) getNeededUsersFromRequest(ctx context.Context,
 
 	// TODO: Check if this is working better than before.
 	if err := p.notifier.LoadRooms(ctx, p.DB, newlyJoined); err != nil {
-		req.Log.WithError(err).Error("unable to refresh notifier lists")
-		return getPresenceForUsers, err
+		return getPresenceForUsers, fmt.Errorf("unable to refresh notifier lists: %w", err)
 	}
 	for _, roomID := range newlyJoined {
 		roomUsers := p.notifier.JoinedUsers(roomID)