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

Do not make snapshots for lone leave events #235

Merged
merged 2 commits into from
Aug 2, 2023

Conversation

kegsay
Copy link
Member

@kegsay kegsay commented Jul 31, 2023

Specifically this is targetting invite rejections, where the leave event is inside the leave block of the sync v2 response.

Previously, we would make a snapshot with this leave event. If the proxy wasn't in this room, it would mean the room state would just be the leave event, which is wrong. If the proxy was in the room, then state would correctly be rolled forward.

Specifically this is targetting invite rejections, where the leave
event is inside the leave block of the sync v2 response.

Previously, we would make a snapshot with this leave event. If the
proxy wasn't in this room, it would mean the room state would just
be the leave event, which is wrong. If the proxy was in the room,
then state would correctly be rolled forward.
@kegsay kegsay added the bug Something isn't working label Jul 31, 2023
@kegsay kegsay requested a review from DMRobertson July 31, 2023 16:54
state/accumulator.go Outdated Show resolved Hide resolved
state/accumulator.go Outdated Show resolved Hide resolved
Copy link
Contributor

@DMRobertson DMRobertson left a comment

Choose a reason for hiding this comment

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

Seems sane. Is this worth an integration test? Something like

  • be invited to unknown public room
  • reject invite
  • check nothing in the snapshots table with that room ID
  • join said room
  • check you get sensible state down /sync
  • check there is exactly one snapshot for that room ID? (except you might get more if the timeline has state events)

@kegsay kegsay merged commit a61a3fd into main Aug 2, 2023
6 checks passed
@kegsay kegsay mentioned this pull request Aug 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants