From ddfdbf9777730210322cbad06e4d90ed2d08cf48 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 21 Aug 2020 17:59:24 +0200 Subject: [PATCH 1/9] implement heroes logic to calculate the room name --- src/matrix/room/members/Heroes.js | 101 ++++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 src/matrix/room/members/Heroes.js diff --git a/src/matrix/room/members/Heroes.js b/src/matrix/room/members/Heroes.js new file mode 100644 index 00000000..79902579 --- /dev/null +++ b/src/matrix/room/members/Heroes.js @@ -0,0 +1,101 @@ +/* +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. +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 {RoomMember} from "./RoomMember.js"; + +function calculateRoomName(sortedMembers, summary) { + const countWithoutMe = summary.joinCount + summary.inviteCount - 1; + if (sortedMembers.length >= countWithoutMe) { + if (sortedMembers.length > 1) { + const lastMember = sortedMembers[sortedMembers.length - 1]; + const firstMembers = sortedMembers.slice(0, sortedMembers.length - 1); + return firstMembers.map(m => m.displayName).join(", ") + " and " + lastMember.displayName; + } else { + return sortedMembers[0].displayName; + } + } else if (sortedMembers.length < countWithoutMe) { + return sortedMembers.map(m => m.displayName).join(", ") + ` and ${countWithoutMe} others`; + } else { + // Empty Room + return null; + } +} + +export class Heroes { + constructor(roomId) { + this._roomId = roomId; + this._members = new Map(); + } + + /** + * @param {string[]} newHeroes array of user ids + * @param {RoomMember[]} changedMembers array of changed members in this sync + * @param {Transaction} txn + * @return {Promise} + */ + async calculateChanges(newHeroes, changedMembers, txn) { + const updatedHeroMembers = new Map(); + const removedUserIds = []; + // remove non-present members + for (const existingUserId of this._members.keys()) { + if (newHeroes.indexOf(existingUserId) === -1) { + removedUserIds.push(existingUserId); + } + } + // update heroes with synced member changes + for (const member of changedMembers) { + if (this._members.has(member.userId) || newHeroes.indexOf(member.userId) !== -1) { + updatedHeroMembers.set(member.userId, member); + } + } + // load member for new heroes from storage + for (const userId of newHeroes) { + if (!this._members.has(userId) && !updatedHeroMembers.has(userId)) { + const memberData = await txn.roomMembers.get(this._roomId, userId); + if (memberData) { + const member = new RoomMember(memberData); + updatedHeroMembers.set(member.userId, member); + } + } + } + return {updatedHeroMembers: updatedHeroMembers.values(), removedUserIds}; + } + + applyChanges({updatedHeroMembers, removedUserIds}, summary) { + for (const userId of removedUserIds) { + this._members.delete(userId); + } + for (const member of updatedHeroMembers) { + this._members.set(member.userId, member); + } + const sortedMembers = Array.from(this._members.values()).sort((a, b) => a.displayName.localeCompare(b.displayName)); + this._roomName = calculateRoomName(sortedMembers, summary); + } + + get roomName() { + return this._roomName; + } + + get roomAvatarUrl() { + if (this._members.size === 1) { + for (const member of this._members.values()) { + console.log("roomAvatarUrl", member, member.avatarUrl); + return member.avatarUrl; + } + } + return null; + } +} From e5cdf061cbc1382271ec82419946b8e4a17cdd0e Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 21 Aug 2020 18:11:07 +0200 Subject: [PATCH 2/9] create, update and remove heroes as they come from sync --- src/matrix/room/Room.js | 54 +++++++++++++++++++++++++++++++--- src/matrix/room/RoomSummary.js | 6 +++- 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/src/matrix/room/Room.js b/src/matrix/room/Room.js index 7d482201..b7a70db5 100644 --- a/src/matrix/room/Room.js +++ b/src/matrix/room/Room.js @@ -24,6 +24,7 @@ import {SendQueue} from "./sending/SendQueue.js"; import {WrappedError} from "../error.js" import {fetchOrLoadMembers} from "./members/load.js"; import {MemberList} from "./members/MemberList.js"; +import {Heroes} from "./members/Heroes.js"; export class Room extends EventEmitter { constructor({roomId, storage, hsApi, emitCollectionChange, sendScheduler, pendingEvents, user}) { @@ -50,15 +51,32 @@ export class Room extends EventEmitter { isInitialSync, isTimelineOpen, txn); const {entries, newLiveKey, changedMembers} = await this._syncWriter.writeSync(roomResponse, txn); + // room name disappeared, open heroes + if (!summaryChanges.name && summaryChanges.heroes && !this._heroes) { + this._heroes = new Heroes(this._roomId); + } + // fetch new members while we have txn open, + // but don't make any in-memory changes yet + let heroChanges; + if (summaryChanges.heroes && this._heroes) { + heroChanges = await this._heroes.calculateChanges(summaryChanges.heroes, changedMembers, txn); + } let removedPendingEvents; if (roomResponse.timeline && roomResponse.timeline.events) { removedPendingEvents = this._sendQueue.removeRemoteEchos(roomResponse.timeline.events, txn); } - return {summaryChanges, newTimelineEntries: entries, newLiveKey, removedPendingEvents, changedMembers}; + return { + summaryChanges, + newTimelineEntries: entries, + newLiveKey, + removedPendingEvents, + changedMembers, + heroChanges + }; } /** @package */ - afterSync({summaryChanges, newTimelineEntries, newLiveKey, removedPendingEvents, changedMembers}) { + afterSync({summaryChanges, newTimelineEntries, newLiveKey, removedPendingEvents, changedMembers, heroChanges}) { this._syncWriter.afterSync(newLiveKey); if (changedMembers.length) { if (this._changedMembersDuringSync) { @@ -70,8 +88,22 @@ export class Room extends EventEmitter { this._memberList.afterSync(changedMembers); } } + let emitChange = false; if (summaryChanges) { this._summary.applyChanges(summaryChanges); + if (this._summary.name && this._heroes) { + this._heroes = null; + } + emitChange = true; + } + if (this._heroes && heroChanges) { + const oldName = this.name; + this._heroes.applyChanges(heroChanges, this._summary); + if (oldName !== this.name) { + emitChange = true; + } + } + if (emitChange) { this.emit("change"); this._emitCollectionChange(this); } @@ -89,9 +121,15 @@ export class Room extends EventEmitter { } /** @package */ - load(summary, txn) { + async load(summary, txn) { try { this._summary.load(summary); + // need to load members for name? + if (!this._summary.name && this._summary.heroes) { + this._heroes = new Heroes(this._roomId); + const changes = await this._heroes.calculateChanges(this._summary.heroes, [], txn); + this._heroes.applyChanges(changes, this._summary); + } return this._syncWriter.load(txn); } catch (err) { throw new WrappedError(`Could not load room ${this._roomId}`, err); @@ -177,6 +215,9 @@ export class Room extends EventEmitter { /** @public */ get name() { + if (this._heroes) { + return this._heroes.roomName; + } return this._summary.name; } @@ -186,7 +227,12 @@ export class Room extends EventEmitter { } get avatarUrl() { - return this._summary.avatarUrl; + if (this._summary.avatarUrl) { + return this._summary.avatarUrl; + } else if (this._heroes) { + return this._heroes.roomAvatarUrl; + } + return null; } get lastMessageTimestamp() { diff --git a/src/matrix/room/RoomSummary.js b/src/matrix/room/RoomSummary.js index 3f13bd1e..de181c41 100644 --- a/src/matrix/room/RoomSummary.js +++ b/src/matrix/room/RoomSummary.js @@ -100,7 +100,7 @@ function updateSummary(data, summary) { const inviteCount = summary["m.joined_member_count"]; const joinCount = summary["m.invited_member_count"]; - if (heroes) { + if (heroes && Array.isArray(heroes)) { data = data.cloneIfNeeded(); data.heroes = heroes; } @@ -174,6 +174,10 @@ export class RoomSummary { return this._data.roomId; } + get heroes() { + return this._data.heroes; + } + get isUnread() { return this._data.isUnread; } From d5d015487360ddb84c8268a503123d9f83669715 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 21 Aug 2020 18:11:26 +0200 Subject: [PATCH 3/9] join and invited count were mixed up... oops --- src/matrix/room/RoomSummary.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/matrix/room/RoomSummary.js b/src/matrix/room/RoomSummary.js index de181c41..d76fc294 100644 --- a/src/matrix/room/RoomSummary.js +++ b/src/matrix/room/RoomSummary.js @@ -97,8 +97,8 @@ function processTimelineEvent(data, event, isInitialSync, isTimelineOpen, ownUse function updateSummary(data, summary) { const heroes = summary["m.heroes"]; - const inviteCount = summary["m.joined_member_count"]; - const joinCount = summary["m.invited_member_count"]; + const joinCount = summary["m.joined_member_count"]; + const inviteCount = summary["m.invited_member_count"]; if (heroes && Array.isArray(heroes)) { data = data.cloneIfNeeded(); From acec7c8f3314d79752b0cf5c76e4a2e907bc861b Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 21 Aug 2020 18:11:53 +0200 Subject: [PATCH 4/9] remove alt_aliases, as we should not use it for the room name --- src/matrix/room/RoomSummary.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/matrix/room/RoomSummary.js b/src/matrix/room/RoomSummary.js index d76fc294..9e9c99dd 100644 --- a/src/matrix/room/RoomSummary.js +++ b/src/matrix/room/RoomSummary.js @@ -73,7 +73,6 @@ function processStateEvent(data, event) { const content = event.content; data = data.cloneIfNeeded(); data.canonicalAlias = content.alias; - data.altAliases = content.alt_aliases; } return data; } @@ -129,7 +128,6 @@ class SummaryData { 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.hasFetchedMembers = copy ? copy.hasFetchedMembers : false; this.lastPaginationToken = copy ? copy.lastPaginationToken : null; this.avatarUrl = copy ? copy.avatarUrl : null; @@ -165,9 +163,6 @@ export class RoomSummary { if (this._data.canonicalAlias) { return this._data.canonicalAlias; } - if (Array.isArray(this._data.altAliases) && this._data.altAliases.length !== 0) { - return this._data.altAliases[0]; - } if (Array.isArray(this._data.heroes) && this._data.heroes.length !== 0) { return this._data.heroes.join(", "); } From 2c14373b13c71ed04f0759e5aae6eeb47ad07dee Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 21 Aug 2020 18:12:11 +0200 Subject: [PATCH 5/9] allow falling back to heroes name if we don't have one --- src/matrix/room/RoomSummary.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/matrix/room/RoomSummary.js b/src/matrix/room/RoomSummary.js index 9e9c99dd..f9848158 100644 --- a/src/matrix/room/RoomSummary.js +++ b/src/matrix/room/RoomSummary.js @@ -163,10 +163,7 @@ export class RoomSummary { if (this._data.canonicalAlias) { return this._data.canonicalAlias; } - if (Array.isArray(this._data.heroes) && this._data.heroes.length !== 0) { - return this._data.heroes.join(", "); - } - return this._data.roomId; + return null; } get heroes() { From 70e89a3dd6d28e08e1fce2995142d6acbb62fc8e Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 21 Aug 2020 18:13:53 +0200 Subject: [PATCH 6/9] expose name/avatar on member --- src/matrix/room/members/RoomMember.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/matrix/room/members/RoomMember.js b/src/matrix/room/members/RoomMember.js index 4c38f66b..e6303fe4 100644 --- a/src/matrix/room/members/RoomMember.js +++ b/src/matrix/room/members/RoomMember.js @@ -54,6 +54,14 @@ export class RoomMember { }); } + get displayName() { + return this._data.displayName; + } + + get avatarUrl() { + return this._data.avatarUrl; + } + get roomId() { return this._data.roomId; } From e4758d06519641cbc182ff3bc9dab10162c29e88 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 21 Aug 2020 18:14:07 +0200 Subject: [PATCH 7/9] we need to read from members now during load, for the heroes and were actually not reading from room state --- src/matrix/Session.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/matrix/Session.js b/src/matrix/Session.js index 80ef342b..7a7dea52 100644 --- a/src/matrix/Session.js +++ b/src/matrix/Session.js @@ -36,7 +36,7 @@ export class Session { const txn = await this._storage.readTxn([ this._storage.storeNames.session, this._storage.storeNames.roomSummary, - this._storage.storeNames.roomState, + this._storage.storeNames.roomMembers, this._storage.storeNames.timelineEvents, this._storage.storeNames.timelineFragments, this._storage.storeNames.pendingEvents, From b39c15d88d502f3870fb3218f61e2adcdc0bd1ef Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 21 Aug 2020 18:14:32 +0200 Subject: [PATCH 8/9] Room.name can actually return null now so protect against this, and fall back to "Empty Room" --- src/domain/session/room/RoomViewModel.js | 4 ++-- src/domain/session/roomlist/RoomTileViewModel.js | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/domain/session/room/RoomViewModel.js b/src/domain/session/room/RoomViewModel.js index b5c05be7..32e09fbe 100644 --- a/src/domain/session/room/RoomViewModel.js +++ b/src/domain/session/room/RoomViewModel.js @@ -84,7 +84,7 @@ export class RoomViewModel extends ViewModel { } get name() { - return this._room.name; + return this._room.name || this.i18n`Empty Room`; } get timelineViewModel() { @@ -102,7 +102,7 @@ export class RoomViewModel extends ViewModel { } get avatarLetter() { - return avatarInitials(this._room.name); + return avatarInitials(this.name); } get avatarColorNumber() { diff --git a/src/domain/session/roomlist/RoomTileViewModel.js b/src/domain/session/roomlist/RoomTileViewModel.js index 9fa7df0f..ec344f79 100644 --- a/src/domain/session/roomlist/RoomTileViewModel.js +++ b/src/domain/session/roomlist/RoomTileViewModel.js @@ -70,7 +70,7 @@ export class RoomTileViewModel extends ViewModel { const timeDiff = theirTimestamp - myTimestamp; if (timeDiff === 0) { // sort alphabetically - const nameCmp = this._room.name.localeCompare(other._room.name); + const nameCmp = this.name.localeCompare(other.name); if (nameCmp === 0) { return this._room.id.localeCompare(other._room.id); } @@ -88,12 +88,12 @@ export class RoomTileViewModel extends ViewModel { } get name() { - return this._room.name; + return this._room.name || this.i18n`Empty Room`; } // Avatar view model contract get avatarLetter() { - return avatarInitials(this._room.name); + return avatarInitials(this.name); } get avatarColorNumber() { From 3d5b69f60ab77ebe3b778cdcd546759946cbc5b2 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 21 Aug 2020 18:14:57 +0200 Subject: [PATCH 9/9] remove obsolete comment --- src/matrix/room/RoomSummary.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/matrix/room/RoomSummary.js b/src/matrix/room/RoomSummary.js index f9848158..e924f6fe 100644 --- a/src/matrix/room/RoomSummary.js +++ b/src/matrix/room/RoomSummary.js @@ -236,12 +236,6 @@ export class RoomSummary { isInitialSync, isTimelineOpen, this._ownUserId); 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; }