Add `depth` parameter and pipe through the event creation methods #10049

MadLittleMods · 2021-05-24T05:37:39Z

Add depth parameter and pipe through the event creation methods. Also add auth_event_ids and prev_event_ids in some places as well so I don't have to go through PR splitting conflict hell.

Split out from #9247

Call stacks:

create_and_send_nonmember_event
create_event
create_new_client_event
builder.build

update_membership
update_membership_locked
_local_membership_update
create_event

Pull Request Checklist

Pull request is based on the develop branch
Pull request includes a changelog file. The entry should:
- Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
- Use markdown where necessary, mostly for code blocks.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
Pull request includes a sign off
Code style is correct (run the linters)

MadLittleMods · 2021-05-24T05:38:40Z

synapse/handlers/room_member.py

@@ -496,160 +561,170 @@ async def update_membership_locked(
            if block_invite:
                raise SynapseError(403, "Invites have been disabled on this server")

-        latest_event_ids = await self.store.get_prev_events_for_room(room_id)
+        if prev_event_ids:
+            latest_event_ids = prev_event_ids


New case is here where we override the prev_event_ids if passed in explicitly

https://github.com/matrix-org/synapse/pull/9247/files#r610994535

MadLittleMods · 2021-05-24T05:39:10Z

synapse/handlers/room_member.py

+        if prev_event_ids:
+            latest_event_ids = prev_event_ids
+        else:
+            latest_event_ids = await self.store.get_prev_events_for_room(room_id)


All of this code is just nested under the else conditional and not changed.

https://github.com/matrix-org/synapse/pull/9247/files#r610994518

Also add auth_event_ids and prev_event_ids in some places as well. Split out from #9247 Call stacks: ``` create_and_send_nonmember_event create_event create_new_client_event builder.build ``` ``` update_membership update_membership_locked _local_membership_update create_event ```

synapse/storage/databases/main/event_federation.py

MadLittleMods · 2021-05-24T06:26:45Z

synapse/events/builder.py

+                    oldest_successor_depth,
+                ) = await self._store.get_min_depth_of(successor_event_ids)
+
+                depth = oldest_successor_depth


The smarts of inherit_depth are here 🎈

erikjohnston

These look like changes that would work :)

synapse/handlers/room_member.py

See #10049 (comment)

MadLittleMods · 2021-05-28T06:58:46Z

I feel like the remaining failing Sytest tests are just flakey. They are different when I re-run them.

Trying to get it running locally to confirm, matrix-org/sytest#1054

erikjohnston

Thanks, this looks good other than tiny nit

synapse/handlers/message.py

…the event dictionary See #10049 (comment)

MadLittleMods · 2021-06-02T18:30:12Z

@erikjohnston Thanks for all the review so far 🎢

erikjohnston · 2021-06-03T07:43:08Z

I think this is good to merge, but I'm going to hold off a bit until we #9247 is a bit closer (in particular I'm wondering if we should pass in the depth explicitly, rather than have inherit_depth). Thanks for splitting it out from that PR though, it was helpful to be able to review it separately :)

See https://github.com/matrix-org/synapse/pull/9247/files#r636778295

See #10049 (comment)

…the event dictionary See #10049 (comment)

MadLittleMods · 2021-06-06T21:25:47Z

@erikjohnston I've updated this PR and the base PR to pass around depth directly 👍

erikjohnston

This looks good, thanks, but again let's wait for the other PR before landing this

synapse/events/builder.py

synapse/storage/databases/main/event_federation.py

tests/handlers/test_presence.py

See #9247 (comment)

MadLittleMods commented May 24, 2021

View reviewed changes

MadLittleMods force-pushed the madlittlemods/pipe-inherit_depth branch 3 times, most recently from 697949c to b9952cf Compare May 24, 2021 06:06

MadLittleMods changed the title ~~Pipe the inherit_depth parameter through the event creation methods~~ Add inherit_depth parameter and pipe through the event creation methods May 24, 2021

MadLittleMods force-pushed the madlittlemods/pipe-inherit_depth branch from b9952cf to 280d6da Compare May 24, 2021 06:11

MadLittleMods force-pushed the madlittlemods/pipe-inherit_depth branch from 280d6da to 3e747bc Compare May 24, 2021 06:17

MadLittleMods commented May 24, 2021

View reviewed changes

synapse/storage/databases/main/event_federation.py Show resolved Hide resolved

