From f4a77822980d6d560e385ae99c8e048cf87359ac Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 5 Mar 2021 17:03:45 +0100 Subject: [PATCH] add MemberWriter, and only return MemberChange's if something changed --- src/matrix/room/members/RoomMember.js | 34 ++- .../room/timeline/persistence/MemberWriter.js | 203 ++++++++++++++++++ .../room/timeline/persistence/SyncWriter.js | 81 +++---- 3 files changed, 246 insertions(+), 72 deletions(-) create mode 100644 src/matrix/room/timeline/persistence/MemberWriter.js diff --git a/src/matrix/room/members/RoomMember.js b/src/matrix/room/members/RoomMember.js index 324f0744..4013f556 100644 --- a/src/matrix/room/members/RoomMember.js +++ b/src/matrix/room/members/RoomMember.js @@ -103,36 +103,34 @@ export class RoomMember { serialize() { return this._data; } + + equals(other) { + const data = this._data; + const otherData = other._data; + return data.roomId === otherData.roomId && + data.userId === otherData.userId && + data.membership === otherData.membership && + data.displayName === otherData.displayName && + data.avatarUrl === otherData.avatarUrl; + } } export class MemberChange { - constructor(roomId, memberEvent) { - this._roomId = roomId; - this._memberEvent = memberEvent; - this._member = null; - } - - get member() { - if (!this._member) { - this._member = RoomMember.fromMemberEvent(this._roomId, this._memberEvent); - } - return this._member; + constructor(member, previousMembership) { + this.member = member; + this.previousMembership = previousMembership; } get roomId() { - return this._roomId; + return this.member.roomId; } get userId() { - return this._memberEvent.state_key; - } - - get previousMembership() { - return getPrevContentFromStateEvent(this._memberEvent)?.membership; + return this.member.userId; } get membership() { - return this._memberEvent.content?.membership; + return this.member.membership; } get hasLeft() { diff --git a/src/matrix/room/timeline/persistence/MemberWriter.js b/src/matrix/room/timeline/persistence/MemberWriter.js new file mode 100644 index 00000000..f72fd617 --- /dev/null +++ b/src/matrix/room/timeline/persistence/MemberWriter.js @@ -0,0 +1,203 @@ +/* +Copyright 2021 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import {MemberChange, RoomMember, EVENT_TYPE as MEMBER_EVENT_TYPE} from "../../members/RoomMember.js"; +import {LRUCache} from "../../../../utils/LRUCache.js"; + +export class MemberWriter { + constructor(roomId) { + this._roomId = roomId; + this._cache = new LRUCache(5, member => member.userId); + } + + writeTimelineMemberEvent(event, txn) { + return this._writeMemberEvent(event, false, txn); + } + + writeStateMemberEvent(event, isLimited, txn) { + // member events in the state section when the room response + // is not limited must always be lazy loaded members. + // If they are not, they will be repeated in the timeline anyway. + return this._writeMemberEvent(event, !isLimited, txn); + } + + async _writeMemberEvent(event, isLazyLoadingMember, txn) { + const userId = event.state_key; + if (!userId) { + return; + } + const member = RoomMember.fromMemberEvent(this._roomId, event); + if (!member) { + return; + } + + let existingMember = this._cache.get(userId); + if (!existingMember) { + const memberData = await txn.roomMembers.get(this._roomId, userId); + if (memberData) { + existingMember = new RoomMember(memberData); + } + } + + // either never heard of the member, or something changed + if (!existingMember || !existingMember.equals(member)) { + txn.roomMembers.set(member.serialize()); + this._cache.set(member); + + if (!isLazyLoadingMember) { + return new MemberChange(member, existingMember?.membership); + } + } + } + + async lookupMember(userId, timelineEvents, txn) { + let member = this._cache.get(userId); + if (!member) { + const memberData = await txn.roomMembers.get(this._roomId, userId); + if (memberData) { + member = new RoomMember(memberData); + this._cache.set(member); + } + } + if (!member) { + // sometimes the member event isn't included in state, but rather in the timeline, + // even if it is not the first event in the timeline. In this case, go look for the + // first occurence + const memberEvent = timelineEvents.find(e => { + return e.type === MEMBER_EVENT_TYPE && e.state_key === userId; + }); + if (memberEvent) { + member = RoomMember.fromMemberEvent(this._roomId, memberEvent); + // adding it to the cache, but not storing it for now; + // we'll do that when we get to the event + this._cache.set(member); + } + } + return member; + } +} + +export function tests() { + + function createMemberEvent(membership, userId, displayName, avatarUrl) { + return { + content: { + membership, + "displayname": displayName, + "avatar_url": avatarUrl + }, + sender: userId, + "state_key": userId, + type: "m.room.member" + }; + } + + + function createStorage(initialMembers = []) { + const members = new Map(); + for (const m of initialMembers) { + members.set(m.userId, m); + } + return { + members, + roomMembers: { + async get(_, userId) { + return members.get(userId); + }, + set(member) { + members.set(member.userId, member); + } + } + } + } + + function member(...args) { + return RoomMember.fromMemberEvent(roomId, createMemberEvent.apply(null, args)); + } + + const roomId = "abc"; + const alice = "@alice:hs.tld"; + const avatar = "mxc://hs.tld/def"; + + return { + "new join through state": async assert => { + const writer = new MemberWriter(roomId); + const txn = createStorage(); + const change = await writer.writeStateMemberEvent(createMemberEvent("join", alice), true, txn); + assert(change.hasJoined); + assert.equal(txn.members.get(alice).membership, "join"); + }, + "accept invite through state": async assert => { + const writer = new MemberWriter(roomId); + const txn = createStorage([member("invite", alice)]); + const change = await writer.writeStateMemberEvent(createMemberEvent("join", alice), true, txn); + assert.equal(change.previousMembership, "invite"); + assert(change.hasJoined); + assert.equal(txn.members.get(alice).membership, "join"); + }, + "change display name through timeline": async assert => { + const writer = new MemberWriter(roomId); + const txn = createStorage([member("join", alice, "Alice")]); + const change = await writer.writeTimelineMemberEvent(createMemberEvent("join", alice, "Alies"), txn); + assert(!change.hasJoined); + assert.equal(change.member.displayName, "Alies"); + assert.equal(txn.members.get(alice).displayName, "Alies"); + }, + "set avatar through timeline": async assert => { + const writer = new MemberWriter(roomId); + const txn = createStorage([member("join", alice, "Alice")]); + const change = await writer.writeTimelineMemberEvent(createMemberEvent("join", alice, "Alice", avatar), txn); + assert(!change.hasJoined); + assert.equal(change.member.avatarUrl, avatar); + assert.equal(txn.members.get(alice).avatarUrl, avatar); + }, + "ignore redundant member event": async assert => { + const writer = new MemberWriter(roomId); + const txn = createStorage([member("join", alice, "Alice", avatar)]); + const change = await writer.writeTimelineMemberEvent(createMemberEvent("join", alice, "Alice", avatar), txn); + assert(!change); + }, + "leave": async assert => { + const writer = new MemberWriter(roomId); + const txn = createStorage([member("join", alice, "Alice")]); + const change = await writer.writeTimelineMemberEvent(createMemberEvent("leave", alice, "Alice"), txn); + assert(change.hasLeft); + assert(!change.hasJoined); + }, + "ban": async assert => { + const writer = new MemberWriter(roomId); + const txn = createStorage([member("join", alice, "Alice")]); + const change = await writer.writeTimelineMemberEvent(createMemberEvent("ban", alice, "Alice"), txn); + assert(change.hasLeft); + assert(!change.hasJoined); + }, + "reject invite": async assert => { + const writer = new MemberWriter(roomId); + const txn = createStorage([member("invite", alice, "Alice")]); + const change = await writer.writeTimelineMemberEvent(createMemberEvent("leave", alice, "Alice"), txn); + assert(!change.hasLeft); + assert(!change.hasJoined); + }, + "lazy loaded member is written but no change returned": async assert => { + const writer = new MemberWriter(roomId); + const txn = createStorage(); + const change = await writer.writeStateMemberEvent(createMemberEvent("join", alice, "Alice"), false, txn); + assert(!change); + assert.equal(txn.members.get(alice).displayName, "Alice"); + }, + + }; +} diff --git a/src/matrix/room/timeline/persistence/SyncWriter.js b/src/matrix/room/timeline/persistence/SyncWriter.js index b12f52a7..a7675993 100644 --- a/src/matrix/room/timeline/persistence/SyncWriter.js +++ b/src/matrix/room/timeline/persistence/SyncWriter.js @@ -1,5 +1,6 @@ /* Copyright 2020 Bruno Windels +Copyright 2020 The Matrix.org Foundation C.I.C. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -18,7 +19,8 @@ import {EventKey} from "../EventKey.js"; import {EventEntry} from "../entries/EventEntry.js"; import {FragmentBoundaryEntry} from "../entries/FragmentBoundaryEntry.js"; import {createEventEntry} from "./common.js"; -import {MemberChange, RoomMember, EVENT_TYPE as MEMBER_EVENT_TYPE} from "../../members/RoomMember.js"; +import {EVENT_TYPE as MEMBER_EVENT_TYPE} from "../../members/RoomMember.js"; +import {MemberWriter} from "./MemberWriter.js"; // Synapse bug? where the m.room.create event appears twice in sync response // when first syncing the room @@ -37,6 +39,7 @@ function deduplicateEvents(events) { export class SyncWriter { constructor({roomId, fragmentIdComparer}) { this._roomId = roomId; + this._memberWriter = new MemberWriter(roomId); this._fragmentIdComparer = fragmentIdComparer; this._lastLiveKey = null; } @@ -130,37 +133,19 @@ export class SyncWriter { return currentKey; } - _writeMember(event, txn) { - const userId = event.state_key; - if (userId) { - const memberChange = new MemberChange(this._roomId, event); - const {member} = memberChange; - if (member) { - // TODO: can we avoid writing redundant members here by checking - // if this is not a limited sync and the state is not in the timeline? - txn.roomMembers.set(member.serialize()); - return memberChange; - } - } - } - - _writeStateEvent(event, txn) { - if (event.type === MEMBER_EVENT_TYPE) { - return this._writeMember(event, txn); - } else { - txn.roomState.set(this._roomId, event); - } - } - - _writeStateEvents(roomResponse, memberChanges, txn, log) { + async _writeStateEvents(roomResponse, memberChanges, isLimited, txn, log) { // persist state const {state} = roomResponse; if (Array.isArray(state?.events)) { log.set("stateEvents", state.events.length); for (const event of state.events) { - const memberChange = this._writeStateEvent(event, txn); - if (memberChange) { - memberChanges.set(memberChange.userId, memberChange); + if (event.type === MEMBER_EVENT_TYPE) { + const memberChange = await this._memberWriter.writeStateMemberEvent(event, isLimited, txn); + if (memberChange) { + memberChanges.set(memberChange.userId, memberChange); + } + } else { + txn.roomState.set(this._roomId, event); } } } @@ -177,20 +162,26 @@ export class SyncWriter { // store event in timeline currentKey = currentKey.nextKey(); const entry = createEventEntry(currentKey, this._roomId, event); - let memberData = await this._findMemberData(event.sender, events, txn); - if (memberData) { - entry.displayName = memberData.displayName; - entry.avatarUrl = memberData.avatarUrl; + let member = await this._memberWriter.lookupMember(event.sender, events, txn); + if (member) { + entry.displayName = member.displayName; + entry.avatarUrl = member.avatarUrl; } txn.timelineEvents.insert(entry); entries.push(new EventEntry(entry, this._fragmentIdComparer)); - // process live state events first, so new member info is available + // update state events after writing event, so for a member event, + // we only update the member info after having written the member event + // to the timeline, as we want that event to have the old profile info if (typeof event.state_key === "string") { timelineStateEventCount += 1; - const memberChange = this._writeStateEvent(event, txn); - if (memberChange) { - memberChanges.set(memberChange.userId, memberChange); + if (event.type === MEMBER_EVENT_TYPE) { + const memberChange = await this._memberWriter.writeTimelineMemberEvent(event, txn); + if (memberChange) { + memberChanges.set(memberChange.userId, memberChange); + } + } else { + txn.roomState.set(this._roomId, event); } } } @@ -199,24 +190,6 @@ export class SyncWriter { return currentKey; } - async _findMemberData(userId, events, txn) { - // TODO: perhaps add a small cache here? - const memberData = await txn.roomMembers.get(this._roomId, userId); - if (memberData) { - return memberData; - } else { - // sometimes the member event isn't included in state, but rather in the timeline, - // even if it is not the first event in the timeline. In this case, go look for the - // first occurence - const memberEvent = events.find(e => { - return e.type === MEMBER_EVENT_TYPE && e.state_key === userId; - }); - if (memberEvent) { - return RoomMember.fromMemberEvent(this._roomId, memberEvent)?.serialize(); - } - } - } - /** * @type {SyncWriterResult} * @property {Array} entries new timeline entries written @@ -233,7 +206,7 @@ export class SyncWriter { const memberChanges = new Map(); // important this happens before _writeTimeline so // members are available in the transaction - this._writeStateEvents(roomResponse, memberChanges, txn, log); + await this._writeStateEvents(roomResponse, memberChanges, timeline?.limited, txn, log); const currentKey = await this._writeTimeline(entries, timeline, this._lastLiveKey, memberChanges, txn, log); log.set("memberChanges", memberChanges.size); return {entries, newLiveKey: currentKey, memberChanges};