Prevent unnecessary m.direct updates (#9805)

* Prevent unnecessary m.direct updates

Signed-off-by: Michael Weimann <michaelw@matrix.org>

* 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 be1e5753677bdf84b3657994dc5ad20d250038bd.

* Revert "Fix rte flaky test (#9811)"

This reverts commit 20d9eb3924c5ebc63f03cb61b172035725e3474d.

* Update src/Rooms.ts

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

Signed-off-by: Michael Weimann <michaelw@matrix.org>
Co-authored-by: Florian Duros <florianduros@element.io>
Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
pull/28788/head^2
Michael Weimann 2022-12-21 17:30:08 +01:00 committed by GitHub
parent dd88fbbc3b
commit 93dd010809
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 167 additions and 11 deletions

View File

@ -57,42 +57,47 @@ export function guessAndSetDMRoom(room: Room, isDirect: boolean): Promise<void>
/**
* Marks or unmarks the given room as being as a DM room.
* @param {string} roomId The ID of the room to modify
* @param {string} userId The user ID of the desired DM
room target user or null to un-mark
this room as a DM room
* @param {string | null} userId The user ID of the desired DM room target user or
* null to un-mark this room as a DM room
* @returns {object} A promise
*/
export async function setDMRoom(roomId: string, userId: string): Promise<void> {
export async function setDMRoom(roomId: string, userId: string | null): Promise<void> {
if (MatrixClientPeg.get().isGuest()) return;
const mDirectEvent = MatrixClientPeg.get().getAccountData(EventType.Direct);
let dmRoomMap = {};
const currentContent = mDirectEvent?.getContent() || {};
if (mDirectEvent !== undefined) dmRoomMap = { ...mDirectEvent.getContent() }; // copy as we will mutate
const dmRoomMap = new Map(Object.entries(currentContent));
let modified = false;
// remove it from the lists of any others users
// (it can only be a DM room for one person)
for (const thisUserId of Object.keys(dmRoomMap)) {
const roomList = dmRoomMap[thisUserId];
for (const thisUserId of dmRoomMap.keys()) {
const roomList = dmRoomMap.get(thisUserId) || [];
if (thisUserId != userId) {
const indexOfRoom = roomList.indexOf(roomId);
if (indexOfRoom > -1) {
roomList.splice(indexOfRoom, 1);
modified = true;
}
}
}
// now add it, if it's not already there
if (userId) {
const roomList = dmRoomMap[userId] || [];
const roomList = dmRoomMap.get(userId) || [];
if (roomList.indexOf(roomId) == -1) {
roomList.push(roomId);
modified = true;
}
dmRoomMap[userId] = roomList;
dmRoomMap.set(userId, roomList);
}
await MatrixClientPeg.get().setAccountData(EventType.Direct, dmRoomMap);
// prevent unnecessary calls to setAccountData
if (!modified) return;
await MatrixClientPeg.get().setAccountData(EventType.Direct, Object.fromEntries(dmRoomMap));
}
/**

151
test/Rooms-test.ts Normal file
View File

@ -0,0 +1,151 @@
/*
Copyright 2022 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 { mocked } from "jest-mock";
import { EventType, MatrixClient, MatrixEvent } from "matrix-js-sdk/src/matrix";
import { setDMRoom } from "../src/Rooms";
import { mkEvent, stubClient } from "./test-utils";
describe("setDMRoom", () => {
const userId1 = "@user1:example.com";
const userId2 = "@user2:example.com";
const userId3 = "@user3:example.com";
const roomId1 = "!room1:example.com";
const roomId2 = "!room2:example.com";
const roomId3 = "!room3:example.com";
const roomId4 = "!room4:example.com";
let client: MatrixClient;
beforeEach(() => {
client = mocked(stubClient());
client.getAccountData = jest.fn().mockImplementation((eventType: string): MatrixEvent | undefined => {
if (eventType === EventType.Direct) {
return mkEvent({
event: true,
content: {
[userId1]: [roomId1, roomId2],
[userId2]: [roomId3],
},
type: EventType.Direct,
user: client.getSafeUserId(),
});
}
return undefined;
});
});
describe("when logged in as a guest and marking a room as DM", () => {
beforeEach(() => {
mocked(client.isGuest).mockReturnValue(true);
setDMRoom(roomId1, userId1);
});
it("should not update the account data", () => {
expect(client.setAccountData).not.toHaveBeenCalled();
});
});
describe("when adding a new room to an existing DM relation", () => {
beforeEach(() => {
setDMRoom(roomId4, userId1);
});
it("should update the account data accordingly", () => {
expect(client.setAccountData).toHaveBeenCalledWith(EventType.Direct, {
[userId1]: [roomId1, roomId2, roomId4],
[userId2]: [roomId3],
});
});
});
describe("when adding a new DM room", () => {
beforeEach(() => {
setDMRoom(roomId4, userId3);
});
it("should update the account data accordingly", () => {
expect(client.setAccountData).toHaveBeenCalledWith(EventType.Direct, {
[userId1]: [roomId1, roomId2],
[userId2]: [roomId3],
[userId3]: [roomId4],
});
});
});
describe("when trying to add a DM, that already exists", () => {
beforeEach(() => {
setDMRoom(roomId1, userId1);
});
it("should not update the account data", () => {
expect(client.setAccountData).not.toHaveBeenCalled();
});
});
describe("when removing an existing DM", () => {
beforeEach(() => {
setDMRoom(roomId1, null);
});
it("should update the account data accordingly", () => {
expect(client.setAccountData).toHaveBeenCalledWith(EventType.Direct, {
[userId1]: [roomId2],
[userId2]: [roomId3],
});
});
});
describe("when removing an unknown room", () => {
beforeEach(() => {
setDMRoom(roomId4, null);
});
it("should not update the account data", () => {
expect(client.setAccountData).not.toHaveBeenCalled();
});
});
describe("when the direct event is undefined", () => {
beforeEach(() => {
mocked(client.getAccountData).mockReturnValue(undefined);
setDMRoom(roomId1, userId1);
});
it("should update the account data accordingly", () => {
expect(client.setAccountData).toHaveBeenCalledWith(EventType.Direct, {
[userId1]: [roomId1],
});
});
});
describe("when the current content is undefined", () => {
beforeEach(() => {
// @ts-ignore
mocked(client.getAccountData).mockReturnValue({
getContent: jest.fn(),
});
setDMRoom(roomId1, userId1);
});
it("should update the account data accordingly", () => {
expect(client.setAccountData).toHaveBeenCalledWith(EventType.Direct, {
[userId1]: [roomId1],
});
});
});
});