nikrozman/element-web
1
0
Fork 0
Code Issues Actions 3 Packages Projects Releases Wiki Activity

Merge commit from fork

Browse Source 
* Validate room upgrade relationships properly

Ensures only correctly related rooms are left when leaving the latest version of a room.
Ensures the room list does not wrongly hide rooms which have not yet been upgraded.
Ensures the breadcrumbs store finds the correct latest version of a room for a given stored room.

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Tests

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

---------

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
This commit is contained in:
Michael Telatynski
2025-09-16 12:43:49 +01:00
committed by GitHub
parent 9a11a80483
commit 8e9a43d70c
8 changed files with 75 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/stores/BreadcrumbsStore.ts
View File

@@ -143,7 +143,7 @@ export class BreadcrumbsStore extends AsyncStoreWithClient<IState> {
// If the room is upgraded, use that room instead. We'll also splice out
// any children of the room.
const history = this.matrixClient?.getRoomUpgradeHistory(room.roomId, false, msc3946ProcessDynamicPredecessor);
const history = this.matrixClient?.getRoomUpgradeHistory(room.roomId, true, msc3946ProcessDynamicPredecessor);
if (history && history.length > 1) {
room = history[history.length - 1]; // Last room is most recent in history

17
src/stores/room-list-v3/RoomListStoreV3.ts
View File

@@ -8,7 +8,7 @@ Please see LICENSE files in the repository root for full details.
import { logger } from "matrix-js-sdk/src/logger";
import { EventType, KnownMembership } from "matrix-js-sdk/src/matrix";
import type { EmptyObject, Room, RoomState } from "matrix-js-sdk/src/matrix";
import type { EmptyObject, Room } from "matrix-js-sdk/src/matrix";
import type { MatrixDispatcher } from "../../dispatcher/dispatcher";
import type { ActionPayload } from "../../dispatcher/payloads";
import type { FilterKey } from "./skip-list/filters";
@@ -250,12 +250,15 @@ export class RoomListStoreV3Class extends AsyncStoreWithClient<EmptyObject> {
// If we're joining an upgraded room, we'll want to make sure we don't proliferate
// the dead room in the list.
if (oldMembership !== EffectiveMembership.Join && newMembership === EffectiveMembership.Join) {
const roomState: RoomState = payload.room.currentState;
const predecessor = roomState.findPredecessor(this.msc3946ProcessDynamicPredecessor);
if (predecessor) {
const prevRoom = this.matrixClient?.getRoom(predecessor.roomId);
if (prevRoom) this.roomSkipList.removeRoom(prevRoom);
else logger.warn(`Unable to find predecessor room with id ${predecessor.roomId}`);
const room: Room = payload.room;
const roomUpgradeHistory = room.client.getRoomUpgradeHistory(
room.roomId,
true,
this.msc3946ProcessDynamicPredecessor,
);
const predecessors = roomUpgradeHistory.slice(0, roomUpgradeHistory.indexOf(room));
for (const predecessor of predecessors) {
this.roomSkipList.removeRoom(predecessor);
}
}

34
src/stores/room-list/RoomListStore.ts
View File

@@ -6,7 +6,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com
Please see LICENSE files in the repository root for full details.
*/
import { type MatrixClient, type Room, type RoomState, EventType, type EmptyObject } from "matrix-js-sdk/src/matrix";
import { type MatrixClient, type Room, EventType, type EmptyObject } from "matrix-js-sdk/src/matrix";
import { logger } from "matrix-js-sdk/src/logger";
import SettingsStore from "../../settings/SettingsStore";
@@ -308,24 +308,22 @@ export class RoomListStoreClass extends AsyncStoreWithClient<EmptyObject> implem
const oldMembership = getEffectiveMembership(membershipPayload.oldMembership);
const newMembership = getEffectiveMembershipTag(membershipPayload.room, membershipPayload.membership);
if (oldMembership !== EffectiveMembership.Join && newMembership === EffectiveMembership.Join) {
// If we're joining an upgraded room, we'll want to make sure we don't proliferate
// the dead room in the list.
const roomState: RoomState = membershipPayload.room.currentState;
const predecessor = roomState.findPredecessor(this.msc3946ProcessDynamicPredecessor);
if (predecessor) {
const prevRoom = this.matrixClient?.getRoom(predecessor.roomId);
if (prevRoom) {
const isSticky = this.algorithm.stickyRoom === prevRoom;
if (isSticky) {
this.algorithm.setStickyRoom(null);
}
// Note: we hit the algorithm instead of our handleRoomUpdate() function to
// avoid redundant updates.
this.algorithm.handleRoomUpdate(prevRoom, RoomUpdateCause.RoomRemoved);
} else {
logger.warn(`Unable to find predecessor room with id ${predecessor.roomId}`);
// If we're joining an upgraded room, we'll want to make sure we don't proliferate the dead room in the list.
const room: Room = membershipPayload.room;
const roomUpgradeHistory = room.client.getRoomUpgradeHistory(
room.roomId,
true,
this.msc3946ProcessDynamicPredecessor,
);
const predecessors = roomUpgradeHistory.slice(0, roomUpgradeHistory.indexOf(room));
for (const predecessor of predecessors) {
const isSticky = this.algorithm.stickyRoom === predecessor;
if (isSticky) {
this.algorithm.setStickyRoom(null);
}
// Note: we hit the algorithm instead of our handleRoomUpdate() function to
// avoid redundant updates.
this.algorithm.handleRoomUpdate(predecessor, RoomUpdateCause.RoomRemoved);
}
await this.handleRoomUpdate(membershipPayload.room, RoomUpdateCause.NewRoom);

2
src/utils/leave-behaviour.ts
View File

@@ -40,7 +40,7 @@ export async function leaveRoomBehaviour(
let leavingAllVersions = true;
const history = matrixClient.getRoomUpgradeHistory(
roomId,
false,
true,
SettingsStore.getValue("feature_dynamic_room_predecessors"),
);
if (history && history.length > 0) {

4
test/unit-tests/stores/BreadcrumbsStore-test.ts
View File

@@ -112,7 +112,7 @@ describe("BreadcrumbsStore", () => {
await dispatchJoinRoom(room.roomId);
// We pass the value of the dynamic predecessor setting through
expect(client.getRoomUpgradeHistory).toHaveBeenCalledWith(room.roomId, false, false);
expect(client.getRoomUpgradeHistory).toHaveBeenCalledWith(room.roomId, true, false);
});
});
@@ -134,7 +134,7 @@ describe("BreadcrumbsStore", () => {
await dispatchJoinRoom(room.roomId);
// We pass the value of the dynamic predecessor setting through
expect(client.getRoomUpgradeHistory).toHaveBeenCalledWith(room.roomId, false, true);
expect(client.getRoomUpgradeHistory).toHaveBeenCalledWith(room.roomId, true, true);
});
});

39
test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts
View File

@@ -28,6 +28,7 @@ import SettingsStore from "../../../../src/settings/SettingsStore";
import * as utils from "../../../../src/utils/notifications";
import * as roomMute from "../../../../src/stores/room-list/utils/roomMute";
import { Action } from "../../../../src/dispatcher/actions";
import { mocked } from "jest-mock";
describe("RoomListStoreV3", () => {
async function getRoomListStore() {
@@ -197,6 +198,9 @@ describe("RoomListStoreV3", () => {
const oldRoom = rooms[32];
// Create a new room with a predecessor event that points to oldRoom
const newRoom = new Room("!foonew:matrix.org", client, client.getSafeUserId(), {});
mocked(client.getRoomUpgradeHistory).mockImplementation((roomId) =>
roomId === newRoom.roomId ? [oldRoom, newRoom] : [],
);
const createWithPredecessor = new MatrixEvent({
type: EventType.RoomCreate,
sender: "@foo:foo.org",
@@ -227,6 +231,41 @@ describe("RoomListStoreV3", () => {
expect(roomIds).toContain(newRoom.roomId);
});
it("should not remove predecessor room based on non-reciprocated relationship", async () => {
const { store, rooms, client, dispatcher } = await getRoomListStore();
const oldRoom = rooms[32];
// Create a new room with a predecessor event that points to oldRoom, but oldRoom does not point back
const newRoom = new Room("!nefarious:matrix.org", client, client.getSafeUserId(), {});
const createWithPredecessor = new MatrixEvent({
type: EventType.RoomCreate,
sender: "@foo:foo.org",
room_id: newRoom.roomId,
content: {
predecessor: { room_id: oldRoom.roomId, event_id: "tombstone_event_id" },
},
event_id: "$create",
state_key: "",
});
upsertRoomStateEvents(newRoom, [createWithPredecessor]);
const fn = jest.fn();
store.on(LISTS_UPDATE_EVENT, fn);
dispatcher.dispatch(
{
action: "MatrixActions.Room.myMembership",
oldMembership: KnownMembership.Invite,
membership: KnownMembership.Join,
room: newRoom,
},
true,
);
expect(fn).toHaveBeenCalled();
const roomIds = store.getSortedRooms().map((r) => r.roomId);
expect(roomIds).toContain(oldRoom.roomId);
expect(roomIds).toContain(newRoom.roomId);
});
it("Rooms are re-inserted on m.direct event", async () => {
const { store, dispatcher, client } = await getRoomListStore();

4
test/unit-tests/stores/room-list/RoomListStore-test.ts
View File

@@ -115,6 +115,10 @@ describe("RoomListStore", () => {
// Given a store we can spy on
const { store, handleRoomUpdate } = createStore();
mocked(client.getRoomUpgradeHistory).mockImplementation((roomId) =>
roomId === roomWithCreatePredecessor.roomId ? [oldRoom, roomWithCreatePredecessor] : [],
);
// When we tell it we joined a new room that has an old room as
// predecessor in the create event
const payload = {

4
test/unit-tests/utils/leave-behaviour-test.ts
View File

@@ -129,7 +129,7 @@ describe("leaveRoomBehaviour", () => {
it("Passes through the dynamic predecessor setting", async () => {
await leaveRoomBehaviour(client, room.roomId);
expect(client.getRoomUpgradeHistory).toHaveBeenCalledWith(room.roomId, false, false);
expect(client.getRoomUpgradeHistory).toHaveBeenCalledWith(room.roomId, true, false);
});
});
@@ -143,7 +143,7 @@ describe("leaveRoomBehaviour", () => {
it("Passes through the dynamic predecessor setting", async () => {
await leaveRoomBehaviour(client, room.roomId);
expect(client.getRoomUpgradeHistory).toHaveBeenCalledWith(room.roomId, false, true);
expect(client.getRoomUpgradeHistory).toHaveBeenCalledWith(room.roomId, true, true);
});
});
});