From 0f909c37db0400e98e6b352227dfb0d7d93d193e Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 5 Sep 2022 14:21:25 +0100 Subject: [PATCH 1/7] Fix handling of remote echoes doubling up --- src/models/event-timeline-set.ts | 14 ++++++-------- src/models/room.ts | 31 +++++++++++++++++-------------- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index 8d11f4a6343..48fe25456ba 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -729,18 +729,16 @@ export class EventTimelineSet extends TypedEventEmitter } } } - - if (event.getUnsigned().transaction_id) { - const existingEvent = this.txnToEvent[event.getUnsigned().transaction_id]; - if (existingEvent) { - // remote echo of an event we sent earlier - this.handleRemoteEcho(event, existingEvent); - } - } } /** @@ -1996,7 +1988,7 @@ export class Room extends TypedEventEmitter * "Room.timeline". * * @param {MatrixEvent} event Event to be added - * @param {IAddLiveEventOptions} options addLiveEvent options + * @param {IAddLiveEventOptions} addLiveEventOptions addLiveEvent options * @fires module:client~MatrixClient#event:"Room.timeline" * @private */ @@ -2383,10 +2375,25 @@ export class Room extends TypedEventEmitter const threadRoots = this.findThreadRoots(events); const eventsByThread: { [threadId: string]: MatrixEvent[] } = {}; + const options: IAddLiveEventOptions = { + duplicateStrategy, + fromCache, + timelineWasEmpty, + }; + for (const event of events) { // TODO: We should have a filter to say "only add state event types X Y Z to the timeline". this.processLiveEvent(event); + if (event.getUnsigned().transaction_id) { + const existingEvent = this.txnToEvent[event.getUnsigned().transaction_id]; + if (existingEvent) { + // remote echo of an event we sent earlier + this.handleRemoteEcho(event, existingEvent); + continue; // we can skip adding the event to the timeline sets, it is already there + } + } + const { shouldLiveInRoom, shouldLiveInThread, @@ -2399,11 +2406,7 @@ export class Room extends TypedEventEmitter eventsByThread[threadId]?.push(event); if (shouldLiveInRoom) { - this.addLiveEvent(event, { - duplicateStrategy, - fromCache, - timelineWasEmpty, - }); + this.addLiveEvent(event, options); } } From c0b61ad90bef380e2914b15cb81ed136fdcc877a Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 5 Sep 2022 14:24:13 +0100 Subject: [PATCH 2/7] Simplify code --- src/models/event-timeline-set.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index 48fe25456ba..a951a399a4a 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -729,13 +729,7 @@ export class EventTimelineSet extends TypedEventEmitter Date: Mon, 5 Sep 2022 14:25:07 +0100 Subject: [PATCH 3/7] Make TSC strict happier --- src/models/room.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/models/room.ts b/src/models/room.ts index fe0dc8bbc10..92af3fe65bd 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -2336,7 +2336,7 @@ export class Room extends TypedEventEmitter fromCache = false, ): void { let duplicateStrategy = duplicateStrategyOrOpts as DuplicateStrategy; - let timelineWasEmpty: boolean; + let timelineWasEmpty = false; if (typeof (duplicateStrategyOrOpts) === 'object') { ({ duplicateStrategy, @@ -2386,7 +2386,7 @@ export class Room extends TypedEventEmitter this.processLiveEvent(event); if (event.getUnsigned().transaction_id) { - const existingEvent = this.txnToEvent[event.getUnsigned().transaction_id]; + const existingEvent = this.txnToEvent[event.getUnsigned().transaction_id!]; if (existingEvent) { // remote echo of an event we sent earlier this.handleRemoteEcho(event, existingEvent); From 7f7ca0cbafa5fe16c09505cbb58e4575433db584 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 5 Sep 2022 14:32:49 +0100 Subject: [PATCH 4/7] Update `timelineWasEmpty` to match type --- spec/unit/room.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/unit/room.spec.ts b/spec/unit/room.spec.ts index 695bc12271f..7f02e840de5 100644 --- a/spec/unit/room.spec.ts +++ b/spec/unit/room.spec.ts @@ -288,11 +288,11 @@ describe("Room", function() { room.addLiveEvents(events); expect(room.currentState.setStateEvents).toHaveBeenCalledWith( [events[0]], - { timelineWasEmpty: undefined }, + { timelineWasEmpty: false }, ); expect(room.currentState.setStateEvents).toHaveBeenCalledWith( [events[1]], - { timelineWasEmpty: undefined }, + { timelineWasEmpty: false }, ); expect(events[0].forwardLooking).toBe(true); expect(events[1].forwardLooking).toBe(true); From 97efa28c9979ef24fba87d4e153af42889833ac9 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 5 Sep 2022 15:23:51 +0100 Subject: [PATCH 5/7] Add tests --- spec/unit/room.spec.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/spec/unit/room.spec.ts b/spec/unit/room.spec.ts index 7f02e840de5..800415d2b8d 100644 --- a/spec/unit/room.spec.ts +++ b/spec/unit/room.spec.ts @@ -426,6 +426,17 @@ describe("Room", function() { // but without the event ID matching we will still have the local event in pending events expect(room.getEventForTxnId(txnId)).toBeUndefined(); }); + + it("should correctly handle remote echoes from other devices", () => { + const remoteEvent = utils.mkMessage({ + room: roomId, user: userA, event: true, + }); + remoteEvent.event.unsigned = { transaction_id: "TXN_ID" }; + + // add the remoteEvent + room.addLiveEvents([remoteEvent]); + expect(room.timeline.length).toEqual(1); + }); }); describe('addEphemeralEvents', () => { From 8aec86961763cbf997dbee2fb5a4a8f6c664a0ba Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 6 Sep 2022 10:52:19 +0100 Subject: [PATCH 6/7] Add tests --- spec/unit/event-timeline-set.spec.ts | 33 +++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/spec/unit/event-timeline-set.spec.ts b/spec/unit/event-timeline-set.spec.ts index 42f4bca4de2..51cf23f6d5e 100644 --- a/spec/unit/event-timeline-set.spec.ts +++ b/spec/unit/event-timeline-set.spec.ts @@ -16,14 +16,15 @@ limitations under the License. import * as utils from "../test-utils/test-utils"; import { + DuplicateStrategy, EventTimeline, EventTimelineSet, EventType, + Filter, MatrixClient, MatrixEvent, MatrixEventEvent, Room, - DuplicateStrategy, } from '../../src'; import { Thread } from "../../src/models/thread"; import { ReEmitter } from "../../src/ReEmitter"; @@ -291,4 +292,34 @@ describe('EventTimelineSet', () => { expect(eventTimelineSet.canContain(event)).toBeTruthy(); }); }); + + describe("handleRemoteEcho", () => { + it("should add to liveTimeline only if the event matches the filter", () => { + const filter = new Filter(client.getUserId(), "test_filter"); + filter.setDefinition({ + room: { + timeline: { + types: [EventType.RoomMessage], + }, + }, + }); + const eventTimelineSet = new EventTimelineSet(room, { filter }, client); + + const roomMessageEvent = new MatrixEvent({ + type: EventType.RoomMessage, + content: { body: "test" }, + event_id: "!test1:server", + }); + eventTimelineSet.handleRemoteEcho(roomMessageEvent, "~!local-event-id:server", roomMessageEvent.getId()); + expect(eventTimelineSet.getLiveTimeline().getEvents()).toContain(roomMessageEvent); + + const roomFilteredEvent = new MatrixEvent({ + type: "other_event_type", + content: { body: "test" }, + event_id: "!test2:server", + }); + eventTimelineSet.handleRemoteEcho(roomFilteredEvent, "~!local-event-id:server", roomFilteredEvent.getId()); + expect(eventTimelineSet.getLiveTimeline().getEvents()).not.toContain(roomFilteredEvent); + }); + }); }); From 9464dd915436d0558aff99753bef3c4e7a509a3a Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 6 Sep 2022 10:55:43 +0100 Subject: [PATCH 7/7] Add lowly `!` --- spec/unit/event-timeline-set.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/unit/event-timeline-set.spec.ts b/spec/unit/event-timeline-set.spec.ts index 51cf23f6d5e..e6c45fbd460 100644 --- a/spec/unit/event-timeline-set.spec.ts +++ b/spec/unit/event-timeline-set.spec.ts @@ -295,7 +295,7 @@ describe('EventTimelineSet', () => { describe("handleRemoteEcho", () => { it("should add to liveTimeline only if the event matches the filter", () => { - const filter = new Filter(client.getUserId(), "test_filter"); + const filter = new Filter(client.getUserId()!, "test_filter"); filter.setDefinition({ room: { timeline: {