MadLittleMods commented May 24, 2021

View reviewed changes

MadLittleMods requested a review from erikjohnston May 24, 2021 06:52

MadLittleMods mentioned this pull request May 27, 2021

Add endpoints for backfilling history (MSC2716) #9247

Merged

9 tasks

erikjohnston reviewed May 27, 2021

View reviewed changes

synapse/handlers/room_member.py Outdated Show resolved Hide resolved

MadLittleMods added 2 commits May 27, 2021 10:43

Return early instead of big if nesting

9f5ed44

See #10049 (comment)

Merge branch 'develop' into madlittlemods/pipe-inherit_depth

62219a3

MadLittleMods requested a review from erikjohnston May 28, 2021 06:58

MadLittleMods added 2 commits May 29, 2021 00:02

Merge branch 'develop' into madlittlemods/pipe-inherit_depth

e1e52ba

Merge branch 'develop' into madlittlemods/pipe-inherit_depth

7912697

erikjohnston reviewed Jun 2, 2021

View reviewed changes

synapse/handlers/message.py Outdated Show resolved Hide resolved

MadLittleMods requested a review from erikjohnston June 2, 2021 18:28

Pass in prev_events as function parameter so clients can't set it in …

bd28bda

…the event dictionary See #10049 (comment)

MadLittleMods force-pushed the madlittlemods/pipe-inherit_depth branch from 63bf8f6 to bd28bda Compare June 2, 2021 18:29

Switch to passing depth directly instead of inherit_depth

8e62928

See https://github.com/matrix-org/synapse/pull/9247/files#r636778295

MadLittleMods added a commit that referenced this pull request Jun 6, 2021

Return early instead of big if nesting

25aef56

See #10049 (comment)

MadLittleMods added a commit that referenced this pull request Jun 6, 2021

Pass in prev_events as function parameter so clients can't set it in …

0580d09

…the event dictionary See #10049 (comment)

MadLittleMods requested review from erikjohnston and removed request for erikjohnston June 6, 2021 21:26

erikjohnston approved these changes Jun 7, 2021

View reviewed changes

synapse/events/builder.py Outdated Show resolved Hide resolved

synapse/storage/databases/main/event_federation.py Show resolved Hide resolved

MadLittleMods changed the title ~~Add inherit_depth parameter and pipe through the event creation methods~~ Add depth parameter and pipe through the event creation methods Jun 7, 2021

Only get_max_depth_of where we use it

170240b

MadLittleMods commented Jun 9, 2021

View reviewed changes

tests/handlers/test_presence.py Outdated Show resolved Hide resolved

Fix tests using False as a depth value (should be None)

604d2a9

See #9247 (comment)

MadLittleMods closed this Jun 22, 2021

MadLittleMods deleted the madlittlemods/pipe-inherit_depth branch September 7, 2021 20:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `depth` parameter and pipe through the event creation methods #10049

Add `depth` parameter and pipe through the event creation methods #10049

MadLittleMods commented May 24, 2021 •

edited

Loading

MadLittleMods May 24, 2021

MadLittleMods May 24, 2021

MadLittleMods May 24, 2021

erikjohnston left a comment

MadLittleMods commented May 28, 2021

erikjohnston left a comment

MadLittleMods commented Jun 2, 2021

erikjohnston commented Jun 3, 2021

MadLittleMods commented Jun 6, 2021

erikjohnston left a comment

Add depth parameter and pipe through the event creation methods #10049

Add depth parameter and pipe through the event creation methods #10049

Conversation

MadLittleMods commented May 24, 2021 • edited Loading

Pull Request Checklist

MadLittleMods May 24, 2021

Choose a reason for hiding this comment

MadLittleMods May 24, 2021

Choose a reason for hiding this comment

MadLittleMods May 24, 2021

Choose a reason for hiding this comment

erikjohnston left a comment

Choose a reason for hiding this comment

MadLittleMods commented May 28, 2021

erikjohnston left a comment

Choose a reason for hiding this comment

MadLittleMods commented Jun 2, 2021

erikjohnston commented Jun 3, 2021

MadLittleMods commented Jun 6, 2021

erikjohnston left a comment

Choose a reason for hiding this comment

Add `depth` parameter and pipe through the event creation methods #10049

Add `depth` parameter and pipe through the event creation methods #10049

MadLittleMods commented May 24, 2021 •

edited

Loading