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

refactor(sdk-crypto): Room key sharing, introduce extensible strategy #3605

Merged
merged 6 commits into from
Jun 25, 2024

Conversation

BillCarsonFr
Copy link
Member

Fixes: #3562

Extracted the logic to collect the devices that should receive a room key.
Introduce a new CollectStrategy that will define how to sort devices that should receive the room_key and those that should receive a witheld code.

There is no functional changes in this PR, a following PR will introduce an IdentityBased strategy that will select devices depending on if they are signed by their owner.

  • Public API changes documented in changelogs (optional)

Signed-off-by:

@BillCarsonFr BillCarsonFr marked this pull request as ready for review June 25, 2024 09:32
@BillCarsonFr BillCarsonFr requested a review from a team as a code owner June 25, 2024 09:32
@BillCarsonFr BillCarsonFr requested review from andybalaam and removed request for a team June 25, 2024 09:32
Copy link

codecov bot commented Jun 25, 2024

Codecov Report

Attention: Patch coverage is 79.03226% with 13 lines in your changes missing coverage. Please review.

Project coverage is 83.86%. Comparing base (b34a2cd) to head (ca6537b).
Report is 73 commits behind head on main.

Files Patch % Lines
...c/session_manager/group_sessions/share_strategy.rs 77.96% 13 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3605      +/-   ##
==========================================
+ Coverage   83.84%   83.86%   +0.02%     
==========================================
  Files         253      254       +1     
  Lines       25884    25892       +8     
==========================================
+ Hits        21702    21714      +12     
+ Misses       4182     4178       -4     

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

@BillCarsonFr BillCarsonFr requested a review from poljar June 25, 2024 10:06
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.

Left a small nit about our module convention, looks good otherwise.

@@ -12,23 +12,25 @@
// See the License for the specific language governing permissions and
// limitations under the License.

mod share_strategy;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please run:

$ git mv crates/matrix-sdk-crypto/src/session_manager/group_sessions.rs crates/matrix-sdk-crypto/src/session_manager/group_sessions/mod.rs

to rename and move group_sessions.rs file into the group_sessions folder, we're using this module convention across all our crates. For more info about Rust modules check out: https://doc.rust-lang.org/reference/items/modules.html.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you need a small change in that file after it was moved, CI will let us know.

@poljar poljar enabled auto-merge June 25, 2024 14:53
@poljar poljar merged commit 3be84a5 into main Jun 25, 2024
38 checks passed
@poljar poljar deleted the valere/invisible_crypto/refactor_share_2 branch June 25, 2024 14:54
@richvdh
Copy link
Member

richvdh commented Jul 9, 2024

Please do remember to update the changelog when landing changes like this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants