This repository has been archived by the owner on Apr 26, 2024. It is now read-only.
Refactor event building into EventBuilder #4481
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
erikjohnston
force-pushed
the
erikj/event_builder
branch
from
b0b1161 to
9fb9205
Compare
erikjohnston
commented
Jan 25, 2019
Codecov Report
@@ Coverage Diff @@
## develop #4481 +/- ##
===========================================
+ Coverage 67.87% 74.76% +6.89%
===========================================
Files 336 336
Lines 34163 34198 +35
Branches 5556 5564 +8
===========================================
+ Hits 23188 25568 +2380
+ Misses 9362 7054 -2308
+ Partials 1613 1576 -37 |
richvdh
suggested changes
Jan 28, 2019
This is so that everything is done in one place, making it easier to change the event format based on room version
`.user_id` is proxed to `.sender` in FrozenEvent, so this has no functional change
erikjohnston
force-pushed
the
erikj/event_builder
branch
from
6b70f5e to
aee39f7
Compare
richvdh
reviewed
Jan 29, 2019
| Deferred[FrozenEvent] | ||
| """ | ||
|
|
||
| state_ids = yield self._state.get_current_state_ids( |
There was a problem hiding this comment.
Slightly concerned that we used to do context.get_prev_state_ids, which might return something different to this?
There was a problem hiding this comment.
In practice, they're exactly the same, compute_event_context just resolves the state across the prev_events to get prev_state_ids. From a protocol perspective, auth_events should be based on the state at the event.
We could make compute_event_context work with an EventBuilder, but I was slightly shying away from doing so in the interests of not changing too much. We could probably do so though
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is so that everything is done in one place, making it easier to change the event format based on room version.
The crux of what's happening here is that the
EventBuilderis now meant to be independent of the format version, allowing the rest of the code to build up events without worrying about different versions. This is done by making it anattrclass with well defined fields that are of interest when building an event (mainlytype/state_key/content/etc), and only adding the rest of the fields when we finally build the event (which handles things like adding prev events, signing and hashing it, using the right format etc)Hopefully the commits are relatively stand alone.