From 998d9e010ec9eac606361c9b9e13ceab108b03bf Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 4 Mar 2019 21:23:55 -0700 Subject: [PATCH 1/8] Support flushing the cache on calculated push rules Needed for encrypted events to be able to pass some push rules. --- src/client.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/client.js b/src/client.js index a472d17b7..434a24d34 100644 --- a/src/client.js +++ b/src/client.js @@ -2299,10 +2299,11 @@ function _membershipChange(client, roomId, userId, membership, reason, callback) * Obtain a dict of actions which should be performed for this event according * to the push rules for this user. Caches the dict on the event. * @param {MatrixEvent} event The event to get push actions for. + * @param {boolean} ignoreCache True to skip the cache and recalculate the push rules. * @return {module:pushprocessor~PushAction} A dict of actions to perform. */ -MatrixClient.prototype.getPushActionsForEvent = function(event) { - if (!event.getPushActions()) { +MatrixClient.prototype.getPushActionsForEvent = function(event, ignoreCache = false) { + if (!event.getPushActions() || ignoreCache) { event.setPushActions(this._pushProcessor.actionsForEvent(event)); } return event.getPushActions(); From bcd4ad130c8df3d6c1d25e98c278cc75b62e501a Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 4 Mar 2019 21:25:04 -0700 Subject: [PATCH 2/8] Use the decrypted event content when checking the push rules Otherwise we'll be looking at the encrypted source, and that doesn't help anyone. --- src/pushprocessor.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/pushprocessor.js b/src/pushprocessor.js index 712c45399..445fce4be 100644 --- a/src/pushprocessor.js +++ b/src/pushprocessor.js @@ -184,7 +184,16 @@ function PushProcessor(client) { }; const eventFulfillsDisplayNameCondition = function(cond, ev) { - const content = ev.getContent(); + let content = ev.getContent(); + // TODO: Don't use private variable access + if (ev.isEncrypted() && ev._clearEvent) { + // Sometimes the event content is nested for some reason, so unpack that. + if (ev._clearEvent.content) { + content = ev._clearEvent.content; + } else { + content = ev._clearEvent; + } + } if (!content || !content.body || typeof content.body != 'string') { return false; } From 4834e12a3a88c41977c6e62f1a0b5f98728f3dfc Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 4 Mar 2019 21:41:20 -0700 Subject: [PATCH 3/8] Calculate unread badges for encrypted events --- src/client.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/client.js b/src/client.js index 434a24d34..fe3433fc4 100644 --- a/src/client.js +++ b/src/client.js @@ -2304,7 +2304,20 @@ function _membershipChange(client, roomId, userId, membership, reason, callback) */ MatrixClient.prototype.getPushActionsForEvent = function(event, ignoreCache = false) { if (!event.getPushActions() || ignoreCache) { - event.setPushActions(this._pushProcessor.actionsForEvent(event)); + const oldActions = event.getPushActions(); + const actions = this._pushProcessor.actionsForEvent(event); + event.setPushActions(actions); + + // Ensure the unread counts are kept up to date if the event is encrypted + const oldHighlight = oldActions && oldActions.tweaks ? !!oldActions.tweaks.highlight : false; + const newHighlight = actions && actions.tweaks ? !!actions.tweaks.highlight : false; + if (oldHighlight !== newHighlight && event.isEncrypted()) { + const room = this.getRoom(event.getRoomId()); + if (room) { + const current = room.getUnreadNotificationCount("highlight"); + room.setUnreadNotificationCount("highlight", newHighlight ? current + 1 : current - 1); + } + } } return event.getPushActions(); }; From 829cd05cba3d53483bb2bf095d668ce3b29aec14 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 4 Mar 2019 21:46:48 -0700 Subject: [PATCH 4/8] Appease the linter --- src/client.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/client.js b/src/client.js index fe3433fc4..86ee232e8 100644 --- a/src/client.js +++ b/src/client.js @@ -2309,13 +2309,16 @@ MatrixClient.prototype.getPushActionsForEvent = function(event, ignoreCache = fa event.setPushActions(actions); // Ensure the unread counts are kept up to date if the event is encrypted - const oldHighlight = oldActions && oldActions.tweaks ? !!oldActions.tweaks.highlight : false; - const newHighlight = actions && actions.tweaks ? !!actions.tweaks.highlight : false; + const oldHighlight = oldActions && oldActions.tweaks + ? !!oldActions.tweaks.highlight : false; + const newHighlight = actions && actions.tweaks + ? !!actions.tweaks.highlight : false; if (oldHighlight !== newHighlight && event.isEncrypted()) { const room = this.getRoom(event.getRoomId()); if (room) { const current = room.getUnreadNotificationCount("highlight"); - room.setUnreadNotificationCount("highlight", newHighlight ? current + 1 : current - 1); + const newCount = newHighlight ? current + 1 : current - 1; + room.setUnreadNotificationCount("highlight", newCount); } } } From 37f106d4af206e6bcb7f1accd611b666c8e13c84 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 5 Mar 2019 14:04:39 -0700 Subject: [PATCH 5/8] More safely set the push actions for an encrypted event --- src/client.js | 47 ++++++++++++++++++++++++++------------------- src/models/event.js | 8 ++++++++ src/models/room.js | 33 +++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 20 deletions(-) diff --git a/src/client.js b/src/client.js index 86ee232e8..d87ec2441 100644 --- a/src/client.js +++ b/src/client.js @@ -228,6 +228,30 @@ function MatrixClient(opts) { this._serverSupportsLazyLoading = null; this._cachedCapabilities = null; // { capabilities: {}, lastUpdated: timestamp } + + // The SDK doesn't really provide a clean way for events to recalculate the push + // actions for themselves, so we have to kinda help them out when they are encrypted. + // We do this so that push rules are correctly executed on events in their decrypted + // state, such as highlights when the user's name is mentioned. + this.on("Event.decrypted", (event) => { + const oldActions = event.getPushActions(); + const actions = this._pushProcessor.actionsForEvent(event); + event.setPushActions(actions); // Might as well while we're here + + // Ensure the unread counts are kept up to date if the event is encrypted + const oldHighlight = oldActions && oldActions.tweaks + ? !!oldActions.tweaks.highlight : false; + const newHighlight = actions && actions.tweaks + ? !!actions.tweaks.highlight : false; + if (oldHighlight !== newHighlight) { + const room = this.getRoom(event.getRoomId()); + if (room && !room.hasUserReadEvent(this.getUserId(), event.getId())) { + const current = room.getUnreadNotificationCount("highlight"); + const newCount = newHighlight ? current + 1 : current - 1; + room.setUnreadNotificationCount("highlight", newCount); + } + } + }); } utils.inherits(MatrixClient, EventEmitter); utils.extend(MatrixClient.prototype, MatrixBaseApis.prototype); @@ -2299,28 +2323,11 @@ function _membershipChange(client, roomId, userId, membership, reason, callback) * Obtain a dict of actions which should be performed for this event according * to the push rules for this user. Caches the dict on the event. * @param {MatrixEvent} event The event to get push actions for. - * @param {boolean} ignoreCache True to skip the cache and recalculate the push rules. * @return {module:pushprocessor~PushAction} A dict of actions to perform. */ -MatrixClient.prototype.getPushActionsForEvent = function(event, ignoreCache = false) { - if (!event.getPushActions() || ignoreCache) { - const oldActions = event.getPushActions(); - const actions = this._pushProcessor.actionsForEvent(event); - event.setPushActions(actions); - - // Ensure the unread counts are kept up to date if the event is encrypted - const oldHighlight = oldActions && oldActions.tweaks - ? !!oldActions.tweaks.highlight : false; - const newHighlight = actions && actions.tweaks - ? !!actions.tweaks.highlight : false; - if (oldHighlight !== newHighlight && event.isEncrypted()) { - const room = this.getRoom(event.getRoomId()); - if (room) { - const current = room.getUnreadNotificationCount("highlight"); - const newCount = newHighlight ? current + 1 : current - 1; - room.setUnreadNotificationCount("highlight", newCount); - } - } +MatrixClient.prototype.getPushActionsForEvent = function(event) { + if (!event.getPushActions()) { + event.setPushActions(this._pushProcessor.actionsForEvent(event)); } return event.getPushActions(); }; diff --git a/src/models/event.js b/src/models/event.js index 823ced97f..025ec9619 100644 --- a/src/models/event.js +++ b/src/models/event.js @@ -471,6 +471,14 @@ utils.extend(module.exports.MatrixEvent.prototype, { this._retryDecryption = false; this._setClearData(res); + // Before we emit the event, clear the push actions so that they can be recalculated + // by relevant code. We do this because the clear event has now changed, making it + // so that existing rules can be re-run over the applicable properties. Stuff like + // highlighting when the user's name is mentioned rely on this happening. We also want + // to set the push actions before emitting so that any notification listeners don't + // pick up the wrong contents. + this.setPushActions(null); + this.emit("Event.decrypted", this, err); return; diff --git a/src/models/room.js b/src/models/room.js index 15477a9c5..3bc58ccc5 100644 --- a/src/models/room.js +++ b/src/models/room.js @@ -1424,6 +1424,39 @@ Room.prototype.getEventReadUpTo = function(userId, ignoreSynthesized) { return receipts["m.read"][userId].eventId; }; +/** + * Determines if the given user has read a particular event ID with the known + * history of the room. This is not a definitive check as it relies only on + * what is available to the room at the time of execution. + * @param {String} userId The user ID to check the read state of. + * @param {String} eventId The event ID to check if the user read. + */ +Room.prototype.hasUserReadEvent = function(userId, eventId) { + const readUpToId = this.getEventReadUpTo(userId, false); + if (readUpToId === eventId) return true; + + if (this.timeline.length + && this.timeline[this.timeline.length - 1].getSender() + && this.timeline[this.timeline.length - 1].getSender() === userId) { + // It doesn't matter where the event is in the timeline, the user has read + // it because they've sent the latest event. + return true; + } + + for (let i = this.timeline.length - 1; i >= 0; --i) { + const ev = this.timeline[i]; + + // If we encounter the target event first, the user hasn't read it + // however if we encounter the readUpToId first then the user has read + // it. These rules apply because we're iterating bottom-up. + if (ev.getId() === eventId) return false; + if (ev.getId() === readUpToId) return true; + } + + // We don't know if the user has read it, so assume not. + return false; +}; + /** * Get a list of receipts for the given event. * @param {MatrixEvent} event the event to get receipts for From 2f2deb5333c06dee1e64bf52e960afac7c899e54 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 5 Mar 2019 14:04:51 -0700 Subject: [PATCH 6/8] Expose the clear event content directly from an event --- src/models/event.js | 11 +++++++++++ src/pushprocessor.js | 10 ++-------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/models/event.js b/src/models/event.js index 025ec9619..ff386f9c7 100644 --- a/src/models/event.js +++ b/src/models/event.js @@ -519,6 +519,17 @@ utils.extend(module.exports.MatrixEvent.prototype, { decryptionResult.forwardingCurve25519KeyChain || []; }, + /** + * Gets the cleartext content for this event. If the event is not encrypted, + * or encryption has not been completed, this will return null. + * + * @returns {Object} The cleartext (decrypted) content for the event + */ + getClearContent: function() { + const ev = this._clearEvent; + return ev && ev.content ? ev.content : null; + }, + /** * Check if the event is encrypted. * @return {boolean} True if this event is encrypted. diff --git a/src/pushprocessor.js b/src/pushprocessor.js index 445fce4be..f7ee4721f 100644 --- a/src/pushprocessor.js +++ b/src/pushprocessor.js @@ -185,14 +185,8 @@ function PushProcessor(client) { const eventFulfillsDisplayNameCondition = function(cond, ev) { let content = ev.getContent(); - // TODO: Don't use private variable access - if (ev.isEncrypted() && ev._clearEvent) { - // Sometimes the event content is nested for some reason, so unpack that. - if (ev._clearEvent.content) { - content = ev._clearEvent.content; - } else { - content = ev._clearEvent; - } + if (ev.isEncrypted() && ev.getClearContent()) { + content = ev.getClearContent(); } if (!content || !content.body || typeof content.body != 'string') { return false; From 54769d91369c4d7e4dce4a1fbca4abead12924ea Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 5 Mar 2019 14:25:32 -0700 Subject: [PATCH 7/8] Add jsdoc --- src/models/room.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/models/room.js b/src/models/room.js index 3bc58ccc5..3c03b8ad7 100644 --- a/src/models/room.js +++ b/src/models/room.js @@ -1430,6 +1430,7 @@ Room.prototype.getEventReadUpTo = function(userId, ignoreSynthesized) { * what is available to the room at the time of execution. * @param {String} userId The user ID to check the read state of. * @param {String} eventId The event ID to check if the user read. + * @returns {Boolean} True if the user has read the event, false otherwise. */ Room.prototype.hasUserReadEvent = function(userId, eventId) { const readUpToId = this.getEventReadUpTo(userId, false); From 61989439762e839ec09c373ed7630020192eb963 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 6 Mar 2019 09:56:36 -0700 Subject: [PATCH 8/8] Add a mention that we should be handling gaps in /sync --- src/client.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/client.js b/src/client.js index d87ec2441..5e2a1cca1 100644 --- a/src/client.js +++ b/src/client.js @@ -245,6 +245,8 @@ function MatrixClient(opts) { ? !!actions.tweaks.highlight : false; if (oldHighlight !== newHighlight) { const room = this.getRoom(event.getRoomId()); + // TODO: Handle mentions received while the client is offline + // See also https://github.com/vector-im/riot-web/issues/9069 if (room && !room.hasUserReadEvent(this.getUserId(), event.getId())) { const current = room.getUnreadNotificationCount("highlight"); const newCount = newHighlight ? current + 1 : current - 1;