matrix/matrix-js-sdk
1
0
Fork 0
mirror of https://github.com/matrix-org/matrix-js-sdk.git synced 2025-08-09 10:22:46 +03:00
Code Activity

Timeline needs to refresh when we see a MSC2716 marker event (#2299)

Browse Source 
Inform the client that historical messages were imported in the timeline and they should refresh the timeline in order to see the new events.

Companion `matrix-react-sdk` PR: https://github.com/matrix-org/matrix-react-sdk/pull/8354

The `marker` events are being used as state now because this way they can't be lost in a timeline gap. Regardless of when they were sent, we will still have the latest version of the state to compare against. Any time we see our latest state value change for marker events, prompt the user that the timeline needs to refresh.

> In a [sync meeting with @ara4n](https://docs.google.com/document/d/1KCEmpnGr4J-I8EeaVQ8QJZKBDu53ViI7V62y5BzfXr0/edit#bookmark=id.67nio1ka8znc), we came up with the idea to make the `marker` events as state events. When the client sees that the `m.room.marker` state changed to a different event ID, it can throw away all of the timeline and re-fetch as needed.
>
> For homeservers where the [same problem](https://github.com/matrix-org/matrix-doc/pull/2716#discussion_r782499674) can happen, we probably don't want to throw away the whole timeline but it can go up the `unsigned.replaces_state` chain of the `m.room.marker` state events to get them all.
>
> In terms of state performance, there could be thousands of `marker` events in a room but it's no different than room members joining and leaving over and over like an IRC room.
>
> *-- https://github.com/matrix-org/matrix-spec-proposals/pull/2716#discussion_r782629097*


### Why are we just setting `timlineNeedsRefresh` (and [prompting the user](https://github.com/matrix-org/matrix-react-sdk/pull/8354)) instead of automatically refreshing the timeline for the user?

If we refreshed the timeline automatically, someone could cause your Element client to constantly refresh the timeline by just sending marker events over and over. Granted, you probably want to leave a room like this 🤷. Perhaps also some sort of DOS vector since everyone will be refreshing and hitting the server at the exact same time.

In order to avoid the timeline maybe going blank during the refresh, we could re-fetch the new events first, then replace the timeline. But the points above still stand on why we shouldn't.
This commit is contained in:
Eric Eastwood
2022-06-01 16:31:20 -05:00
committed by GitHub
parent 2e27a4134c
commit b64dbdce74
16 changed files with 1635 additions and 125 deletions
Download Patch File Download Diff File Expand all files Collapse all files

88
spec/unit/room.spec.ts
View File

@@ -133,6 +133,27 @@ describe("Room", function() {
room.currentState = room.getLiveTimeline().endState = utils.mock(RoomState, "currentState");
});
describe('getCreator', () => {
it("should return the creator from m.room.create", function() {
room.currentState.getStateEvents.mockImplementation(function(type, key) {
if (type === EventType.RoomCreate && key === "") {
return utils.mkEvent({
event: true,
type: EventType.RoomCreate,
skey: "",
room: roomId,
user: userA,
content: {
creator: userA,
},
});
}
});
const roomCreator = room.getCreator();
expect(roomCreator).toStrictEqual(userA);
});
});
describe("getAvatarUrl", function() {
const hsUrl = "https://my.home.server";
@@ -196,22 +217,17 @@ describe("Room", function() {
}) as MatrixEvent,
];
it("should call RoomState.setTypingEvent on m.typing events", function() {
const typing = utils.mkEvent({
room: roomId,
type: EventType.Typing,
event: true,
content: {
user_ids: [userA],
},
});
room.addEphemeralEvents([typing]);
expect(room.currentState.setTypingEvent).toHaveBeenCalledWith(typing);
it("Make sure legacy overload passing options directly as parameters still works", () => {
expect(() => room.addLiveEvents(events, DuplicateStrategy.Replace, false)).not.toThrow();
expect(() => room.addLiveEvents(events, DuplicateStrategy.Ignore, true)).not.toThrow();
expect(() => room.addLiveEvents(events, "shouldfailbecauseinvalidduplicatestrategy", false)).toThrow();
});
it("should throw if duplicateStrategy isn't 'replace' or 'ignore'", function() {
expect(function() {
room.addLiveEvents(events, "foo");
room.addLiveEvents(events, {
duplicateStrategy: "foo",
});
}).toThrow();
});
@@ -223,7 +239,9 @@ describe("Room", function() {
dupe.event.event_id = events[0].getId();
room.addLiveEvents(events);
expect(room.timeline[0]).toEqual(events[0]);
room.addLiveEvents([dupe], DuplicateStrategy.Replace);
room.addLiveEvents([dupe], {
duplicateStrategy: DuplicateStrategy.Replace,
});
expect(room.timeline[0]).toEqual(dupe);
});
@@ -235,7 +253,9 @@ describe("Room", function() {
dupe.event.event_id = events[0].getId();
room.addLiveEvents(events);
expect(room.timeline[0]).toEqual(events[0]);
room.addLiveEvents([dupe], "ignore");
room.addLiveEvents([dupe], {
duplicateStrategy: "ignore",
});
expect(room.timeline[0]).toEqual(events[0]);
});
@@ -268,9 +288,11 @@ describe("Room", function() {
room.addLiveEvents(events);
expect(room.currentState.setStateEvents).toHaveBeenCalledWith(
[events[0]],
{ timelineWasEmpty: undefined },
);
expect(room.currentState.setStateEvents).toHaveBeenCalledWith(
[events[1]],
{ timelineWasEmpty: undefined },
);
expect(events[0].forwardLooking).toBe(true);
expect(events[1].forwardLooking).toBe(true);
@@ -341,6 +363,21 @@ describe("Room", function() {
});
});
describe('addEphemeralEvents', () => {
it("should call RoomState.setTypingEvent on m.typing events", function() {
const typing = utils.mkEvent({
room: roomId,
type: EventType.Typing,
event: true,
content: {
user_ids: [userA],
},
});
room.addEphemeralEvents([typing]);
expect(room.currentState.setTypingEvent).toHaveBeenCalledWith(typing);
});
});
describe("addEventsToTimeline", function() {
const events = [
utils.mkMessage({
@@ -472,9 +509,11 @@ describe("Room", function() {
room.addEventsToTimeline(events, true, room.getLiveTimeline());
expect(room.oldState.setStateEvents).toHaveBeenCalledWith(
[events[0]],
{ timelineWasEmpty: undefined },
);
expect(room.oldState.setStateEvents).toHaveBeenCalledWith(
[events[1]],
{ timelineWasEmpty: undefined },
);
expect(events[0].forwardLooking).toBe(false);
expect(events[1].forwardLooking).toBe(false);
@@ -520,6 +559,23 @@ describe("Room", function() {
it("should reset the legacy timeline fields", function() {
room.addLiveEvents([events[0], events[1]]);
expect(room.timeline.length).toEqual(2);
const oldStateBeforeRunningReset = room.oldState;
let oldStateUpdateEmitCount = 0;
room.on(RoomEvent.OldStateUpdated, function(room, previousOldState, oldState) {
expect(previousOldState).toBe(oldStateBeforeRunningReset);
expect(oldState).toBe(room.oldState);
oldStateUpdateEmitCount += 1;
});
const currentStateBeforeRunningReset = room.currentState;
let currentStateUpdateEmitCount = 0;
room.on(RoomEvent.CurrentStateUpdated, function(room, previousCurrentState, currentState) {
expect(previousCurrentState).toBe(currentStateBeforeRunningReset);
expect(currentState).toBe(room.currentState);
currentStateUpdateEmitCount += 1;
});
room.resetLiveTimeline('sometoken', 'someothertoken');
room.addLiveEvents([events[2]]);
@@ -529,6 +585,10 @@ describe("Room", function() {
newLiveTimeline.getState(EventTimeline.BACKWARDS));
expect(room.currentState).toEqual(
newLiveTimeline.getState(EventTimeline.FORWARDS));
// Make sure `RoomEvent.OldStateUpdated` was emitted
expect(oldStateUpdateEmitCount).toEqual(1);
// Make sure `RoomEvent.OldStateUpdated` was emitted if necessary
expect(currentStateUpdateEmitCount).toEqual(timelineSupport ? 1 : 0);
});
it("should emit Room.timelineReset event and set the correct " +