-
Notifications
You must be signed in to change notification settings - Fork 51
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
Move /send_leave
to GMSL
#387
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #387 +/- ##
==========================================
+ Coverage 58.99% 59.24% +0.25%
==========================================
Files 51 51
Lines 7113 7194 +81
==========================================
+ Hits 4196 4262 +66
- Misses 2528 2538 +10
- Partials 389 394 +5 ☔ View full report in Codecov by Sentry. |
@@ -221,3 +222,177 @@ func TestHandleMakeLeave(t *testing.T) { | |||
}) | |||
} | |||
} | |||
|
|||
type dummyQuerier struct { | |||
pdu PDU |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to assert the type/state key are correct when queried via CurrentStateEvent
.
eventID, roomID string, | ||
querier CurrentStateQuerier, | ||
verifier JSONVerifier, | ||
) (PDU, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sidenote: this function and others like it will likely need to be modified to support pseudo IDs, as in many cases we pull out the state key and check that it is a user ID, and then check that the domain is correct on it. I would probably structure this as:
- static function
HandleSendLeave
as it is in this PR - Pull out the room version immediately.
- Call
roomVer.HandleSendLeave(args...)
to let room versions decide how to implement this. - Most room versions will call a private function which does the below code, checking user IDs etc.
- But pseudo IDs will call a custom function which omits these checks.
Whilst the caller could call roomVer.HandleSendLeave
this then creates bad symmetry, as there are cases where you don't know the room version at this point (e.g invites).
eventversion.go
Outdated
@@ -431,6 +447,17 @@ func (v RoomVersionImpl) RedactEventJSON(eventJSON []byte) ([]byte, error) { | |||
return v.redactionAlgorithm(eventJSON) | |||
} | |||
|
|||
// HandleSendLeave handles requests to `/send_leave` | |||
func (v RoomVersionImpl) HandleSendLeave(ctx context.Context, | |||
event PDU, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is problematic if v != event.Version()
.
handleleave.go
Outdated
// handleSendLeave handles requests to `/send_leave | ||
// Returns the parsed event or an error. | ||
func handleSendLeave(ctx context.Context, | ||
event PDU, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The proposal I mentioned in #387 (comment) was to keep this function public, and to serve as the entry point for callers, because:
Whilst the caller could call roomVer.HandleSendLeave this then creates bad symmetry, as there are cases where you don't know the room version at this point (e.g invites).
This reverts commit 540062a.
No description provided.