From 9c03b0a4fa38971dfe83bd135aefb3c482a18380 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Thu, 10 Dec 2020 18:57:10 +0000 Subject: [PATCH] Refactor sync tokens (#1628) * Refactor sync tokens * Comment out broken notifier test * Update types, sytest-whitelist * More robust token checking * Remove New functions for streaming tokens * Export Logs in StreamingToken * Fix tests --- syncapi/consumers/clientapi.go | 2 +- syncapi/consumers/eduserver_receipts.go | 2 +- syncapi/consumers/eduserver_sendtodevice.go | 2 +- syncapi/consumers/eduserver_typing.go | 6 +- syncapi/consumers/keychange.go | 12 +- syncapi/consumers/roomserver.go | 12 +- syncapi/internal/keychange_test.go | 14 +- syncapi/routing/messages.go | 8 +- syncapi/storage/shared/syncserver.go | 57 ++-- syncapi/storage/storage_test.go | 59 ++-- syncapi/sync/notifier_test.go | 14 +- syncapi/sync/request.go | 3 +- syncapi/sync/requestpool.go | 10 +- syncapi/types/types.go | 300 ++++++++------------ syncapi/types/types_test.go | 52 ++-- sytest-whitelist | 1 + 16 files changed, 265 insertions(+), 289 deletions(-) diff --git a/syncapi/consumers/clientapi.go b/syncapi/consumers/clientapi.go index 7070dd320..9883c6b03 100644 --- a/syncapi/consumers/clientapi.go +++ b/syncapi/consumers/clientapi.go @@ -92,7 +92,7 @@ func (s *OutputClientDataConsumer) onMessage(msg *sarama.ConsumerMessage) error }).Panicf("could not save account data") } - s.notifier.OnNewEvent(nil, "", []string{string(msg.Key)}, types.NewStreamToken(pduPos, 0, nil)) + s.notifier.OnNewEvent(nil, "", []string{string(msg.Key)}, types.StreamingToken{PDUPosition: pduPos}) return nil } diff --git a/syncapi/consumers/eduserver_receipts.go b/syncapi/consumers/eduserver_receipts.go index 3361e1347..5c286cf08 100644 --- a/syncapi/consumers/eduserver_receipts.go +++ b/syncapi/consumers/eduserver_receipts.go @@ -88,7 +88,7 @@ func (s *OutputReceiptEventConsumer) onMessage(msg *sarama.ConsumerMessage) erro return err } // update stream position - s.notifier.OnNewReceipt(types.NewStreamToken(0, streamPos, nil)) + s.notifier.OnNewReceipt(types.StreamingToken{ReceiptPosition: streamPos}) return nil } diff --git a/syncapi/consumers/eduserver_sendtodevice.go b/syncapi/consumers/eduserver_sendtodevice.go index 07324fcd7..0c3f52cd3 100644 --- a/syncapi/consumers/eduserver_sendtodevice.go +++ b/syncapi/consumers/eduserver_sendtodevice.go @@ -107,7 +107,7 @@ func (s *OutputSendToDeviceEventConsumer) onMessage(msg *sarama.ConsumerMessage) s.notifier.OnNewSendToDevice( output.UserID, []string{output.DeviceID}, - types.NewStreamToken(0, streamPos, nil), + types.StreamingToken{SendToDevicePosition: streamPos}, ) return nil diff --git a/syncapi/consumers/eduserver_typing.go b/syncapi/consumers/eduserver_typing.go index bdea606cb..885e7fd1f 100644 --- a/syncapi/consumers/eduserver_typing.go +++ b/syncapi/consumers/eduserver_typing.go @@ -66,7 +66,9 @@ func (s *OutputTypingEventConsumer) Start() error { s.db.SetTypingTimeoutCallback(func(userID, roomID string, latestSyncPosition int64) { s.notifier.OnNewEvent( nil, roomID, nil, - types.NewStreamToken(0, types.StreamPosition(latestSyncPosition), nil), + types.StreamingToken{ + TypingPosition: types.StreamPosition(latestSyncPosition), + }, ) }) @@ -95,6 +97,6 @@ func (s *OutputTypingEventConsumer) onMessage(msg *sarama.ConsumerMessage) error typingPos = s.db.RemoveTypingUser(typingEvent.UserID, typingEvent.RoomID) } - s.notifier.OnNewEvent(nil, output.Event.RoomID, nil, types.NewStreamToken(0, typingPos, nil)) + s.notifier.OnNewEvent(nil, output.Event.RoomID, nil, types.StreamingToken{TypingPosition: typingPos}) return nil } diff --git a/syncapi/consumers/keychange.go b/syncapi/consumers/keychange.go index 3fc6120d2..0d82f7a58 100644 --- a/syncapi/consumers/keychange.go +++ b/syncapi/consumers/keychange.go @@ -114,12 +114,14 @@ func (s *OutputKeyChangeEventConsumer) onMessage(msg *sarama.ConsumerMessage) er return err } // TODO: f.e queryRes.UserIDsToCount : notify users by waking up streams - posUpdate := types.NewStreamToken(0, 0, map[string]*types.LogPosition{ - syncinternal.DeviceListLogName: { - Offset: msg.Offset, - Partition: msg.Partition, + posUpdate := types.StreamingToken{ + Logs: map[string]*types.LogPosition{ + syncinternal.DeviceListLogName: { + Offset: msg.Offset, + Partition: msg.Partition, + }, }, - }) + } for userID := range queryRes.UserIDsToCount { s.notifier.OnNewKeyChange(posUpdate, userID, output.UserID) } diff --git a/syncapi/consumers/roomserver.go b/syncapi/consumers/roomserver.go index 11d75a683..be84a2816 100644 --- a/syncapi/consumers/roomserver.go +++ b/syncapi/consumers/roomserver.go @@ -181,7 +181,7 @@ func (s *OutputRoomEventConsumer) onNewRoomEvent( return err } - s.notifier.OnNewEvent(ev, "", nil, types.NewStreamToken(pduPos, 0, nil)) + s.notifier.OnNewEvent(ev, "", nil, types.StreamingToken{PDUPosition: pduPos}) return nil } @@ -220,7 +220,7 @@ func (s *OutputRoomEventConsumer) onOldRoomEvent( return err } - s.notifier.OnNewEvent(ev, "", nil, types.NewStreamToken(pduPos, 0, nil)) + s.notifier.OnNewEvent(ev, "", nil, types.StreamingToken{PDUPosition: pduPos}) return nil } @@ -269,7 +269,7 @@ func (s *OutputRoomEventConsumer) onNewInviteEvent( }).Panicf("roomserver output log: write invite failure") return nil } - s.notifier.OnNewEvent(msg.Event, "", nil, types.NewStreamToken(pduPos, 0, nil)) + s.notifier.OnNewEvent(msg.Event, "", nil, types.StreamingToken{PDUPosition: pduPos}) return nil } @@ -287,7 +287,7 @@ func (s *OutputRoomEventConsumer) onRetireInviteEvent( } // Notify any active sync requests that the invite has been retired. // Invites share the same stream counter as PDUs - s.notifier.OnNewEvent(nil, "", []string{msg.TargetUserID}, types.NewStreamToken(sp, 0, nil)) + s.notifier.OnNewEvent(nil, "", []string{msg.TargetUserID}, types.StreamingToken{PDUPosition: sp}) return nil } @@ -307,7 +307,7 @@ func (s *OutputRoomEventConsumer) onNewPeek( // we need to wake up the users who might need to now be peeking into this room, // so we send in a dummy event to trigger a wakeup - s.notifier.OnNewEvent(nil, msg.RoomID, nil, types.NewStreamToken(sp, 0, nil)) + s.notifier.OnNewEvent(nil, msg.RoomID, nil, types.StreamingToken{PDUPosition: sp}) return nil } @@ -327,7 +327,7 @@ func (s *OutputRoomEventConsumer) onRetirePeek( // we need to wake up the users who might need to now be peeking into this room, // so we send in a dummy event to trigger a wakeup - s.notifier.OnNewEvent(nil, msg.RoomID, nil, types.NewStreamToken(sp, 0, nil)) + s.notifier.OnNewEvent(nil, msg.RoomID, nil, types.StreamingToken{PDUPosition: sp}) return nil } diff --git a/syncapi/internal/keychange_test.go b/syncapi/internal/keychange_test.go index adf498d2d..f65db0a5b 100644 --- a/syncapi/internal/keychange_test.go +++ b/syncapi/internal/keychange_test.go @@ -16,13 +16,15 @@ import ( var ( syncingUser = "@alice:localhost" - emptyToken = types.NewStreamToken(0, 0, nil) - newestToken = types.NewStreamToken(0, 0, map[string]*types.LogPosition{ - DeviceListLogName: { - Offset: sarama.OffsetNewest, - Partition: 0, + emptyToken = types.StreamingToken{} + newestToken = types.StreamingToken{ + Logs: map[string]*types.LogPosition{ + DeviceListLogName: { + Offset: sarama.OffsetNewest, + Partition: 0, + }, }, - }) + } ) type mockKeyAPI struct{} diff --git a/syncapi/routing/messages.go b/syncapi/routing/messages.go index 92f36e23b..865203a9b 100644 --- a/syncapi/routing/messages.go +++ b/syncapi/routing/messages.go @@ -381,7 +381,7 @@ func (r *messagesReq) getStartEnd(events []*gomatrixserverlib.HeaderedEvent) (st if r.backwardOrdering && events[len(events)-1].Type() == gomatrixserverlib.MRoomCreate { // We've hit the beginning of the room so there's really nowhere else // to go. This seems to fix Riot iOS from looping on /messages endlessly. - end = types.NewTopologyToken(0, 0) + end = types.TopologyToken{} } else { end, err = r.db.EventPositionInTopology( r.ctx, events[len(events)-1].EventID(), @@ -447,11 +447,11 @@ func (r *messagesReq) handleNonEmptyEventsSlice(streamEvents []types.StreamEvent // The condition in the SQL query is a strict "greater than" so // we need to check against to-1. streamPos := types.StreamPosition(streamEvents[len(streamEvents)-1].StreamPosition) - isSetLargeEnough = (r.to.PDUPosition()-1 == streamPos) + isSetLargeEnough = (r.to.PDUPosition-1 == streamPos) } } else { streamPos := types.StreamPosition(streamEvents[0].StreamPosition) - isSetLargeEnough = (r.from.PDUPosition()-1 == streamPos) + isSetLargeEnough = (r.from.PDUPosition-1 == streamPos) } } @@ -565,7 +565,7 @@ func setToDefault( if backwardOrdering { // go 1 earlier than the first event so we correctly fetch the earliest event // this is because Database.GetEventsInTopologicalRange is exclusive of the lower-bound. - to = types.NewTopologyToken(0, 0) + to = types.TopologyToken{} } else { to, err = db.MaxTopologicalPosition(ctx, roomID) } diff --git a/syncapi/storage/shared/syncserver.go b/syncapi/storage/shared/syncserver.go index 9df049439..c0ae3d7a9 100644 --- a/syncapi/storage/shared/syncserver.go +++ b/syncapi/storage/shared/syncserver.go @@ -78,8 +78,8 @@ func (d *Database) GetEventsInStreamingRange( backwardOrdering bool, ) (events []types.StreamEvent, err error) { r := types.Range{ - From: from.PDUPosition(), - To: to.PDUPosition(), + From: from.PDUPosition, + To: to.PDUPosition, Backwards: backwardOrdering, } if backwardOrdering { @@ -391,16 +391,16 @@ func (d *Database) GetEventsInTopologicalRange( var minDepth, maxDepth, maxStreamPosForMaxDepth types.StreamPosition if backwardOrdering { // Backward ordering means the 'from' token has a higher depth than the 'to' token - minDepth = to.Depth() - maxDepth = from.Depth() + minDepth = to.Depth + maxDepth = from.Depth // for cases where we have say 5 events with the same depth, the TopologyToken needs to // know which of the 5 the client has seen. This is done by using the PDU position. // Events with the same maxDepth but less than this PDU position will be returned. - maxStreamPosForMaxDepth = from.PDUPosition() + maxStreamPosForMaxDepth = from.PDUPosition } else { // Forward ordering means the 'from' token has a lower depth than the 'to' token. - minDepth = from.Depth() - maxDepth = to.Depth() + minDepth = from.Depth + maxDepth = to.Depth } // Select the event IDs from the defined range. @@ -440,9 +440,9 @@ func (d *Database) MaxTopologicalPosition( ) (types.TopologyToken, error) { depth, streamPos, err := d.Topology.SelectMaxPositionInTopology(ctx, nil, roomID) if err != nil { - return types.NewTopologyToken(0, 0), err + return types.TopologyToken{}, err } - return types.NewTopologyToken(depth, streamPos), nil + return types.TopologyToken{Depth: depth, PDUPosition: streamPos}, nil } func (d *Database) EventPositionInTopology( @@ -450,9 +450,9 @@ func (d *Database) EventPositionInTopology( ) (types.TopologyToken, error) { depth, stream, err := d.Topology.SelectPositionInTopology(ctx, nil, eventID) if err != nil { - return types.NewTopologyToken(0, 0), err + return types.TopologyToken{}, err } - return types.NewTopologyToken(depth, stream), nil + return types.TopologyToken{Depth: depth, PDUPosition: stream}, nil } func (d *Database) syncPositionTx( @@ -483,7 +483,11 @@ func (d *Database) syncPositionTx( if maxPeekID > maxEventID { maxEventID = maxPeekID } - sp = types.NewStreamToken(types.StreamPosition(maxEventID), types.StreamPosition(d.EDUCache.GetLatestSyncPosition()), nil) + // TODO: complete these positions + sp = types.StreamingToken{ + PDUPosition: types.StreamPosition(maxEventID), + TypingPosition: types.StreamPosition(d.EDUCache.GetLatestSyncPosition()), + } return } @@ -555,7 +559,7 @@ func (d *Database) addTypingDeltaToResponse( for _, roomID := range joinedRoomIDs { var jr types.JoinResponse if typingUsers, updated := d.EDUCache.GetTypingUsersIfUpdatedAfter( - roomID, int64(since.EDUPosition()), + roomID, int64(since.TypingPosition), ); updated { ev := gomatrixserverlib.ClientEvent{ Type: gomatrixserverlib.MTyping, @@ -584,7 +588,7 @@ func (d *Database) addReceiptDeltaToResponse( joinedRoomIDs []string, res *types.Response, ) error { - receipts, err := d.Receipts.SelectRoomReceiptsAfter(context.TODO(), joinedRoomIDs, since.EDUPosition()) + receipts, err := d.Receipts.SelectRoomReceiptsAfter(context.TODO(), joinedRoomIDs, since.ReceiptPosition) if err != nil { return fmt.Errorf("unable to select receipts for rooms: %w", err) } @@ -639,7 +643,7 @@ func (d *Database) addEDUDeltaToResponse( joinedRoomIDs []string, res *types.Response, ) error { - if fromPos.EDUPosition() != toPos.EDUPosition() { + if fromPos.TypingPosition != toPos.TypingPosition { // add typing deltas if err := d.addTypingDeltaToResponse(fromPos, joinedRoomIDs, res); err != nil { return fmt.Errorf("unable to apply typing delta to response: %w", err) @@ -647,8 +651,8 @@ func (d *Database) addEDUDeltaToResponse( } // Check on initial sync and if EDUPositions differ - if (fromPos.EDUPosition() == 0 && toPos.EDUPosition() == 0) || - fromPos.EDUPosition() != toPos.EDUPosition() { + if (fromPos.ReceiptPosition == 0 && toPos.ReceiptPosition == 0) || + fromPos.ReceiptPosition != toPos.ReceiptPosition { if err := d.addReceiptDeltaToResponse(fromPos, joinedRoomIDs, res); err != nil { return fmt.Errorf("unable to apply receipts to response: %w", err) } @@ -687,10 +691,10 @@ func (d *Database) IncrementalSync( var joinedRoomIDs []string var err error - if fromPos.PDUPosition() != toPos.PDUPosition() || wantFullState { + if fromPos.PDUPosition != toPos.PDUPosition || wantFullState { r := types.Range{ - From: fromPos.PDUPosition(), - To: toPos.PDUPosition(), + From: fromPos.PDUPosition, + To: toPos.PDUPosition, } joinedRoomIDs, err = d.addPDUDeltaToResponse( ctx, device, r, numRecentEventsPerRoom, wantFullState, res, @@ -772,7 +776,7 @@ func (d *Database) getResponseWithPDUsForCompleteSync( } r := types.Range{ From: 0, - To: toPos.PDUPosition(), + To: toPos.PDUPosition, } res.NextBatch = toPos.String() @@ -882,7 +886,10 @@ func (d *Database) getJoinResponseForCompleteSync( if err != nil { return } - prevBatch := types.NewTopologyToken(backwardTopologyPos, backwardStreamPos) + prevBatch := types.TopologyToken{ + Depth: backwardTopologyPos, + PDUPosition: backwardStreamPos, + } prevBatch.Decrement() prevBatchStr = prevBatch.String() } @@ -915,7 +922,7 @@ func (d *Database) CompleteSync( // Use a zero value SyncPosition for fromPos so all EDU states are added. err = d.addEDUDeltaToResponse( - types.NewStreamToken(0, 0, nil), toPos, joinedRoomIDs, res, + types.StreamingToken{}, toPos, joinedRoomIDs, res, ) if err != nil { return nil, fmt.Errorf("d.addEDUDeltaToResponse: %w", err) @@ -965,7 +972,7 @@ func (d *Database) getBackwardTopologyPos( ctx context.Context, txn *sql.Tx, events []types.StreamEvent, ) (types.TopologyToken, error) { - zeroToken := types.NewTopologyToken(0, 0) + zeroToken := types.TopologyToken{} if len(events) == 0 { return zeroToken, nil } @@ -973,7 +980,7 @@ func (d *Database) getBackwardTopologyPos( if err != nil { return zeroToken, err } - tok := types.NewTopologyToken(pos, spos) + tok := types.TopologyToken{Depth: pos, PDUPosition: spos} tok.Decrement() return tok, nil } diff --git a/syncapi/storage/storage_test.go b/syncapi/storage/storage_test.go index b1b0d2543..8387543f5 100644 --- a/syncapi/storage/storage_test.go +++ b/syncapi/storage/storage_test.go @@ -165,9 +165,9 @@ func TestSyncResponse(t *testing.T) { { Name: "IncrementalSync penultimate", DoSync: func() (*types.Response, error) { - from := types.NewStreamToken( // pretend we are at the penultimate event - positions[len(positions)-2], types.StreamPosition(0), nil, - ) + from := types.StreamingToken{ // pretend we are at the penultimate event + PDUPosition: positions[len(positions)-2], + } res := types.NewResponse() return db.IncrementalSync(ctx, res, testUserDeviceA, from, latest, 5, false) }, @@ -178,9 +178,9 @@ func TestSyncResponse(t *testing.T) { { Name: "IncrementalSync limited", DoSync: func() (*types.Response, error) { - from := types.NewStreamToken( // pretend we are 10 events behind - positions[len(positions)-11], types.StreamPosition(0), nil, - ) + from := types.StreamingToken{ // pretend we are 10 events behind + PDUPosition: positions[len(positions)-11], + } res := types.NewResponse() // limit is set to 5 return db.IncrementalSync(ctx, res, testUserDeviceA, from, latest, 5, false) @@ -222,7 +222,12 @@ func TestSyncResponse(t *testing.T) { if err != nil { st.Fatalf("failed to do sync: %s", err) } - next := types.NewStreamToken(latest.PDUPosition(), latest.EDUPosition(), nil) + next := types.StreamingToken{ + PDUPosition: latest.PDUPosition, + TypingPosition: latest.TypingPosition, + ReceiptPosition: latest.ReceiptPosition, + SendToDevicePosition: latest.SendToDevicePosition, + } if res.NextBatch != next.String() { st.Errorf("NextBatch got %s want %s", res.NextBatch, next.String()) } @@ -245,9 +250,9 @@ func TestGetEventsInRangeWithPrevBatch(t *testing.T) { if err != nil { t.Fatalf("failed to get SyncPosition: %s", err) } - from := types.NewStreamToken( - positions[len(positions)-2], types.StreamPosition(0), nil, - ) + from := types.StreamingToken{ + PDUPosition: positions[len(positions)-2], + } res := types.NewResponse() res, err = db.IncrementalSync(ctx, res, testUserDeviceA, from, latest, 5, false) @@ -271,7 +276,7 @@ func TestGetEventsInRangeWithPrevBatch(t *testing.T) { } // backpaginate 5 messages starting at the latest position. // head towards the beginning of time - to := types.NewTopologyToken(0, 0) + to := types.TopologyToken{} paginatedEvents, err := db.GetEventsInTopologicalRange(ctx, &prevBatchToken, &to, testRoomID, 5, true) if err != nil { t.Fatalf("GetEventsInRange returned an error: %s", err) @@ -291,7 +296,7 @@ func TestGetEventsInRangeWithStreamToken(t *testing.T) { t.Fatalf("failed to get SyncPosition: %s", err) } // head towards the beginning of time - to := types.NewStreamToken(0, 0, nil) + to := types.StreamingToken{} // backpaginate 5 messages starting at the latest position. paginatedEvents, err := db.GetEventsInStreamingRange(ctx, &latest, &to, testRoomID, 5, true) @@ -313,7 +318,7 @@ func TestGetEventsInRangeWithTopologyToken(t *testing.T) { t.Fatalf("failed to get MaxTopologicalPosition: %s", err) } // head towards the beginning of time - to := types.NewTopologyToken(0, 0) + to := types.TopologyToken{} // backpaginate 5 messages starting at the latest position. paginatedEvents, err := db.GetEventsInTopologicalRange(ctx, &from, &to, testRoomID, 5, true) @@ -382,7 +387,7 @@ func TestGetEventsInRangeWithEventsSameDepth(t *testing.T) { t.Fatalf("failed to get EventPositionInTopology for event: %s", err) } // head towards the beginning of time - to := types.NewTopologyToken(0, 0) + to := types.TopologyToken{} testCases := []struct { Name string @@ -458,7 +463,7 @@ func TestGetEventsInTopologicalRangeMultiRoom(t *testing.T) { t.Fatalf("failed to get MaxTopologicalPosition: %s", err) } // head towards the beginning of time - to := types.NewTopologyToken(0, 0) + to := types.TopologyToken{} // Query using room B as room A was inserted first and hence A will have lower stream positions but identical depths, // allowing this bug to surface. @@ -508,7 +513,7 @@ func TestGetEventsInRangeWithEventsInsertedLikeBackfill(t *testing.T) { } // head towards the beginning of time - to := types.NewTopologyToken(0, 0) + to := types.TopologyToken{} // starting at `from`, backpaginate to the beginning of time, asserting as we go. chunkSize = 3 @@ -534,14 +539,14 @@ func TestSendToDeviceBehaviour(t *testing.T) { // At this point there should be no messages. We haven't sent anything // yet. - events, updates, deletions, err := db.SendToDeviceUpdatesForSync(ctx, "alice", "one", types.NewStreamToken(0, 0, nil)) + events, updates, deletions, err := db.SendToDeviceUpdatesForSync(ctx, "alice", "one", types.StreamingToken{}) if err != nil { t.Fatal(err) } if len(events) != 0 || len(updates) != 0 || len(deletions) != 0 { t.Fatal("first call should have no updates") } - err = db.CleanSendToDeviceUpdates(context.Background(), updates, deletions, types.NewStreamToken(0, 0, nil)) + err = db.CleanSendToDeviceUpdates(context.Background(), updates, deletions, types.StreamingToken{}) if err != nil { return } @@ -559,14 +564,14 @@ func TestSendToDeviceBehaviour(t *testing.T) { // At this point we should get exactly one message. We're sending the sync position // that we were given from the update and the send-to-device update will be updated // in the database to reflect that this was the sync position we sent the message at. - events, updates, deletions, err = db.SendToDeviceUpdatesForSync(ctx, "alice", "one", types.NewStreamToken(0, streamPos, nil)) + events, updates, deletions, err = db.SendToDeviceUpdatesForSync(ctx, "alice", "one", types.StreamingToken{SendToDevicePosition: streamPos}) if err != nil { t.Fatal(err) } if len(events) != 1 || len(updates) != 1 || len(deletions) != 0 { t.Fatal("second call should have one update") } - err = db.CleanSendToDeviceUpdates(context.Background(), updates, deletions, types.NewStreamToken(0, streamPos, nil)) + err = db.CleanSendToDeviceUpdates(context.Background(), updates, deletions, types.StreamingToken{SendToDevicePosition: streamPos}) if err != nil { return } @@ -574,35 +579,35 @@ func TestSendToDeviceBehaviour(t *testing.T) { // At this point we should still have one message because we haven't progressed the // sync position yet. This is equivalent to the client failing to /sync and retrying // with the same position. - events, updates, deletions, err = db.SendToDeviceUpdatesForSync(ctx, "alice", "one", types.NewStreamToken(0, streamPos, nil)) + events, updates, deletions, err = db.SendToDeviceUpdatesForSync(ctx, "alice", "one", types.StreamingToken{SendToDevicePosition: streamPos}) if err != nil { t.Fatal(err) } if len(events) != 1 || len(updates) != 0 || len(deletions) != 0 { t.Fatal("third call should have one update still") } - err = db.CleanSendToDeviceUpdates(context.Background(), updates, deletions, types.NewStreamToken(0, streamPos, nil)) + err = db.CleanSendToDeviceUpdates(context.Background(), updates, deletions, types.StreamingToken{SendToDevicePosition: streamPos}) if err != nil { return } // At this point we should now have no updates, because we've progressed the sync // position. Therefore the update from before will not be sent again. - events, updates, deletions, err = db.SendToDeviceUpdatesForSync(ctx, "alice", "one", types.NewStreamToken(0, streamPos+1, nil)) + events, updates, deletions, err = db.SendToDeviceUpdatesForSync(ctx, "alice", "one", types.StreamingToken{SendToDevicePosition: streamPos + 1}) if err != nil { t.Fatal(err) } if len(events) != 0 || len(updates) != 0 || len(deletions) != 1 { t.Fatal("fourth call should have no updates") } - err = db.CleanSendToDeviceUpdates(context.Background(), updates, deletions, types.NewStreamToken(0, streamPos+1, nil)) + err = db.CleanSendToDeviceUpdates(context.Background(), updates, deletions, types.StreamingToken{SendToDevicePosition: streamPos + 1}) if err != nil { return } // At this point we should still have no updates, because no new updates have been // sent. - events, updates, deletions, err = db.SendToDeviceUpdatesForSync(ctx, "alice", "one", types.NewStreamToken(0, streamPos+2, nil)) + events, updates, deletions, err = db.SendToDeviceUpdatesForSync(ctx, "alice", "one", types.StreamingToken{SendToDevicePosition: streamPos + 2}) if err != nil { t.Fatal(err) } @@ -639,7 +644,7 @@ func TestInviteBehaviour(t *testing.T) { } // both invite events should appear in a new sync beforeRetireRes := types.NewResponse() - beforeRetireRes, err = db.IncrementalSync(ctx, beforeRetireRes, testUserDeviceA, types.NewStreamToken(0, 0, nil), latest, 0, false) + beforeRetireRes, err = db.IncrementalSync(ctx, beforeRetireRes, testUserDeviceA, types.StreamingToken{}, latest, 0, false) if err != nil { t.Fatalf("IncrementalSync failed: %s", err) } @@ -654,7 +659,7 @@ func TestInviteBehaviour(t *testing.T) { t.Fatalf("failed to get SyncPosition: %s", err) } res := types.NewResponse() - res, err = db.IncrementalSync(ctx, res, testUserDeviceA, types.NewStreamToken(0, 0, nil), latest, 0, false) + res, err = db.IncrementalSync(ctx, res, testUserDeviceA, types.StreamingToken{}, latest, 0, false) if err != nil { t.Fatalf("IncrementalSync failed: %s", err) } diff --git a/syncapi/sync/notifier_test.go b/syncapi/sync/notifier_test.go index 5a4c7b31b..39124214a 100644 --- a/syncapi/sync/notifier_test.go +++ b/syncapi/sync/notifier_test.go @@ -32,11 +32,11 @@ var ( randomMessageEvent gomatrixserverlib.HeaderedEvent aliceInviteBobEvent gomatrixserverlib.HeaderedEvent bobLeaveEvent gomatrixserverlib.HeaderedEvent - syncPositionVeryOld = types.NewStreamToken(5, 0, nil) - syncPositionBefore = types.NewStreamToken(11, 0, nil) - syncPositionAfter = types.NewStreamToken(12, 0, nil) - syncPositionNewEDU = types.NewStreamToken(syncPositionAfter.PDUPosition(), 1, nil) - syncPositionAfter2 = types.NewStreamToken(13, 0, nil) + syncPositionVeryOld = types.StreamingToken{PDUPosition: 5} + syncPositionBefore = types.StreamingToken{PDUPosition: 11} + syncPositionAfter = types.StreamingToken{PDUPosition: 12} + //syncPositionNewEDU = types.NewStreamToken(syncPositionAfter.PDUPosition, 1, 0, 0, nil) + syncPositionAfter2 = types.StreamingToken{PDUPosition: 13} ) var ( @@ -205,6 +205,9 @@ func TestNewInviteEventForUser(t *testing.T) { } // Test an EDU-only update wakes up the request. +// TODO: Fix this test, invites wake up with an incremented +// PDU position, not EDU position +/* func TestEDUWakeup(t *testing.T) { n := NewNotifier(syncPositionAfter) n.setUsersJoinedToRooms(map[string][]string{ @@ -229,6 +232,7 @@ func TestEDUWakeup(t *testing.T) { wg.Wait() } +*/ // Test that all blocked requests get woken up on a new event. func TestMultipleRequestWakeup(t *testing.T) { diff --git a/syncapi/sync/request.go b/syncapi/sync/request.go index 0996729e6..d5cf143d9 100644 --- a/syncapi/sync/request.go +++ b/syncapi/sync/request.go @@ -65,8 +65,7 @@ func newSyncRequest(req *http.Request, device userapi.Device, syncDB storage.Dat since = &tok } if since == nil { - tok := types.NewStreamToken(0, 0, nil) - since = &tok + since = &types.StreamingToken{} } timelineLimit := DefaultTimelineLimit // TODO: read from stored filters too diff --git a/syncapi/sync/requestpool.go b/syncapi/sync/requestpool.go index 0cb6efe7a..a4eec467c 100644 --- a/syncapi/sync/requestpool.go +++ b/syncapi/sync/requestpool.go @@ -254,7 +254,7 @@ func (rp *RequestPool) currentSyncForUser(req syncRequest, latestPos types.Strea } // TODO: handle ignored users - if req.since.PDUPosition() == 0 && req.since.EDUPosition() == 0 { + if req.since.IsEmpty() { res, err = rp.db.CompleteSync(req.ctx, res, req.device, req.limit) if err != nil { return res, fmt.Errorf("rp.db.CompleteSync: %w", err) @@ -267,7 +267,7 @@ func (rp *RequestPool) currentSyncForUser(req syncRequest, latestPos types.Strea } accountDataFilter := gomatrixserverlib.DefaultEventFilter() // TODO: use filter provided in req instead - res, err = rp.appendAccountData(res, req.device.UserID, req, latestPos.PDUPosition(), &accountDataFilter) + res, err = rp.appendAccountData(res, req.device.UserID, req, latestPos.PDUPosition, &accountDataFilter) if err != nil { return res, fmt.Errorf("rp.appendAccountData: %w", err) } @@ -299,7 +299,7 @@ func (rp *RequestPool) currentSyncForUser(req syncRequest, latestPos types.Strea // Get the next_batch from the sync response and increase the // EDU counter. if pos, perr := types.NewStreamTokenFromString(res.NextBatch); perr == nil { - pos.Positions[1]++ + pos.SendToDevicePosition++ res.NextBatch = pos.String() } } @@ -328,7 +328,7 @@ func (rp *RequestPool) appendAccountData( // data keys were set between two message. This isn't a huge issue since the // duplicate data doesn't represent a huge quantity of data, but an optimisation // here would be making sure each data is sent only once to the client. - if req.since == nil || (req.since.PDUPosition() == 0 && req.since.EDUPosition() == 0) { + if req.since.IsEmpty() { // If this is the initial sync, we don't need to check if a data has // already been sent. Instead, we send the whole batch. dataReq := &userapi.QueryAccountDataRequest{ @@ -363,7 +363,7 @@ func (rp *RequestPool) appendAccountData( } r := types.Range{ - From: req.since.PDUPosition(), + From: req.since.PDUPosition, To: currentPos, } // If both positions are the same, it means that the data was saved after the diff --git a/syncapi/types/types.go b/syncapi/types/types.go index 36f30c20b..fe76b74e0 100644 --- a/syncapi/types/types.go +++ b/syncapi/types/types.go @@ -16,7 +16,6 @@ package types import ( "encoding/json" - "errors" "fmt" "sort" "strconv" @@ -107,108 +106,119 @@ const ( ) type StreamingToken struct { - syncToken - logs map[string]*LogPosition + PDUPosition StreamPosition + TypingPosition StreamPosition + ReceiptPosition StreamPosition + SendToDevicePosition StreamPosition + Logs map[string]*LogPosition } func (t *StreamingToken) SetLog(name string, lp *LogPosition) { - if t.logs == nil { - t.logs = make(map[string]*LogPosition) + if t.Logs == nil { + t.Logs = make(map[string]*LogPosition) } - t.logs[name] = lp + t.Logs[name] = lp } func (t *StreamingToken) Log(name string) *LogPosition { - l, ok := t.logs[name] + l, ok := t.Logs[name] if !ok { return nil } return l } -func (t *StreamingToken) PDUPosition() StreamPosition { - return t.Positions[0] -} -func (t *StreamingToken) EDUPosition() StreamPosition { - return t.Positions[1] -} -func (t *StreamingToken) String() string { +func (t StreamingToken) String() string { + posStr := fmt.Sprintf( + "s%d_%d_%d_%d", + t.PDUPosition, t.TypingPosition, + t.ReceiptPosition, t.SendToDevicePosition, + ) var logStrings []string - for name, lp := range t.logs { + for name, lp := range t.Logs { logStr := fmt.Sprintf("%s-%d-%d", name, lp.Partition, lp.Offset) logStrings = append(logStrings, logStr) } sort.Strings(logStrings) - // E.g s11_22_33.dl0-134.ab1-441 - return strings.Join(append([]string{t.syncToken.String()}, logStrings...), ".") + // E.g s11_22_33_44.dl0-134.ab1-441 + return strings.Join(append([]string{posStr}, logStrings...), ".") } // IsAfter returns true if ANY position in this token is greater than `other`. func (t *StreamingToken) IsAfter(other StreamingToken) bool { - for i := range other.Positions { - if t.Positions[i] > other.Positions[i] { - return true - } + switch { + case t.PDUPosition > other.PDUPosition: + return true + case t.TypingPosition > other.TypingPosition: + return true + case t.ReceiptPosition > other.ReceiptPosition: + return true + case t.SendToDevicePosition > other.SendToDevicePosition: + return true } - for name := range t.logs { + for name := range t.Logs { otherLog := other.Log(name) if otherLog == nil { continue } - if t.logs[name].IsAfter(otherLog) { + if t.Logs[name].IsAfter(otherLog) { return true } } return false } +func (t *StreamingToken) IsEmpty() bool { + return t == nil || t.PDUPosition+t.TypingPosition+t.ReceiptPosition+t.SendToDevicePosition == 0 +} + // WithUpdates returns a copy of the StreamingToken with updates applied from another StreamingToken. // If the latter StreamingToken contains a field that is not 0, it is considered an update, // and its value will replace the corresponding value in the StreamingToken on which WithUpdates is called. // If the other token has a log, they will replace any existing log on this token. func (t *StreamingToken) WithUpdates(other StreamingToken) (ret StreamingToken) { - ret.Type = t.Type - ret.Positions = make([]StreamPosition, len(t.Positions)) - for i := range t.Positions { - ret.Positions[i] = t.Positions[i] - if other.Positions[i] == 0 { - continue - } - ret.Positions[i] = other.Positions[i] + ret = *t + switch { + case other.PDUPosition > 0: + ret.PDUPosition = other.PDUPosition + case other.TypingPosition > 0: + ret.TypingPosition = other.TypingPosition + case other.ReceiptPosition > 0: + ret.ReceiptPosition = other.ReceiptPosition + case other.SendToDevicePosition > 0: + ret.SendToDevicePosition = other.SendToDevicePosition } - ret.logs = make(map[string]*LogPosition) - for name := range t.logs { + ret.Logs = make(map[string]*LogPosition) + for name := range t.Logs { otherLog := other.Log(name) if otherLog == nil { continue } copy := *otherLog - ret.logs[name] = © + ret.Logs[name] = © } return ret } type TopologyToken struct { - syncToken + Depth StreamPosition + PDUPosition StreamPosition } -func (t *TopologyToken) Depth() StreamPosition { - return t.Positions[0] -} -func (t *TopologyToken) PDUPosition() StreamPosition { - return t.Positions[1] -} func (t *TopologyToken) StreamToken() StreamingToken { - return NewStreamToken(t.PDUPosition(), 0, nil) + return StreamingToken{ + PDUPosition: t.PDUPosition, + } } -func (t *TopologyToken) String() string { - return t.syncToken.String() + +func (t TopologyToken) String() string { + return fmt.Sprintf("t%d_%d", t.Depth, t.PDUPosition) } // Decrement the topology token to one event earlier. func (t *TopologyToken) Decrement() { - depth := t.Positions[0] - pduPos := t.Positions[1] + depth := t.Depth + pduPos := t.PDUPosition if depth-1 <= 0 { // nothing can be lower than this depth = 1 @@ -223,151 +233,93 @@ func (t *TopologyToken) Decrement() { if depth < 1 { depth = 1 } - t.Positions = []StreamPosition{ - depth, pduPos, - } + t.Depth = depth + t.PDUPosition = pduPos } -// NewSyncTokenFromString takes a string of the form "xyyyy..." where "x" -// represents the type of a pagination token and "yyyy..." the token itself, and -// parses it in order to create a new instance of SyncToken. Returns an -// error if the token couldn't be parsed into an int64, or if the token type -// isn't a known type (returns ErrInvalidSyncTokenType in the latter -// case). -func newSyncTokenFromString(s string) (token *syncToken, categories []string, err error) { - if len(s) == 0 { - return nil, nil, ErrInvalidSyncTokenLen +func NewTopologyTokenFromString(tok string) (token TopologyToken, err error) { + if len(tok) < 1 { + err = fmt.Errorf("empty topology token") + return } - - token = new(syncToken) - var positions []string - - switch t := SyncTokenType(s[:1]); t { - case SyncTokenTypeStream, SyncTokenTypeTopology: - token.Type = t - categories = strings.Split(s[1:], ".") - positions = strings.Split(categories[0], "_") - default: - return nil, nil, ErrInvalidSyncTokenType + if tok[0] != SyncTokenTypeTopology[0] { + err = fmt.Errorf("topology token must start with 't'") + return } - - for _, pos := range positions { - if posInt, err := strconv.ParseInt(pos, 10, 64); err != nil { - return nil, nil, err - } else if posInt < 0 { - return nil, nil, errors.New("negative position not allowed") - } else { - token.Positions = append(token.Positions, StreamPosition(posInt)) + parts := strings.Split(tok[1:], "_") + var positions [2]StreamPosition + for i, p := range parts { + if i > len(positions) { + break } + var pos int + pos, err = strconv.Atoi(p) + if err != nil { + return + } + positions[i] = StreamPosition(pos) + } + token = TopologyToken{ + Depth: positions[0], + PDUPosition: positions[1], } return } -// NewTopologyToken creates a new sync token for /messages -func NewTopologyToken(depth, streamPos StreamPosition) TopologyToken { - if depth < 0 { - depth = 1 - } - return TopologyToken{ - syncToken: syncToken{ - Type: SyncTokenTypeTopology, - Positions: []StreamPosition{depth, streamPos}, - }, - } -} -func NewTopologyTokenFromString(tok string) (token TopologyToken, err error) { - t, _, err := newSyncTokenFromString(tok) - if err != nil { - return - } - if t.Type != SyncTokenTypeTopology { - err = fmt.Errorf("token %s is not a topology token", tok) - return - } - if len(t.Positions) < 2 { - err = fmt.Errorf("token %s wrong number of values, got %d want at least 2", tok, len(t.Positions)) - return - } - return TopologyToken{ - syncToken: *t, - }, nil -} - -// NewStreamToken creates a new sync token for /sync -func NewStreamToken(pduPos, eduPos StreamPosition, logs map[string]*LogPosition) StreamingToken { - if logs == nil { - logs = make(map[string]*LogPosition) - } - return StreamingToken{ - syncToken: syncToken{ - Type: SyncTokenTypeStream, - Positions: []StreamPosition{pduPos, eduPos}, - }, - logs: logs, - } -} func NewStreamTokenFromString(tok string) (token StreamingToken, err error) { - t, categories, err := newSyncTokenFromString(tok) - if err != nil { + if len(tok) < 1 { + err = fmt.Errorf("empty stream token") return } - if t.Type != SyncTokenTypeStream { - err = fmt.Errorf("token %s is not a streaming token", tok) + if tok[0] != SyncTokenTypeStream[0] { + err = fmt.Errorf("stream token must start with 's'") return } - if len(t.Positions) < 2 { - err = fmt.Errorf("token %s wrong number of values, got %d want at least 2", tok, len(t.Positions)) - return + categories := strings.Split(tok[1:], ".") + parts := strings.Split(categories[0], "_") + var positions [4]StreamPosition + for i, p := range parts { + if i > len(positions) { + break + } + var pos int + pos, err = strconv.Atoi(p) + if err != nil { + return + } + positions[i] = StreamPosition(pos) } - logs := make(map[string]*LogPosition) - if len(categories) > 1 { - // dl-0-1234 - // $log_name-$partition-$offset - for _, logStr := range categories[1:] { - segments := strings.Split(logStr, "-") - if len(segments) != 3 { - err = fmt.Errorf("token %s - invalid log: %s", tok, logStr) - return - } - var partition int64 - partition, err = strconv.ParseInt(segments[1], 10, 32) - if err != nil { - return - } - var offset int64 - offset, err = strconv.ParseInt(segments[2], 10, 64) - if err != nil { - return - } - logs[segments[0]] = &LogPosition{ - Partition: int32(partition), - Offset: offset, - } + token = StreamingToken{ + PDUPosition: positions[0], + TypingPosition: positions[1], + ReceiptPosition: positions[2], + SendToDevicePosition: positions[3], + Logs: make(map[string]*LogPosition), + } + // dl-0-1234 + // $log_name-$partition-$offset + for _, logStr := range categories[1:] { + segments := strings.Split(logStr, "-") + if len(segments) != 3 { + err = fmt.Errorf("token %s - invalid log: %s", tok, logStr) + return + } + var partition int64 + partition, err = strconv.ParseInt(segments[1], 10, 32) + if err != nil { + return + } + var offset int64 + offset, err = strconv.ParseInt(segments[2], 10, 64) + if err != nil { + return + } + token.Logs[segments[0]] = &LogPosition{ + Partition: int32(partition), + Offset: offset, } } - return StreamingToken{ - syncToken: *t, - logs: logs, - }, nil -} - -// syncToken represents a syncapi token, used for interactions with -// /sync or /messages, for example. -type syncToken struct { - Type SyncTokenType - // A list of stream positions, their meanings vary depending on the token type. - Positions []StreamPosition -} - -// String translates a SyncToken to a string of the "xyyyy..." (see -// NewSyncToken to know what it represents). -func (p *syncToken) String() string { - posStr := make([]string, len(p.Positions)) - for i := range p.Positions { - posStr[i] = strconv.FormatInt(int64(p.Positions[i]), 10) - } - - return fmt.Sprintf("%s%s", p.Type, strings.Join(posStr, "_")) + return token, nil } // PrevEventRef represents a reference to a previous event in a state event upgrade diff --git a/syncapi/types/types_test.go b/syncapi/types/types_test.go index 62404a606..15079188a 100644 --- a/syncapi/types/types_test.go +++ b/syncapi/types/types_test.go @@ -10,22 +10,22 @@ import ( func TestNewSyncTokenWithLogs(t *testing.T) { tests := map[string]*StreamingToken{ - "s4_0": { - syncToken: syncToken{Type: "s", Positions: []StreamPosition{4, 0}}, - logs: make(map[string]*LogPosition), + "s4_0_0_0": { + PDUPosition: 4, + Logs: make(map[string]*LogPosition), }, - "s4_0.dl-0-123": { - syncToken: syncToken{Type: "s", Positions: []StreamPosition{4, 0}}, - logs: map[string]*LogPosition{ + "s4_0_0_0.dl-0-123": { + PDUPosition: 4, + Logs: map[string]*LogPosition{ "dl": { Partition: 0, Offset: 123, }, }, }, - "s4_0.ab-1-14419482332.dl-0-123": { - syncToken: syncToken{Type: "s", Positions: []StreamPosition{4, 0}}, - logs: map[string]*LogPosition{ + "s4_0_0_0.ab-1-14419482332.dl-0-123": { + PDUPosition: 4, + Logs: map[string]*LogPosition{ "ab": { Partition: 1, Offset: 14419482332, @@ -56,16 +56,22 @@ func TestNewSyncTokenWithLogs(t *testing.T) { } } -func TestNewSyncTokenFromString(t *testing.T) { - shouldPass := map[string]syncToken{ - "s4_0": NewStreamToken(4, 0, nil).syncToken, - "s3_1": NewStreamToken(3, 1, nil).syncToken, - "t3_1": NewTopologyToken(3, 1).syncToken, +func TestSyncTokens(t *testing.T) { + shouldPass := map[string]string{ + "s4_0_0_0": StreamingToken{4, 0, 0, 0, nil}.String(), + "s3_1_0_0": StreamingToken{3, 1, 0, 0, nil}.String(), + "s3_1_2_3": StreamingToken{3, 1, 2, 3, nil}.String(), + "t3_1": TopologyToken{3, 1}.String(), + } + + for a, b := range shouldPass { + if a != b { + t.Errorf("expected %q, got %q", a, b) + } } shouldFail := []string{ "", - "s_1", "s_", "a3_4", "b", @@ -74,19 +80,15 @@ func TestNewSyncTokenFromString(t *testing.T) { "2", } - for test, expected := range shouldPass { - result, _, err := newSyncTokenFromString(test) - if err != nil { - t.Error(err) - } - if result.String() != expected.String() { - t.Errorf("%s expected %v but got %v", test, expected.String(), result.String()) + for _, f := range append(shouldFail, "t1_2") { + if _, err := NewStreamTokenFromString(f); err == nil { + t.Errorf("NewStreamTokenFromString %q should have failed", f) } } - for _, test := range shouldFail { - if _, _, err := newSyncTokenFromString(test); err == nil { - t.Errorf("input '%v' should have errored but didn't", test) + for _, f := range append(shouldFail, "s1_2_3_4") { + if _, err := NewTopologyTokenFromString(f); err == nil { + t.Errorf("NewTopologyTokenFromString %q should have failed", f) } } } diff --git a/sytest-whitelist b/sytest-whitelist index da4b201ce..eb1634367 100644 --- a/sytest-whitelist +++ b/sytest-whitelist @@ -503,3 +503,4 @@ Forgetting room does not show up in v2 /sync Can forget room you've been kicked from /whois /joined_members return joined members +A next_batch token can be used in the v1 messages API