diff --git a/lib/models/event-timeline.js b/lib/models/event-timeline.js index 6913a9eb5..2d3cb8e93 100644 --- a/lib/models/event-timeline.js +++ b/lib/models/event-timeline.js @@ -5,6 +5,8 @@ */ var RoomState = require("./room-state"); +var utils = require("../utils"); +var MatrixEvent = require("./event").MatrixEvent; /** * Construct a new EventTimeline @@ -31,7 +33,9 @@ function EventTimeline(roomId) { this._events = []; this._baseIndex = -1; this._startState = new RoomState(roomId); + this._startState.paginationToken = null; this._endState = new RoomState(roomId); + this._endState.paginationToken = null; this._prevTimeline = null; this._nextTimeline = null; @@ -54,8 +58,14 @@ EventTimeline.prototype.initialiseState = function(stateEvents) { throw new Error("Cannot initialise state after events are added"); } - // do we need to copy here? sync thinks we do but I can't see why - this._startState.setStateEvents(stateEvents); + // we deep-copy the events here, in case they get changed later - we don't + // want changes to the start state leaking through to the end state. + var oldStateEvents = utils.map( + utils.deepCopy( + stateEvents.map(function(mxEvent) { return mxEvent.event; }) + ), function(ev) { return new MatrixEvent(ev); }); + + this._startState.setStateEvents(oldStateEvents); this._endState.setStateEvents(stateEvents); }; @@ -72,7 +82,9 @@ EventTimeline.prototype.getRoomId = function() { * *

This is an index which is incremented when events are prepended to the * timeline. An individual event therefore stays at the same index in the array - * relative to the base index. + * relative to the base index (although note that a given event's index may + * well be less than the base index, thus giving that event a negative relative + * index). * * @return {number} */ diff --git a/lib/models/room.js b/lib/models/room.js index 02fe59ace..7287f3436 100644 --- a/lib/models/room.js +++ b/lib/models/room.js @@ -177,8 +177,20 @@ Room.prototype.resetLiveTimeline = function() { } if (this._liveTimeline) { + // we have an existing timeline. This will always be true, except when + // this method is called by our own constructor. + // initialise the state in the new timeline from our last known state - newTimeline.initialiseState(this._liveTimeline.getState(false).events); + var evMap = this._liveTimeline.getState(false).events; + var events = []; + for (var evtype in evMap) { + if (!evMap.hasOwnProperty(evtype)) { continue; } + for (var stateKey in evMap[evtype]) { + if (!evMap[evtype].hasOwnProperty(stateKey)) { continue; } + events.push(evMap[evtype][stateKey]); + } + } + newTimeline.initialiseState(events); } this._liveTimeline = newTimeline; @@ -196,13 +208,14 @@ Room.prototype.resetLiveTimeline = function() { * * @param {string} eventId event ID to look for * @return {?module:models/event-timeline~EventTimeline} timeline containing - * the given event, or undefined if unknown + * the given event, or null if unknown */ Room.prototype.getTimelineForEvent = function(eventId) { - return this._eventIdToTimeline[eventId]; + var res = this._eventIdToTimeline[eventId]; + return (res === undefined) ? null : res; }; -/* +/** * Get one of the notification counts for this room * @param {String} type The type of notification count to get. default: 'total' * @return {Number} The notification count, or undefined if there is no count @@ -364,105 +377,125 @@ Room.prototype.addEventsToTimeline = function(events, toStartOfTimeline, return; } - // we need to handle the following case: + // Adding events to timelines can be quite complicated. The following + // illustrates some of the corner-cases. // - // We already know about two timelines. One of these contains just - // event M, and one contains just event P: + // Let's say we start by knowing about four timelines. timeline3 and + // timeline4 are neighbours: // - // timeline1 timeline2 - // [M] [P] + // timeline1 timeline2 timeline3 timeline4 + // [M] [P] [S] <------> [T] // - // We are now processing a new pagination response from the server. - // This contains the following events: [M, N, P, R, S]. + // Now we paginate timeline1, and get the following events from the server: + // [M, N, P, R, S, T, U]. // // 1. First, we ignore event M, since we already know about it. // // 2. Next, we append N to timeline 1. // // 3. Next, we don't add event P, since we already know about it, - // but we do link togehter the timelines. We now have: + // but we do link together the timelines. We now have: // - // timeline1 timeline2 - // [M, N] <----> [P] + // timeline1 timeline2 timeline3 timeline4 + // [M, N] <---> [P] [S] <------> [T] // - // 4. We now need to append R and S to timeline2. So finally, we have: + // 4. Now we add event R to timeline2: // - // timeline1 timeline2 - // [M, N] <----> [P, R, S] + // timeline1 timeline2 timeline3 timeline4 + // [M, N] <---> [P, R] [S] <------> [T] // - // The important thing to note in the above is that we switched the - // timeline we were adding events to, after we found a neighbour. - - // XXX: TODO: but timeline2 might already be joined to timeline3: we need - // to skip all the way to the end of the linked list! + // Note that we have switched the timeline we are working on from + // timeline1 to timeline2. + // + // 5. We ignore event S, but again join the timelines: + // + // timeline1 timeline2 timeline3 timeline4 + // [M, N] <---> [P, R] <---> [S] <------> [T] + // + // 6. We ignore event T, and the timelines are already joined, so there + // is nothing to do. + // + // 7. Finally, we add event U to timeline4: + // + // timeline1 timeline2 timeline3 timeline4 + // [M, N] <---> [P, R] <---> [S] <------> [T, U] + // + // The important thing to note in the above is what happened when we + // already knew about a given event: + // + // - if it was appropriate, we joined up the timelines (steps 3, 5). + // - in any case, we started adding further events to the timeline which + // contained the event we knew about (steps 3, 5, 6). + // + // + // Finally, consider what we want to do with the pagination token. In the + // case above, we will be given a pagination token which tells us how to + // get events beyond 'U' - in this case, it makes sense to store this + // against timeline4. But what if timeline4 already had 'U' and beyond? in + // that case, our best bet is to throw away the pagination token we were + // given and stick with whatever token timeline4 had previously. In short, + // we want to only store the pagination token if the last event we receive + // is one we didn't previously know about. var updateToken = false; for (var i = 0; i < events.length; i++) { - var existingTimeline = this._checkForNewEventInExistingTimeline( - events[i], timeline, toStartOfTimeline); + var event = events[i]; + var eventId = event.getId(); - if (existingTimeline) { - timeline = existingTimeline; - updateToken = false; - } else { - this._addEventToTimeline(events[i], timeline, toStartOfTimeline); + var existingTimeline = this._eventIdToTimeline[eventId]; + + if (!existingTimeline) { + // we don't know about this event yet. Just add it to the timeline. + this._addEventToTimeline(event, timeline, toStartOfTimeline); updateToken = true; + continue; } + + // we already know about this event. We don't want to use the pagination + // token if this is the last event, as per the text above. + updateToken = false; + + if (existingTimeline == timeline) { + console.log("Event " + eventId + " already in timeline " + timeline); + continue; + } + + var neighbour = timeline.getNeighbouringTimeline(toStartOfTimeline); + if (neighbour) { + // this timeline already has a neighbour in the relevant direction; + // let's assume the timelines are already correctly linked up, and + // skip over to it. + // + // there's probably some edge-case here where we end up with an + // event which is in a timeline a way down the chain, and there is + // a break in the chain somewhere. But I can't really imagine how + // that would happen, so I'm going to ignore it for now. + // + if (existingTimeline == neighbour) { + console.log("Event " + eventId + " in neighbouring timeline - " + + "switching to " + existingTimeline); + } else { + console.log("Event " + eventId + " already in a different " + + "timeline " + existingTimeline); + } + timeline = existingTimeline; + continue; + } + + // time to join the timelines. + console.info("Already have timeline for " + eventId + + " - joining timeline " + timeline + " to " + + existingTimeline); + timeline.setNeighbouringTimeline(existingTimeline, toStartOfTimeline); + existingTimeline.setNeighbouringTimeline(timeline, !toStartOfTimeline); + timeline = existingTimeline; } + if (updateToken) { timeline.setPaginationToken(paginationToken, toStartOfTimeline); } }; -/** - * Check if this event is already in a timeline, and join up the timelines if - * necessary - * - * @param {MatrixEvent} event event to add - * @param {EventTimeline} timeline timeline we think we should add to - * @param {boolean} toStartOfTimeline true if we're adding to the start of - * the timeline - * @return {?EventTimeline} the timeline with the event already in, or null if - * none - * @private - */ -Room.prototype._checkForNewEventInExistingTimeline = function( - event, timeline, toStartOfTimeline) { - var eventId = event.getId(); - - var existingTimeline = this._eventIdToTimeline[eventId]; - if (!existingTimeline) { - return null; - } - - // we already know about this event. Hopefully it's in this timeline, or - // its neighbour - if (existingTimeline == timeline) { - console.log("Event " + eventId + " already in timeline " + timeline); - return timeline; - } - - var neighbour = timeline.getNeighbouringTimeline(toStartOfTimeline); - if (neighbour) { - if (existingTimeline == neighbour) { - console.log("Event " + eventId + " in neighbouring timeline - " + - "switching to " + existingTimeline); - } else { - console.warn("Event " + eventId + " already in a different " + - "timeline " + existingTimeline); - } - return existingTimeline; - } - - // time to join the timelines. - console.info("Already have timeline for " + eventId + - " - joining timeline " + timeline + " to " + - existingTimeline); - timeline.setNeighbouringTimeline(existingTimeline, toStartOfTimeline); - existingTimeline.setNeighbouringTimeline(timeline, !toStartOfTimeline); - return existingTimeline; -}; - /** * Check for redactions, and otherwise add event to the given timeline. Assumes * we have already checked we don't know about this event. @@ -472,7 +505,11 @@ Room.prototype._checkForNewEventInExistingTimeline = function( * @param {MatrixEvent} event * @param {EventTimeline} timeline * @param {boolean} toStartOfTimeline - * @param {boolean} spliceBeforeLocalEcho + * + * @param {boolean} spliceBeforeLocalEcho if true, insert this event before + * any localecho events at the end of the timeline. Ignored if + * toStartOfTimeline == true. + * * @fires module:client~MatrixClient#event:"Room.timeline" * * @private diff --git a/lib/sync.js b/lib/sync.js index 1ba77e6cc..36bd260ea 100644 --- a/lib/sync.js +++ b/lib/sync.js @@ -216,6 +216,13 @@ SyncApi.prototype.peek = function(roomId) { response.messages.chunk, client.getEventMapper() ); + // set the pagination token before adding the events in case people + // fire off pagination requests in response to the Room.timeline + // events. + if (response.messages.start) { + peekRoom.oldState.paginationToken = response.messages.start; + } + // set the state of the room to as it was after the timeline executes peekRoom.oldState.setStateEvents(oldStateEvents); peekRoom.currentState.setStateEvents(stateEvents); @@ -223,14 +230,11 @@ SyncApi.prototype.peek = function(roomId) { self._resolveInvites(peekRoom); peekRoom.recalculate(self.client.credentials.userId); - // roll backwards to diverge old state: - peekRoom.addEventsToTimeline(messages.reverse(), true); - - // set the pagination token. Make sure this happens after adding - // events to the timeline, otherwise it will get reset. - if (response.messages.start) { - peekRoom.oldState.paginationToken = response.messages.start; - } + // roll backwards to diverge old state. addEventsToTimeline + // will overwrite the pagination token, so make sure it overwrites + // it with the right thing. + peekRoom.addEventsToTimeline(messages.reverse(), true, + undefined, response.messages.start); client.store.storeRoom(peekRoom); client.emit("Room", peekRoom); diff --git a/spec/integ/matrix-client-room-timeline.spec.js b/spec/integ/matrix-client-room-timeline.spec.js index 2f6bb4005..9548fda2d 100644 --- a/spec/integ/matrix-client-room-timeline.spec.js +++ b/spec/integ/matrix-client-room-timeline.spec.js @@ -481,4 +481,36 @@ describe("MatrixClient room timelines", function() { httpBackend.flush("/sync", 1); }); }); + + describe("gappy sync", function() { + it("should copy the last known state to the new timeline", function(done) { + var eventData = [ + utils.mkMessage({user: userId, room: roomId}), + ]; + setNextSyncData(eventData); + NEXT_SYNC_DATA.rooms.join[roomId].timeline.limited = true; + + client.on("sync", function(state) { + if (state !== "PREPARED") { return; } + var room = client.getRoom(roomId); + + httpBackend.flush("/messages", 1); + httpBackend.flush("/sync", 1).done(function() { + expect(room.timeline.length).toEqual(1); + expect(room.timeline[0].event).toEqual(eventData[0]); + expect(room.currentState.getMembers().length).toEqual(2); + expect(room.currentState.getMember(userId).name).toEqual(userName); + expect(room.currentState.getMember(userId).membership).toEqual( + "join" + ); + expect(room.currentState.getMember(otherUserId).name).toEqual("Bob"); + expect(room.currentState.getMember(otherUserId).membership).toEqual( + "join" + ); + done(); + }); + }); + httpBackend.flush("/sync", 1); + }); + }); }); diff --git a/spec/unit/event-timeline.spec.js b/spec/unit/event-timeline.spec.js index 5b3b759b8..cd0dc0163 100644 --- a/spec/unit/event-timeline.spec.js +++ b/spec/unit/event-timeline.spec.js @@ -54,11 +54,12 @@ describe("EventTimeline", function() { event: true, }); - var state = + var state = [ utils.mkMembership({ room: roomId, mship: "invite", user: userB, skey: userA, event: true, - }); + }) + ]; expect(function() { timeline.initialiseState(state); }).not.toThrow(); timeline.addEvent(event, false);