diff --git a/roomserver/internal/input/input_membership.go b/roomserver/internal/input/input_membership.go index d3723aa17..77a2ae8c4 100644 --- a/roomserver/internal/input/input_membership.go +++ b/roomserver/internal/input/input_membership.go @@ -99,6 +99,16 @@ func (r *Inputer) updateMembership( } } + var targetLocal bool + if add != nil { + targetLocal = r.isLocalTarget(add) + } + + mu, err := updater.MembershipUpdater(targetUserNID, targetLocal) + if err != nil { + return nil, err + } + // In an ideal world, we shouldn't ever have "add" be nil and "remove" be // set, as this implies that we're deleting a state event without replacing // it (a thing that ordinarily shouldn't happen in Matrix). However, state @@ -108,17 +118,10 @@ func (r *Inputer) updateMembership( // after a state reset, often thinking that the user was still joined to // the room even though the room state said otherwise, and this would prevent // the user from being able to attempt to rejoin the room without modifying - // the database. So instead what we'll do is we'll just update the membership - // table to say that the user is "leave" and we'll use the old event to - // avoid nil pointer exceptions on the code path that follows. - if add == nil { - add = remove - newMembership = gomatrixserverlib.Leave - } - - mu, err := updater.MembershipUpdater(targetUserNID, r.isLocalTarget(add)) - if err != nil { - return nil, err + // the database. So instead we're going to remove the membership from the + // database altogether, so that it doesn't create future problems. + if add == nil && remove != nil { + return nil, mu.Delete() } switch newMembership { diff --git a/roomserver/internal/perform/perform_leave.go b/roomserver/internal/perform/perform_leave.go index 74ccd7884..56e7240d0 100644 --- a/roomserver/internal/perform/perform_leave.go +++ b/roomserver/internal/perform/perform_leave.go @@ -30,7 +30,6 @@ import ( "github.com/matrix-org/dendrite/roomserver/internal/helpers" "github.com/matrix-org/dendrite/roomserver/internal/input" "github.com/matrix-org/dendrite/roomserver/storage" - "github.com/matrix-org/dendrite/roomserver/storage/tables" "github.com/matrix-org/dendrite/setup/config" userapi "github.com/matrix-org/dendrite/userapi/api" ) @@ -229,14 +228,14 @@ func (r *Leaver) performFederatedRejectInvite( util.GetLogger(ctx).WithError(err).Errorf("failed to get MembershipUpdater, still retiring invite event") } if updater != nil { - if _, _, err = updater.Update(tables.MembershipStateLeaveOrBan, leaveRes.Event.Unwrap()); err != nil { - util.GetLogger(ctx).WithError(err).Errorf("failed to set membership to leave, still retiring invite event") + if err = updater.Delete(); err != nil { + util.GetLogger(ctx).WithError(err).Errorf("failed to delete membership, still retiring invite event") if err = updater.Rollback(); err != nil { - util.GetLogger(ctx).WithError(err).Errorf("failed to rollback membership leave, still retiring invite event") + util.GetLogger(ctx).WithError(err).Errorf("failed to rollback deleting membership, still retiring invite event") } } else { if err = updater.Commit(); err != nil { - util.GetLogger(ctx).WithError(err).Errorf("failed to commit membership update, still retiring invite event") + util.GetLogger(ctx).WithError(err).Errorf("failed to commit deleting membership, still retiring invite event") } } } diff --git a/roomserver/storage/postgres/membership_table.go b/roomserver/storage/postgres/membership_table.go index c01753c3a..8d455a7f3 100644 --- a/roomserver/storage/postgres/membership_table.go +++ b/roomserver/storage/postgres/membership_table.go @@ -112,6 +112,9 @@ const updateMembershipForgetRoom = "" + "UPDATE roomserver_membership SET forgotten = $3" + " WHERE room_nid = $1 AND target_nid = $2" +const deleteMembershipSQL = "" + + "DELETE FROM roomserver_membership WHERE room_nid = $1 AND target_nid = $2" + const selectRoomsWithMembershipSQL = "" + "SELECT room_nid FROM roomserver_membership WHERE membership_nid = $1 AND target_nid = $2 and forgotten = false" @@ -158,6 +161,7 @@ type membershipStatements struct { updateMembershipForgetRoomStmt *sql.Stmt selectLocalServerInRoomStmt *sql.Stmt selectServerInRoomStmt *sql.Stmt + deleteMembershipStmt *sql.Stmt } func CreateMembershipTable(db *sql.DB) error { @@ -183,6 +187,7 @@ func PrepareMembershipTable(db *sql.DB) (tables.Membership, error) { {&s.updateMembershipForgetRoomStmt, updateMembershipForgetRoom}, {&s.selectLocalServerInRoomStmt, selectLocalServerInRoomSQL}, {&s.selectServerInRoomStmt, selectServerInRoomSQL}, + {&s.deleteMembershipStmt, deleteMembershipSQL}, }.Prepare(db) } @@ -394,3 +399,13 @@ func (s *membershipStatements) SelectServerInRoom( } return roomNID == nid, nil } + +func (s *membershipStatements) DeleteMembership( + ctx context.Context, txn *sql.Tx, + roomNID types.RoomNID, targetUserNID types.EventStateKeyNID, +) error { + _, err := sqlutil.TxStmt(txn, s.deleteMembershipStmt).ExecContext( + ctx, roomNID, targetUserNID, + ) + return err +} diff --git a/roomserver/storage/shared/membership_updater.go b/roomserver/storage/shared/membership_updater.go index 67905d8fc..9b61fa572 100644 --- a/roomserver/storage/shared/membership_updater.go +++ b/roomserver/storage/shared/membership_updater.go @@ -90,6 +90,10 @@ func (u *MembershipUpdater) IsKnock() bool { return u.oldMembership == tables.MembershipStateKnock } +func (u *MembershipUpdater) Delete() error { + return u.d.MembershipTable.DeleteMembership(u.ctx, u.txn, u.roomNID, u.targetUserNID) +} + func (u *MembershipUpdater) Update(newMembership tables.MembershipState, event *gomatrixserverlib.Event) (bool, []string, error) { var inserted bool var retired []string diff --git a/roomserver/storage/sqlite3/membership_table.go b/roomserver/storage/sqlite3/membership_table.go index 6f0fe8b64..02cd9215d 100644 --- a/roomserver/storage/sqlite3/membership_table.go +++ b/roomserver/storage/sqlite3/membership_table.go @@ -119,6 +119,9 @@ const selectServerInRoomSQL = "" + " JOIN roomserver_event_state_keys ON roomserver_membership.target_nid = roomserver_event_state_keys.event_state_key_nid" + " WHERE membership_nid = $1 AND room_nid = $2 AND event_state_key LIKE '%:' || $3 LIMIT 1" +const deleteMembershipSQL = "" + + "DELETE FROM roomserver_membership WHERE room_nid = $1 AND target_nid = $2" + type membershipStatements struct { db *sql.DB insertMembershipStmt *sql.Stmt @@ -134,6 +137,7 @@ type membershipStatements struct { updateMembershipForgetRoomStmt *sql.Stmt selectLocalServerInRoomStmt *sql.Stmt selectServerInRoomStmt *sql.Stmt + deleteMembershipStmt *sql.Stmt } func CreateMembershipTable(db *sql.DB) error { @@ -160,6 +164,7 @@ func PrepareMembershipTable(db *sql.DB) (tables.Membership, error) { {&s.updateMembershipForgetRoomStmt, updateMembershipForgetRoom}, {&s.selectLocalServerInRoomStmt, selectLocalServerInRoomSQL}, {&s.selectServerInRoomStmt, selectServerInRoomSQL}, + {&s.deleteMembershipStmt, deleteMembershipSQL}, }.Prepare(db) } @@ -373,3 +378,13 @@ func (s *membershipStatements) SelectServerInRoom(ctx context.Context, txn *sql. } return roomNID == nid, nil } + +func (s *membershipStatements) DeleteMembership( + ctx context.Context, txn *sql.Tx, + roomNID types.RoomNID, targetUserNID types.EventStateKeyNID, +) error { + _, err := sqlutil.TxStmt(txn, s.deleteMembershipStmt).ExecContext( + ctx, roomNID, targetUserNID, + ) + return err +} diff --git a/roomserver/storage/tables/interface.go b/roomserver/storage/tables/interface.go index 116e11c4e..d228257b7 100644 --- a/roomserver/storage/tables/interface.go +++ b/roomserver/storage/tables/interface.go @@ -133,6 +133,7 @@ type Membership interface { UpdateForgetMembership(ctx context.Context, txn *sql.Tx, roomNID types.RoomNID, targetUserNID types.EventStateKeyNID, forget bool) error SelectLocalServerInRoom(ctx context.Context, txn *sql.Tx, roomNID types.RoomNID) (bool, error) SelectServerInRoom(ctx context.Context, txn *sql.Tx, roomNID types.RoomNID, serverName gomatrixserverlib.ServerName) (bool, error) + DeleteMembership(ctx context.Context, txn *sql.Tx, roomNID types.RoomNID, targetUserNID types.EventStateKeyNID) error } type Published interface {