Skip to content
This repository was archived by the owner on Sep 11, 2024. It is now read-only.

Commit bf72b91

Browse files
RiotRobotweeman1337turt2live
authored
[Backport staging] Prevent unnecessary m.direct updates (#9817)
* Prevent unnecessary m.direct updates (#9805) * Prevent unnecessary m.direct updates Signed-off-by: Michael Weimann <[email protected]> * Replace object with Map * Clean up comment; simplify code * Fix rte flaky test (#9811) * Snap in PiP widget when content changed (#9797) * Check modified at the end of setDMRoom * Revert "Snap in PiP widget when content changed (#9797)" This reverts commit be1e575. * Revert "Fix rte flaky test (#9811)" This reverts commit 20d9eb3. * Update src/Rooms.ts Co-authored-by: Richard van der Hoff <[email protected]> Signed-off-by: Michael Weimann <[email protected]> Co-authored-by: Florian Duros <[email protected]> Co-authored-by: Richard van der Hoff <[email protected]> (cherry picked from commit 93dd010) * Use a function that exists on staging Co-authored-by: Michael Weimann <[email protected]> Co-authored-by: Travis Ralston <[email protected]>
1 parent e0fe177 commit bf72b91

File tree

2 files changed

+167
-11
lines changed

2 files changed

+167
-11
lines changed

src/Rooms.ts

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -57,42 +57,47 @@ export function guessAndSetDMRoom(room: Room, isDirect: boolean): Promise<void>
5757
/**
5858
* Marks or unmarks the given room as being as a DM room.
5959
* @param {string} roomId The ID of the room to modify
60-
* @param {string} userId The user ID of the desired DM
61-
room target user or null to un-mark
62-
this room as a DM room
60+
* @param {string | null} userId The user ID of the desired DM room target user or
61+
* null to un-mark this room as a DM room
6362
* @returns {object} A promise
6463
*/
65-
export async function setDMRoom(roomId: string, userId: string): Promise<void> {
64+
export async function setDMRoom(roomId: string, userId: string | null): Promise<void> {
6665
if (MatrixClientPeg.get().isGuest()) return;
6766

6867
const mDirectEvent = MatrixClientPeg.get().getAccountData(EventType.Direct);
69-
let dmRoomMap = {};
68+
const currentContent = mDirectEvent?.getContent() || {};
7069

71-
if (mDirectEvent !== undefined) dmRoomMap = { ...mDirectEvent.getContent() }; // copy as we will mutate
70+
const dmRoomMap = new Map(Object.entries(currentContent));
71+
let modified = false;
7272

7373
// remove it from the lists of any others users
7474
// (it can only be a DM room for one person)
75-
for (const thisUserId of Object.keys(dmRoomMap)) {
76-
const roomList = dmRoomMap[thisUserId];
75+
for (const thisUserId of dmRoomMap.keys()) {
76+
const roomList = dmRoomMap.get(thisUserId) || [];
7777

7878
if (thisUserId != userId) {
7979
const indexOfRoom = roomList.indexOf(roomId);
8080
if (indexOfRoom > -1) {
8181
roomList.splice(indexOfRoom, 1);
82+
modified = true;
8283
}
8384
}
8485
}
8586

8687
// now add it, if it's not already there
8788
if (userId) {
88-
const roomList = dmRoomMap[userId] || [];
89+
const roomList = dmRoomMap.get(userId) || [];
8990
if (roomList.indexOf(roomId) == -1) {
9091
roomList.push(roomId);
92+
modified = true;
9193
}
92-
dmRoomMap[userId] = roomList;
94+
dmRoomMap.set(userId, roomList);
9395
}
9496

95-
await MatrixClientPeg.get().setAccountData(EventType.Direct, dmRoomMap);
97+
// prevent unnecessary calls to setAccountData
98+
if (!modified) return;
99+
100+
await MatrixClientPeg.get().setAccountData(EventType.Direct, Object.fromEntries(dmRoomMap));
96101
}
97102

98103
/**

test/Rooms-test.ts

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
/*
2+
Copyright 2022 The Matrix.org Foundation C.I.C.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
import { mocked } from "jest-mock";
18+
import { EventType, MatrixClient, MatrixEvent } from "matrix-js-sdk/src/matrix";
19+
20+
import { setDMRoom } from "../src/Rooms";
21+
import { mkEvent, stubClient } from "./test-utils";
22+
23+
describe("setDMRoom", () => {
24+
const userId1 = "@user1:example.com";
25+
const userId2 = "@user2:example.com";
26+
const userId3 = "@user3:example.com";
27+
const roomId1 = "!room1:example.com";
28+
const roomId2 = "!room2:example.com";
29+
const roomId3 = "!room3:example.com";
30+
const roomId4 = "!room4:example.com";
31+
let client: MatrixClient;
32+
33+
beforeEach(() => {
34+
client = mocked(stubClient());
35+
client.getAccountData = jest.fn().mockImplementation((eventType: string): MatrixEvent | undefined => {
36+
if (eventType === EventType.Direct) {
37+
return mkEvent({
38+
event: true,
39+
content: {
40+
[userId1]: [roomId1, roomId2],
41+
[userId2]: [roomId3],
42+
},
43+
type: EventType.Direct,
44+
user: client.getUserId(),
45+
});
46+
}
47+
48+
return undefined;
49+
});
50+
});
51+
52+
describe("when logged in as a guest and marking a room as DM", () => {
53+
beforeEach(() => {
54+
mocked(client.isGuest).mockReturnValue(true);
55+
setDMRoom(roomId1, userId1);
56+
});
57+
58+
it("should not update the account data", () => {
59+
expect(client.setAccountData).not.toHaveBeenCalled();
60+
});
61+
});
62+
63+
describe("when adding a new room to an existing DM relation", () => {
64+
beforeEach(() => {
65+
setDMRoom(roomId4, userId1);
66+
});
67+
68+
it("should update the account data accordingly", () => {
69+
expect(client.setAccountData).toHaveBeenCalledWith(EventType.Direct, {
70+
[userId1]: [roomId1, roomId2, roomId4],
71+
[userId2]: [roomId3],
72+
});
73+
});
74+
});
75+
76+
describe("when adding a new DM room", () => {
77+
beforeEach(() => {
78+
setDMRoom(roomId4, userId3);
79+
});
80+
81+
it("should update the account data accordingly", () => {
82+
expect(client.setAccountData).toHaveBeenCalledWith(EventType.Direct, {
83+
[userId1]: [roomId1, roomId2],
84+
[userId2]: [roomId3],
85+
[userId3]: [roomId4],
86+
});
87+
});
88+
});
89+
90+
describe("when trying to add a DM, that already exists", () => {
91+
beforeEach(() => {
92+
setDMRoom(roomId1, userId1);
93+
});
94+
95+
it("should not update the account data", () => {
96+
expect(client.setAccountData).not.toHaveBeenCalled();
97+
});
98+
});
99+
100+
describe("when removing an existing DM", () => {
101+
beforeEach(() => {
102+
setDMRoom(roomId1, null);
103+
});
104+
105+
it("should update the account data accordingly", () => {
106+
expect(client.setAccountData).toHaveBeenCalledWith(EventType.Direct, {
107+
[userId1]: [roomId2],
108+
[userId2]: [roomId3],
109+
});
110+
});
111+
});
112+
113+
describe("when removing an unknown room", () => {
114+
beforeEach(() => {
115+
setDMRoom(roomId4, null);
116+
});
117+
118+
it("should not update the account data", () => {
119+
expect(client.setAccountData).not.toHaveBeenCalled();
120+
});
121+
});
122+
123+
describe("when the direct event is undefined", () => {
124+
beforeEach(() => {
125+
mocked(client.getAccountData).mockReturnValue(undefined);
126+
setDMRoom(roomId1, userId1);
127+
});
128+
129+
it("should update the account data accordingly", () => {
130+
expect(client.setAccountData).toHaveBeenCalledWith(EventType.Direct, {
131+
[userId1]: [roomId1],
132+
});
133+
});
134+
});
135+
136+
describe("when the current content is undefined", () => {
137+
beforeEach(() => {
138+
// @ts-ignore
139+
mocked(client.getAccountData).mockReturnValue({
140+
getContent: jest.fn(),
141+
});
142+
setDMRoom(roomId1, userId1);
143+
});
144+
145+
it("should update the account data accordingly", () => {
146+
expect(client.setAccountData).toHaveBeenCalledWith(EventType.Direct, {
147+
[userId1]: [roomId1],
148+
});
149+
});
150+
});
151+
});

0 commit comments

Comments
 (0)