From 79aefe9707f5b2ded13469d45a49d7fda3f22d45 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Mon, 12 Apr 2021 09:37:19 +0100 Subject: [PATCH 1/8] Fix timeline jumpiness by setting correct txnId --- src/models/room.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/models/room.js b/src/models/room.js index 302d571a2..93ed22f32 100644 --- a/src/models/room.js +++ b/src/models/room.js @@ -195,9 +195,10 @@ export function Room(roomId, client, myUserId, opts) { if (serializedPendingEventList) { JSON.parse(serializedPendingEventList) .forEach(serializedEvent => { + const txnId = client.makeTxnId(); const event = new MatrixEvent(serializedEvent); event.setStatus(EventStatus.NOT_SENT); - const txnId = client.makeTxnId(); + event.setTxnId(txnId); this.addPendingEvent(event, txnId); }); } @@ -1472,9 +1473,6 @@ Room.prototype.updatePendingEvent = function(event, newStatus, newEventId) { for (let i = 0; i < this._timelineSets.length; i++) { this._timelineSets[i].replaceEventId(oldEventId, newEventId); } - if (this._opts.pendingEventOrdering === "detached") { - this.removePendingEvent(event.event.event_id); - } } else if (newStatus == EventStatus.CANCELLED) { // remove it from the pending event list, or the timeline. if (this._pendingEventList) { From 7371f8dd3a58d3e44262a4d9617d67196b769346 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Mon, 12 Apr 2021 09:55:01 +0100 Subject: [PATCH 2/8] Persist txnId to ensure idempotency --- src/models/event.js | 3 ++- src/models/room.js | 4 +--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/models/event.js b/src/models/event.js index 4124df708..b959f6df4 100644 --- a/src/models/event.js +++ b/src/models/event.js @@ -168,7 +168,7 @@ export const MatrixEvent = function( /* The txnId with which this event was sent if it was during this session, allows for a unique ID which does not change when the event comes back down sync. */ - this._txnId = null; + this._txnId = event.txn_id || null; /* Set an approximate timestamp for the event relative the local clock. * This will inherently be approximate because it doesn't take into account @@ -1098,6 +1098,7 @@ utils.extend(MatrixEvent.prototype, { origin_server_ts: this.getTs(), unsigned: this.getUnsigned(), room_id: this.getRoomId(), + txn_id: this.getTxnId(), }; // if this is a redaction then attach the redacts key diff --git a/src/models/room.js b/src/models/room.js index 93ed22f32..85403b4b2 100644 --- a/src/models/room.js +++ b/src/models/room.js @@ -195,11 +195,9 @@ export function Room(roomId, client, myUserId, opts) { if (serializedPendingEventList) { JSON.parse(serializedPendingEventList) .forEach(serializedEvent => { - const txnId = client.makeTxnId(); const event = new MatrixEvent(serializedEvent); event.setStatus(EventStatus.NOT_SENT); - event.setTxnId(txnId); - this.addPendingEvent(event, txnId); + this.addPendingEvent(event, event.getTxnId()); }); } } From 2f05be599c73ae523d46362e4dde6aed786e6041 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Mon, 12 Apr 2021 12:26:27 +0100 Subject: [PATCH 3/8] undo changes to event#toJSON and persist encrypted events --- src/models/event.js | 1 - src/models/room.js | 44 +++++++++++++++++++++++++++++--------------- 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/src/models/event.js b/src/models/event.js index b959f6df4..767539895 100644 --- a/src/models/event.js +++ b/src/models/event.js @@ -1098,7 +1098,6 @@ utils.extend(MatrixEvent.prototype, { origin_server_ts: this.getTs(), unsigned: this.getUnsigned(), room_id: this.getRoomId(), - txn_id: this.getTxnId(), }; // if this is a redaction then attach the redacts key diff --git a/src/models/room.js b/src/models/room.js index 85403b4b2..41002f5a1 100644 --- a/src/models/room.js +++ b/src/models/room.js @@ -194,8 +194,11 @@ export function Room(roomId, client, myUserId, opts) { const serializedPendingEventList = client._sessionStore.store.getItem(pendingEventsKey(this.roomId)); if (serializedPendingEventList) { JSON.parse(serializedPendingEventList) - .forEach(serializedEvent => { + .forEach(async serializedEvent => { const event = new MatrixEvent(serializedEvent); + if (event.getType() === "m.room.encrypted") { + await event.attemptDecryption(this._client._crypto); + } event.setStatus(EventStatus.NOT_SENT); this.addPendingEvent(event, event.getTxnId()); }); @@ -395,15 +398,7 @@ Room.prototype.removePendingEvent = function(eventId) { }, false, ); - const { store } = this._client._sessionStore; - if (this._pendingEventList.length > 0) { - store.setItem( - pendingEventsKey(this.roomId), - JSON.stringify(this._pendingEventList), - ); - } else { - store.removeItem(pendingEventsKey(this.roomId)); - } + this._savePendingEvents(); return removed; }; @@ -1270,11 +1265,7 @@ Room.prototype.addPendingEvent = function(event, txnId) { event.setStatus(EventStatus.NOT_SENT); } this._pendingEventList.push(event); - const { store } = this._client._sessionStore; - store.setItem( - pendingEventsKey(this.roomId), - JSON.stringify(this._pendingEventList), - ); + this._savePendingEvents(); if (event.isRelation()) { // For pending events, add them to the relations collection immediately. // (The alternate case below already covers this as part of adding to @@ -1311,6 +1302,28 @@ Room.prototype.addPendingEvent = function(event, txnId) { this.emit("Room.localEchoUpdated", event, this, null, null); }; + +Room.prototype._savePendingEvents = async function() { + const pendingEvents = await Promise.all( + this._pendingEventList.map(async (event) => { + return { + ...event.event, + txn_id: event.getTxnId(), + }; + }), + ); + + const { store } = this._client._sessionStore; + if (this._pendingEventList.length > 0) { + store.setItem( + pendingEventsKey(this.roomId), + JSON.stringify(pendingEvents), + ); + } else { + store.removeItem(pendingEventsKey(this.roomId)); + } +}; + /** * Used to aggregate the local echo for a relation, and also * for re-applying a relation after it's redaction has been cancelled, @@ -1484,6 +1497,7 @@ Room.prototype.updatePendingEvent = function(event, newStatus, newEventId) { } this.removeEvent(oldEventId); } + this._savePendingEvents(); this.emit("Room.localEchoUpdated", event, this, oldEventId, oldStatus); }; From 64aaed833b622caf61d2d4dab14e9434a6f0efc9 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Mon, 12 Apr 2021 12:53:27 +0100 Subject: [PATCH 4/8] Only persist encrypted events for encrypted rooms --- src/models/room.js | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/models/room.js b/src/models/room.js index 41002f5a1..25dfe7978 100644 --- a/src/models/room.js +++ b/src/models/room.js @@ -1303,15 +1303,19 @@ Room.prototype.addPendingEvent = function(event, txnId) { this.emit("Room.localEchoUpdated", event, this, null, null); }; -Room.prototype._savePendingEvents = async function() { - const pendingEvents = await Promise.all( - this._pendingEventList.map(async (event) => { - return { - ...event.event, - txn_id: event.getTxnId(), - }; - }), - ); +Room.prototype._savePendingEvents = function() { + const pendingEvents = this._pendingEventList.map(event => { + return { + ...event.event, + txn_id: event.getTxnId(), + }; + }).filter(event => { + // Filter out the unencrypted messages if the room is encrypted + const isEventEncrypted = event.getType !== "m.room.encrypted"; + const isRoomEncrypted = this._client.isRoomEncrypted(this.roomId); + + return !isRoomEncrypted || (isRoomEncrypted && isEventEncrypted); + }); const { store } = this._client._sessionStore; if (this._pendingEventList.length > 0) { From d534ab18c790af9bc2c222cb7ae609ffb9e895a5 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Mon, 12 Apr 2021 13:07:57 +0100 Subject: [PATCH 5/8] fix event filtering logic --- src/models/room.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/models/room.js b/src/models/room.js index 25dfe7978..5a5a2318c 100644 --- a/src/models/room.js +++ b/src/models/room.js @@ -1311,10 +1311,9 @@ Room.prototype._savePendingEvents = function() { }; }).filter(event => { // Filter out the unencrypted messages if the room is encrypted - const isEventEncrypted = event.getType !== "m.room.encrypted"; + const isEventEncrypted = event.getType() === "m.room.encrypted"; const isRoomEncrypted = this._client.isRoomEncrypted(this.roomId); - - return !isRoomEncrypted || (isRoomEncrypted && isEventEncrypted); + return isEventEncrypted || !isRoomEncrypted; }); const { store } = this._client._sessionStore; From 4c2a83c47097132e70561d9507254a3c7891ba0d Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Mon, 12 Apr 2021 14:29:10 +0100 Subject: [PATCH 6/8] Add explanation for events persistence --- src/models/room.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/models/room.js b/src/models/room.js index 5a5a2318c..7400fda68 100644 --- a/src/models/room.js +++ b/src/models/room.js @@ -1303,6 +1303,19 @@ Room.prototype.addPendingEvent = function(event, txnId) { this.emit("Room.localEchoUpdated", event, this, null, null); }; +/** + * Persists all pending events to local storage + * + * If the current room is encrypted only encrypted events will be persisted + * all messages that are not yet encrypted will be discarded + * + * This is because the flow of EVENT_STATUS transition is + * queued => sending => encrypting => sending => sent + * + * Steps 3 and 4 are skipped for unencrypted room. + * It is better to discard an unencrypted message rather than persisting + * it locally for everyone to read + */ Room.prototype._savePendingEvents = function() { const pendingEvents = this._pendingEventList.map(event => { return { From 466f749b7175a76d91698b8bc54daa75d6843a9b Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Mon, 12 Apr 2021 14:38:27 +0100 Subject: [PATCH 7/8] fix typo --- src/models/room.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/models/room.js b/src/models/room.js index 7400fda68..db41996f6 100644 --- a/src/models/room.js +++ b/src/models/room.js @@ -1324,7 +1324,7 @@ Room.prototype._savePendingEvents = function() { }; }).filter(event => { // Filter out the unencrypted messages if the room is encrypted - const isEventEncrypted = event.getType() === "m.room.encrypted"; + const isEventEncrypted = event.type === "m.room.encrypted"; const isRoomEncrypted = this._client.isRoomEncrypted(this.roomId); return isEventEncrypted || !isRoomEncrypted; }); From 2666a271a5c3a513e18a3fea5b1aaa2690e492db Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Mon, 12 Apr 2021 14:51:45 +0100 Subject: [PATCH 8/8] fix tests when pendingEventsList does not exist --- src/models/room.js | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/src/models/room.js b/src/models/room.js index db41996f6..d6b6a8d9d 100644 --- a/src/models/room.js +++ b/src/models/room.js @@ -1317,26 +1317,28 @@ Room.prototype.addPendingEvent = function(event, txnId) { * it locally for everyone to read */ Room.prototype._savePendingEvents = function() { - const pendingEvents = this._pendingEventList.map(event => { - return { - ...event.event, - txn_id: event.getTxnId(), - }; - }).filter(event => { - // Filter out the unencrypted messages if the room is encrypted - const isEventEncrypted = event.type === "m.room.encrypted"; - const isRoomEncrypted = this._client.isRoomEncrypted(this.roomId); - return isEventEncrypted || !isRoomEncrypted; - }); + if (this._pendingEventList) { + const pendingEvents = this._pendingEventList.map(event => { + return { + ...event.event, + txn_id: event.getTxnId(), + }; + }).filter(event => { + // Filter out the unencrypted messages if the room is encrypted + const isEventEncrypted = event.type === "m.room.encrypted"; + const isRoomEncrypted = this._client.isRoomEncrypted(this.roomId); + return isEventEncrypted || !isRoomEncrypted; + }); - const { store } = this._client._sessionStore; - if (this._pendingEventList.length > 0) { - store.setItem( - pendingEventsKey(this.roomId), - JSON.stringify(pendingEvents), - ); - } else { - store.removeItem(pendingEventsKey(this.roomId)); + const { store } = this._client._sessionStore; + if (this._pendingEventList.length > 0) { + store.setItem( + pendingEventsKey(this.roomId), + JSON.stringify(pendingEvents), + ); + } else { + store.removeItem(pendingEventsKey(this.roomId)); + } } };