client/server: set 'initial' flag on a per-room basis and use it when rendering

Previously the 'initial' flag was set on the entire response which was pointless
as the client can detect this by the presence/absence of `?pos=`.

Instead, move the `initial` flag to be present on the Room object, so clients know
when to replace or update their databases. Use this to fix a bug where the timeline
would show incorrectly when clicking on rooms due to appending to the timeline when
the room subscription data was collected.
This commit is contained in:
Kegan Dougal 2022-02-24 16:48:39 +00:00
parent 923d3e084c
commit 4a03b12287
8 changed files with 110 additions and 93 deletions

View File

@ -21,9 +21,7 @@
<input id="syncButton" type="button" value="Start Sync" /> <input id="syncButton" type="button" value="Start Sync" />
<input id="debugButton" type="button" value="DEBUG" style="display: none;" /> <input id="debugButton" type="button" value="DEBUG" style="display: none;" />
<span id="errorMsg"></span> <span id="errorMsg"></span>
<div> <input id="roomfilter" type="text" placeholder="Filter rooms..." />
<input id="roomfilter" type="text" placeholder="Filter rooms..." />
</div>
</div> </div>
<div class="roomheading"> <div class="roomheading">
<img class="roomavatar" decoding="async" id="selectedroomavatar" src="/client/placeholder.svg"/> <img class="roomavatar" decoding="async" id="selectedroomavatar" src="/client/placeholder.svg"/>

View File

