From 3db9e98456b3580f230035c186dc4216f2043908 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Wed, 2 Nov 2022 09:34:19 +0000 Subject: [PATCH] Don't limit `"state"` (#2849) This is apparently some incorrect behaviour that we built as a result of a spec bug (matrix-org/matrix-spec#1314) where we were applying a filter to the `"state"` section of the `/sync` response incorrectly. The client then has no way to know that the state was limited. This PR removes the state limiting, which probably also helps #2842. --- syncapi/routing/context.go | 1 - syncapi/routing/search.go | 2 +- syncapi/storage/postgres/current_room_state_table.go | 4 +--- syncapi/storage/postgres/output_room_events_table.go | 8 ++------ syncapi/storage/sqlite3/current_room_state_table.go | 3 ++- syncapi/storage/sqlite3/filtering.go | 6 ++++-- syncapi/storage/sqlite3/output_room_events_table.go | 2 +- syncapi/streams/stream_pdu.go | 9 ++++----- syncapi/sync/request.go | 1 - 9 files changed, 15 insertions(+), 21 deletions(-) diff --git a/syncapi/routing/context.go b/syncapi/routing/context.go index 0ed164c7e..095a868c7 100644 --- a/syncapi/routing/context.go +++ b/syncapi/routing/context.go @@ -93,7 +93,6 @@ func Context( } stateFilter := gomatrixserverlib.StateFilter{ - Limit: 100, NotSenders: filter.NotSenders, NotTypes: filter.NotTypes, Senders: filter.Senders, diff --git a/syncapi/routing/search.go b/syncapi/routing/search.go index aef355def..081ec6cb1 100644 --- a/syncapi/routing/search.go +++ b/syncapi/routing/search.go @@ -294,7 +294,7 @@ type SearchRequest struct { BeforeLimit int `json:"before_limit,omitempty"` IncludeProfile bool `json:"include_profile,omitempty"` } `json:"event_context"` - Filter gomatrixserverlib.StateFilter `json:"filter"` + Filter gomatrixserverlib.RoomEventFilter `json:"filter"` Groupings struct { GroupBy []struct { Key string `json:"key"` diff --git a/syncapi/storage/postgres/current_room_state_table.go b/syncapi/storage/postgres/current_room_state_table.go index 2ccf0be1a..48ed20021 100644 --- a/syncapi/storage/postgres/current_room_state_table.go +++ b/syncapi/storage/postgres/current_room_state_table.go @@ -91,8 +91,7 @@ const selectCurrentStateSQL = "" + " AND ( $4::text[] IS NULL OR type LIKE ANY($4) )" + " AND ( $5::text[] IS NULL OR NOT(type LIKE ANY($5)) )" + " AND ( $6::bool IS NULL OR contains_url = $6 )" + - " AND (event_id = ANY($7)) IS NOT TRUE" + - " LIMIT $8" + " AND (event_id = ANY($7)) IS NOT TRUE" const selectJoinedUsersSQL = "" + "SELECT room_id, state_key FROM syncapi_current_room_state WHERE type = 'm.room.member' AND membership = 'join'" @@ -290,7 +289,6 @@ func (s *currentRoomStateStatements) SelectCurrentState( pq.StringArray(filterConvertTypeWildcardToSQL(stateFilter.NotTypes)), stateFilter.ContainsURL, pq.StringArray(excludeEventIDs), - stateFilter.Limit, ) if err != nil { return nil, err diff --git a/syncapi/storage/postgres/output_room_events_table.go b/syncapi/storage/postgres/output_room_events_table.go index 0ecbdf4d2..3b69b26f6 100644 --- a/syncapi/storage/postgres/output_room_events_table.go +++ b/syncapi/storage/postgres/output_room_events_table.go @@ -144,8 +144,7 @@ const selectStateInRangeFilteredSQL = "" + " AND ( $6::text[] IS NULL OR type LIKE ANY($6) )" + " AND ( $7::text[] IS NULL OR NOT(type LIKE ANY($7)) )" + " AND ( $8::bool IS NULL OR contains_url = $8 )" + - " ORDER BY id ASC" + - " LIMIT $9" + " ORDER BY id ASC" // In order for us to apply the state updates correctly, rows need to be ordered in the order they were received (id). const selectStateInRangeSQL = "" + @@ -153,8 +152,7 @@ const selectStateInRangeSQL = "" + " FROM syncapi_output_room_events" + " WHERE (id > $1 AND id <= $2) AND (add_state_ids IS NOT NULL OR remove_state_ids IS NOT NULL)" + " AND room_id = ANY($3)" + - " ORDER BY id ASC" + - " LIMIT $4" + " ORDER BY id ASC" const deleteEventsForRoomSQL = "" + "DELETE FROM syncapi_output_room_events WHERE room_id = $1" @@ -264,13 +262,11 @@ func (s *outputRoomEventsStatements) SelectStateInRange( pq.StringArray(filterConvertTypeWildcardToSQL(stateFilter.Types)), pq.StringArray(filterConvertTypeWildcardToSQL(stateFilter.NotTypes)), stateFilter.ContainsURL, - stateFilter.Limit, ) } else { stmt := sqlutil.TxStmt(txn, s.selectStateInRangeStmt) rows, err = stmt.QueryContext( ctx, r.Low(), r.High(), pq.StringArray(roomIDs), - r.High()-r.Low(), ) } diff --git a/syncapi/storage/sqlite3/current_room_state_table.go b/syncapi/storage/sqlite3/current_room_state_table.go index ff45e786e..7a381f68b 100644 --- a/syncapi/storage/sqlite3/current_room_state_table.go +++ b/syncapi/storage/sqlite3/current_room_state_table.go @@ -277,7 +277,8 @@ func (s *currentRoomStateStatements) SelectCurrentState( }, stateFilter.Senders, stateFilter.NotSenders, stateFilter.Types, stateFilter.NotTypes, - excludeEventIDs, stateFilter.ContainsURL, stateFilter.Limit, FilterOrderNone, + excludeEventIDs, stateFilter.ContainsURL, 0, + FilterOrderNone, ) if err != nil { return nil, fmt.Errorf("s.prepareWithFilters: %w", err) diff --git a/syncapi/storage/sqlite3/filtering.go b/syncapi/storage/sqlite3/filtering.go index 05edb7b8c..17a37a2df 100644 --- a/syncapi/storage/sqlite3/filtering.go +++ b/syncapi/storage/sqlite3/filtering.go @@ -84,8 +84,10 @@ func prepareWithFilters( case FilterOrderDesc: query += " ORDER BY id DESC" } - query += fmt.Sprintf(" LIMIT $%d", offset+1) - params = append(params, limit) + if limit > 0 { + query += fmt.Sprintf(" LIMIT $%d", offset+1) + params = append(params, limit) + } var stmt *sql.Stmt var err error diff --git a/syncapi/storage/sqlite3/output_room_events_table.go b/syncapi/storage/sqlite3/output_room_events_table.go index 77c692ff0..1aa4bfff7 100644 --- a/syncapi/storage/sqlite3/output_room_events_table.go +++ b/syncapi/storage/sqlite3/output_room_events_table.go @@ -200,7 +200,7 @@ func (s *outputRoomEventsStatements) SelectStateInRange( s.db, txn, stmtSQL, inputParams, stateFilter.Senders, stateFilter.NotSenders, stateFilter.Types, stateFilter.NotTypes, - nil, stateFilter.ContainsURL, stateFilter.Limit, FilterOrderAsc, + nil, stateFilter.ContainsURL, 0, FilterOrderAsc, ) } else { stmt, params, err = prepareWithFilters( diff --git a/syncapi/streams/stream_pdu.go b/syncapi/streams/stream_pdu.go index 81f32301f..5ea2732f4 100644 --- a/syncapi/streams/stream_pdu.go +++ b/syncapi/streams/stream_pdu.go @@ -301,7 +301,7 @@ func (p *PDUStreamProvider) addRoomDeltaToResponse( } // Applies the history visibility rules - events, err := applyHistoryVisibilityFilter(ctx, snapshot, p.rsAPI, delta.RoomID, device.UserID, eventFilter.Limit, recentEvents) + events, err := applyHistoryVisibilityFilter(ctx, snapshot, p.rsAPI, delta.RoomID, device.UserID, recentEvents) if err != nil { logrus.WithError(err).Error("unable to apply history visibility filter") } @@ -378,12 +378,12 @@ func applyHistoryVisibilityFilter( snapshot storage.DatabaseTransaction, rsAPI roomserverAPI.SyncRoomserverAPI, roomID, userID string, - limit int, recentEvents []*gomatrixserverlib.HeaderedEvent, ) ([]*gomatrixserverlib.HeaderedEvent, error) { // We need to make sure we always include the latest states events, if they are in the timeline. // We grep at least limit * 2 events, to ensure we really get the needed events. - stateEvents, err := snapshot.CurrentState(ctx, roomID, &gomatrixserverlib.StateFilter{Limit: limit * 2}, nil) + filter := gomatrixserverlib.DefaultStateFilter() + stateEvents, err := snapshot.CurrentState(ctx, roomID, &filter, nil) if err != nil { // Not a fatal error, we can continue without the stateEvents, // they are only needed if there are state events in the timeline. @@ -521,7 +521,7 @@ func (p *PDUStreamProvider) getJoinResponseForCompleteSync( events := recentEvents // Only apply history visibility checks if the response is for joined rooms if !isPeek { - events, err = applyHistoryVisibilityFilter(ctx, snapshot, p.rsAPI, roomID, device.UserID, eventFilter.Limit, recentEvents) + events, err = applyHistoryVisibilityFilter(ctx, snapshot, p.rsAPI, roomID, device.UserID, recentEvents) if err != nil { logrus.WithError(err).Error("unable to apply history visibility filter") } @@ -601,7 +601,6 @@ func (p *PDUStreamProvider) lazyLoadMembers( } // Query missing membership events filter := gomatrixserverlib.DefaultStateFilter() - filter.Limit = stateFilter.Limit filter.Senders = &wantUsers filter.Types = &[]string{gomatrixserverlib.MRoomMember} memberships, err := snapshot.GetStateEventsForRoom(ctx, roomID, &filter) diff --git a/syncapi/sync/request.go b/syncapi/sync/request.go index 620dfdcdb..e5e5fdb5b 100644 --- a/syncapi/sync/request.go +++ b/syncapi/sync/request.go @@ -79,7 +79,6 @@ func newSyncRequest(req *http.Request, device userapi.Device, syncDB storage.Dat // for the rest of the data to trickle down. filter.AccountData.Limit = math.MaxInt32 filter.Room.AccountData.Limit = math.MaxInt32 - filter.Room.State.Limit = math.MaxInt32 } logger := util.GetLogger(req.Context()).WithFields(logrus.Fields{