OpenCloud
/
riot-web
mirror of https://github.com/vector-im/riot-web
1
0
Fork 0
Code Issues Releases Wiki Activity

Merge pull request #4828 from matrix-org/travis/room-list/proliferation 

Fix a number of proliferation issues in the new room list
pull/21833/head
Travis Ralston 2020-06-30 16:10:50 -06:00 committed by GitHub
parent b456529c3e 9de42513dc
commit 2eaaf6a7bd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 209 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

57
src/stores/room-list/RoomListStore2.ts
View File

@ -29,6 +29,7 @@ import { IFilterCondition } from "./filters/IFilterCondition";
import { TagWatcher } from "./TagWatcher";
import RoomViewStore from "../RoomViewStore";
import { Algorithm, LIST_UPDATED_EVENT } from "./algorithms/Algorithm";
import { EffectiveMembership, getEffectiveMembership } from "./membership";
interface IState {
tagsEnabled?: boolean;
@ -97,8 +98,10 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> {
this.algorithm.stickyRoom = null;
} else if (activeRoomId) {
const activeRoom = this.matrixClient.getRoom(activeRoomId);
if (!activeRoom) throw new Error(`${activeRoomId} is current in RVS but missing from client`);
if (activeRoom !== this.algorithm.stickyRoom) {
if (!activeRoom) {
console.warn(`${activeRoomId} is current in RVS but missing from client - clearing sticky room`);
this.algorithm.stickyRoom = null;
} else if (activeRoom !== this.algorithm.stickyRoom) {
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log(`Changing sticky room to ${activeRoomId}`);
this.algorithm.stickyRoom = activeRoom;
@ -187,10 +190,13 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> {
console.log(`[RoomListDebug] Live timeline event ${eventPayload.event.getId()} in ${updatedRoom.roomId}`);
if (eventPayload.event.getType() === 'm.room.tombstone' && eventPayload.event.getStateKey() === '') {
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log(`[RoomListDebug] Got tombstone event - regenerating room list`);
// TODO: We could probably be smarter about this: https://github.com/vector-im/riot-web/issues/14035
await this.regenerateAllLists();
return; // don't pass the update down - we will have already handled it in the regen
console.log(`[RoomListDebug] Got tombstone event - trying to remove now-dead room`);
const newRoom = this.matrixClient.getRoom(eventPayload.event.getContent()['replacement_room']);
if (newRoom) {
// If we have the new room, then the new room check will have seen the predecessor
// and did the required updates, so do nothing here.
return;
}
}
await this.handleRoomUpdate(updatedRoom, RoomUpdateCause.Timeline);
};
@ -242,18 +248,49 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> {
}
} else if (payload.action === 'MatrixActions.Room.myMembership') {
const membershipPayload = (<any>payload); // TODO: Type out the dispatcher types
if (membershipPayload.oldMembership !== "join" && membershipPayload.membership === "join") {
const oldMembership = getEffectiveMembership(membershipPayload.oldMembership);
const newMembership = getEffectiveMembership(membershipPayload.membership);
if (oldMembership !== EffectiveMembership.Join && newMembership === EffectiveMembership.Join) {
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log(`[RoomListDebug] Handling new room ${membershipPayload.room.roomId}`);
await this.algorithm.handleRoomUpdate(membershipPayload.room, RoomUpdateCause.NewRoom);
// If we're joining an upgraded room, we'll want to make sure we don't proliferate
// the dead room in the list.
const createEvent = membershipPayload.room.currentState.getStateEvents("m.room.create", "");
if (createEvent && createEvent.getContent()['predecessor']) {
console.log(`[RoomListDebug] Room has a predecessor`);
const prevRoom = this.matrixClient.getRoom(createEvent.getContent()['predecessor']['room_id']);
if (prevRoom) {
const isSticky = this.algorithm.stickyRoom === prevRoom;
if (isSticky) {
console.log(`[RoomListDebug] Clearing sticky room due to room upgrade`);
await this.algorithm.setStickyRoomAsync(null);
}
// Note: we hit the algorithm instead of our handleRoomUpdate() function to
// avoid redundant updates.
console.log(`[RoomListDebug] Removing previous room from room list`);
await this.algorithm.handleRoomUpdate(prevRoom, RoomUpdateCause.RoomRemoved);
}
}
console.log(`[RoomListDebug] Adding new room to room list`);
await this.handleRoomUpdate(membershipPayload.room, RoomUpdateCause.NewRoom);
return;
}
if (oldMembership !== EffectiveMembership.Invite && newMembership === EffectiveMembership.Invite) {
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log(`[RoomListDebug] Handling invite to ${membershipPayload.room.roomId}`);
await this.handleRoomUpdate(membershipPayload.room, RoomUpdateCause.NewRoom);
return;
}
// If it's not a join, it's transitioning into a different list (possibly historical)
if (membershipPayload.oldMembership !== membershipPayload.membership) {
if (oldMembership !== newMembership) {
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log(`[RoomListDebug] Handling membership change in ${membershipPayload.room.roomId}`);
await this.algorithm.handleRoomUpdate(membershipPayload.room, RoomUpdateCause.PossibleTagChange);
await this.handleRoomUpdate(membershipPayload.room, RoomUpdateCause.PossibleTagChange);
return;
}
}

104
src/stores/room-list/algorithms/Algorithm.ts
View File

@ -30,7 +30,7 @@ import {
SortAlgorithm
} from "./models";
import { FILTER_CHANGED, FilterPriority, IFilterCondition } from "../filters/IFilterCondition";
import { EffectiveMembership, splitRoomsByMembership } from "../membership";
import { EffectiveMembership, getEffectiveMembership, splitRoomsByMembership } from "../membership";
import { OrderingAlgorithm } from "./list-ordering/OrderingAlgorithm";
import { getListAlgorithmInstance } from "./list-ordering";
@ -99,6 +99,14 @@ export class Algorithm extends EventEmitter {
return this._cachedRooms;
}
/**
* Awaitable version of the sticky room setter.
* @param val The new room to sticky.
*/
public async setStickyRoomAsync(val: Room) {
await this.updateStickyRoom(val);
}
public getTagSorting(tagId: TagID): SortAlgorithm {
return this.sortAlgorithms[tagId];
}
@ -160,10 +168,13 @@ export class Algorithm extends EventEmitter {
// It's possible to have no selected room. In that case, clear the sticky room
if (!val) {
if (this._stickyRoom) {
const stickyRoom = this._stickyRoom.room;
this._stickyRoom = null; // clear before we go to update the algorithm
// Lie to the algorithm and re-add the room to the algorithm
await this.handleRoomUpdate(this._stickyRoom.room, RoomUpdateCause.NewRoom);
await this.handleRoomUpdate(stickyRoom, RoomUpdateCause.NewRoom);
return;
}
this._stickyRoom = null;
return;
}
@ -289,6 +300,8 @@ export class Algorithm extends EventEmitter {
}
protected recalculateFilteredRoomsForTag(tagId: TagID): void {
if (!this.hasFilters) return; // don't bother doing work if there's nothing to do
console.log(`Recalculating filtered rooms for ${tagId}`);
delete this.filteredRooms[tagId];
const rooms = this.cachedRooms[tagId].map(r => r); // cheap clone
@ -428,6 +441,13 @@ export class Algorithm extends EventEmitter {
if (isNullOrUndefined(rooms)) throw new Error(`Array of rooms cannot be null`);
if (!this.sortAlgorithms) throw new Error(`Cannot set known rooms without a tag sorting map`);
console.warn("Resetting known rooms, initiating regeneration");
// Before we go any further we need to clear (but remember) the sticky room to
// avoid accidentally duplicating it in the list.
const oldStickyRoom = this._stickyRoom;
await this.updateStickyRoom(null);
this.rooms = rooms;
const newTags: ITagMap = {};
@ -458,14 +478,7 @@ export class Algorithm extends EventEmitter {
// Now process all the joined rooms. This is a bit more complicated
for (const room of memberships[EffectiveMembership.Join]) {
let tags = Object.keys(room.tags || {});
if (tags.length === 0) {
// Check to see if it's a DM if it isn't anything else
if (DMRoomMap.shared().getUserIdForRoomId(room.roomId)) {
tags = [DefaultTagID.DM];
}
}
const tags = this.getTagsOfJoinedRoom(room);
let inTag = false;
if (tags.length > 0) {
@ -494,6 +507,54 @@ export class Algorithm extends EventEmitter {
this.cachedRooms = newTags;
this.updateTagsFromCache();
this.recalculateFilteredRooms();
// Now that we've finished generation, we need to update the sticky room to what
// it was. It's entirely possible that it changed lists though, so if it did then
// we also have to update the position of it.
if (oldStickyRoom && oldStickyRoom.room) {
await this.updateStickyRoom(oldStickyRoom.room);
if (this._stickyRoom && this._stickyRoom.room) { // just in case the update doesn't go according to plan
if (this._stickyRoom.tag !== oldStickyRoom.tag) {
// We put the sticky room at the top of the list to treat it as an obvious tag change.
this._stickyRoom.position = 0;
this.recalculateStickyRoom(this._stickyRoom.tag);
}
}
}
}
private getTagsForRoom(room: Room): TagID[] {
// XXX: This duplicates a lot of logic from setKnownRooms above, but has a slightly
// different use case and therefore different performance curve
const tags: TagID[] = [];
const membership = getEffectiveMembership(room.getMyMembership());
if (membership === EffectiveMembership.Invite) {
tags.push(DefaultTagID.Invite);
} else if (membership === EffectiveMembership.Leave) {
tags.push(DefaultTagID.Archived);
} else {
tags.push(...this.getTagsOfJoinedRoom(room));
}
if (!tags.length) tags.push(DefaultTagID.Untagged);
return tags;
}
private getTagsOfJoinedRoom(room: Room): TagID[] {
let tags = Object.keys(room.tags || {});
if (tags.length === 0) {
// Check to see if it's a DM if it isn't anything else
if (DMRoomMap.shared().getUserIdForRoomId(room.roomId)) {
tags = [DefaultTagID.DM];
}
}
return tags;
}
/**
@ -548,6 +609,14 @@ export class Algorithm extends EventEmitter {
public async handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise<boolean> {
if (!this.algorithms) throw new Error("Not ready: no algorithms to determine tags from");
if (cause === RoomUpdateCause.NewRoom) {
const roomTags = this.roomIdsToTags[room.roomId];
if (roomTags && roomTags.length > 0) {
console.warn(`${room.roomId} is reportedly new but is already known - assuming TagChange instead`);
cause = RoomUpdateCause.PossibleTagChange;
}
}
if (cause === RoomUpdateCause.PossibleTagChange) {
// TODO: Be smarter and splice rather than regen the planet. https://github.com/vector-im/riot-web/issues/14035
// TODO: No-op if no change. https://github.com/vector-im/riot-web/issues/14035
@ -566,6 +635,19 @@ export class Algorithm extends EventEmitter {
}
}
if (cause === RoomUpdateCause.NewRoom && !this.roomIdsToTags[room.roomId]) {
console.log(`[RoomListDebug] Updating tags for new room ${room.roomId} (${room.name})`);
// Get the tags for the room and populate the cache
const roomTags = this.getTagsForRoom(room).filter(t => !isNullOrUndefined(this.cachedRooms[t]));
// "This should never happen" condition - we specify DefaultTagID.Untagged in getTagsForRoom(),
// which means we should *always* have a tag to go off of.
if (!roomTags.length) throw new Error(`Tags cannot be determined for ${room.roomId}`);
this.roomIdsToTags[room.roomId] = roomTags;
}
let tags = this.roomIdsToTags[room.roomId];
if (!tags) {
console.warn(`No tags known for "${room.name}" (${room.roomId})`);

82
src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts
View File

@ -179,45 +179,51 @@ export class ImportanceAlgorithm extends OrderingAlgorithm {
}
public async handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise<boolean> {
if (cause === RoomUpdateCause.NewRoom || cause === RoomUpdateCause.RoomRemoved) {
return this.handleSplice(room, cause);
try {
await this.updateLock.acquireAsync();
if (cause === RoomUpdateCause.NewRoom || cause === RoomUpdateCause.RoomRemoved) {
return this.handleSplice(room, cause);
}
if (cause !== RoomUpdateCause.Timeline && cause !== RoomUpdateCause.ReadReceipt) {
throw new Error(`Unsupported update cause: ${cause}`);
}
const category = this.getRoomCategory(room);
if (this.sortingAlgorithm === SortAlgorithm.Manual) {
return; // Nothing to do here.
}
const roomIdx = this.getRoomIndex(room);
if (roomIdx === -1) {
throw new Error(`Room ${room.roomId} has no index in ${this.tagId}`);
}
// Try to avoid doing array operations if we don't have to: only move rooms within
// the categories if we're jumping categories
const oldCategory = this.getCategoryFromIndices(roomIdx, this.indices);
if (oldCategory !== category) {
// Move the room and update the indices
this.moveRoomIndexes(1, oldCategory, category, this.indices);
this.cachedOrderedRooms.splice(roomIdx, 1); // splice out the old index (fixed position)
this.cachedOrderedRooms.splice(this.indices[category], 0, room); // splice in the new room (pre-adjusted)
// Note: if moveRoomIndexes() is called after the splice then the insert operation
// will happen in the wrong place. Because we would have already adjusted the index
// for the category, we don't need to determine how the room is moving in the list.
// If we instead tried to insert before updating the indices, we'd have to determine
// whether the room was moving later (towards IDLE) or earlier (towards RED) from its
// current position, as it'll affect the category's start index after we remove the
// room from the array.
}
// Sort the category now that we've dumped the room in
await this.sortCategory(category);
return true; // change made
} finally {
await this.updateLock.release();
}
if (cause !== RoomUpdateCause.Timeline && cause !== RoomUpdateCause.ReadReceipt) {
throw new Error(`Unsupported update cause: ${cause}`);
}
const category = this.getRoomCategory(room);
if (this.sortingAlgorithm === SortAlgorithm.Manual) {
return; // Nothing to do here.
}
const roomIdx = this.getRoomIndex(room);
if (roomIdx === -1) {
throw new Error(`Room ${room.roomId} has no index in ${this.tagId}`);
}
// Try to avoid doing array operations if we don't have to: only move rooms within
// the categories if we're jumping categories
const oldCategory = this.getCategoryFromIndices(roomIdx, this.indices);
if (oldCategory !== category) {
// Move the room and update the indices
this.moveRoomIndexes(1, oldCategory, category, this.indices);
this.cachedOrderedRooms.splice(roomIdx, 1); // splice out the old index (fixed position)
this.cachedOrderedRooms.splice(this.indices[category], 0, room); // splice in the new room (pre-adjusted)
// Note: if moveRoomIndexes() is called after the splice then the insert operation
// will happen in the wrong place. Because we would have already adjusted the index
// for the category, we don't need to determine how the room is moving in the list.
// If we instead tried to insert before updating the indices, we'd have to determine
// whether the room was moving later (towards IDLE) or earlier (towards RED) from its
// current position, as it'll affect the category's start index after we remove the
// room from the array.
}
// Sort the category now that we've dumped the room in
await this.sortCategory(category);
return true; // change made
}
private async sortCategory(category: Category) {

40
src/stores/room-list/algorithms/list-ordering/NaturalAlgorithm.ts
View File

@ -38,23 +38,29 @@ export class NaturalAlgorithm extends OrderingAlgorithm {
}
public async handleRoomUpdate(room, cause): Promise<boolean> {
const isSplice = cause === RoomUpdateCause.NewRoom || cause === RoomUpdateCause.RoomRemoved;
const isInPlace = cause === RoomUpdateCause.Timeline || cause === RoomUpdateCause.ReadReceipt;
if (!isSplice && !isInPlace) {
throw new Error(`Unsupported update cause: ${cause}`);
try {
await this.updateLock.acquireAsync();
const isSplice = cause === RoomUpdateCause.NewRoom || cause === RoomUpdateCause.RoomRemoved;
const isInPlace = cause === RoomUpdateCause.Timeline || cause === RoomUpdateCause.ReadReceipt;
if (!isSplice && !isInPlace) {
throw new Error(`Unsupported update cause: ${cause}`);
}
if (cause === RoomUpdateCause.NewRoom) {
this.cachedOrderedRooms.push(room);
} else if (cause === RoomUpdateCause.RoomRemoved) {
const idx = this.cachedOrderedRooms.indexOf(room);
if (idx >= 0) this.cachedOrderedRooms.splice(idx, 1);
}
// TODO: Optimize this to avoid useless operations: https://github.com/vector-im/riot-web/issues/14035
// For example, we can skip updates to alphabetic (sometimes) and manually ordered tags
this.cachedOrderedRooms = await sortRoomsWithAlgorithm(this.cachedOrderedRooms, this.tagId, this.sortingAlgorithm);
return true;
} finally {
await this.updateLock.release();
}
if (cause === RoomUpdateCause.NewRoom) {
this.cachedOrderedRooms.push(room);
} else if (cause === RoomUpdateCause.RoomRemoved) {
const idx = this.cachedOrderedRooms.indexOf(room);
if (idx >= 0) this.cachedOrderedRooms.splice(idx, 1);
}
// TODO: Optimize this to avoid useless operations: https://github.com/vector-im/riot-web/issues/14035
// For example, we can skip updates to alphabetic (sometimes) and manually ordered tags
this.cachedOrderedRooms = await sortRoomsWithAlgorithm(this.cachedOrderedRooms, this.tagId, this.sortingAlgorithm);
return true;
}
}

2
src/stores/room-list/algorithms/list-ordering/OrderingAlgorithm.ts
View File

@ -17,6 +17,7 @@ limitations under the License.
import { Room } from "matrix-js-sdk/src/models/room";
import { RoomUpdateCause, TagID } from "../../models";
import { SortAlgorithm } from "../models";
import AwaitLock from "await-lock";
/**
* Represents a list ordering algorithm. Subclasses should populate the
@ -25,6 +26,7 @@ import { SortAlgorithm } from "../models";
export abstract class OrderingAlgorithm {
protected cachedOrderedRooms: Room[];
protected sortingAlgorithm: SortAlgorithm;
protected readonly updateLock = new AwaitLock();
protected constructor(protected tagId: TagID, initialSortingAlgorithm: SortAlgorithm) {
// noinspection JSIgnoredPromiseFromCall