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

ffi: Expose encryption settings via FFI #3326

Merged
merged 1 commit into from
Apr 25, 2024

Conversation

andybalaam
Copy link
Contributor

@andybalaam andybalaam commented Apr 11, 2024

Fixes #3130

This allows providing encryption settings while constructing a client via FFI, instead of hard-coding specific values.

We do it by holding on to an EncryptionSettings inside ClientBuilder and updating it when certain methods on the builder are called. During build_inner() we provide those EncryptionSettings by calling with_encryption_settings on inner_builder.

I added an enum BackupDownloadStrategy to be passed as an argument to backup_download_strategy. Let me know if there's a better way.

@andybalaam andybalaam requested a review from a team as a code owner April 11, 2024 16:08
@andybalaam andybalaam requested review from bnjbvr and removed request for a team April 11, 2024 16:09
Copy link

codecov bot commented Apr 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.67%. Comparing base (7c68096) to head (ebcf1c4).
Report is 38 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3326      +/-   ##
==========================================
+ Coverage   83.65%   83.67%   +0.01%     
==========================================
  Files         238      238              
  Lines       24668    24668              
==========================================
+ Hits        20637    20640       +3     
+ Misses       4031     4028       -3     

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

@andybalaam andybalaam marked this pull request as draft April 12, 2024 08:51
Copy link
Member

@bnjbvr bnjbvr left a comment

Choose a reason for hiding this comment

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

Oh I see the PR is marked as draft, so I'll do another round of review when ready.

@andybalaam andybalaam force-pushed the andybalaam/allow-setting-encryption-settings branch from 875a1cf to 516e9a1 Compare April 12, 2024 09:36
@andybalaam andybalaam changed the title ffi: Expose with_encryption_settings via FFI ffi: Expose encryption settings via FFI Apr 12, 2024
@andybalaam andybalaam marked this pull request as ready for review April 12, 2024 13:29
@andybalaam andybalaam requested a review from bnjbvr April 12, 2024 13:29
@andybalaam andybalaam marked this pull request as draft April 12, 2024 13:30
@andybalaam andybalaam force-pushed the andybalaam/allow-setting-encryption-settings branch 2 times, most recently from c5f9207 to 0f86532 Compare April 12, 2024 13:50
@andybalaam andybalaam marked this pull request as ready for review April 12, 2024 13:52
bnjbvr
bnjbvr previously approved these changes Apr 15, 2024
bindings/matrix-sdk-ffi/src/client_builder.rs Outdated Show resolved Hide resolved
bindings/matrix-sdk-ffi/src/client_builder.rs Outdated Show resolved Hide resolved
bindings/matrix-sdk-ffi/src/client_builder.rs Outdated Show resolved Hide resolved
@andybalaam andybalaam force-pushed the andybalaam/allow-setting-encryption-settings branch from 71c6878 to 7c2d77c Compare April 15, 2024 16:12
@andybalaam andybalaam force-pushed the andybalaam/allow-setting-encryption-settings branch from 7c2d77c to e22a7c7 Compare April 17, 2024 11:52
@andybalaam andybalaam force-pushed the andybalaam/allow-setting-encryption-settings branch from e22a7c7 to ebcf1c4 Compare April 23, 2024 14:45
@bnjbvr bnjbvr dismissed their stale review April 23, 2024 14:52

Review was one week ago, and there's been many pushes since then

@andybalaam andybalaam requested a review from poljar April 24, 2024 11:55
@andybalaam andybalaam merged commit 2c7afc2 into main Apr 25, 2024
35 checks passed
@andybalaam andybalaam deleted the andybalaam/allow-setting-encryption-settings branch April 25, 2024 07:55
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.

FFI: BackupDownloadStrategy cannot be set in ClientBuilder
3 participants