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

crypto: Update SenderData on /keys/query response #3849

Merged
merged 6 commits into from
Sep 4, 2024

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Aug 16, 2024

Fixes #3753

Builds on #3806, #3920, #3921

@richvdh richvdh force-pushed the andyrich/sender-data-from-keys-query branch 5 times, most recently from b40fefe to d785bb8 Compare August 29, 2024 10:15
@richvdh richvdh force-pushed the rav/update_on_keys_query branch from b1e0aca to 92950d7 Compare August 29, 2024 11:18
@richvdh richvdh force-pushed the andyrich/sender-data-from-keys-query branch 2 times, most recently from 99f672e to 7929242 Compare August 30, 2024 09:21
@richvdh richvdh force-pushed the rav/update_on_keys_query branch 2 times, most recently from be0b80f to c332fa2 Compare August 30, 2024 16:10
@richvdh richvdh changed the title WIP: Update SenderData on /keys/query response Update SenderData on /keys/query response Aug 30, 2024
@richvdh richvdh marked this pull request as ready for review September 2, 2024 10:47
@richvdh richvdh requested review from a team as code owners September 2, 2024 10:47
@richvdh richvdh requested review from stefanceriu and andybalaam and removed request for a team September 2, 2024 10:47
@stefanceriu stefanceriu removed their request for review September 2, 2024 10:48
@richvdh richvdh changed the base branch from andyrich/sender-data-from-keys-query to main September 2, 2024 11:31
@richvdh richvdh force-pushed the rav/update_on_keys_query branch from c332fa2 to 09e77a4 Compare September 2, 2024 12:18
Copy link

codecov bot commented Sep 2, 2024

Codecov Report

Attention: Patch coverage is 89.18919% with 4 lines in your changes missing coverage. Please review.

Project coverage is 84.15%. Comparing base (d8b0f9f) to head (dbc852b).
Report is 23 commits behind head on main.

Files with missing lines Patch % Lines
crates/matrix-sdk-crypto/src/identities/manager.rs 88.23% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3849      +/-   ##
==========================================
- Coverage   84.16%   84.15%   -0.01%     
==========================================
  Files         267      267              
  Lines       28107    28140      +33     
==========================================
+ Hits        23656    23681      +25     
- Misses       4451     4459       +8     

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

@richvdh richvdh force-pushed the rav/update_on_keys_query branch 3 times, most recently from 3befbec to 0e295aa Compare September 3, 2024 08:48
@bnjbvr bnjbvr changed the title Update SenderData on /keys/query response crypto: Update SenderData on /keys/query response Sep 3, 2024
Copy link
Member

@andybalaam andybalaam left a comment

Choose a reason for hiding this comment

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

One conversation to be had about unbounded memory usage; all good otherwise!

crates/matrix-sdk-crypto/src/identities/manager.rs Outdated Show resolved Hide resolved
crates/matrix-sdk-crypto/src/identities/manager.rs Outdated Show resolved Hide resolved
crates/matrix-sdk-crypto/src/identities/manager.rs Outdated Show resolved Hide resolved
crates/matrix-sdk-crypto/src/identities/manager.rs Outdated Show resolved Hide resolved
crates/matrix-sdk-crypto/src/identities/manager.rs Outdated Show resolved Hide resolved
Copy link
Member

@andybalaam andybalaam left a comment

Choose a reason for hiding this comment

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

Great, thanks!

We need write access to this in the integration tests
If the previous session is removed from the list, we should still be able to
continue iterating through the *rest* of the list.
This module has a number of useful types (in particular, error types). Rather
than addding even more types to the top level module, let's export the
`sender_data_finder` module as a whole.
When we receive an `/keys/query` response, look for existing
inboundgroupsessions created by updated devices, and see if we can update any
of their senderdata settings.
Extend the integration tests for megolm sender data to check that we update
existing inbound group sessions when we get a `/keys/query` response.
@richvdh richvdh force-pushed the rav/update_on_keys_query branch from edfc611 to dbc852b Compare September 4, 2024 14:25
@richvdh richvdh merged commit f7ee643 into main Sep 4, 2024
38 checks passed
@richvdh richvdh deleted the rav/update_on_keys_query branch September 4, 2024 15:07
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.

Sender Data: Populate missing SenderData when we receive device info via /keys/query
2 participants