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

Groundwork for fixes to one-time-key fetching #2803

Merged
merged 6 commits into from
Nov 3, 2023
Merged

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Nov 1, 2023

Some non-functional refactoring in preparation for fixing #281. Suggest review commit-by-commit. The salient changes are:

  • Account::create_outbound_session no longer takes an entire list of keys; rather it takes a single key and it is up to the caller to pick a key out of the list. This in turn means that SessionCreationError loses one of its reason codes.

  • SessionManager::create_sessions is factored out of receive_keys_claim_response. This is mostly helpful in tests, where our goal is to establish a session with a given device, rather than actually to test parsing of the entire /keys/claim response.

@richvdh richvdh requested a review from a team as a code owner November 1, 2023 15:58
@richvdh richvdh requested review from poljar and removed request for a team November 1, 2023 15:58
`Account::create_outbound_session` no longer takes an entire list of keys;
rather it takes a single key and it is up to the caller to pick a key out of
the list.

This in turn means that `SessionCreationError` loses one of its reason codes.
... and use it in some tests.

Simplify some of the test code by not building a whole keys/claim response.
Copy link

codecov bot commented Nov 1, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f945a50) 81.39% compared to head (fdcbd84) 81.73%.
Report is 28 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2803      +/-   ##
==========================================
+ Coverage   81.39%   81.73%   +0.33%     
==========================================
  Files         204      204              
  Lines       20998    21008      +10     
==========================================
+ Hits        17092    17171      +79     
+ Misses       3906     3837      -69     
Files Coverage Δ
crates/matrix-sdk-crypto/src/dehydrated_devices.rs 100.00% <ø> (ø)
crates/matrix-sdk-crypto/src/error.rs 0.00% <ø> (ø)
crates/matrix-sdk-crypto/src/machine.rs 84.40% <100.00%> (+0.12%) ⬆️
crates/matrix-sdk-crypto/src/olm/account.rs 88.19% <100.00%> (+0.65%) ⬆️
.../matrix-sdk-crypto/src/session_manager/sessions.rs 91.90% <100.00%> (+3.29%) ⬆️

... and 19 files with indirect coverage changes

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

Copy link
Contributor

@poljar poljar left a comment

Choose a reason for hiding this comment

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

Most of the commits here look fine, though I don't understand the final one.

@richvdh richvdh requested a review from poljar November 2, 2023 15:49
@poljar poljar merged commit c2f4222 into main Nov 3, 2023
35 checks passed
@poljar poljar deleted the rav/claim_keys_prep branch November 3, 2023 09:43
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.

2 participants