Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement 'pendingEventList' #111

Merged
merged 3 commits into from
Mar 18, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
13 changes: 8 additions & 5 deletions lib/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 "<b>chronological</b>", messages will appear in the timeline
* when the call to <code>sendEvent</code> was made. If "<b>end</b>", 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 "<b>chronological</b>", messages will appear
* in the timeline when the call to <code>sendEvent</code> was made. If
* "<b>detached</b>", 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).
*/
Expand Down
15 changes: 1 addition & 14 deletions lib/models/event-timeline.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down
91 changes: 63 additions & 28 deletions lib/models/room.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 "<b>chronological</b>", messages will appear in the timeline
* when the call to <code>sendEvent</code> was made. If "<b>end</b>", 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 "<b>chronological</b>", messages will appear
* in the timeline when the call to <code>sendEvent</code> was made. If
* "<b>detached</b>", 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.
*
Expand All @@ -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 + "'"
);
}

Expand Down Expand Up @@ -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 <code>opts.pendingEventOrdering</code> 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.
*
Expand Down Expand Up @@ -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 = {
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -643,9 +660,12 @@ Room.prototype._addLiveEvents = function(events) {
/**
* Add a pending outgoing event to this room.
*
* <p>The event is added to either the pendingEventList, or the live timeline,
* depending on the setting of opts.pendingEventOrdering.
*
* <p>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
*
Expand Down Expand Up @@ -674,18 +694,25 @@ 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);
};

/**
* Deal with the echo of a message we sent.
*
* <p>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
Expand All @@ -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.)
Expand All @@ -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 = {};
Expand Down Expand Up @@ -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, " +
Expand Down Expand Up @@ -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];
Expand Down
64 changes: 16 additions & 48 deletions spec/unit/room.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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
Expand All @@ -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"
});
Expand All @@ -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]
);
});
});
});