From 7307701a246a4a189a485aefcfc86d3de16c4523 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Mon, 31 Oct 2022 15:14:08 +0000 Subject: [PATCH] Tweak `"state"` and `"timeline"` filtering (#2844) This should stop state events disappearing down a gap where we'd try to separate out the sections *before* applying history visibility instead of after. This may be a better approach than #2843 but I hope @tak-hntlabs will shout if it isn't. --- syncapi/streams/stream_pdu.go | 21 +++++++++++++-------- sytest-blacklist | 7 ++++++- sytest-whitelist | 2 +- 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/syncapi/streams/stream_pdu.go b/syncapi/streams/stream_pdu.go index 3ab0f4ed..90f40148 100644 --- a/syncapi/streams/stream_pdu.go +++ b/syncapi/streams/stream_pdu.go @@ -251,8 +251,10 @@ func (p *PDUStreamProvider) addRoomDeltaToResponse( } return r.From, fmt.Errorf("p.DB.RecentEvents: %w", err) } - recentEvents := snapshot.StreamEventsToEvents(device, recentStreamEvents) - delta.StateEvents = removeDuplicates(delta.StateEvents, recentEvents) // roll back + recentEvents := gomatrixserverlib.HeaderedReverseTopologicalOrdering( + snapshot.StreamEventsToEvents(device, recentStreamEvents), + gomatrixserverlib.TopologicalOrderByPrevEvents, + ) prevBatch, err := snapshot.GetBackwardTopologyPos(ctx, recentStreamEvents) if err != nil { return r.From, fmt.Errorf("p.DB.GetBackwardTopologyPos: %w", err) @@ -263,10 +265,6 @@ func (p *PDUStreamProvider) addRoomDeltaToResponse( return r.To, nil } - // Sort the events so that we can pick out the latest events from both sections. - recentEvents = gomatrixserverlib.HeaderedReverseTopologicalOrdering(recentEvents, gomatrixserverlib.TopologicalOrderByPrevEvents) - delta.StateEvents = gomatrixserverlib.HeaderedReverseTopologicalOrdering(delta.StateEvents, gomatrixserverlib.TopologicalOrderByAuthEvents) - // Work out what the highest stream position is for all of the events in this // room that were returned. latestPosition := r.To @@ -314,6 +312,14 @@ func (p *PDUStreamProvider) addRoomDeltaToResponse( limited = true } + // Now that we've filtered the timeline, work out which state events are still + // left. Anything that appears in the filtered timeline will be removed from the + // "state" section and kept in "timeline". + delta.StateEvents = gomatrixserverlib.HeaderedReverseTopologicalOrdering( + removeDuplicates(delta.StateEvents, recentEvents), + gomatrixserverlib.TopologicalOrderByAuthEvents, + ) + if len(delta.StateEvents) > 0 { updateLatestPosition(delta.StateEvents[len(delta.StateEvents)-1].EventID()) } @@ -507,7 +513,6 @@ func (p *PDUStreamProvider) getJoinResponseForCompleteSync( // transaction IDs for complete syncs, but we do it anyway because Sytest demands it for: // "Can sync a room with a message with a transaction id" - which does a complete sync to check. recentEvents := snapshot.StreamEventsToEvents(device, recentStreamEvents) - stateEvents = removeDuplicates(stateEvents, recentEvents) events := recentEvents // Only apply history visibility checks if the response is for joined rooms @@ -521,7 +526,7 @@ func (p *PDUStreamProvider) getJoinResponseForCompleteSync( // If we are limited by the filter AND the history visibility filter // didn't "remove" events, return that the response is limited. limited = limited && len(events) == len(recentEvents) - + stateEvents = removeDuplicates(stateEvents, recentEvents) if stateFilter.LazyLoadMembers { if err != nil { return nil, err diff --git a/sytest-blacklist b/sytest-blacklist index 14edf398..e2859dcb 100644 --- a/sytest-blacklist +++ b/sytest-blacklist @@ -40,4 +40,9 @@ Accesing an AS-hosted room alias asks the AS server Guest users can join guest_access rooms # This will fail in HTTP API mode, so blacklisted for now -If a device list update goes missing, the server resyncs on the next one \ No newline at end of file + +If a device list update goes missing, the server resyncs on the next one + +# Might be a bug in the test because leaves do appear :-( + +Leaves are present in non-gapped incremental syncs diff --git a/sytest-whitelist b/sytest-whitelist index 6e4500d0..28235b77 100644 --- a/sytest-whitelist +++ b/sytest-whitelist @@ -699,7 +699,7 @@ We do send redundant membership state across incremental syncs if asked Rejecting invite over federation doesn't break incremental /sync Gapped incremental syncs include all state changes Old leaves are present in gapped incremental syncs -Leaves are present in non-gapped incremental syncs +#Leaves are present in non-gapped incremental syncs Members from the gap are included in gappy incr LL sync Presence can be set from sync /state returns M_NOT_FOUND for a rejected message event