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

[pseudoIDs] More pseudo ID fixes - Part 2 #3181

Merged
merged 14 commits into from
Aug 24, 2023
Merged

Conversation

swedgwood
Copy link
Contributor

@swedgwood swedgwood commented Aug 16, 2023

Totals before Totals: 620 passed, 44 expected fail, 118 failed
Totals after (0c76e61) Totals: 643 passed, 49 expected fail, 90 failed

Fixes include:

  • Translating state keys that contain user IDs to their respective room keys for both querying and sending state events
    • NOTE: there may be design discussion needed on what should happen when sender keys cannot be found for users
  • A simple fix for kicking guests from rooms properly
  • Logic for boundary history visibilities was slightly off (I'm surprised this only manifested in pseudo ID room versions)

Signed-off-by: Sam Wedgwood <sam@wedgwood.dev>

- so state events that use user IDs translate to pseudo IDs in pseudo ID rooms
- e.g. allowing to query m.room.member by user ID in a pseudo ID room
@codecov
Copy link

codecov bot commented Aug 16, 2023

Codecov Report

Patch coverage: 42.85% and project coverage change: +0.01% 🎉

Comparison is base (a721294) 64.20% compared to head (def3c74) 64.22%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3181      +/-   ##
==========================================
+ Coverage   64.20%   64.22%   +0.01%     
==========================================
  Files         506      506              
  Lines       57277    57346      +69     
==========================================
+ Hits        36776    36829      +53     
- Misses      16636    16651      +15     
- Partials     3865     3866       +1     
Flag Coverage Δ
unittests 49.09% <42.85%> (+0.13%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
clientapi/routing/state.go 66.88% <25.92%> (-4.03%) ⬇️
clientapi/routing/sendevent.go 58.59% <30.00%> (+0.43%) ⬆️
syncapi/synctypes/clientevent.go 73.07% <30.00%> (-9.69%) ⬇️
roomserver/internal/input/input_events.go 56.71% <100.00%> (+0.63%) ⬆️
syncapi/internal/history_visibility.go 87.30% <100.00%> (+0.63%) ⬆️

... and 20 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@swedgwood swedgwood marked this pull request as ready for review August 22, 2023 17:22
@swedgwood swedgwood requested a review from a team as a code owner August 22, 2023 17:22
Copy link
Collaborator

@devonh devonh left a comment

Choose a reason for hiding this comment

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

Changes look good!

@swedgwood swedgwood merged commit 9b5be6b into main Aug 24, 2023
16 of 20 checks passed
@swedgwood swedgwood deleted the swedgwood/pseudo-id-fixes-ii branch August 24, 2023 15:43
wrenix pushed a commit to wrenix/dendrite that referenced this pull request Aug 26, 2023
Fixes include:
- Translating state keys that contain user IDs to their respective room
keys for both querying and sending state events
- **NOTE**: there may be design discussion needed on what should happen
when sender keys cannot be found for users
- A simple fix for kicking guests from rooms properly
- Logic for boundary history visibilities was slightly off (I'm
surprised this only manifested in pseudo ID room versions)

Signed-off-by: `Sam Wedgwood <sam@wedgwood.dev>`
@swedgwood swedgwood mentioned this pull request Sep 8, 2023
swedgwood added a commit that referenced this pull request Sep 8, 2023
In a [previous PR](#3181) I
accidentally left GMSL on a dev branch, this PR fixes it by bringing it
back to the main branch of GMSL

Signed-off-by: `Sam Wedgwood <sam@wedgwood.dev>`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants