Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft: Refresh room timeline when we see a MSC2716 marker event #2282

Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
2b029c9
Refresh room timeline when we see a MSC2716 marker event
MadLittleMods Apr 8, 2022
ffbcbff
Don't refresh timeline when first sync or from cache
MadLittleMods Apr 8, 2022
39cd6e9
Fix docstring typo
MadLittleMods Apr 8, 2022
e5f34db
Only notify client that timeline should be refreshed instead of doing…
MadLittleMods Apr 13, 2022
e070b3c
Remove console.log
MadLittleMods Apr 13, 2022
21f6aa2
Fixup docstrings
MadLittleMods Apr 13, 2022
9bfcf4b
Fix some lints
MadLittleMods Apr 13, 2022
1c2b955
Merge branch 'develop' into madlittlemods/refresh-timeline-when-we-se…
MadLittleMods Apr 13, 2022
05b9800
Fix some lints
MadLittleMods Apr 13, 2022
8b82bb5
Rename to roomState to align with other options objects
MadLittleMods Apr 13, 2022
35f3f04
Rename to toStartOfTimeline to align with other options objects
MadLittleMods Apr 13, 2022
b2636c3
Revert arg rename change for unused
MadLittleMods Apr 13, 2022
da57a74
Fix up tests
MadLittleMods Apr 13, 2022
c539b64
Fix timeline-window specs
MadLittleMods Apr 13, 2022
11040e5
Stop prompting to refresh timeline when syncing from cache
MadLittleMods Apr 13, 2022
07fcf27
Fix lints and move to logger
MadLittleMods Apr 13, 2022
701bf34
Remove todo
MadLittleMods Apr 13, 2022
c080c6c
Remove fixme in favor of aspirational explanation of what we could do…
MadLittleMods Apr 13, 2022
0694b84
Add some explanation comments
MadLittleMods Apr 13, 2022
26fb2b6
Add some tests
MadLittleMods Apr 13, 2022
8ca2645
WIP: messy timelineWasEmpty
MadLittleMods Apr 14, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/@types/event.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ export enum EventType {
RoomKeyRequest = "m.room_key_request",
ForwardedRoomKey = "m.forwarded_room_key",
Dummy = "m.dummy",

Marker = "org.matrix.msc2716.marker", // MSC2716
}

