diff --git a/lib/client.js b/lib/client.js index f610bfffc..eeb25d73c 100644 --- a/lib/client.js +++ b/lib/client.js @@ -2072,8 +2072,8 @@ MatrixClient.prototype.paginateEventContext = function(eventContext, opts) { * Get an EventTimeline for the given event * *

If the room object already has the given event in its store, the - * corresponding timeline will be returned. Otherwise, we make a /context - * request, and use it to construct an EventTimeline. + * corresponding timeline will be returned. Otherwise, a /context request is + * made, and used to construct an EventTimeline. * * @param {Room} room The room to look for the event in * @param {string} eventId The ID of the event to look for @@ -2085,7 +2085,7 @@ MatrixClient.prototype.paginateEventContext = function(eventContext, opts) { MatrixClient.prototype.getEventTimeline = function(room, eventId) { // don't allow any timeline support unless it's been enabled. if (!this.timelineSupport) { - throw Error("timeline support is disabled. Set the 'timelineSupport'" + + throw new Error("timeline support is disabled. Set the 'timelineSupport'" + " parameter to true when creating MatrixClient to enable" + " it."); } @@ -2108,7 +2108,7 @@ MatrixClient.prototype.getEventTimeline = function(room, eventId) { self._http.authedRequest(undefined, "GET", path ).then(function(res) { if (!res.event) { - throw Error("'event' not in '/context' result - homeserver too old?"); + throw new Error("'event' not in '/context' result - homeserver too old?"); } // by the time the request completes, the event might have ended up in @@ -2127,7 +2127,7 @@ MatrixClient.prototype.getEventTimeline = function(room, eventId) { var timeline = room.getTimelineForEvent(matrixEvents[0].getId()); if (!timeline) { - timeline = room.createTimeline(); + timeline = room.addTimeline(); timeline.initialiseState(utils.map(res.state, self.getEventMapper())); timeline.getState(false).paginationToken = res.end; @@ -2155,8 +2155,8 @@ MatrixClient.prototype.getEventTimeline = function(room, eventId) { * false to go forwards * @param {number} [opts.limit = 30] number of events to request * - * @return {module:client.Promise} Resolves: false if there are no events and - * we reached the end of the timeline; else true. Rejects: Error with an error response + * @return {module:client.Promise} Resolves to a boolean: false if there are no + * events and we reached either end of the timeline; else true. */ MatrixClient.prototype.paginateEventTimeline = function(eventTimeline, opts) { // TODO: we should implement a backoff (as per scrollback()) to deal more @@ -2164,10 +2164,15 @@ MatrixClient.prototype.paginateEventTimeline = function(eventTimeline, opts) { opts = opts || {}; var backwards = opts.backwards || false; + var room = this.getRoom(eventTimeline.getRoomId()); + if (!room) { + throw new Error("Unknown room " + eventTimeline.getRoomId()); + } + var token = eventTimeline.getPaginationToken(backwards); if (!token) { - // no more results. - return q.reject(new Error("No paginate token")); + // no token - no results. + return q(false); } var dir = backwards ? 'b' : 'f'; @@ -2193,9 +2198,6 @@ MatrixClient.prototype.paginateEventTimeline = function(eventTimeline, opts) { this._http.authedRequest(undefined, "GET", path, params ).then(function(res) { var token = res.end; - console.log("client: completed paginate; backwards=" + backwards + - "; token=" + token); - var room = self.getRoom(eventTimeline.getRoomId()); var matrixEvents = utils.map(res.chunk, self.getEventMapper()); room.addEventsToTimeline(matrixEvents, backwards, eventTimeline, token); diff --git a/lib/models/event-timeline.js b/lib/models/event-timeline.js index 43b3124ed..6913a9eb5 100644 --- a/lib/models/event-timeline.js +++ b/lib/models/event-timeline.js @@ -15,12 +15,12 @@ var RoomState = require("./room-state"); * the room at the beginning and end of the timeline, and pagination tokens for * going backwards and forwards in the timeline. * - *

In order that clients can meaningfully maintain an index into a timeline, we - * track a 'baseIndex'. This starts at zero, but is incremented when events are - * prepended to the timeline. The index of an event relative to baseIndex - * therefore remains constant. + *

In order that clients can meaningfully maintain an index into a timeline, + * the EventTimeline object tracks a 'baseIndex'. This starts at zero, but is + * incremented when events are prepended to the timeline. The index of an event + * relative to baseIndex therefore remains constant. * - *

Once a timeline joins up with its neighbour, we link them together into a + *

Once a timeline joins up with its neighbour, they are linked together into a * doubly-linked list. * * @param {string} roomId the ID of the room where this timeline came from @@ -45,8 +45,9 @@ function EventTimeline(roomId) { * *

This can only be called before any events are added. * - * @param {MatrixEvent[]} stateEvents list of state events to intialise the + * @param {MatrixEvent[]} stateEvents list of state events to initialise the * state with. + * @throws {Error} if an attempt is made to call this after addEvent is called. */ EventTimeline.prototype.initialiseState = function(stateEvents) { if (this._events.length > 0) { @@ -67,7 +68,11 @@ EventTimeline.prototype.getRoomId = function() { }; /** - * Get the base index + * Get the base index. + * + *

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. * * @return {number} */ @@ -137,6 +142,9 @@ EventTimeline.prototype.getNeighbouringTimeline = function(before) { * * @param {boolean} before true to set the previous timeline; false to set * following one. + * + * @throws {Error} if an attempt is made to set the neighbouring timeline when + * it is already set. */ EventTimeline.prototype.setNeighbouringTimeline = function(neighbour, before) { if (this.getNeighbouringTimeline(before)) { diff --git a/lib/models/room.js b/lib/models/room.js index 30b8e804d..44e9ff39a 100644 --- a/lib/models/room.js +++ b/lib/models/room.js @@ -171,7 +171,7 @@ Room.prototype.resetLiveTimeline = function() { this._timelines = [newTimeline]; this._eventIdToTimeline = {}; } else { - newTimeline = this.createTimeline(); + newTimeline = this.addTimeline(); } if (this._liveTimeline) { @@ -292,12 +292,11 @@ Room.prototype.getAvatarUrl = function(baseUrl, width, height, resizeMethod, }; /** - * Create a new timeline for this room + * Add a new timeline to this room * - * @param {Array} stateBeforeEvent state of the room before this event * @return {module:models/event-timeline~EventTimeline} newly-created timeline */ -Room.prototype.createTimeline = function() { +Room.prototype.addTimeline = function() { if (!this._timelineSupport) { throw Error("timeline support is disabled. Set the 'timelineSupport'" + " parameter to true when creating MatrixClient to enable" + @@ -341,12 +340,44 @@ Room.prototype.addEventsToTimeline = function(events, toStartOfTimeline, return; } + // we need to handle the following case: + // + // We already know about two timelines. One of these contains just + // event M, and one contains just event P: + // + // timeline1 timeline2 + // [M] [P] + // + // We are now processing a new pagination response from the server. + // This contains the following events: [M, N, P, R, S]. + // + // 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: + // + // timeline1 timeline2 + // [M, N] <----> [P] + // + // 4. We now need to append R and S to timeline2. So finally, we have: + // + // timeline1 timeline2 + // [M, N] <----> [P, R, S] + // + // 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! + var updateToken = false; for (var i = 0; i < events.length; i++) { - var existingTimeline = this._checkExistingTimeline(events[i], timeline, - toStartOfTimeline); + var existingTimeline = this._checkForNewEventInExistingTimeline( + events[i], timeline, toStartOfTimeline); + if (existingTimeline) { - // switch to the other timeline timeline = existingTimeline; updateToken = false; } else { @@ -371,8 +402,8 @@ Room.prototype.addEventsToTimeline = function(events, toStartOfTimeline, * none * @private */ -Room.prototype._checkExistingTimeline = function(event, timeline, - toStartOfTimeline) { +Room.prototype._checkForNewEventInExistingTimeline = function( + event, timeline, toStartOfTimeline) { var eventId = event.getId(); var existingTimeline = this._eventIdToTimeline[eventId]; @@ -410,7 +441,7 @@ Room.prototype._checkExistingTimeline = function(event, timeline, /** * Check for redactions, and otherwise add event to the given timeline. Assumes - * we have already checked we don't lnow about this event. + * we have already checked we don't know about this event. * * Will fire "Room.timeline" for each event added. * @@ -592,13 +623,16 @@ Room.prototype.removeEvents = function(event_ids) { /** * Removes a single event from this room. + * * @param {String} eventId The id of the event to remove - * @return {?MatrixEvent} the removed event, or null if none + * + * @return {?MatrixEvent} the removed event, or null if the event was not found + * in this room. */ Room.prototype.removeEvent = function(eventId) { var timeline = this._eventIdToTimeline[eventId]; if (!timeline) { - return false; + return null; } var removed = timeline.removeEvent(eventId);