@ -16,6 +16,9 @@ import * as matrix from "./matrix.js";
let syncv2ServerUrl; // will be populated with the URL of the CS API e.g 'https://matrix-client.matrix.org' let syncv2ServerUrl; // will be populated with the URL of the CS API e.g 'https://matrix-client.matrix.org'
let slidingSync; let slidingSync;
let syncConnection = new SlidingSyncConnection(); let syncConnection = new SlidingSyncConnection();
// The lists present on the UI.
// You can add/remove these at will to experiment with sliding sync filters.
let activeLists = [ let activeLists = [
new SlidingList("Direct Messages", { new SlidingList("Direct Messages", {
is_dm: true, is_dm: true,
@ -34,59 +37,72 @@ let rooms = {
window.rooms = rooms; window.rooms = rooms;
window.activeLists = activeLists; window.activeLists = activeLists;
const accumulateRoomData = (r, isUpdate) => { /**
let room = r; * Set top-level properties on the response which corresponds to state fields the UI requires.
if (isUpdate) { * The normal response returns whole state events in an array which isn't useful when rendering as we
// use what we already have, if any * often just need a single key within the content, so pre-emptively find the fields we want and set them.
let existingRoom = rooms.roomIdToRoom[r.room_id]; * Modifies the input object.
if (existingRoom) { * @param {object} r The room response JSON
if (r.name) { */
existingRoom.name = r.name; const setRoomStateFields = (r) => {
} if (!r.required_state) {
if (r.highlight_count !== undefined) { return;
existingRoom.highlight_count = r.highlight_count; }
} for (let i = 0; i < r.required_state.length; i++) {
if (r.notification_count !== undefined) { const ev = r.required_state[i];
existingRoom.notification_count = r.notification_count; switch (ev.type) {
} case "m.room.avatar":
if (r.timeline) { r.avatar = ev.content.url;
r.timeline.forEach((e) => { break;
existingRoom.timeline.push(e); case "m.room.topic":
}); r.topic = ev.content.topic;
} break;
room = existingRoom; case "m.room.tombstone":
r.obsolete = ev.content.body || "m.room.tombstone";
break;
} }
} }
// pull out avatar and topic if it exists };
let avatar;
let topic; /**
let obsolete; * Accumulate room data for this room. This is done by inspecting the 'initial' flag on the response.
if (r.required_state) { * If it is set, the entire room is replaced with this data. If it is false, the room data is
for (let i = 0; i < r.required_state.length; i++) { * merged together with whatever is there in the store.
const ev = r.required_state[i]; * @param {object} r The room response JSON
switch (ev.type) { */
case "m.room.avatar": const accumulateRoomData = (r) => {
avatar = ev.content.url; setRoomStateFields(r);
break; if (r.initial) {
case "m.room.topic": rooms.roomIdToRoom[r.room_id] = r;
topic = ev.content.topic; return;
break; }
case "m.room.tombstone": // use what we already have, if any
obsolete = ev.content.body || "m.room.tombstone"; let existingRoom = rooms.roomIdToRoom[r.room_id];
break; if (existingRoom) {
} if (r.name) {
existingRoom.name = r.name;
} }
if (r.highlight_count !== undefined) {
existingRoom.highlight_count = r.highlight_count;
}
if (r.notification_count !== undefined) {
existingRoom.notification_count = r.notification_count;
}
if (r.timeline) {
r.timeline.forEach((e) => {
existingRoom.timeline.push(e);
});
}
} else {
// we don't know this room but apparently we should. Whine about it and use what we have.
console.error(
"room initial flag not set but we have no memory of this room",
r
);
existingRoom = r;
} }
if (avatar !== undefined) {
room.avatar = avatar; rooms.roomIdToRoom[existingRoom.room_id] = existingRoom;
}
if (topic !== undefined) {
room.topic = topic;
}
if (obsolete !== undefined) {
room.obsolete = obsolete;
}
rooms.roomIdToRoom[room.room_id] = room;
}; };
let debounceTimeoutId; let debounceTimeoutId;
@ -165,11 +181,6 @@ const intersectionObserver = new IntersectionObserver(
const renderMessage = (container, ev) => { const renderMessage = (container, ev) => {
const eventIdKey = "msg" + ev.event_id; const eventIdKey = "msg" + ev.event_id;
// try to find the element. If it exists then don't re-render.
const existing = document.getElementById(eventIdKey);
if (existing) {
return;
}
const msgCell = render.renderEvent(eventIdKey, ev); const msgCell = render.renderEvent(eventIdKey, ev);
container.appendChild(msgCell); container.appendChild(msgCell);
}; };
@ -350,19 +361,14 @@ const doSyncLoop = async (accessToken) => {
} }
}); });
slidingSync.addRoomDataListener((roomId, roomData, isIncremental) => { slidingSync.addRoomDataListener((roomId, roomData) => {
accumulateRoomData( accumulateRoomData(roomData);
roomData,
isIncremental
? isIncremental
: rooms.roomIdToRoom[roomId] !== undefined
);
// render the right-hand side section with the room timeline if we are viewing it. // render the right-hand side section with the room timeline if we are viewing it.
if (roomId !== slidingSync.roomSubscription) { if (roomId !== slidingSync.roomSubscription) {
return; return;
} }
let room = rooms.roomIdToRoom[slidingSync.roomSubscription]; let room = rooms.roomIdToRoom[slidingSync.roomSubscription];
renderRoomTimeline(room, false); renderRoomTimeline(room, roomData.initial);
}); });
// begin the sliding sync loop // begin the sliding sync loop
@ -450,6 +456,7 @@ window.addEventListener("load", async (event) => {
"Error sending message: " + err; "Error sending message: " + err;
} }
ev.target.removeAttribute("disabled"); ev.target.removeAttribute("disabled");
ev.target.focus();
} }
}); });
}); });

View File

