feat(ffi): Add methods for observing account data changes #4994
Conversation
Signed-off-by: Johannes Marbach <[email protected]>
| }; | ||
| use tracing::info; | ||
|
|
||
| use crate::{ | ||
| error::{ClientError, MediaInfoError}, | ||
| helpers::unwrap_or_clone_arc, | ||
| notification_settings::{Action, PushCondition}, |
There was a problem hiding this comment.
The mapping between matrix-sdk-ffi types and ruma types for pushes is now split between notification_settings.rs and ruma.rs. I wasn't sure if maybe we'd want all of the mappers in ruma.rs?
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4994 +/- ##
=======================================
Coverage 85.86% 85.86%
=======================================
Files 325 325
Lines 35851 35851
=======================================
+ Hits 30783 30785 +2
+ Misses 5068 5066 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| #[derive(Clone, uniffi::Enum)] | ||
| pub enum AccountDataEventType { | ||
| /// m.direct | ||
| Direct, | ||
| /// m.identity_server | ||
| IdentityServer, | ||
| /// m.ignored_user_list | ||
| IgnoredUserList, | ||
| /// m.push_rules | ||
| PushRules, | ||
| /// m.secret_storage.default_key | ||
| SecretStorageDefaultKey, | ||
| /// m.secret_storage.key.* | ||
| SecretStorageKey { key_id: String }, | ||
| } |
There was a problem hiding this comment.
There's a lot of duplication of Ruma types here. I remember the Ruma folks being open towards adding uniffi attributes to Ruma types directly.
It's prbobably a big ask, but would you be open on attempting this?
The account data types might be sufficiently small that this might not be too big of a problem.
There was a problem hiding this comment.
I remember the Ruma folks being open towards adding uniffi attributes to Ruma types directly.
Oh, I wasn't aware. That would actually be amazing. The mapping of Ruma to FFI types adds a lot of boilerplate in many places.
I can try to push things done. It might just take me a little. Just saying in case there's any urgency on #5014 (or its successor).
There was a problem hiding this comment.
I think we can also merge this as is, and if/when uniffi attributes are added to the Ruma type we can just switch over.
There was a problem hiding this comment.
Ok, cool. 👍
I should be able to make the macro change you suggested within the next few days. So this hopefully shouldn't block you much longer.
|
I have a question about this PR since it may be very useful for: #5014 I see that you can subscribe to either stable or unstable content, couldn't the SDK unify the stable and unstable and internally use the appropriate the stable one when available, or how would that be achieved without having to do the check client side? |
Good point. I think we can map both the stable and the unstable type from Ruma to a single FFI type. I can make that change for
Hm, I'm not sure. Maybe we could return |
Err, actually that doesn't work because then you might get two competing updates on the listener when both events exist. I think the logic needs to go down into the SDK. It should only trigger the observer for the unstable event if the stable one doesn't exist. Not sure how hard this would be given the generic way that event listening is set up though. |
Uh oh!
There was an error while loading. Please reload this page.