diff --git a/CHANGELOG.md b/CHANGELOG.md index 684f76a90e9..b98dde02858 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,13 @@ +Unreleased Changes +================== + +**BREAKING CHANGES**: + * `opts.pendingEventOrdering`==`end` is no longer supported in the arguments to + `MatrixClient.startClient()`. Instead we provide a `detached` option, which + puts pending events into a completely separate list in the Room, accessible + via `Room.getPendingEvents()`. + [\#111](https://github.com/matrix-org/matrix-js-sdk/pull/111) + Changes in [0.4.2](https://github.com/matrix-org/matrix-js-sdk/releases/tag/v0.4.2) (2016-03-17) ================================================================================================ [Full Changelog](https://github.com/matrix-org/matrix-js-sdk/compare/v0.4.1...v0.4.2) diff --git a/lib/client.js b/lib/client.js index be453083e85..ead84d6a44a 100644 --- a/lib/client.js +++ b/lib/client.js @@ -2900,11 +2900,14 @@ MatrixClient.prototype.isLoggedIn = function() { * @param {Boolean=} opts.resolveInvitesToProfiles True to do /profile requests * on every invite event if the displayname/avatar_url is not known for this user ID. * Default: false. - * @param {String=} opts.pendingEventOrdering Controls where pending messages appear - * in a room's timeline. If "chronological", messages will appear in the timeline - * when the call to sendEvent was made. If "end", pending messages - * will always appear at the end of the timeline (multiple pending messages will be sorted - * chronologically). Default: "chronological". + * + * @param {String=} opts.pendingEventOrdering Controls where pending messages + * appear in a room's timeline. If "chronological", messages will appear + * in the timeline when the call to sendEvent was made. If + * "detached", pending messages will appear in a separate list, + * accessbile via {@link module:models/room~Room#getPendingEvents}. Default: + * "chronological". + * * @param {Number=} opts.pollTimeout The number of milliseconds to wait on /events. * Default: 30000 (30 seconds). */ diff --git a/lib/models/event-timeline.js b/lib/models/event-timeline.js index 673e32e9248..06300972e07 100644 --- a/lib/models/event-timeline.js +++ b/lib/models/event-timeline.js @@ -211,10 +211,8 @@ EventTimeline.prototype.setNeighbouringTimeline = function(neighbour, direction) * * @param {MatrixEvent} event new event * @param {boolean} atStart true to insert new event at the start - * @param {boolean} [spliceBeforeLocalEcho = false] insert this event before any - * localecho events at the end of the timeline. Ignored if atStart == true */ -EventTimeline.prototype.addEvent = function(event, atStart, spliceBeforeLocalEcho) { +EventTimeline.prototype.addEvent = function(event, atStart) { var stateContext = atStart ? this._startState : this._endState; setEventMetadata(event, stateContext, atStart); @@ -243,17 +241,6 @@ EventTimeline.prototype.addEvent = function(event, atStart, spliceBeforeLocalEch insertIndex = 0; } else { insertIndex = this._events.length; - - // if this is a real event, we might need to splice it in before any pending - // local echo events. - if (spliceBeforeLocalEcho) { - for (var j = this._events.length - 1; j >= 0; j--) { - if (!this._events[j].status) { // real events don't have a status - insertIndex = j + 1; - break; - } - } - } } this._events.splice(insertIndex, 0, event); // insert element diff --git a/lib/models/room.js b/lib/models/room.js index dc4a9ed74db..f00fb48f5bc 100644 --- a/lib/models/room.js +++ b/lib/models/room.js @@ -68,11 +68,14 @@ function synthesizeReceipt(userId, event, receiptType) { * @param {*} opts.storageToken Optional. The token which a data store can use * to remember the state of the room. What this means is dependent on the store * implementation. - * @param {String=} opts.pendingEventOrdering Controls where pending messages appear - * in a room's timeline. If "chronological", messages will appear in the timeline - * when the call to sendEvent was made. If "end", pending messages - * will always appear at the end of the timeline (multiple pending messages will be sorted - * chronologically). Default: "chronological". + * + * @param {String=} opts.pendingEventOrdering Controls where pending messages + * appear in a room's timeline. If "chronological", messages will appear + * in the timeline when the call to sendEvent was made. If + * "detached", pending messages will appear in a separate list, + * accessbile via {@link module:models/room~Room#getPendingEvents}. Default: + * "chronological". + * * @param {boolean} [opts.timelineSupport = false] Set to true to enable improved * timeline support. * @@ -99,10 +102,10 @@ function Room(roomId, opts) { opts = opts || {}; opts.pendingEventOrdering = opts.pendingEventOrdering || "chronological"; - if (["chronological", "end"].indexOf(opts.pendingEventOrdering) === -1) { + if (["chronological", "detached"].indexOf(opts.pendingEventOrdering) === -1) { throw new Error( "opts.pendingEventOrdering MUST be either 'chronological' or " + - "'end'. Got: '" + opts.pendingEventOrdering + "'" + "'detached'. Got: '" + opts.pendingEventOrdering + "'" ); } @@ -151,9 +154,31 @@ function Room(roomId, opts) { this._eventIdToTimeline = {}; this._timelineSupport = Boolean(opts.timelineSupport); + if (this._opts.pendingEventOrdering == "detached") { + this._pendingEventList = []; + } } utils.inherits(Room, EventEmitter); +/** + * Get the list of pending sent events for this room + * + * @return {module:models/event.MatrixEvent[]} A list of the sent events + * waiting for remote echo. + * + * @throws If opts.pendingEventOrdering was not 'detached' + */ +Room.prototype.getPendingEvents = function() { + if (this._opts.pendingEventOrdering !== "detached") { + throw new Error( + "Cannot call getPendingEventList with pendingEventOrdering == " + + this._opts.pendingEventOrdering); + } + + return this._pendingEventList; +}; + + /** * Get the live timeline for this room. * @@ -558,18 +583,13 @@ Room.prototype.addEventsToTimeline = function(events, toStartOfTimeline, * @param {EventTimeline} timeline * @param {boolean} toStartOfTimeline * - * @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 */ -Room.prototype._addEventToTimeline = function(event, timeline, toStartOfTimeline, - spliceBeforeLocalEcho) { +Room.prototype._addEventToTimeline = function(event, timeline, toStartOfTimeline) { var eventId = event.getId(); - timeline.addEvent(event, toStartOfTimeline, spliceBeforeLocalEcho); + timeline.addEvent(event, toStartOfTimeline); this._eventIdToTimeline[eventId] = timeline; var data = { @@ -589,8 +609,6 @@ Room.prototype._addEventToTimeline = function(event, timeline, toStartOfTimeline * @private */ Room.prototype._addLiveEvents = function(events) { - var addLocalEchoToEnd = this._opts.pendingEventOrdering === "end"; - for (var i = 0; i < events.length; i++) { if (events[i].getType() === "m.room.redaction") { var redactId = events[i].event.redacts; @@ -624,8 +642,7 @@ Room.prototype._addLiveEvents = function(events) { if (!this._eventIdToTimeline[events[i].getId()]) { // TODO: pass through filter to see if this should be added to the timeline. - this._addEventToTimeline(events[i], this._liveTimeline, false, - addLocalEchoToEnd); + this._addEventToTimeline(events[i], this._liveTimeline, false); } // synthesize and inject implicit read receipts @@ -643,9 +660,12 @@ Room.prototype._addLiveEvents = function(events) { /** * Add a pending outgoing event to this room. * + *

The event is added to either the pendingEventList, or the live timeline, + * depending on the setting of opts.pendingEventOrdering. + * *

This is an internal method, intended for use by MatrixClient. * - * @param {module:models/event~MatrixEvent} event The event to add. + * @param {module:models/event.MatrixEvent} event The event to add. * * @param {string} txnId Transaction id for this outgoing event * @@ -674,7 +694,11 @@ Room.prototype.addPendingEvent = function(event, txnId) { this._txnToEvent[txnId] = event; - this._addEventToTimeline(event, this._liveTimeline, false); + if (this._opts.pendingEventOrdering == "detached") { + this._pendingEventList.push(event); + } else { + this._addEventToTimeline(event, this._liveTimeline, false); + } this.emit("Room.localEchoUpdated", event, this, null, null); }; @@ -682,10 +706,13 @@ Room.prototype.addPendingEvent = function(event, txnId) { /** * Deal with the echo of a message we sent. * + *

We move the event to the live timeline if it isn't there already, and + * update it. + * * @param {module:models/event~MatrixEvent} remoteEvent The event received from * /sync * @param {module:models/event~MatrixEvent} localEvent The local echo, which - * should already be in the timeline. + * should be either in the _pendingEventList or the timeline. * * @fires module:client~MatrixClient#event:"Room.localEchoUpdated" * @private @@ -698,6 +725,15 @@ Room.prototype._handleRemoteEcho = function(remoteEvent, localEvent) { // no longer pending delete this._txnToEvent[remoteEvent.transaction_id]; + // if it's in the pending list, remove it + if (this._pendingEventList) { + utils.removeElement( + this._pendingEventList, + function(ev) { return ev.getId() == oldEventId; }, + false + ); + } + // replace the event source, but preserve the original content // and type in case it was encrypted (we won't be able to // decrypt it, even though we sent it.) @@ -709,18 +745,19 @@ Room.prototype._handleRemoteEcho = function(remoteEvent, localEvent) { // successfully sent. localEvent.status = null; - // Update the timeline map. + // if it's already in the timeline, update the timeline map. If it's not, add it. var existingTimeline = this._eventIdToTimeline[oldEventId]; if (existingTimeline) { delete this._eventIdToTimeline[oldEventId]; this._eventIdToTimeline[newEventId] = existingTimeline; + } else { + this._addEventToTimeline(localEvent, this._liveTimeline, false); } this.emit("Room.localEchoUpdated", localEvent, this, oldEventId, oldStatus); }; - /* a map from current event status to a list of allowed next statuses */ var ALLOWED_TRANSITIONS = {}; @@ -750,10 +787,6 @@ ALLOWED_TRANSITIONS[EventStatus.NOT_SENT] = * @fires module:client~MatrixClient#event:"Room.localEchoUpdated" */ Room.prototype.updatePendingEvent = function(event, newStatus, newEventId) { - if (!this.getTimelineForEvent(event.getId())) { - throw new Error("updateLocalEchoStatus called on an unknown event."); - } - // if the message was sent, we expect an event id if (newStatus == EventStatus.SENT && !newEventId) { throw new Error("updatePendingEvent called with status=SENT, " + @@ -790,7 +823,9 @@ Room.prototype.updatePendingEvent = function(event, newStatus, newEventId) { // update the event id event.event.event_id = newEventId; - // Update the timeline map + // if the event was already in the timeline (which will be the case if + // opts.pendingEventOrdering==chronological), we need to update the + // timeline map. var existingTimeline = this._eventIdToTimeline[oldEventId]; if (existingTimeline) { delete this._eventIdToTimeline[oldEventId]; diff --git a/spec/unit/room.spec.js b/spec/unit/room.spec.js index e7a5848fa98..342b7d83264 100644 --- a/spec/unit/room.spec.js +++ b/spec/unit/room.spec.js @@ -333,7 +333,6 @@ describe("Room", function() { var localEvent = utils.mkMessage({ room: roomId, user: userA, event: true, }); - localEvent._txnId = "TXN_ID"; localEvent.status = EventStatus.SENDING; var localEventId = localEvent.getId(); @@ -1144,10 +1143,11 @@ describe("Room", function() { }); }); - describe("pendingEventOrdering", function() { - it("should sort pending events to the end of the timeline if 'end'", function() { + describe("addPendingEvent", function() { + it("should add pending events to the pendingEventList if " + + "pendingEventOrdering == 'detached'", function() { var room = new Room(roomId, { - pendingEventOrdering: "end" + pendingEventOrdering: "detached" }); var eventA = utils.mkMessage({ room: roomId, user: userA, msg: "remote 1", event: true @@ -1159,13 +1159,19 @@ describe("Room", function() { var eventC = utils.mkMessage({ room: roomId, user: userA, msg: "remote 2", event: true }); - room.addEvents([eventA, eventB, eventC]); + room.addEvents([eventA]); + room.addPendingEvent(eventB, "TXN1"); + room.addEvents([eventC]); expect(room.timeline).toEqual( - [eventA, eventC, eventB] + [eventA, eventC] + ); + expect(room.getPendingEvents()).toEqual( + [eventB] ); }); - it("should sort pending events chronologically if 'chronological'", function() { + it("should add pending events to the timeline if " + + "pendingEventOrdering == 'chronological'", function() { room = new Room(roomId, { pendingEventOrdering: "chronological" }); @@ -1179,50 +1185,12 @@ describe("Room", function() { var eventC = utils.mkMessage({ room: roomId, user: userA, msg: "remote 2", event: true }); - room.addEvents([eventA, eventB, eventC]); + room.addEvents([eventA]); + room.addPendingEvent(eventB, "TXN1"); + room.addEvents([eventC]); expect(room.timeline).toEqual( [eventA, eventB, eventC] ); }); - - it("should treat NOT_SENT events as local echo", function() { - var room = new Room(roomId, { - pendingEventOrdering: "end" - }); - var eventA = utils.mkMessage({ - room: roomId, user: userA, msg: "remote 1", event: true - }); - var eventB = utils.mkMessage({ - room: roomId, user: userA, msg: "local 1", event: true - }); - eventB.status = EventStatus.NOT_SENT; - var eventC = utils.mkMessage({ - room: roomId, user: userA, msg: "remote 2", event: true - }); - room.addEvents([eventA, eventB, eventC]); - expect(room.timeline).toEqual( - [eventA, eventC, eventB] - ); - }); - - it("should treat QUEUED events as local echo", function() { - var room = new Room(roomId, { - pendingEventOrdering: "end" - }); - var eventA = utils.mkMessage({ - room: roomId, user: userA, msg: "remote 1", event: true - }); - var eventB = utils.mkMessage({ - room: roomId, user: userA, msg: "local 1", event: true - }); - eventB.status = EventStatus.QUEUED; - var eventC = utils.mkMessage({ - room: roomId, user: userA, msg: "remote 2", event: true - }); - room.addEvents([eventA, eventB, eventC]); - expect(room.timeline).toEqual( - [eventA, eventC, eventB] - ); - }); }); });