Merge pull request #91 from matrix-org/rav/fix_stuck_pagination_after_join

Don't reset the timeline when we join a room after peeking

lib/sync.js

@@ -593,22 +593,60 @@ SyncApi.prototype._processSyncResponse = function(syncToken, data) {
 
         joinObj.timeline = joinObj.timeline || {};
 
+        var limited = false;
         if (joinObj.timeline.limited) {
-            // nuke the timeline so we don't get holes
+            limited = true;
+
+            // we've got a limited sync, so we *probably* have a gap in the
+            // timeline, so should reset. But we might have been peeking or
+            // paginating and already have some of the events, in which
+            // case we just want to append any subsequent events to the end
+            // of the existing timeline.
             //
-            // save the old 'next_batch' token as the
+            // This is particularly important in the case that we already have
-            // forward-pagination token for the previously-active
+            // *all* of the events in the timeline - in that case, if we reset
-            // timeline.
+            // the timeline, we'll end up with an entirely empty timeline,
-            room.currentState.paginationToken = syncToken;
+            // which we'll try to paginate but not get any new events (which
-            self._deregisterStateListeners(room);
+            // will stop us linking the empty timeline into the chain).
-            room.resetLiveTimeline();
+            //
-            self._registerStateListeners(room);
+            for (var i = timelineEvents.length - 1; i >= 0; i--) {
+                var eventId = timelineEvents[i].getId();
+                if (room.getTimelineForEvent(eventId)) {
+                    debuglog("Already have event " + eventId + " in limited " +
+                             "sync - not resetting");
+                    limited = false;
+
+                    // we might still be missing some of the events before i;
+                    // we don't want to be adding them to the end of the
+                    // timeline because that would put them out of order.
+                    timelineEvents.splice(0, i);
+
+                    // XXX: there's a problem here if the skipped part of the
+                    // timeline modifies the state set in stateEvents, because
+                    // we'll end up using the state from stateEvents rather
+                    // than the later state from timelineEvents. We probably
+                    // need to wind stateEvents forward over the events we're
+                    // skipping.
+
+                    break;
+                }
+            }
+
+            if (limited) {
+                // save the old 'next_batch' token as the
+                // forward-pagination token for the previously-active
+                // timeline.
+                room.currentState.paginationToken = syncToken;
+                self._deregisterStateListeners(room);
+                room.resetLiveTimeline();
+                self._registerStateListeners(room);
+            }
         }
 
         // we want to set a new pagination token if this is the first time
         // we've made this room or if we're nuking the timeline
         var paginationToken = null;
-        if (joinObj.isBrandNewRoom || joinObj.timeline.limited) {
+        if (joinObj.isBrandNewRoom || limited) {
             paginationToken = joinObj.timeline.prev_batch;
         }
 
@@ -862,6 +900,11 @@ SyncApi.prototype._processRoomEvents = function(room, stateEventList,
     }
 
     // set the state of the room to as it was before the timeline executes
+    //
+    // XXX: what if we've already seen (some of) the events in the timeline,
+    // and they modify some of the state set in stateEvents? In that case we'll
+    // end up with the state from stateEvents, instead of the more recent state
+    // from the timeline.
     room.oldState.setStateEvents(oldStateEvents);
     room.currentState.setStateEvents(stateEvents);