From 0f0e8f9038af965e372f769d979b18c43c4636fd Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 3 Nov 2022 19:02:36 -0500 Subject: [PATCH 1/9] Extra insurance that we don't mix events in the wrong timelines Split out from https://github.com/matrix-org/matrix-js-sdk/pull/2521 --- .DS_Store | Bin 0 -> 6148 bytes spec/unit/event-timeline-set.spec.ts | 59 +++++++++++++++++++++++++++ src/models/event-timeline-set.ts | 13 ++++++ 3 files changed, 72 insertions(+) create mode 100644 .DS_Store diff --git a/.DS_Store b/.DS_Store new file mode 100644 index 0000000000000000000000000000000000000000..8c6875c741d7013ebf68f1a62758ebca25db1fed GIT binary patch literal 6148 zcmeHK%}T>S5Z<*_6N-?7LXQhx3%1l&ikDF93mDOZN=;1BV9b^zHHT8jSzpK}@p+ut z-GIfMMeGdhe)GGV{UH0p7~|tb*kjCLj9JhSIVv@R?%L3nNk-&2Mm7&(8G!W>%uVdC z1AcphWh`Y6LGk_j<0#9!-A}&NXm0Q9T9(za?z|^ic)6c1vaz4upmix_98|g=Tt%~D z>g=D%B=@6ama2j%oI%R%b(Dm%7|TTxW~$cH0n4`SsncFAhbR4x=nV&}j#!RHqmCH# zk5?<(IygK!y_i17FNu89baG%@$)3Rq-a#>|dG*pHmdPV{s_ZI@kQg8ahyh|?vl%ew zg4Nn=8ff*z05MR*0PYV08lr2k)Tp)&=7p`UpzfkFnyBet{28e-m1{&IQ@cciAU#9kvzg|KX zF+dFbGX{8b;!Vb|D08;{SRS6W0@^(^6wE780ResO5&#D7BW)Gbae+GIxduy(I12hz QIUrpG6d}|R1HZt)7X)QVng9R* literal 0 HcmV?d00001 diff --git a/spec/unit/event-timeline-set.spec.ts b/spec/unit/event-timeline-set.spec.ts index ce8fbf32dc4..28502515654 100644 --- a/spec/unit/event-timeline-set.spec.ts +++ b/spec/unit/event-timeline-set.spec.ts @@ -55,6 +55,23 @@ describe('EventTimelineSet', () => { }); }; + const mkThreadResponse = (root: MatrixEvent) => utils.mkEvent({ + event: true, + type: EventType.RoomMessage, + user: userA, + room: roomId, + content: { + "body": "Thread response :: " + Math.random(), + "m.relates_to": { + "event_id": root.getId(), + "m.in_reply_to": { + "event_id": root.getId()!, + }, + "rel_type": "m.thread", + }, + }, + }, room.client); + beforeEach(() => { client = utils.mock(MatrixClient, 'MatrixClient'); client.reEmitter = utils.mock(ReEmitter, 'ReEmitter'); @@ -117,6 +134,13 @@ describe('EventTimelineSet', () => { }); describe('addEventToTimeline', () => { + let thread: Thread; + + beforeEach(() => { + (client.supportsExperimentalThreads as jest.Mock).mockReturnValue(true); + thread = new Thread("!thread_id:server", messageEvent, { room, client }); + }); + it("Adds event to timeline", () => { const liveTimeline = eventTimelineSet.getLiveTimeline(); expect(liveTimeline.getEvents().length).toStrictEqual(0); @@ -144,6 +168,41 @@ describe('EventTimelineSet', () => { ); }).not.toThrow(); }); + + it("should not add an event to a timeline that does not belong to the timelineSet", () => { + const eventTimelineSet2 = new EventTimelineSet(room); + const liveTimeline2 = eventTimelineSet2.getLiveTimeline(); + expect(liveTimeline2.getEvents().length).toStrictEqual(0); + + expect(() => { + eventTimelineSet.addEventToTimeline(messageEvent, liveTimeline2, { + toStartOfTimeline: true, + }); + }).toThrowError(); + }); + + it("should not add a threaded reply to the main room timeline", () => { + const liveTimeline = eventTimelineSet.getLiveTimeline(); + expect(liveTimeline.getEvents().length).toStrictEqual(0); + + const threadedReplyEvent = mkThreadResponse(messageEvent); + + eventTimelineSet.addEventToTimeline(threadedReplyEvent, liveTimeline, { + toStartOfTimeline: true, + }); + expect(liveTimeline.getEvents().length).toStrictEqual(0); + }); + + it("should not add a normal message to the timelineSet representing a thread", () => { + const eventTimelineSetForThread = new EventTimelineSet(room, {}, client, thread); + const liveTimeline = eventTimelineSetForThread.getLiveTimeline(); + expect(liveTimeline.getEvents().length).toStrictEqual(0); + + eventTimelineSetForThread.addEventToTimeline(messageEvent, liveTimeline, { + toStartOfTimeline: true, + }); + expect(liveTimeline.getEvents().length).toStrictEqual(0); + }); }); describe('aggregateRelations', () => { diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index e41eec15881..fd9dc1ff833 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -702,6 +702,19 @@ export class EventTimelineSet extends TypedEventEmitter Date: Thu, 3 Nov 2022 19:18:50 -0500 Subject: [PATCH 2/9] Fix null client error in tests even though it should not be null according to the signature --- src/models/room.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/models/room.ts b/src/models/room.ts index 3bfa308c5c0..0079aba9da9 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -1862,7 +1862,7 @@ export class Room extends ReadReceipt { shouldLiveInThread: boolean; threadId?: string; } { - if (!this.client.supportsExperimentalThreads()) { + if (!this.client?.supportsExperimentalThreads()) { return { shouldLiveInRoom: true, shouldLiveInThread: false, From b91b03b605185b3fd33ba8e613666fc5455d2842 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 3 Nov 2022 19:25:50 -0500 Subject: [PATCH 3/9] Content is not required --- spec/unit/event-timeline-set.spec.ts | 2 +- src/models/event.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/unit/event-timeline-set.spec.ts b/spec/unit/event-timeline-set.spec.ts index 28502515654..d4b0f621382 100644 --- a/spec/unit/event-timeline-set.spec.ts +++ b/spec/unit/event-timeline-set.spec.ts @@ -65,7 +65,7 @@ describe('EventTimelineSet', () => { "m.relates_to": { "event_id": root.getId(), "m.in_reply_to": { - "event_id": root.getId()!, + "event_id": root.getId(), }, "rel_type": "m.thread", }, diff --git a/src/models/event.ts b/src/models/event.ts index 6ba8a65110e..21c5c4cc2b9 100644 --- a/src/models/event.ts +++ b/src/models/event.ts @@ -107,7 +107,7 @@ export interface IEventRelation { event_id?: string; is_falling_back?: boolean; "m.in_reply_to"?: { - event_id: string; + event_id?: string; }; key?: string; } From 7fd36a47ccbc65bc8898dffa49765a1686d71245 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 4 Nov 2022 13:20:59 -0500 Subject: [PATCH 4/9] Make sure to remove macOS garbage --- .DS_Store | Bin 6148 -> 0 bytes 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 .DS_Store diff --git a/.DS_Store b/.DS_Store deleted file mode 100644 index 8c6875c741d7013ebf68f1a62758ebca25db1fed..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 6148 zcmeHK%}T>S5Z<*_6N-?7LXQhx3%1l&ikDF93mDOZN=;1BV9b^zHHT8jSzpK}@p+ut z-GIfMMeGdhe)GGV{UH0p7~|tb*kjCLj9JhSIVv@R?%L3nNk-&2Mm7&(8G!W>%uVdC z1AcphWh`Y6LGk_j<0#9!-A}&NXm0Q9T9(za?z|^ic)6c1vaz4upmix_98|g=Tt%~D z>g=D%B=@6ama2j%oI%R%b(Dm%7|TTxW~$cH0n4`SsncFAhbR4x=nV&}j#!RHqmCH# zk5?<(IygK!y_i17FNu89baG%@$)3Rq-a#>|dG*pHmdPV{s_ZI@kQg8ahyh|?vl%ew zg4Nn=8ff*z05MR*0PYV08lr2k)Tp)&=7p`UpzfkFnyBet{28e-m1{&IQ@cciAU#9kvzg|KX zF+dFbGX{8b;!Vb|D08;{SRS6W0@^(^6wE780ResO5&#D7BW)Gbae+GIxduy(I12hz QIUrpG6d}|R1HZt)7X)QVng9R* From 52d4280081c24ce3c240438751f784f890f5498f Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 4 Nov 2022 15:38:22 -0500 Subject: [PATCH 5/9] Only run canContain for room timelines Fix test failure in `sliding-sync/sliding-sync.ts` -> `"should show the right unread notifications"` ``` Cannot call `EventTimelineSet::canContain without a `room` set. Set the room when creating the EventTimelineSet to call this method. at EventTimelineSet.canContain (VM4503 200:726:13) at EventTimelineSet.addEventToTimeline (VM4503 200:576:15) at EventTimelineSet.addLiveEvent (VM4503 200:527:10) at eval (VM4587 281:722:145) at Array.forEach () at SlidingSyncSdk.purgeNotifications (VM4587 281:720:22) at SlidingSyncSdk.onLifecycle (VM4587 281:263:14) at SlidingSync.emit (VM4407 21:153:5) at SlidingSync.emit (VM4472 169:45:18) at SlidingSync.invokeLifecycleListeners (VM4588 282:378:10) ``` --- src/models/event-timeline-set.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index fd9dc1ff833..5caba0bb5ff 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -707,9 +707,12 @@ export class EventTimelineSet extends TypedEventEmitter Date: Fri, 4 Nov 2022 16:31:39 -0500 Subject: [PATCH 6/9] More info in error --- src/models/event-timeline-set.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index 5caba0bb5ff..f403e7b16a1 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -713,8 +713,11 @@ export class EventTimelineSet extends TypedEventEmitter Date: Fri, 4 Nov 2022 17:16:35 -0500 Subject: [PATCH 7/9] Add test that would fail in the JS sdk --- spec/unit/event-timeline-set.spec.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/spec/unit/event-timeline-set.spec.ts b/spec/unit/event-timeline-set.spec.ts index d4b0f621382..21d2c6ef080 100644 --- a/spec/unit/event-timeline-set.spec.ts +++ b/spec/unit/event-timeline-set.spec.ts @@ -203,6 +203,23 @@ describe('EventTimelineSet', () => { }); expect(liveTimeline.getEvents().length).toStrictEqual(0); }); + + describe('non-room timeline', () => { + fit('Adds event to timeline', () => { + const nonRoomEventTimelineSet = new EventTimelineSet( + // This is what we're specifically testing against, a timeline + // without a `room` defined + undefined, + ); + const nonRoomEventTimeline = new EventTimeline(nonRoomEventTimelineSet); + + expect(nonRoomEventTimeline.getEvents().length).toStrictEqual(0); + nonRoomEventTimelineSet.addEventToTimeline(messageEvent, nonRoomEventTimeline, { + toStartOfTimeline: true, + }); + expect(nonRoomEventTimeline.getEvents().length).toStrictEqual(1); + }); + }); }); describe('aggregateRelations', () => { From f2f37e6b65d19f688df0214af30e29d3808cb26a Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 4 Nov 2022 17:29:53 -0500 Subject: [PATCH 8/9] SonarQube: Refactor this code to not use nested template literals. See https://sonarcloud.io/project/issues?resolved=false&sinceLeakPeriod=true&types=CODE_SMELL&pullRequest=2856&id=matrix-js-sdk&open=AYRElaAdNY5nTGOdJo3a --- src/models/event-timeline-set.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index f403e7b16a1..addce6e46ae 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -713,9 +713,12 @@ export class EventTimelineSet extends TypedEventEmitter Date: Mon, 7 Nov 2022 11:08:02 -0600 Subject: [PATCH 9/9] Remove fit --- 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 21d2c6ef080..2bc068af660 100644 --- a/spec/unit/event-timeline-set.spec.ts +++ b/spec/unit/event-timeline-set.spec.ts @@ -205,7 +205,7 @@ describe('EventTimelineSet', () => { }); describe('non-room timeline', () => { - fit('Adds event to timeline', () => { + it('Adds event to timeline', () => { const nonRoomEventTimelineSet = new EventTimelineSet( // This is what we're specifically testing against, a timeline // without a `room` defined