review

 - fix types (`msc4354_sticky:number` -> `msc4354_sticky?: { duration_ms: number };`)
  - add `MultiKeyMap`

Signed-off-by: Timo K <[email protected]>

Author: toger5

src/models/event.ts

@@ -96,7 +96,7 @@ export interface IEvent {
     membership?: Membership;
     unsigned: IUnsigned;
     redacts?: string;
-    msc4354_sticky?: number;
+    msc4354_sticky?: { duration_ms: number };
 }
 
 export interface IAggregatedRelation {

src/models/room.ts

@@ -25,7 +25,7 @@ import {
 import { Direction, EventTimeline } from "./event-timeline.ts";
 import { getHttpUriForMxc } from "../content-repo.ts";
 import * as utils from "../utils.ts";
-import { normalize, noUnsafeEventProps, removeElement } from "../utils.ts";
+import { normalize, noUnsafeEventProps, removeElement, MultiKeyMap } from "../utils.ts";
 import {
     type IEvent,
     type IThreadBundledRelationship,
@@ -312,7 +312,19 @@ export type RoomEventHandlerMap = {
      * @param summary - the room summary object
      */
     [RoomEvent.Summary]: (summary: IRoomSummary) => void;
-    [RoomEvent.StickyEvents]: (stickyEventMap: Map<string, MatrixEvent>, room: Room) => void;
+    /**
+     * Fires when sticky events are updated for a room.
+     * For a list of all updated events use:
+     * `const updated = added.filter(e => removed.includes(e));`
+     * for a list of all new events use:
+     * `const addedNew = added.filter(e => !removed.includes(e));`
+     * for a list of all removed events use:
+     * `const removedOnly = removed.filter(e => !added.includes(e));`
+     * @param added - The events that were added to the map of sticky events (can be updated events for existing keys or new keys)
+     * @param removed - The events that were removed from the map of sticky events (caused by expiration or updated keys)
+     * @param room - The room containing the sticky events
+     */
+    [RoomEvent.StickyEvents]: (added: MatrixEvent[], removed: MatrixEvent[], room: Room) => void;
     [ThreadEvent.New]: (thread: Thread, toStartOfTimeline: boolean) => void;
     /**
      * Fires when a new poll instance is added to the room state
@@ -378,7 +390,8 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
     private getTypeWarning = false;
     private membersPromise?: Promise<boolean>;
 
-    private stickyEventsMap = new Map<string, MatrixEvent>();
+    private stickyEventsMap = new MultiKeyMap<MatrixEvent & { event: { msc4354_sticky: { duration_ms: number } } }>();
+
     // XXX: These should be read-only
     /**
      * The human-readable display name for this room.
@@ -3417,24 +3430,56 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
         return this.accountData.get(type);
     }
 
-    public getStickyEventsMap(): Map<string, MatrixEvent> {
-        return this.stickyEventsMap;
+    public getStickyEvents(): MapIterator<MatrixEvent> {
+        return this.stickyEventsMap.values();
     }
 
     public addStickyEvents(event: MatrixEvent[]): void {
+        const added = [];
+        const removed = [];
         for (const e of event) {
-            const stickyKey = Room.stickyKey(e);
-            if (!stickyKey) {
+            const mapStickyKey = Room.stickyKey(e);
+            if (!mapStickyKey) {
                 logger.warn("Event from sticky sync section is missing sticky_key, ignoring", e);
                 continue;
             }
-            if ("sticky" in e.event) {
+            // With this we have the guarantee, that all events in stickyEventsMap are correctly formatted
+            if (
+                !(
+                    e.event.msc4354_sticky &&
+                    "duration_ms" in e.event.msc4354_sticky &&
+                    e.event.msc4354_sticky.duration_ms !== undefined &&
+                    typeof e.event.msc4354_sticky.duration_ms === "number"
+                )
+            ) {
                 logger.warn("Event from sticky sync section is missing sticky timeout", e);
                 continue;
             }
-            this.stickyEventsMap.set(stickyKey, e);
-            this.emit(RoomEvent.StickyEvents, this.stickyEventsMap, this);
+            const ev = e as MatrixEvent & { event: { msc4354_sticky: { duration_ms: number } } };
+            const prevEv = this.stickyEventsMap.get(mapStickyKey);
+            const sender = e.getSender();
+            if (!sender) {
+                logger.warn("Event from sticky sync section is missing sender", e);
+                continue;
+            } else if (prevEv && ev.getTs() < prevEv.getTs()) {
+                logger.info(
+                    "ignored sticky event with older timestamp for on sticky_key",
+                    ev.getContent().sticky_key,
+                    ev,
+                );
+            } else if (prevEv && ev.getTs() === prevEv.getTs() && (ev.getId() ?? "") < (prevEv.getId() ?? "")) {
+                logger.info(
+                    "ignored sticky event due to 'id tie break rule' on sticky_key",
+                    ev.getContent().sticky_key,
+                    ev,
+                );
+            } else {
+                added.push(ev);
+                if (prevEv) removed.push(prevEv);
+                this.stickyEventsMap.set(mapStickyKey, ev);
+            }
         }
+        if (added.length) this.emit(RoomEvent.StickyEvents, added, removed, this);
         this.checkStickyTimer();
     }
     private stickyEventTimer?: NodeJS.Timeout;
@@ -3454,7 +3499,7 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
     private getNextStickyExpiryTs(): number | null {
         let nextExpiry = null;
         for (const event of this.stickyEventsMap.values()) {
-            const expiry = event.getTs() + event.event.msc4354_sticky!;
+            const expiry = event.getTs() + event.event.msc4354_sticky.duration_ms;
             if (nextExpiry === null || expiry < nextExpiry) {
                 nextExpiry = expiry;
             }
@@ -3463,29 +3508,31 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
     }
     private cleanExpiredStickyEvents(): void {
         const now = Date.now();
-        const toRemove: string[] = [];
+        const toRemove: string[][] = [];
+        const removedEvents: MatrixEvent[] = [];
         for (const [key, event] of this.stickyEventsMap.entries()) {
             const creationTs = event.getTs();
             // we only added items with `sticky` into this map so we can assert non-null here
-            if (now > creationTs + event.event.msc4354_sticky!) {
+            if (now > creationTs + event.event.msc4354_sticky.duration_ms) {
                 toRemove.push(key);
+                removedEvents.push(event);
             }
         }
         for (const key of toRemove) {
             this.stickyEventsMap.delete(key);
         }
-        if (toRemove.length) {
-            this.emit(RoomEvent.StickyEvents, this.stickyEventsMap, this);
+        if (removedEvents.length) {
+            this.emit(RoomEvent.StickyEvents, [], removedEvents, this);
         }
         this.checkStickyTimer();
     }
 
-    private static stickyKey(event: MatrixEvent): string | undefined {
-        const k = event.getContent().sticky_key;
-        if (!k) {
+    private static stickyKey(event: MatrixEvent): string[] | undefined {
+        const stickyKey = event.getContent().sticky_key;
+        if (!stickyKey) {
             return undefined;
         }
-        return event.getSender() + event.getType() + k;
+        return [event.getSender(), event.getType(), stickyKey];
     }
 
     /**

src/sync.ts

@@ -1422,8 +1422,8 @@ export class SyncApi {
             this.processEventsForNotifs(room, timelineEvents);
 
             const emitEvent = (e: MatrixEvent): boolean => client.emit(ClientEvent.Event, e);
-            // this fires a couple of times for some events. (eg state events are in the timeline and the state or sticky events)
-            // should this get a the sync section as an additional event emission param (e, syncSection))?
+            // this fires a couple of times for some events. (eg state events are in the timeline and the state)
+            // should this get a sync section as an additional event emission param (e, syncSection))?
             stateEvents.forEach(emitEvent);
             timelineEvents.forEach(emitEvent);
             ephemeralEvents.forEach(emitEvent);

src/utils.ts

@@ -765,3 +765,24 @@ export class MapWithDefault<K, V> extends Map<K, V> {
         return this.get(key)!;
     }
 }
+
+export class MultiKeyMap<V> {
+    private map = new Map<string, V>();
+    public get(key: Array<string>): V | undefined {
+        return this.map.get(JSON.stringify(key));
+    }
+    public set(key: Array<string>, value: V): void {
+        this.map.set(JSON.stringify(key), value);
+    }
+    public values(): MapIterator<V> {
+        return this.map.values();
+    }
+    public entries(): MapIterator<[string[], V]> {
+        return Array.from(this.map.entries())
+            .map<[string[], V]>(([k, v]) => [JSON.parse(k) as string[], v])
+            .values();
+    }
+    public delete(key: Array<string>): boolean {
+        return this.map.delete(JSON.stringify(key));
+    }
+}