Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Fix bug which caused rejected events to be stored with the wrong room state #6320

Merged
merged 4 commits into from
Nov 6, 2019

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Nov 4, 2019

The problem here was the code in StateStore which assumed that prev_group gave the state before the event being persisted. Whilst this may (sometimes) have been true for state events, it was never true for message events.

I'm not entirely sure if this is the best way to fix it, but the general idea is to rewrite compute_event_context to ensure that we always have a state group for the state before the event being persisted (and to store it in a new field in the EventContext). That also allows us to rewrite it to reduce a bunch of duplication between the code paths.

Fixes #6289.

This PR builds on #6319.

@richvdh richvdh changed the title ### Pull Request Checklist Fix bug which caused rejected events to be stored with the wrong room state Nov 4, 2019
@richvdh richvdh force-pushed the rav/event_context/2 branch from 73cd3c2 to a80143f Compare November 4, 2019 18:11
@richvdh richvdh requested a review from a team November 4, 2019 18:11
@erikjohnston
Copy link
Member

Do we have any unit tests for compute_event_context?

@richvdh
Copy link
Member Author

richvdh commented Nov 5, 2019

Do we have any unit tests for compute_event_context?

there are a bunch in https://github.com/matrix-org/synapse/blob/develop/tests/test_state.py.

I guess I know where that question is leading...

@erikjohnston
Copy link
Member

Do we have any unit tests for compute_event_context?

there are a bunch in https://github.com/matrix-org/synapse/blob/develop/tests/test_state.py.

I guess I know where that question is leading...

Just adding some checks for the new fields in those tests would be good 😇

Fixes a bug where rejected events were persisted with the wrong state group.

Also fixes an occasional internal-server-error when receiving events over
federation which are rejected and (possibly because they are
backwards-extremities) have no prev_group.

Fixes #6289.
@richvdh richvdh force-pushed the rav/event_context/2 branch from 37ec6ee to ba3a5e8 Compare November 5, 2019 13:26
@richvdh
Copy link
Member Author

richvdh commented Nov 5, 2019

tests (and a lying docstring) updated in ba3a5e8

@richvdh richvdh merged commit 807ec3b into develop Nov 6, 2019
@richvdh richvdh deleted the rav/event_context/2 branch November 6, 2019 10:01
babolivier pushed a commit that referenced this pull request Sep 1, 2021
… state (#6320)

* commit '807ec3bd9':
  Fix bug which caused rejected events to be stored with the wrong room state  (#6320)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants