Make callback interactions with ClientSessionDelegate async #4549
Conversation
|
I do not understand the compilation error, it seems to only be flagged in tests, and I can't figure out how to run the failing step locally, making debugging very challenging. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4549 +/- ##
=======================================
Coverage 85.40% 85.40%
=======================================
Files 285 285
Lines 32213 32216 +3
=======================================
+ Hits 27511 27514 +3
Misses 4702 4702 ☔ View full report in Codecov by Sentry. |
|
Maybe it's because the complement-crypto tests use a quite outdated version of UniFFI (0.25.0, if I'm not mistaken). I can confirm just building the usual bindings works fine, but if I run the build_rust_sdk.sh script locally to build the go bindings I get the same error as the CI. I'm not sure if it's possible to upgrade the go bindings version in the complement-crypto codebase though 🫤 . |
| @@ -153,9 +154,13 @@ pub trait ClientDelegate: Sync + Send { | |||
| } | |||
|
|
|||
| #[matrix_sdk_ffi_macros::export(callback_interface)] | |||
| #[async_trait::async_trait] | |||
There was a problem hiding this comment.
Do we still need async_trait with our minimum required version of Rust? I'm not sure.
| } | ||
| } | ||
|
|
||
| struct FfiSaveSessionCallback { |
There was a problem hiding this comment.
Please, don't use an Ffi prefix for type names. If it collides with the SDK names, you can rename the SDK names while importing names, by adding an Sdk prefix, as we do in other modules of this crate. Thus:
use matrix_sdk::authentication::SaveSessionCallback as SdkSaveSessionCallback;
|
Forgot to say: Thanks for your contribution! |
See issue: #4516
The client we are using the matrix-rust-sdk (react-native) does not offer synchronous access to the system keychain. In order to make this easier, this change adjusts ClientSessionDelegate to be a CallbackInterface with async methods for storing and retrieving. Retrieving is the only on that strictly speaking needs to be async, but it felt like symmetry was warranted here in case the save operation were to fail at the system level.
Public API changes documented in changelogs (optional)
Signed-off-by: Daniel Salinas