From fc741272ba9ec0fb43872f6ea77e7d54e28e510f Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Sat, 14 Mar 2020 20:45:36 +0100 Subject: [PATCH 1/4] only apply sync changes in session once txn is committed --- src/matrix/session.js | 64 ++++++++++++++++++++++++++++++++++++++++--- src/matrix/sync.js | 4 ++- 2 files changed, 63 insertions(+), 5 deletions(-) diff --git a/src/matrix/session.js b/src/matrix/session.js index b86a8c69..63b2b03c 100644 --- a/src/matrix/session.js +++ b/src/matrix/session.js @@ -77,11 +77,19 @@ export default class Session { return room; } - persistSync(syncToken, syncFilterId, accountData, txn) { + writeSync(syncToken, syncFilterId, accountData, txn) { if (syncToken !== this._session.syncToken) { - this._session.syncToken = syncToken; - this._session.syncFilterId = syncFilterId; - txn.session.set(this._session); + // don't modify this._session because transaction might still fail + const newSessionData = Object.assign({}, this._session, {syncToken, syncFilterId}); + txn.session.set(newSessionData); + return newSessionData; + } + } + + afterSync(newSessionData) { + if (newSessionData) { + // sync transaction succeeded, modify object state now + this._session = newSessionData; } } @@ -97,3 +105,51 @@ export default class Session { return this._user; } } + +export function tests() { + function createStorageMock(session, pendingEvents) { + return { + readTxn() { + return Promise.resolve({ + session: { + get() { + return Promise.resolve(Object.assign({}, session)); + } + }, + pendingEvents: { + getAll() { + return Promise.resolve(pendingEvents); + } + } + }); + } + }; + } + + return { + "session data is not modified until after sync": async (assert) => { + const session = new Session({storage: createStorageMock({ + syncToken: "a", + syncFilterId: 5, + })}); + await session.load(); + let txnSetCalled = false; + const syncTxn = { + session: { + set({syncToken, syncFilterId}) { + txnSetCalled = true; + assert.equals(syncToken, "b"); + assert.equals(syncFilterId, 6); + } + } + }; + const newSessionData = session.writeSync("b", 6, {}, syncTxn); + assert(txnSetCalled); + assert.equals(session.syncToken, "a"); + assert.equals(session.syncFilterId, 5); + session.afterSync(newSessionData); + assert.equals(session.syncToken, "b"); + assert.equals(session.syncFilterId, 6); + } + } +} diff --git a/src/matrix/sync.js b/src/matrix/sync.js index beac1799..52726f01 100644 --- a/src/matrix/sync.js +++ b/src/matrix/sync.js @@ -85,8 +85,9 @@ export default class Sync extends EventEmitter { storeNames.pendingEvents, ]); const roomChanges = []; + let sessionChanges; try { - this._session.persistSync(syncToken, syncFilterId, response.account_data, syncTxn); + sessionChanges = this._session.writeSync(syncToken, syncFilterId, response.account_data, syncTxn); // to_device // presence if (response.rooms) { @@ -116,6 +117,7 @@ export default class Sync extends EventEmitter { console.error("unable to commit sync tranaction"); throw err; } + this._session.afterSync(sessionChanges); // emit room related events after txn has been closed for(let {room, changes} of roomChanges) { room.emitSync(changes); From ac4a7f971b3df0c78a3fcf13c02cd246c6aa270c Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Sat, 14 Mar 2020 20:46:49 +0100 Subject: [PATCH 2/4] only apply sync changes in room summary once txn is committed - also remove m.room.aliases support as they were wrongly implemented and now obsolete - don't count invited and joined members according to m.room.member events anymore as it was also wrongly implemented (only when prev!==new membership, but on initial sync we only get last member event, which might have been a nick change --- src/matrix/room/room.js | 11 +- src/matrix/room/summary.js | 321 +++++++++++++++++-------------------- src/matrix/sync.js | 4 +- 3 files changed, 151 insertions(+), 185 deletions(-) diff --git a/src/matrix/room/room.js b/src/matrix/room/room.js index eaed2378..858ab149 100644 --- a/src/matrix/room/room.js +++ b/src/matrix/room/room.js @@ -20,18 +20,19 @@ export default class Room extends EventEmitter { this._user = user; } - async persistSync(roomResponse, membership, txn) { - const summaryChanged = this._summary.applySync(roomResponse, membership, txn); + async writeSync(roomResponse, membership, txn) { + const summaryChanges = this._summary.writeSync(roomResponse, membership, txn); const {entries, newLiveKey} = await this._syncWriter.writeSync(roomResponse, txn); let removedPendingEvents; if (roomResponse.timeline && roomResponse.timeline.events) { removedPendingEvents = this._sendQueue.removeRemoteEchos(roomResponse.timeline.events, txn); } - return {summaryChanged, newTimelineEntries: entries, newLiveKey, removedPendingEvents}; + return {summaryChanges, newTimelineEntries: entries, newLiveKey, removedPendingEvents}; } - emitSync({summaryChanged, newTimelineEntries, newLiveKey, removedPendingEvents}) { - if (summaryChanged) { + afterSync({summaryChanges, newTimelineEntries, newLiveKey, removedPendingEvents}) { + if (summaryChanges) { + this._summary.afterSync(summaryChanges); this.emit("change"); this._emitCollectionChange(this); } diff --git a/src/matrix/room/summary.js b/src/matrix/room/summary.js index 614b494d..c5e264f2 100644 --- a/src/matrix/room/summary.js +++ b/src/matrix/room/summary.js @@ -1,205 +1,170 @@ -// import SummaryMembers from "./members"; +function applySyncResponse(data, roomResponse, membership) { + if (roomResponse.summary) { + data = updateSummary(data, roomResponse.summary); + } + if (membership !== data.membership) { + data = data.cloneIfNeeded(); + data.membership = membership; + } + // state comes before timeline + if (roomResponse.state) { + data = roomResponse.state.events.reduce(processEvent, data); + } + if (roomResponse.timeline) { + data = roomResponse.timeline.events.reduce(processEvent, data); + } + + return data; +} + +function processEvent(data, event) { + if (event.type === "m.room.encryption") { + if (!data.isEncrypted) { + data = data.cloneIfNeeded(); + data.isEncrypted = true; + } + } + if (event.type === "m.room.name") { + const newName = event.content && event.content.name; + if (newName !== data.name) { + data = data.cloneIfNeeded(); + data.name = newName; + } + } else if (event.type === "m.room.message") { + const content = event.content; + const body = content && content.body; + const msgtype = content && content.msgtype; + if (msgtype === "m.text") { + data = data.cloneIfNeeded(); + data.lastMessageBody = body; + } + } else if (event.type === "m.room.canonical_alias") { + const content = event.content; + data = data.cloneIfNeeded(); + data.canonicalAlias = content.alias; + data.altAliases = content.alt_aliases; + } + return data; +} + +function updateSummary(data, summary) { + const heroes = summary["m.heroes"]; + const inviteCount = summary["m.joined_member_count"]; + const joinCount = summary["m.invited_member_count"]; + + if (heroes) { + data = data.cloneIfNeeded(); + data.heroes = heroes; + } + if (Number.isInteger(inviteCount)) { + data = data.cloneIfNeeded(); + data.inviteCount = inviteCount; + } + if (Number.isInteger(joinCount)) { + data = data.cloneIfNeeded(); + data.joinCount = joinCount; + } + return data; +} + +class SummaryData { + constructor(copy, roomId) { + this.roomId = copy ? copy.roomId : roomId; + this.name = copy ? copy.name : null; + this.lastMessageBody = copy ? copy.lastMessageBody : null; + this.unreadCount = copy ? copy.unreadCount : null; + this.mentionCount = copy ? copy.mentionCount : null; + this.isEncrypted = copy ? copy.isEncrypted : null; + this.isDirectMessage = copy ? copy.isDirectMessage : null; + this.membership = copy ? copy.membership : null; + this.inviteCount = copy ? copy.inviteCount : 0; + this.joinCount = copy ? copy.joinCount : 0; + this.heroes = copy ? copy.heroes : null; + this.canonicalAlias = copy ? copy.canonicalAlias : null; + this.altAliases = copy ? copy.altAliases : null; + this.cloned = copy ? true : false; + } + + cloneIfNeeded() { + if (this.cloned) { + return this; + } else { + return new SummaryData(this); + } + } + + serialize() { + const {cloned, ...serializedProps} = this; + return serializedProps; + } +} export default class RoomSummary { constructor(roomId) { - // this._members = new SummaryMembers(); - this._roomId = roomId; - this._name = null; - this._lastMessageBody = null; - this._unreadCount = null; - this._mentionCount = null; - this._isEncrypted = null; - this._isDirectMessage = null; - this._membership = null; - this._inviteCount = 0; - this._joinCount = 0; - this._readMarkerEventId = null; - this._heroes = null; - this._canonicalAlias = null; - this._aliases = null; + this._data = new SummaryData(null, roomId); } get name() { - if (this._name) { - return this._name; + if (this._data.name) { + return this._data.name; } - if (this._canonicalAlias) { - return this._canonicalAlias; + if (this._data.canonicalAlias) { + return this._data.canonicalAlias; } - if (Array.isArray(this._aliases) && this._aliases.length !== 0) { - return this._aliases[0]; + if (Array.isArray(this._data.altAliases) && this._data.altAliases.length !== 0) { + return this._data.altAliases[0]; } - if (Array.isArray(this._heroes) && this._heroes.length !== 0) { - return this._heroes.join(", "); + if (Array.isArray(this._data.heroes) && this._data.heroes.length !== 0) { + return this._data.heroes.join(", "); } - return this._roomId; + return this._data.roomId; } get lastMessage() { - return this._lastMessageBody; + return this._data.lastMessageBody; } get inviteCount() { - return this._inviteCount; + return this._data.inviteCount; } get joinCount() { - return this._joinCount; + return this._data.joinCount; } - applySync(roomResponse, membership, txn) { - const changed = this._processSyncResponse(roomResponse, membership); - if (changed) { - this._persist(txn); + writeSync(roomResponse, membership, txn) { + // clear cloned flag, so cloneIfNeeded makes a copy and + // this._data is not modified if any field is changed. + this._data.cloned = false; + const data = applySyncResponse(this._data, roomResponse, membership); + if (data !== this._data) { + // need to think here how we want to persist + // things like unread status (as read marker, or unread count)? + // we could very well load additional things in the load method + // ... the trade-off is between constantly writing the summary + // on every sync, or doing a bit of extra reading on load + // and have in-memory only variables for visualization + txn.roomSummary.set(data.serialize()); + return data; } - return changed; } + afterSync(data) { + this._data = data; + } + async load(summary) { - this._roomId = summary.roomId; - this._name = summary.name; - this._lastMessageBody = summary.lastMessageBody; - this._unreadCount = summary.unreadCount; - this._mentionCount = summary.mentionCount; - this._isEncrypted = summary.isEncrypted; - this._isDirectMessage = summary.isDirectMessage; - this._membership = summary.membership; - this._inviteCount = summary.inviteCount; - this._joinCount = summary.joinCount; - this._readMarkerEventId = summary.readMarkerEventId; - this._heroes = summary.heroes; - this._aliases = summary.aliases; - this._canonicalAlias = summary.canonicalAlias; - } - - _persist(txn) { - // need to think here how we want to persist - // things like unread status (as read marker, or unread count)? - // we could very well load additional things in the load method - // ... the trade-off is between constantly writing the summary - // on every sync, or doing a bit of extra reading on load - // and have in-memory only variables for visualization - const summary = { - roomId: this._roomId, - name: this._name, - lastMessageBody: this._lastMessageBody, - unreadCount: this._unreadCount, - mentionCount: this._mentionCount, - isEncrypted: this._isEncrypted, - isDirectMessage: this._isDirectMessage, - membership: this._membership, - inviteCount: this._inviteCount, - joinCount: this._joinCount, - readMarkerEventId: this._readMarkerEventId, - heroes: this._heroes, - aliases: this._aliases, - canonicalAlias: this._canonicalAlias, - }; - return txn.roomSummary.set(summary); - } - - _processSyncResponse(roomResponse, membership) { - let changed = false; - if (roomResponse.summary) { - this._updateSummary(roomResponse.summary); - changed = true; - } - if (membership !== this._membership) { - this._membership = membership; - changed = true; - } - // state comes before timeline - if (roomResponse.state) { - changed = roomResponse.state.events.reduce((changed, e) => { - return this._processEvent(e) || changed; - }, changed); - } - if (roomResponse.timeline) { - changed = roomResponse.timeline.events.reduce((changed, e) => { - return this._processEvent(e) || changed; - }, changed); - } - - return changed; - } - - _processEvent(event) { - if (event.type === "m.room.encryption") { - if (!this._isEncrypted) { - this._isEncrypted = true; - return true; - } - } - if (event.type === "m.room.name") { - const newName = event.content && event.content.name; - if (newName !== this._name) { - this._name = newName; - return true; - } - } else if (event.type === "m.room.member") { - return this._processMembership(event); - } else if (event.type === "m.room.message") { - const content = event.content; - const body = content && content.body; - const msgtype = content && content.msgtype; - if (msgtype === "m.text") { - this._lastMessageBody = body; - return true; - } - } else if (event.type === "m.room.canonical_alias") { - const content = event.content; - this._canonicalAlias = content.alias; - return true; - } else if (event.type === "m.room.aliases") { - const content = event.content; - this._aliases = content.aliases; - return true; - } - return false; - } - - _processMembership(event) { - let changed = false; - const prevMembership = event.prev_content && event.prev_content.membership; - if (!event.content) { - return changed; - } - const content = event.content; - const membership = content.membership; - // danger of a replayed event getting the count out of sync - // but summary api will solve this. - // otherwise we'd have to store all the member ids in here - if (membership !== prevMembership) { - switch (prevMembership) { - case "invite": --this._inviteCount; break; - case "join": --this._joinCount; break; - } - switch (membership) { - case "invite": ++this._inviteCount; break; - case "join": ++this._joinCount; break; - } - changed = true; - } - // if (membership === "join" && content.name) { - // // TODO: avatar_url - // changed = this._members.applyMember(content.name, content.state_key) || changed; - // } - return changed; - } - - _updateSummary(summary) { - const heroes = summary["m.heroes"]; - const inviteCount = summary["m.joined_member_count"]; - const joinCount = summary["m.invited_member_count"]; - - if (heroes) { - this._heroes = heroes; - } - if (Number.isInteger(inviteCount)) { - this._inviteCount = inviteCount; - } - if (Number.isInteger(joinCount)) { - this._joinCount = joinCount; - } + this._data = new SummaryData(summary); } } + +export function tests() { + return { + "membership trigger change": function(assert) { + const summary = new RoomSummary("id"); + const changes = summary.writeSync({}, "join"); + assert(changes); + assert(changes.changed); + } + } +} diff --git a/src/matrix/sync.js b/src/matrix/sync.js index 52726f01..fb31e422 100644 --- a/src/matrix/sync.js +++ b/src/matrix/sync.js @@ -97,7 +97,7 @@ export default class Sync extends EventEmitter { room = this._session.createRoom(roomId); } console.log(` * applying sync response to room ${roomId} ...`); - const changes = await room.persistSync(roomResponse, membership, syncTxn); + const changes = await room.writeSync(roomResponse, membership, syncTxn); roomChanges.push({room, changes}); }); await Promise.all(promises); @@ -120,7 +120,7 @@ export default class Sync extends EventEmitter { this._session.afterSync(sessionChanges); // emit room related events after txn has been closed for(let {room, changes} of roomChanges) { - room.emitSync(changes); + room.afterSync(changes); } return syncToken; From 56c87b3966dabee561a2be895473b8659f3f9a74 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Sat, 14 Mar 2020 20:49:15 +0100 Subject: [PATCH 3/4] better naming --- src/matrix/room/room.js | 2 +- src/matrix/room/timeline/persistence/SyncWriter.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/matrix/room/room.js b/src/matrix/room/room.js index 858ab149..d493bc00 100644 --- a/src/matrix/room/room.js +++ b/src/matrix/room/room.js @@ -31,12 +31,12 @@ export default class Room extends EventEmitter { } afterSync({summaryChanges, newTimelineEntries, newLiveKey, removedPendingEvents}) { + this._syncWriter.afterSync(newLiveKey); if (summaryChanges) { this._summary.afterSync(summaryChanges); this.emit("change"); this._emitCollectionChange(this); } - this._syncWriter.setKeyOnCompleted(newLiveKey); if (this._timeline) { this._timeline.appendLiveEntries(newTimelineEntries); } diff --git a/src/matrix/room/timeline/persistence/SyncWriter.js b/src/matrix/room/timeline/persistence/SyncWriter.js index 537a495f..be7d8f9b 100644 --- a/src/matrix/room/timeline/persistence/SyncWriter.js +++ b/src/matrix/room/timeline/persistence/SyncWriter.js @@ -129,7 +129,7 @@ export default class SyncWriter { return {entries, newLiveKey: currentKey}; } - setKeyOnCompleted(newLiveKey) { + afterSync(newLiveKey) { this._lastLiveKey = newLiveKey; } } From 714892c3d3b54afc0ca138cb3a762f2d1b9bd3a4 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Sat, 14 Mar 2020 20:52:17 +0100 Subject: [PATCH 4/4] this breaks the local server, so don't do it for now --- package.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/package.json b/package.json index 31f7947c..93823aa5 100644 --- a/package.json +++ b/package.json @@ -29,6 +29,5 @@ "postcss-import": "^12.0.1", "rollup": "^1.15.6", "serve-static": "^1.13.2" - }, - "type": "module" + } }