export enum RelationType {
Expand Down
2 changes: 2 additions & 0 deletions src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -790,6 +790,7 @@ type RoomEvents = RoomEvent.Name
| RoomEvent.Receipt
| RoomEvent.Tags
| RoomEvent.LocalEchoUpdated
| RoomEvent.historyImportedWithinTimeline
| RoomEvent.AccountData
| RoomEvent.MyMembership
| RoomEvent.Timeline
Expand All @@ -799,6 +800,7 @@ type RoomStateEvents = RoomStateEvent.Events
| RoomStateEvent.Members
| RoomStateEvent.NewMember
| RoomStateEvent.Update
| RoomStateEvent.Marker
;

type CryptoEvents = CryptoEvent.KeySignatureUploadFailure
Expand Down
82 changes: 66 additions & 16 deletions src/models/event-timeline-set.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,38 @@ export interface IRoomTimelineData {
liveEvent?: boolean;
}

export interface IAddLiveEventOptions {
/** Applies to events in the timeline only. If this is 'replace' then if a
* duplicate is encountered, the event passed to this function will replace
* the existing event in the timeline. If this is not specified, or is
* 'ignore', then the event passed to this function will be ignored
* entirely, preserving the existing event in the timeline. Events are
* identical based on their event ID <b>only</b>. */
duplicateStrategy?: DuplicateStrategy;
/** Whether the sync response came from cache */
fromCache?: boolean;
/** The state events to reconcile metadata from */
roomState?: RoomState;
/** Whether the state is part of the first state snapshot we're seeing in
* the room. This could be happen in a variety of cases:
* 1. From the initial sync
* 2. It's the first state we're seeing after joining the room
* 3. Or whether it's coming from `syncFromCache` */
fromInitialState?: boolean;
MadLittleMods marked this conversation as resolved.
Show resolved Hide resolved
}
Copy link
Contributor Author

@MadLittleMods MadLittleMods Apr 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of dealing with unwieldy long function signatures with unlabeled and optional arguments in the usage, I've refactored some of these into option objects now that I am adding yet another boolean here (adding fromInitialState).

Before:

room.addLiveEvents(mainTimelineEvents, null, fromCache, timelineWasEmpty);

After:

room.addLiveEvents(mainTimelineEvents, {
    fromCache,
    fromInitialState: timelineWasEmpty
});

If necessary, this refactor can be split out to another PR with some effort.


export interface IAddEventToTimelineOptions {
toStartOfTimeline: boolean;
fromCache?: boolean;
roomState?: RoomState;
/** Whether the state is part of the first state snapshot we're seeing in
* the room. This could be happen in a variety of cases:
* 1. From the initial sync
* 2. It's the first state we're seeing after joining the room
* 3. Or whether it's coming from `syncFromCache` */
fromInitialState?: boolean;
}

type EmittedEvents = RoomEvent.Timeline | RoomEvent.TimelineReset;

export type EventTimelineSetHandlerMap = {
Expand Down Expand Up @@ -431,7 +463,9 @@ export class EventTimelineSet extends TypedEventEmitter<EmittedEvents, EventTime

if (!existingTimeline) {
// we don't know about this event yet. Just add it to the timeline.
this.addEventToTimeline(event, timeline, toStartOfTimeline);
this.addEventToTimeline(event, timeline, {
toStartOfTimeline
});
lastEventWasNew = true;
didUpdate = true;
continue;
Expand Down Expand Up @@ -523,15 +557,16 @@ export class EventTimelineSet extends TypedEventEmitter<EmittedEvents, EventTime
* Add an event to the end of this live timeline.
*
* @param {MatrixEvent} event Event to be added
* @param {string?} duplicateStrategy 'ignore' or 'replace'
* @param {boolean} fromCache whether the sync response came from cache
* @param roomState the state events to reconcile metadata from
* @param {IAddLiveEventOptions} options addLiveEvent options
*/
public addLiveEvent(
event: MatrixEvent,
duplicateStrategy: DuplicateStrategy = DuplicateStrategy.Ignore,
fromCache = false,
roomState?: RoomState,
{
duplicateStrategy = DuplicateStrategy.Ignore,
fromCache = false,
roomState,
fromInitialState
}: IAddLiveEventOptions = {}
): void {
if (this.filter) {
const events = this.filter.filterRoomTimeline([event]);
Expand Down Expand Up @@ -570,7 +605,12 @@ export class EventTimelineSet extends TypedEventEmitter<EmittedEvents, EventTime
return;
}

this.addEventToTimeline(event, this.liveTimeline, false, fromCache, roomState);
this.addEventToTimeline(event, this.liveTimeline, {
toStartOfTimeline: false,
fromCache,
roomState,
fromInitialState,
});
}

/**
Expand All @@ -581,20 +621,26 @@ export class EventTimelineSet extends TypedEventEmitter<EmittedEvents, EventTime
*
* @param {MatrixEvent} event
* @param {EventTimeline} timeline
* @param {boolean} toStartOfTimeline
* @param {boolean} fromCache whether the sync response came from cache
* @param {IAddEventToTimelineOptions} options addEventToTimeline options
*
* @fires module:client~MatrixClient#event:"Room.timeline"
*/
public addEventToTimeline(
event: MatrixEvent,
timeline: EventTimeline,
toStartOfTimeline: boolean,
fromCache = false,
roomState?: RoomState,
{
toStartOfTimeline,
fromCache = false,
roomState,
fromInitialState = false
}: IAddEventToTimelineOptions,
) {
const eventId = event.getId();
timeline.addEvent(event, toStartOfTimeline, roomState);
timeline.addEvent(event, {
atStart: toStartOfTimeline,
stateContext: roomState,
fromInitialState,
});
this._eventIdToTimeline[eventId] = timeline;

this.setRelationsTarget(event);
Expand Down Expand Up @@ -630,10 +676,14 @@ export class EventTimelineSet extends TypedEventEmitter<EmittedEvents, EventTime
} else {
if (this.filter) {
if (this.filter.filterRoomTimeline([localEvent]).length) {
this.addEventToTimeline(localEvent, this.liveTimeline, false);
this.addEventToTimeline(localEvent, this.liveTimeline, {
toStartOfTimeline: false
});
}
} else {
this.addEventToTimeline(localEvent, this.liveTimeline, false);
this.addEventToTimeline(localEvent, this.liveTimeline, {
toStartOfTimeline: false
});
}
}
}
Expand Down
29 changes: 24 additions & 5 deletions src/models/event-timeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,19 @@ import { EventTimelineSet } from "./event-timeline-set";
import { MatrixEvent } from "./event";
import { Filter } from "../filter";
import { EventType } from "../@types/event";
import { ISetStateOptions } from "./room-state";

export enum Direction {
Backward = "b",
Forward = "f",
}

export interface IAddEventOptions {
atStart: boolean;
stateContext?: RoomState;
fromInitialState?: boolean;
}

export class EventTimeline {
/**
* Symbolic constant for methods which take a 'direction' argument:
Expand Down Expand Up @@ -152,8 +159,11 @@ export class EventTimeline {
Object.freeze(e);
}

this.startState.setStateEvents(stateEvents);
this.endState.setStateEvents(stateEvents);
const setStateOptions: ISetStateOptions = {
fromInitialState: true
};
this.startState.setStateEvents(stateEvents, setStateOptions);
this.endState.setStateEvents(stateEvents, setStateOptions);
}

/**
Expand Down Expand Up @@ -345,9 +355,16 @@ export class EventTimeline {
* Add a new event to the timeline, and update the state
*
* @param {MatrixEvent} event new event
* @param {boolean} atStart true to insert new event at the start
* @param {IAddEventOptions} options addEvent options
*/
public addEvent(event: MatrixEvent, atStart: boolean, stateContext?: RoomState): void {
public addEvent(
event: MatrixEvent,
{
atStart,
stateContext,
fromInitialState,
}: IAddEventOptions
): void {
if (!stateContext) {
stateContext = atStart ? this.startState : this.endState;
}
Expand All @@ -362,7 +379,9 @@ export class EventTimeline {
event.isState() &&
timelineSet.room.getUnfilteredTimelineSet() === timelineSet
) {
stateContext.setStateEvents([event]);
stateContext.setStateEvents([event], {
fromInitialState
});
// it is possible that the act of setting the state event means we
// can set more metadata (specifically sender/target props), so try
// it again if the prop wasn't previously set. It may also mean that
Expand Down
25 changes: 20 additions & 5 deletions src/models/room-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,22 @@ enum OobStatus {
Finished,
}

export interface ISetStateOptions {
/** Whether the state is part of the first state snapshot we're seeing in
* the room. This could be happen in a variety of cases:
* 1. From the initial sync
* 2. It's the first state we're seeing after joining the room
* 3. Or whether it's coming from `syncFromCache` */
fromInitialState?: boolean;
}

export enum RoomStateEvent {
Events = "RoomState.events",
Members = "RoomState.members",
NewMember = "RoomState.newMember",
Update = "RoomState.update", // signals batches of updates without specificity
BeaconLiveness = "RoomState.BeaconLiveness",
Marker = "RoomState.org.matrix.msc2716.marker",
}

export type RoomStateEventHandlerMap = {
Expand All @@ -51,6 +61,7 @@ export type RoomStateEventHandlerMap = {
[RoomStateEvent.Update]: (state: RoomState) => void;
[RoomStateEvent.BeaconLiveness]: (state: RoomState, hasLiveBeacons: boolean) => void;
[BeaconEvent.New]: (event: MatrixEvent, beacon: Beacon) => void;
[RoomStateEvent.Marker]: (event: MatrixEvent, setStateOptions: ISetStateOptions) => void;
};

type EmittedEvents = RoomStateEvent | BeaconEvent;
Expand Down Expand Up @@ -309,16 +320,18 @@ export class RoomState extends TypedEventEmitter<EmittedEvents, EventHandlerMap>
}

/**
* Add an array of one or more state MatrixEvents, overwriting
* any existing state with the same {type, stateKey} tuple. Will fire
* "RoomState.events" for every event added. May fire "RoomState.members"
* if there are <code>m.room.member</code> events.
* Add an array of one or more state MatrixEvents, overwriting any existing
* state with the same {type, stateKey} tuple. Will fire "RoomState.events"
* for every event added. May fire "RoomState.members" if there are
* <code>m.room.member</code> events.
* @param {MatrixEvent[]} stateEvents a list of state events for this room.
* @param {boolean} fromInitialState whether the stateEvents are from the first
* sync in the room or a sync we already know about (syncFromCache)
* @fires module:client~MatrixClient#event:"RoomState.members"
* @fires module:client~MatrixClient#event:"RoomState.newMember"
* @fires module:client~MatrixClient#event:"RoomState.events"
*/
public setStateEvents(stateEvents: MatrixEvent[]) {
public setStateEvents(stateEvents: MatrixEvent[], setStateOptions?: ISetStateOptions) {
this.updateModifiedTime();

// update the core event dict
Expand Down Expand Up @@ -398,6 +411,8 @@ export class RoomState extends TypedEventEmitter<EmittedEvents, EventHandlerMap>

// assume all our sentinels are now out-of-date
this.sentinels = {};
} else if (event.getType() === EventType.Marker) {
this.emit(RoomStateEvent.Marker, event, setStateOptions);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we see a new marker state event, emit an event so we can listen for it in sync.ts -> registerStateListeners and see whether we need to refresh the timeline

}
});

Expand Down
Loading