From 6650553402af9d1368e5e819799536efbc283523 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Fri, 30 Sep 2022 10:22:16 +0100 Subject: [PATCH] Remove `GetFilter` from snapshot --- syncapi/routing/filter.go | 8 +------- syncapi/storage/interface.go | 8 ++++---- syncapi/storage/shared/snapshot.go | 6 ------ syncapi/storage/shared/syncserver.go | 6 ++++++ syncapi/sync/request.go | 9 +-------- 5 files changed, 12 insertions(+), 25 deletions(-) diff --git a/syncapi/routing/filter.go b/syncapi/routing/filter.go index bb506ec39..f5acdbde3 100644 --- a/syncapi/routing/filter.go +++ b/syncapi/routing/filter.go @@ -45,14 +45,8 @@ func GetFilter( return jsonerror.InternalServerError() } - snapshot, err := syncDB.NewDatabaseSnapshot(req.Context()) - if err != nil { - return jsonerror.InternalServerError() - } - defer snapshot.Rollback() // nolint:errcheck - filter := gomatrixserverlib.DefaultFilter() - if err := snapshot.GetFilter(req.Context(), &filter, localpart, filterID); err != nil { + if err := syncDB.GetFilter(req.Context(), &filter, localpart, filterID); err != nil { //TODO better error handling. This error message is *probably* right, // but if there are obscure db errors, this will also be returned, // even though it is not correct. diff --git a/syncapi/storage/interface.go b/syncapi/storage/interface.go index 14b8dbc0b..3e9a5edb1 100644 --- a/syncapi/storage/interface.go +++ b/syncapi/storage/interface.go @@ -90,10 +90,6 @@ type DatabaseSnapshot interface { // SendToDeviceUpdatesForSync returns a list of send-to-device updates. It returns the // relevant events within the given ranges for the supplied user ID and device ID. SendToDeviceUpdatesForSync(ctx context.Context, userID, deviceID string, from, to types.StreamPosition) (pos types.StreamPosition, events []types.SendToDeviceEvent, err error) - // GetFilter looks up the filter associated with a given local user and filter ID - // and populates the target filter. Otherwise returns an error if no such filter exists - // or if there was an error talking to the database. - GetFilter(ctx context.Context, target *gomatrixserverlib.Filter, localpart string, filterID string) error // GetRoomReceipts gets all receipts for a given roomID GetRoomReceipts(ctx context.Context, roomIDs []string, streamPos types.StreamPosition) ([]types.OutputReceiptEvent, error) SelectContextEvent(ctx context.Context, roomID, eventID string) (int, gomatrixserverlib.HeaderedEvent, error) @@ -162,6 +158,10 @@ type Database interface { // CleanSendToDeviceUpdates removes all send-to-device messages BEFORE the specified // from position, preventing the send-to-device table from growing indefinitely. CleanSendToDeviceUpdates(ctx context.Context, userID, deviceID string, before types.StreamPosition) (err error) + // GetFilter looks up the filter associated with a given local user and filter ID + // and populates the target filter. Otherwise returns an error if no such filter exists + // or if there was an error talking to the database. + GetFilter(ctx context.Context, target *gomatrixserverlib.Filter, localpart string, filterID string) error // PutFilter puts the passed filter into the database. // Returns the filterID as a string. Otherwise returns an error if something // goes wrong. diff --git a/syncapi/storage/shared/snapshot.go b/syncapi/storage/shared/snapshot.go index deac73482..834d9d869 100644 --- a/syncapi/storage/shared/snapshot.go +++ b/syncapi/storage/shared/snapshot.go @@ -253,12 +253,6 @@ func (d *DatabaseSnapshot) StreamToTopologicalPosition( } } -func (d *DatabaseSnapshot) GetFilter( - ctx context.Context, target *gomatrixserverlib.Filter, localpart string, filterID string, -) error { - return d.Filter.SelectFilter(ctx, d.txn, target, localpart, filterID) -} - // GetBackwardTopologyPos retrieves the backward topology position, i.e. the position of the // oldest event in the room's topology. func (d *DatabaseSnapshot) GetBackwardTopologyPos( diff --git a/syncapi/storage/shared/syncserver.go b/syncapi/storage/shared/syncserver.go index 8ad7b32ab..3f5bf938d 100644 --- a/syncapi/storage/shared/syncserver.go +++ b/syncapi/storage/shared/syncserver.go @@ -333,6 +333,12 @@ func (d *Database) updateRoomState( return nil } +func (d *Database) GetFilter( + ctx context.Context, target *gomatrixserverlib.Filter, localpart string, filterID string, +) error { + return d.Filter.SelectFilter(ctx, nil, target, localpart, filterID) +} + func (d *Database) PutFilter( ctx context.Context, localpart string, filter *gomatrixserverlib.Filter, ) (string, error) { diff --git a/syncapi/sync/request.go b/syncapi/sync/request.go index 8b6d78619..268ed70c6 100644 --- a/syncapi/sync/request.go +++ b/syncapi/sync/request.go @@ -48,13 +48,6 @@ func newSyncRequest(req *http.Request, device userapi.Device, syncDB storage.Dat } } - snapshot, err := syncDB.NewDatabaseSnapshot(req.Context()) - if err != nil { - logrus.WithError(err).Error("Failed to acquire database snapshot for sync request") - return nil, err - } - defer snapshot.Rollback() // nolint:errcheck - // Create a default filter and apply a stored filter on top of it (if specified) filter := gomatrixserverlib.DefaultFilter() filterQuery := req.URL.Query().Get("filter") @@ -71,7 +64,7 @@ func newSyncRequest(req *http.Request, device userapi.Device, syncDB storage.Dat util.GetLogger(req.Context()).WithError(err).Error("gomatrixserverlib.SplitID failed") return nil, fmt.Errorf("gomatrixserverlib.SplitID: %w", err) } - if err := snapshot.GetFilter(req.Context(), &filter, localpart, filterQuery); err != nil && err != sql.ErrNoRows { + if err := syncDB.GetFilter(req.Context(), &filter, localpart, filterQuery); err != nil && err != sql.ErrNoRows { util.GetLogger(req.Context()).WithError(err).Error("syncDB.GetFilter failed") return nil, fmt.Errorf("syncDB.GetFilter: %w", err) }