@ -20,6 +20,7 @@ const REQUIRED_STATE_EVENTS_IN_LIST = [
]; ];
const REQUIRED_STATE_EVENTS_IN_ROOM = [ const REQUIRED_STATE_EVENTS_IN_ROOM = [
["m.room.avatar", ""], // need the avatar to display next to the room name ["m.room.avatar", ""], // need the avatar to display next to the room name
["m.room.tombstone", ""], // need to know if this room has been replaced
["m.room.topic", ""], // need the topic to display on the right-hand-side ["m.room.topic", ""], // need the topic to display on the right-hand-side
]; ];
@ -178,11 +179,10 @@ export class SlidingSync {
* Invoke all attached room data listeners. * Invoke all attached room data listeners.
* @param {string} roomId The room which received some data. * @param {string} roomId The room which received some data.
* @param {object} roomData The raw sliding sync response JSON. * @param {object} roomData The raw sliding sync response JSON.
* @param {boolean} isIncremental True if the roomData is a delta. False if this is the complete snapshot.
*/ */
_invokeRoomDataListeners(roomId, roomData, isIncremental) { _invokeRoomDataListeners(roomId, roomData) {
this.roomDataCallbacks.forEach((callback) => { this.roomDataCallbacks.forEach((callback) => {
callback(roomId, roomData, isIncremental); callback(roomId, roomData);
}); });
} }
@ -278,7 +278,6 @@ export class SlidingSync {
); );
} catch (err) { } catch (err) {
if (err.name !== "AbortError") { if (err.name !== "AbortError") {
// XXX: request failed
this._invokeLifecycleListeners( this._invokeLifecycleListeners(
LifecycleSyncRequestFinished, LifecycleSyncRequestFinished,
null, null,
@ -373,13 +372,7 @@ export class SlidingSync {
op.room.room_id, op.room.room_id,
";" ";"
); );
// TODO: move this._invokeRoomDataListeners(op.room.room_id, op.room);
// XXX: room data, room ID
this._invokeRoomDataListeners(
op.room.room_id,
op.room,
true
);
} else if (op.op === "SYNC") { } else if (op.op === "SYNC") {
let syncRooms = []; let syncRooms = [];
const startIndex = op.range[0]; const startIndex = op.range[0];

View File

@ -161,7 +161,6 @@ func (s *ConnState) onIncomingRequest(ctx context.Context, req *sync3.Request) (
response := &sync3.Response{ response := &sync3.Response{
RoomSubscriptions: s.updateRoomSubscriptions(int(sync3.DefaultTimelineLimit), newSubs, newUnsubs), RoomSubscriptions: s.updateRoomSubscriptions(int(sync3.DefaultTimelineLimit), newSubs, newUnsubs),
Extensions: s.extensionsHandler.Handle(ex), Extensions: s.extensionsHandler.Handle(ex),
Initial: prevReq == nil,
} }
responseOperations := []sync3.ResponseOp{} // empty not nil slice responseOperations := []sync3.ResponseOp{} // empty not nil slice
@ -513,6 +512,7 @@ func (s *ConnState) getInitialRoomData(timelineLimit int, roomIDs ...string) []s
HighlightCount: int64(userRoomData.HighlightCount), HighlightCount: int64(userRoomData.HighlightCount),
Timeline: userRoomData.Timeline, Timeline: userRoomData.Timeline,
RequiredState: s.globalCache.LoadRoomState(roomID, s.loadPosition, s.muxedReq.GetRequiredState(0, roomID)), RequiredState: s.globalCache.LoadRoomState(roomID, s.loadPosition, s.muxedReq.GetRequiredState(0, roomID)),
Initial: true,
} }
} }
return rooms return rooms

View File

@ -17,8 +17,7 @@ const (
) )
type Response struct { type Response struct {
Ops []ResponseOp `json:"ops"` Ops []ResponseOp `json:"ops"`
Initial bool `json:"initial,omitempty"`
RoomSubscriptions map[string]Room `json:"room_subscriptions"` RoomSubscriptions map[string]Room `json:"room_subscriptions"`
Counts []int `json:"counts"` Counts []int `json:"counts"`
@ -44,7 +43,6 @@ func (r *Response) UnmarshalJSON(b []byte) error {
Extensions extensions.Response `json:"extensions"` Extensions extensions.Response `json:"extensions"`
Pos string `json:"pos"` Pos string `json:"pos"`
Initial bool `json:"initial"`
Session string `json:"session_id,omitempty"` Session string `json:"session_id,omitempty"`
}{} }{}
if err := json.Unmarshal(b, &temporary); err != nil { if err := json.Unmarshal(b, &temporary); err != nil {
@ -53,7 +51,6 @@ func (r *Response) UnmarshalJSON(b []byte) error {
r.RoomSubscriptions = temporary.RoomSubscriptions r.RoomSubscriptions = temporary.RoomSubscriptions
r.Counts = temporary.Counts r.Counts = temporary.Counts
r.Pos = temporary.Pos r.Pos = temporary.Pos
r.Initial = temporary.Initial
r.Session = temporary.Session r.Session = temporary.Session
r.Extensions = temporary.Extensions r.Extensions = temporary.Extensions

View File

@ -13,6 +13,7 @@ type Room struct {
Timeline []json.RawMessage `json:"timeline,omitempty"` Timeline []json.RawMessage `json:"timeline,omitempty"`
NotificationCount int64 `json:"notification_count"` NotificationCount int64 `json:"notification_count"`
HighlightCount int64 `json:"highlight_count"` HighlightCount int64 `json:"highlight_count"`
Initial bool `json:"initial,omitempty"`
} }
type RoomConnMetadata struct { type RoomConnMetadata struct {

View File

@ -248,11 +248,12 @@ func TestInitialFlag(t *testing.T) {
defer v3.close() defer v3.close()
alice := "@TestInitialFlag_alice:localhost" alice := "@TestInitialFlag_alice:localhost"
aliceToken := "ALICE_BEARER_TOKEN_TestInitialFlag" aliceToken := "ALICE_BEARER_TOKEN_TestInitialFlag"
roomID := "!a:localhost"
v2.addAccount(alice, aliceToken) v2.addAccount(alice, aliceToken)
v2.queueResponse(alice, sync2.SyncResponse{ v2.queueResponse(alice, sync2.SyncResponse{
Rooms: sync2.SyncRoomsResponse{ Rooms: sync2.SyncRoomsResponse{
Join: v2JoinTimeline(roomEvents{ Join: v2JoinTimeline(roomEvents{
roomID: "!a:localhost", roomID: roomID,
state: createRoomState(t, alice, time.Now()), state: createRoomState(t, alice, time.Now()),
}), }),
}, },
@ -265,12 +266,26 @@ func TestInitialFlag(t *testing.T) {
TimelineLimit: 10, TimelineLimit: 10,
}}, }},
}) })
MatchResponse(t, res, func(res *sync3.Response) error { MatchResponse(t, res, MatchV3Ops(
if !res.Initial { MatchV3SyncOpWithMatchers(MatchRoomRange(
return fmt.Errorf("initial flag was not set") []roomMatcher{MatchRoomInitial(true)},
} )),
return nil ))
// send an update
v2.queueResponse(alice, sync2.SyncResponse{
Rooms: sync2.SyncRoomsResponse{
Join: v2JoinTimeline(
roomEvents{
roomID: roomID,
events: []json.RawMessage{
testutils.NewEvent(t, "m.room.message", alice, map[string]interface{}{}, time.Now()),
},
},
),
},
}) })
v2.waitUntilEmpty(t, alice)
res = v3.mustDoV3RequestWithPos(t, aliceToken, res.Pos, sync3.Request{ res = v3.mustDoV3RequestWithPos(t, aliceToken, res.Pos, sync3.Request{
Lists: []sync3.RequestList{{ Lists: []sync3.RequestList{{
Ranges: sync3.SliceRanges{ Ranges: sync3.SliceRanges{
@ -278,12 +293,9 @@ func TestInitialFlag(t *testing.T) {
}, },
}}, }},
}) })
MatchResponse(t, res, func(res *sync3.Response) error { MatchResponse(t, res, MatchV3Ops(
if res.Initial { MatchV3UpdateOp(0, 0, roomID, MatchRoomInitial(false)),
return fmt.Errorf("initial flag was set") ))
}
return nil
})
} }
// Regression test for https://github.com/matrix-org/sliding-sync/commit/39d6e99f967e55b609f8ef8b4271c04ebb053d37 // Regression test for https://github.com/matrix-org/sliding-sync/commit/39d6e99f967e55b609f8ef8b4271c04ebb053d37

View File

@ -361,6 +361,15 @@ func MatchRoomNotificationCount(count int64) roomMatcher {
} }
} }
func MatchRoomInitial(initial bool) roomMatcher {
return func(r sync3.Room) error {
if r.Initial != initial {
return fmt.Errorf("MatchRoomInitial: got %v want %v", r.Initial, initial)
}
return nil
}
}
type roomEvents struct { type roomEvents struct {
roomID string roomID string
name string name string