From 14a48c118271d2ee2f1f1a42a43b40fdecebf1be Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 6 Nov 2015 15:13:30 +0000 Subject: [PATCH 1/5] Synthesize implicit read receipts in recalculateRoom to make them correct when the room is first loaded. --- lib/models/room.js | 66 +++++++++++++++++++++++++++++++++++++++------- 1 file changed, 56 insertions(+), 10 deletions(-) diff --git a/lib/models/room.js b/lib/models/room.js index bf3bdfae7..857f401cf 100644 --- a/lib/models/room.js +++ b/lib/models/room.js @@ -201,16 +201,10 @@ Room.prototype.addEventsToTimeline = function(events, toStartOfTimeline) { // This is really ugly because JS has no way to express an object literal // where the name of a key comes from an expression - if (events[i].sender) { - var fakeReceipt = {content: {}}; - fakeReceipt.content[events[i].getId()] = { - 'm.read': { - } - }; - fakeReceipt.content[events[i].getId()]['m.read'][events[i].sender.userId] = { - ts: events[i].getTs() - }; - this.addReceipt(new MatrixEvent(fakeReceipt)); + if (!toStartOfTimeline && events[i].sender) { + this.addReceipt(new MatrixEvent(this._synthesizeReceipt( + events[i].sender.userId, events[i], "m.read" + ))); } this.emit("Room.timeline", events[i], this, Boolean(toStartOfTimeline), false); @@ -347,8 +341,60 @@ Room.prototype.recalculate = function(userId) { if (oldName !== this.name) { this.emit("Room.name", this); } + + + + // recalculate read receipts, adding implicit ones where necessary + // NB. This is a duplication of logic for injecting implicit receipts, + // it would be technically possible to only ever generate these + // receipts in addEventsToTimeline but doing so means correctly + // choosing whether to keep or replace the existing receipt which + // is complex and slow. This is faster and more understandable. + + var usersFound = {}; + for (var i = this.timeline.length - 1; i >= 0; --i) { + // loop through the timeline backwards looking for either an + // event sent by each user or a real receipt from them. + // Replace the read receipt for that user with whichever + // occurs later in the timeline (ie. first because we're going + // backwards). + var e = this.timeline[i]; + + var readReceiptsForEvent = this.getReceiptsForEvent(e); + + for (var receiptIt = 0; receiptIt < readReceiptsForEvent.length; ++receiptIt) { + var receipt = readReceiptsForEvent[receiptIt]; + if (receipt.type !== "m.read") { continue; } + + var userId = receipt.userId; + if (usersFound[userId]) { continue; } + + // Then this is the receipt we keep for this user + usersFound[userId] = 1; + } + + if (e.sender && usersFound[e.sender.userId] === undefined) { + // no receipt yet for this sender, so we synthesize one. + + this.addReceipt(this._synthesizeReceipt(e.sender.userId, e, "m.read")); + + usersFound[e.sender.userId] = 1; + } + } }; +Room.prototype._synthesizeReceipt = function(userId, event, receiptType) { + // This is really ugly because JS has no way to express an object literal + // where the name of a key comes from an expression + var fakeReceipt = {content: {}}; + fakeReceipt.content[event.getId()] = {}; + fakeReceipt.content[event.getId()][receiptType] = {}; + fakeReceipt.content[event.getId()][receiptType][userId] = { + ts: event + }; + return new MatrixEvent(fakeReceipt); +} + /** * Get a list of user IDs who have read up to the given event. From 77266fe221ec6b711bace6b609eb76d3432933b6 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 6 Nov 2015 15:26:35 +0000 Subject: [PATCH 2/5] Fix lint errors and make thing that didn't need to be a member function not a member function --- lib/models/room.js | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/lib/models/room.js b/lib/models/room.js index 857f401cf..ad586ff3f 100644 --- a/lib/models/room.js +++ b/lib/models/room.js @@ -10,6 +10,19 @@ var MatrixEvent = require("./event").MatrixEvent; var utils = require("../utils"); var ContentRepo = require("../content-repo"); +function synthesizeReceipt(userId, event, receiptType) { + // This is really ugly because JS has no way to express an object literal + // where the name of a key comes from an expression + var fakeReceipt = {content: {}}; + fakeReceipt.content[event.getId()] = {}; + fakeReceipt.content[event.getId()][receiptType] = {}; + fakeReceipt.content[event.getId()][receiptType][userId] = { + ts: event + }; + return new MatrixEvent(fakeReceipt); +} + + /** * Construct a new Room. * @constructor @@ -202,7 +215,7 @@ Room.prototype.addEventsToTimeline = function(events, toStartOfTimeline) { // This is really ugly because JS has no way to express an object literal // where the name of a key comes from an expression if (!toStartOfTimeline && events[i].sender) { - this.addReceipt(new MatrixEvent(this._synthesizeReceipt( + this.addReceipt(new MatrixEvent(synthesizeReceipt( events[i].sender.userId, events[i], "m.read" ))); } @@ -366,35 +379,22 @@ Room.prototype.recalculate = function(userId) { var receipt = readReceiptsForEvent[receiptIt]; if (receipt.type !== "m.read") { continue; } - var userId = receipt.userId; - if (usersFound[userId]) { continue; } + if (usersFound[receipt.userId]) { continue; } // Then this is the receipt we keep for this user - usersFound[userId] = 1; + usersFound[receipt.userId] = 1; } if (e.sender && usersFound[e.sender.userId] === undefined) { // no receipt yet for this sender, so we synthesize one. - this.addReceipt(this._synthesizeReceipt(e.sender.userId, e, "m.read")); + this.addReceipt(synthesizeReceipt(e.sender.userId, e, "m.read")); usersFound[e.sender.userId] = 1; } } }; -Room.prototype._synthesizeReceipt = function(userId, event, receiptType) { - // This is really ugly because JS has no way to express an object literal - // where the name of a key comes from an expression - var fakeReceipt = {content: {}}; - fakeReceipt.content[event.getId()] = {}; - fakeReceipt.content[event.getId()][receiptType] = {}; - fakeReceipt.content[event.getId()][receiptType][userId] = { - ts: event - }; - return new MatrixEvent(fakeReceipt); -} - /** * Get a list of user IDs who have read up to the given event. From c13b1800b9df3a5c9618a4aff29a83cfa389ef19 Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 9 Nov 2015 10:23:37 +0000 Subject: [PATCH 3/5] forEach probably nicer here --- lib/models/room.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/models/room.js b/lib/models/room.js index ad586ff3f..eff95ed79 100644 --- a/lib/models/room.js +++ b/lib/models/room.js @@ -375,15 +375,13 @@ Room.prototype.recalculate = function(userId) { var readReceiptsForEvent = this.getReceiptsForEvent(e); - for (var receiptIt = 0; receiptIt < readReceiptsForEvent.length; ++receiptIt) { - var receipt = readReceiptsForEvent[receiptIt]; - if (receipt.type !== "m.read") { continue; } - - if (usersFound[receipt.userId]) { continue; } + readReceiptsForEvent.forEach(function(receipt) { + if (receipt.type !== "m.read") { return; } + if (usersFound[receipt.userId]) { return; } // Then this is the receipt we keep for this user usersFound[receipt.userId] = 1; - } + }); if (e.sender && usersFound[e.sender.userId] === undefined) { // no receipt yet for this sender, so we synthesize one. From ad24596d3ffaeffce1579756d2874ddacefaaaf8 Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 9 Nov 2015 13:48:05 +0000 Subject: [PATCH 4/5] Revert c13b180 as it fails lint (creating functions in a loop) --- lib/models/room.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/models/room.js b/lib/models/room.js index eff95ed79..ad586ff3f 100644 --- a/lib/models/room.js +++ b/lib/models/room.js @@ -375,13 +375,15 @@ Room.prototype.recalculate = function(userId) { var readReceiptsForEvent = this.getReceiptsForEvent(e); - readReceiptsForEvent.forEach(function(receipt) { - if (receipt.type !== "m.read") { return; } - if (usersFound[receipt.userId]) { return; } + for (var receiptIt = 0; receiptIt < readReceiptsForEvent.length; ++receiptIt) { + var receipt = readReceiptsForEvent[receiptIt]; + if (receipt.type !== "m.read") { continue; } + + if (usersFound[receipt.userId]) { continue; } // Then this is the receipt we keep for this user usersFound[receipt.userId] = 1; - }); + } if (e.sender && usersFound[e.sender.userId] === undefined) { // no receipt yet for this sender, so we synthesize one. From c95b27683f3c1e6389f8f6ee5e9efbcb1c99fbc6 Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 9 Nov 2015 15:05:46 +0000 Subject: [PATCH 5/5] Add higher level keys to fake receipts --- lib/models/room.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/models/room.js b/lib/models/room.js index ad586ff3f..f3f630ac1 100644 --- a/lib/models/room.js +++ b/lib/models/room.js @@ -13,7 +13,11 @@ var ContentRepo = require("../content-repo"); function synthesizeReceipt(userId, event, receiptType) { // This is really ugly because JS has no way to express an object literal // where the name of a key comes from an expression - var fakeReceipt = {content: {}}; + var fakeReceipt = { + content: {}, + type: "m.receipt", + room_id: event.getRoomId() + }; fakeReceipt.content[event.getId()] = {}; fakeReceipt.content[event.getId()][receiptType] = {}; fakeReceipt.content[event.getId()][receiptType][userId] = {