syncapi: Rename and split out tokens (#1025)

* syncapi: Rename and split out tokens

Previously we used the badly named `PaginationToken` which was
used for both `/sync` and `/messages` requests. This quickly
became confusing because named fields like `PDUPosition` meant
different things depending on the token type. Instead, we now have
two token types: `TopologyToken` and `StreamingToken`, both of
which have fields which make more sense for their specific situations.

Updated the codebase to use one or the other. `PaginationToken` still
lives on as `syncToken`, an unexported type which both tokens rely on.
This allows us to guarantee that the specific mappings of positions
to a string remain solely under the control of the `types` package.
This enables us to move high-level conceptual things like
"decrement this topological token" to function calls e.g
`TopologicalToken.Decrement()`.

Currently broken because `/messages` seemingly used both stream and
topological tokens, though I need to confirm this.

* final tweaks/hacks

* spurious logging

* Review comments and linting
This commit is contained in:
Kegsay 2020-05-13 12:14:50 +01:00 committed by GitHub
parent 31e6a7f193
commit 5e9dce1c0c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 457 additions and 439 deletions

View file

@ -36,7 +36,7 @@ type Notifier struct {
// Protects currPos and userStreams.
streamLock *sync.Mutex
// The latest sync position
currPos types.PaginationToken
currPos types.StreamingToken
// A map of user_id => UserStream which can be used to wake a given user's /sync request.
userStreams map[string]*UserStream
// The last time we cleaned out stale entries from the userStreams map
@ -46,7 +46,7 @@ type Notifier struct {
// NewNotifier creates a new notifier set to the given sync position.
// In order for this to be of any use, the Notifier needs to be told all rooms and
// the joined users within each of them by calling Notifier.Load(*storage.SyncServerDatabase).
func NewNotifier(pos types.PaginationToken) *Notifier {
func NewNotifier(pos types.StreamingToken) *Notifier {
return &Notifier{
currPos: pos,
roomIDToJoinedUsers: make(map[string]userIDSet),
@ -68,7 +68,7 @@ func NewNotifier(pos types.PaginationToken) *Notifier {
// event type it handles, leaving other fields as 0.
func (n *Notifier) OnNewEvent(
ev *gomatrixserverlib.HeaderedEvent, roomID string, userIDs []string,
posUpdate types.PaginationToken,
posUpdate types.StreamingToken,
) {
// update the current position then notify relevant /sync streams.
// This needs to be done PRIOR to waking up users as they will read this value.
@ -151,7 +151,7 @@ func (n *Notifier) Load(ctx context.Context, db storage.Database) error {
}
// CurrentPosition returns the current sync position
func (n *Notifier) CurrentPosition() types.PaginationToken {
func (n *Notifier) CurrentPosition() types.StreamingToken {
n.streamLock.Lock()
defer n.streamLock.Unlock()
@ -173,7 +173,7 @@ func (n *Notifier) setUsersJoinedToRooms(roomIDToUserIDs map[string][]string) {
}
}
func (n *Notifier) wakeupUsers(userIDs []string, newPos types.PaginationToken) {
func (n *Notifier) wakeupUsers(userIDs []string, newPos types.StreamingToken) {
for _, userID := range userIDs {
stream := n.fetchUserStream(userID, false)
if stream != nil {

View file

@ -33,11 +33,11 @@ var (
randomMessageEvent gomatrixserverlib.HeaderedEvent
aliceInviteBobEvent gomatrixserverlib.HeaderedEvent
bobLeaveEvent gomatrixserverlib.HeaderedEvent
syncPositionVeryOld types.PaginationToken
syncPositionBefore types.PaginationToken
syncPositionAfter types.PaginationToken
syncPositionNewEDU types.PaginationToken
syncPositionAfter2 types.PaginationToken
syncPositionVeryOld = types.NewStreamToken(5, 0)
syncPositionBefore = types.NewStreamToken(11, 0)
syncPositionAfter = types.NewStreamToken(12, 0)
syncPositionNewEDU = types.NewStreamToken(syncPositionAfter.PDUPosition(), 1)
syncPositionAfter2 = types.NewStreamToken(13, 0)
)
var (
@ -47,26 +47,6 @@ var (
)
func init() {
baseSyncPos := types.PaginationToken{
PDUPosition: 0,
EDUTypingPosition: 0,
}
syncPositionVeryOld = baseSyncPos
syncPositionVeryOld.PDUPosition = 5
syncPositionBefore = baseSyncPos
syncPositionBefore.PDUPosition = 11
syncPositionAfter = baseSyncPos
syncPositionAfter.PDUPosition = 12
syncPositionNewEDU = syncPositionAfter
syncPositionNewEDU.EDUTypingPosition = 1
syncPositionAfter2 = baseSyncPos
syncPositionAfter2.PDUPosition = 13
var err error
err = json.Unmarshal([]byte(`{
"_room_version": "1",
@ -118,6 +98,12 @@ func init() {
}
}
func mustEqualPositions(t *testing.T, got, want types.StreamingToken) {
if got.String() != want.String() {
t.Fatalf("mustEqualPositions got %s want %s", got.String(), want.String())
}
}
// Test that the current position is returned if a request is already behind.
func TestImmediateNotification(t *testing.T) {
n := NewNotifier(syncPositionBefore)
@ -125,9 +111,7 @@ func TestImmediateNotification(t *testing.T) {
if err != nil {
t.Fatalf("TestImmediateNotification error: %s", err)
}
if pos != syncPositionBefore {
t.Fatalf("TestImmediateNotification want %v, got %v", syncPositionBefore, pos)
}
mustEqualPositions(t, pos, syncPositionBefore)
}
// Test that new events to a joined room unblocks the request.
@ -144,9 +128,7 @@ func TestNewEventAndJoinedToRoom(t *testing.T) {
if err != nil {
t.Errorf("TestNewEventAndJoinedToRoom error: %w", err)
}
if pos != syncPositionAfter {
t.Errorf("TestNewEventAndJoinedToRoom want %v, got %v", syncPositionAfter, pos)
}
mustEqualPositions(t, pos, syncPositionAfter)
wg.Done()
}()
@ -172,9 +154,7 @@ func TestNewInviteEventForUser(t *testing.T) {
if err != nil {
t.Errorf("TestNewInviteEventForUser error: %w", err)
}
if pos != syncPositionAfter {
t.Errorf("TestNewInviteEventForUser want %v, got %v", syncPositionAfter, pos)
}
mustEqualPositions(t, pos, syncPositionAfter)
wg.Done()
}()
@ -200,9 +180,7 @@ func TestEDUWakeup(t *testing.T) {
if err != nil {
t.Errorf("TestNewInviteEventForUser error: %w", err)
}
if pos != syncPositionNewEDU {
t.Errorf("TestNewInviteEventForUser want %v, got %v", syncPositionNewEDU, pos)
}
mustEqualPositions(t, pos, syncPositionNewEDU)
wg.Done()
}()
@ -228,9 +206,7 @@ func TestMultipleRequestWakeup(t *testing.T) {
if err != nil {
t.Errorf("TestMultipleRequestWakeup error: %w", err)
}
if pos != syncPositionAfter {
t.Errorf("TestMultipleRequestWakeup want %v, got %v", syncPositionAfter, pos)
}
mustEqualPositions(t, pos, syncPositionAfter)
wg.Done()
}
go poll()
@ -268,9 +244,7 @@ func TestNewEventAndWasPreviouslyJoinedToRoom(t *testing.T) {
if err != nil {
t.Errorf("TestNewEventAndWasPreviouslyJoinedToRoom error: %w", err)
}
if pos != syncPositionAfter {
t.Errorf("TestNewEventAndWasPreviouslyJoinedToRoom want %v, got %v", syncPositionAfter, pos)
}
mustEqualPositions(t, pos, syncPositionAfter)
leaveWG.Done()
}()
bobStream := lockedFetchUserStream(n, bob)
@ -287,9 +261,7 @@ func TestNewEventAndWasPreviouslyJoinedToRoom(t *testing.T) {
if err != nil {
t.Errorf("TestNewEventAndWasPreviouslyJoinedToRoom error: %w", err)
}
if pos != syncPositionAfter2 {
t.Errorf("TestNewEventAndWasPreviouslyJoinedToRoom want %v, got %v", syncPositionAfter2, pos)
}
mustEqualPositions(t, pos, syncPositionAfter2)
aliceWG.Done()
}()
@ -312,13 +284,13 @@ func TestNewEventAndWasPreviouslyJoinedToRoom(t *testing.T) {
time.Sleep(1 * time.Millisecond)
}
func waitForEvents(n *Notifier, req syncRequest) (types.PaginationToken, error) {
func waitForEvents(n *Notifier, req syncRequest) (types.StreamingToken, error) {
listener := n.GetListener(req)
defer listener.Close()
select {
case <-time.After(5 * time.Second):
return types.PaginationToken{}, fmt.Errorf(
return types.StreamingToken{}, fmt.Errorf(
"waitForEvents timed out waiting for %s (pos=%v)", req.device.UserID, req.since,
)
case <-listener.GetNotifyChannel(*req.since):
@ -344,7 +316,7 @@ func lockedFetchUserStream(n *Notifier, userID string) *UserStream {
return n.fetchUserStream(userID, true)
}
func newTestSyncRequest(userID string, since types.PaginationToken) syncRequest {
func newTestSyncRequest(userID string, since types.StreamingToken) syncRequest {
return syncRequest{
device: authtypes.Device{UserID: userID},
timeout: 1 * time.Minute,

View file

@ -36,7 +36,7 @@ type syncRequest struct {
device authtypes.Device
limit int
timeout time.Duration
since *types.PaginationToken // nil means that no since token was supplied
since *types.StreamingToken // nil means that no since token was supplied
wantFullState bool
log *log.Entry
}
@ -45,9 +45,14 @@ func newSyncRequest(req *http.Request, device authtypes.Device) (*syncRequest, e
timeout := getTimeout(req.URL.Query().Get("timeout"))
fullState := req.URL.Query().Get("full_state")
wantFullState := fullState != "" && fullState != "false"
since, err := getPaginationToken(req.URL.Query().Get("since"))
if err != nil {
return nil, err
var since *types.StreamingToken
sinceStr := req.URL.Query().Get("since")
if sinceStr != "" {
tok, err := types.NewStreamTokenFromString(sinceStr)
if err != nil {
return nil, err
}
since = &tok
}
// TODO: Additional query params: set_presence, filter
return &syncRequest{
@ -71,16 +76,3 @@ func getTimeout(timeoutMS string) time.Duration {
}
return time.Duration(i) * time.Millisecond
}
// getSyncStreamPosition tries to parse a 'since' token taken from the API to a
// types.PaginationToken. If the string is empty then (nil, nil) is returned.
// There are two forms of tokens: The full length form containing all PDU and EDU
// positions separated by "_", and the short form containing only the PDU
// position. Short form can be used for, e.g., `prev_batch` tokens.
func getPaginationToken(since string) (*types.PaginationToken, error) {
if since == "" {
return nil, nil
}
return types.NewPaginationTokenFromString(since)
}

View file

@ -132,7 +132,7 @@ func (rp *RequestPool) OnIncomingSyncRequest(req *http.Request, device *authtype
}
}
func (rp *RequestPool) currentSyncForUser(req syncRequest, latestPos types.PaginationToken) (res *types.Response, err error) {
func (rp *RequestPool) currentSyncForUser(req syncRequest, latestPos types.StreamingToken) (res *types.Response, err error) {
// TODO: handle ignored users
if req.since == nil {
res, err = rp.db.CompleteSync(req.ctx, req.device.UserID, req.limit)
@ -145,7 +145,7 @@ func (rp *RequestPool) currentSyncForUser(req syncRequest, latestPos types.Pagin
}
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)
return
}
@ -187,7 +187,7 @@ func (rp *RequestPool) appendAccountData(
// Sync is not initial, get all account data since the latest sync
dataTypes, err := rp.db.GetAccountDataInRange(
req.ctx, userID,
types.StreamPosition(req.since.PDUPosition), types.StreamPosition(currentPos),
types.StreamPosition(req.since.PDUPosition()), types.StreamPosition(currentPos),
accountDataFilter,
)
if err != nil {

View file

@ -34,7 +34,7 @@ type UserStream struct {
// Closed when there is an update.
signalChannel chan struct{}
// The last sync position that there may have been an update for the user
pos types.PaginationToken
pos types.StreamingToken
// The last time when we had some listeners waiting
timeOfLastChannel time.Time
// The number of listeners waiting
@ -50,7 +50,7 @@ type UserStreamListener struct {
}
// NewUserStream creates a new user stream
func NewUserStream(userID string, currPos types.PaginationToken) *UserStream {
func NewUserStream(userID string, currPos types.StreamingToken) *UserStream {
return &UserStream{
UserID: userID,
timeOfLastChannel: time.Now(),
@ -83,7 +83,7 @@ func (s *UserStream) GetListener(ctx context.Context) UserStreamListener {
}
// Broadcast a new sync position for this user.
func (s *UserStream) Broadcast(pos types.PaginationToken) {
func (s *UserStream) Broadcast(pos types.StreamingToken) {
s.lock.Lock()
defer s.lock.Unlock()
@ -116,9 +116,9 @@ func (s *UserStream) TimeOfLastNonEmpty() time.Time {
return s.timeOfLastChannel
}
// GetStreamPosition returns last sync position which the UserStream was
// GetSyncPosition returns last sync position which the UserStream was
// notified about
func (s *UserStreamListener) GetSyncPosition() types.PaginationToken {
func (s *UserStreamListener) GetSyncPosition() types.StreamingToken {
s.userStream.lock.Lock()
defer s.userStream.lock.Unlock()
@ -130,7 +130,7 @@ func (s *UserStreamListener) GetSyncPosition() types.PaginationToken {
// sincePos specifies from which point we want to be notified about. If there
// has already been an update after sincePos we'll return a closed channel
// immediately.
func (s *UserStreamListener) GetNotifyChannel(sincePos types.PaginationToken) <-chan struct{} {
func (s *UserStreamListener) GetNotifyChannel(sincePos types.StreamingToken) <-chan struct{} {
s.userStream.lock.Lock()
defer s.userStream.lock.Unlock()