sdk: Use strongly-typed strings where it makes sense

Where a string is clearly documented as a room ID, event ID or mxc URI,
use OwnedRoomId, OwnedEventId and OwnedMxcURI, respectively.

Signed-off-by: Kévin Commaille <[email protected]>

Author: zecakeh

bindings/matrix-sdk-ffi/src/client.rs

@@ -548,7 +548,7 @@ impl Client {
 
     /// Retrieves an avatar cached from a previous call to [`Self::avatar_url`].
     pub fn cached_avatar_url(&self) -> Result<Option<String>, ClientError> {
-        Ok(RUNTIME.block_on(self.inner.account().get_cached_avatar_url())?)
+        Ok(RUNTIME.block_on(self.inner.account().get_cached_avatar_url())?.map(Into::into))
     }
 
     pub fn device_id(&self) -> Result<String, ClientError> {
@@ -873,11 +873,19 @@ impl Client {
     }
 
     pub async fn get_recently_visited_rooms(&self) -> Result<Vec<String>, ClientError> {
-        Ok(self.inner.account().get_recently_visited_rooms().await?)
+        Ok(self
+            .inner
+            .account()
+            .get_recently_visited_rooms()
+            .await?
+            .into_iter()
+            .map(Into::into)
+            .collect())
     }
 
     pub async fn track_recently_visited_room(&self, room: String) -> Result<(), ClientError> {
-        self.inner.account().track_recently_visited_room(room).await?;
+        let room_id = RoomId::parse(room)?;
+        self.inner.account().track_recently_visited_room(room_id).await?;
         Ok(())
     }
 

bindings/matrix-sdk-ffi/src/room.rs

@@ -4,7 +4,8 @@ use anyhow::{Context, Result};
 use matrix_sdk::{
     event_cache::paginator::PaginatorError,
     room::{power_levels::RoomPowerLevelChanges, Room as SdkRoom, RoomMemberRole},
-    ComposerDraft, RoomHero as SdkRoomHero, RoomMemberships, RoomState,
+    ComposerDraft as SdkComposerDraft, ComposerDraftType as SdkComposerDraftType,
+    RoomHero as SdkRoomHero, RoomMemberships, RoomState,
 };
 use matrix_sdk_ui::timeline::{PaginationError, RoomExt, TimelineFocus};
 use mime::Mime;
@@ -677,12 +678,12 @@ impl Room {
     /// Store the given `ComposerDraft` in the state store using the current
     /// room id, as identifier.
     pub async fn save_composer_draft(&self, draft: ComposerDraft) -> Result<(), ClientError> {
-        Ok(self.inner.save_composer_draft(draft).await?)
+        Ok(self.inner.save_composer_draft(draft.try_into()?).await?)
     }
 
     /// Retrieve the `ComposerDraft` stored in the state store for this room.
     pub async fn load_composer_draft(&self) -> Result<Option<ComposerDraft>, ClientError> {
-        Ok(self.inner.load_composer_draft().await?)
+        Ok(self.inner.load_composer_draft().await?.map(Into::into))
     }
 
     /// Remove the `ComposerDraft` stored in the state store for this room.
@@ -848,3 +849,72 @@ impl From<RtcApplicationType> for notify::ApplicationType {
         }
     }
 }
+
+/// Current draft of the composer for the room.
+#[derive(uniffi::Record)]
+pub struct ComposerDraft {
+    /// The draft content in plain text.
+    pub plain_text: String,
+    /// If the message is formatted in HTML, the HTML representation of the
+    /// message.
+    pub html_text: Option<String>,
+    /// The type of draft.
+    pub draft_type: ComposerDraftType,
+}
+
+impl From<SdkComposerDraft> for ComposerDraft {
+    fn from(value: SdkComposerDraft) -> Self {
+        let SdkComposerDraft { plain_text, html_text, draft_type } = value;
+        Self { plain_text, html_text, draft_type: draft_type.into() }
+    }
+}
+
+impl TryFrom<ComposerDraft> for SdkComposerDraft {
+    type Error = ruma::IdParseError;
+
+    fn try_from(value: ComposerDraft) -> std::result::Result<Self, Self::Error> {
+        let ComposerDraft { plain_text, html_text, draft_type } = value;
+        Ok(Self { plain_text, html_text, draft_type: draft_type.try_into()? })
+    }
+}
+
+/// The type of draft of the composer.
+#[derive(uniffi::Enum)]
+pub enum ComposerDraftType {
+    /// The draft is a new message.
+    NewMessage,
+    /// The draft is a reply to an event.
+    Reply {
+        /// The ID of the event being replied to.
+        event_id: String,
+    },
+    /// The draft is an edit of an event.
+    Edit {
+        /// The ID of the event being edited.
+        event_id: String,
+    },
+}
+
+impl From<SdkComposerDraftType> for ComposerDraftType {
+    fn from(value: SdkComposerDraftType) -> Self {
+        match value {
+            SdkComposerDraftType::NewMessage => Self::NewMessage,
+            SdkComposerDraftType::Reply { event_id } => Self::Reply { event_id: event_id.into() },
+            SdkComposerDraftType::Edit { event_id } => Self::Edit { event_id: event_id.into() },
+        }
+    }
+}
+
+impl TryFrom<ComposerDraftType> for SdkComposerDraftType {
+    type Error = ruma::IdParseError;
+
+    fn try_from(value: ComposerDraftType) -> std::result::Result<Self, Self::Error> {
+        let draft_type = match value {
+            ComposerDraftType::NewMessage => Self::NewMessage,
+            ComposerDraftType::Reply { event_id } => Self::Reply { event_id: event_id.try_into()? },
+            ComposerDraftType::Edit { event_id } => Self::Edit { event_id: event_id.try_into()? },
+        };
+
+        Ok(draft_type)
+    }
+}

crates/matrix-sdk-base/src/lib.rs

@@ -56,7 +56,8 @@ pub use rooms::{
     RoomMember, RoomMemberships, RoomState, RoomStateFilter,
 };
 pub use store::{
-    ComposerDraft, StateChanges, StateStore, StateStoreDataKey, StateStoreDataValue, StoreError,
+    ComposerDraft, ComposerDraftType, StateChanges, StateStore, StateStoreDataKey,
+    StateStoreDataValue, StoreError,
 };
 pub use utils::{
     MinimalRoomMemberEvent, MinimalStateEvent, OriginalMinimalStateEvent, RedactedMinimalStateEvent,

crates/matrix-sdk-base/src/store/integration_tests.rs

@@ -28,7 +28,7 @@ use ruma::{
         AnySyncStateEvent, GlobalAccountDataEventType, RoomAccountDataEventType, StateEventType,
         SyncStateEvent,
     },
-    mxc_uri, room_id,
+    mxc_uri, owned_mxc_uri, room_id,
     serde::Raw,
     uint, user_id, EventId, OwnedEventId, OwnedUserId, RoomId, TransactionId, UserId,
 };
@@ -548,11 +548,11 @@ impl StateStoreIntegrationTests for DynStateStore {
 
     async fn test_user_avatar_url_saving(&self) {
         let user_id = user_id!("@alice:example.org");
-        let url = "https://example.org";
+        let url = owned_mxc_uri!("mxc://example.org/poiuyt098");
 
         self.set_kv_data(
             StateStoreDataKey::UserAvatarUrl(user_id),
-            StateStoreDataValue::UserAvatarUrl(url.to_owned()),
+            StateStoreDataValue::UserAvatarUrl(url.clone()),
         )
         .await
         .unwrap();

crates/matrix-sdk-base/src/store/memory_store.rs

@@ -52,9 +52,9 @@ use crate::{
 #[allow(clippy::type_complexity)]
 #[derive(Debug)]
 pub struct MemoryStore {
-    recently_visited_rooms: StdRwLock<HashMap<String, Vec<String>>>,
+    recently_visited_rooms: StdRwLock<HashMap<OwnedUserId, Vec<OwnedRoomId>>>,
     composer_drafts: StdRwLock<HashMap<OwnedRoomId, ComposerDraft>>,
-    user_avatar_url: StdRwLock<HashMap<String, String>>,
+    user_avatar_url: StdRwLock<HashMap<OwnedUserId, OwnedMxcUri>>,
     sync_token: StdRwLock<Option<String>>,
     filters: StdRwLock<HashMap<String, String>>,
     utd_hook_manager_data: StdRwLock<Option<GrowableBloom>>,
@@ -186,14 +186,14 @@ impl StateStore for MemoryStore {
                 .user_avatar_url
                 .read()
                 .unwrap()
-                .get(user_id.as_str())
+                .get(user_id)
                 .cloned()
                 .map(StateStoreDataValue::UserAvatarUrl),
             StateStoreDataKey::RecentlyVisitedRooms(user_id) => self
                 .recently_visited_rooms
                 .read()
                 .unwrap()
-                .get(user_id.as_str())
+                .get(user_id)
                 .cloned()
                 .map(StateStoreDataValue::RecentlyVisitedRooms),
             StateStoreDataKey::UtdHookManagerData => self
@@ -230,13 +230,13 @@ impl StateStore for MemoryStore {
             }
             StateStoreDataKey::UserAvatarUrl(user_id) => {
                 self.user_avatar_url.write().unwrap().insert(
-                    user_id.to_string(),
+                    user_id.to_owned(),
                     value.into_user_avatar_url().expect("Session data not a user avatar url"),
                 );
             }
             StateStoreDataKey::RecentlyVisitedRooms(user_id) => {
                 self.recently_visited_rooms.write().unwrap().insert(
-                    user_id.to_string(),
+                    user_id.to_owned(),
                     value
                         .into_recently_visited_rooms()
                         .expect("Session data not a list of recently visited rooms"),
@@ -267,10 +267,10 @@ impl StateStore for MemoryStore {
                 self.filters.write().unwrap().remove(filter_name);
             }
             StateStoreDataKey::UserAvatarUrl(user_id) => {
-                self.user_avatar_url.write().unwrap().remove(user_id.as_str());
+                self.user_avatar_url.write().unwrap().remove(user_id);
             }
             StateStoreDataKey::RecentlyVisitedRooms(user_id) => {
-                self.recently_visited_rooms.write().unwrap().remove(user_id.as_str());
+                self.recently_visited_rooms.write().unwrap().remove(user_id);
             }
             StateStoreDataKey::UtdHookManagerData => {
                 *self.utd_hook_manager_data.write().unwrap() = None

crates/matrix-sdk-base/src/store/traits.rs

@@ -34,8 +34,8 @@ use ruma::{
         StateEventType, StaticEventContent, StaticStateEventContent,
     },
     serde::Raw,
-    EventId, MxcUri, OwnedEventId, OwnedRoomId, OwnedTransactionId, OwnedUserId, RoomId,
-    TransactionId, UserId,
+    EventId, MxcUri, OwnedEventId, OwnedMxcUri, OwnedRoomId, OwnedTransactionId, OwnedUserId,
+    RoomId, TransactionId, UserId,
 };
 use serde::{Deserialize, Serialize};
 
@@ -914,10 +914,10 @@ pub enum StateStoreDataValue {
     Filter(String),
 
     /// The user avatar url
-    UserAvatarUrl(String),
+    UserAvatarUrl(OwnedMxcUri),
 
     /// A list of recently visited room identifiers for the current user
-    RecentlyVisitedRooms(Vec<String>),
+    RecentlyVisitedRooms(Vec<OwnedRoomId>),
 
     /// Persistent data for
     /// `matrix_sdk_ui::unable_to_decrypt_hook::UtdHookManager`.
@@ -932,7 +932,6 @@ pub enum StateStoreDataValue {
 
 /// Current draft of the composer for the room.
 #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
-#[cfg_attr(feature = "uniffi", derive(uniffi::Record))]
 pub struct ComposerDraft {
     /// The draft content in plain text.
     pub plain_text: String,
@@ -945,19 +944,18 @@ pub struct ComposerDraft {
 
 /// The type of draft of the composer.
 #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
-#[cfg_attr(feature = "uniffi", derive(uniffi::Enum))]
 pub enum ComposerDraftType {
     /// The draft is a new message.
     NewMessage,
     /// The draft is a reply to an event.
     Reply {
         /// The ID of the event being replied to.
-        event_id: String,
+        event_id: OwnedEventId,
     },
     /// The draft is an edit of an event.
     Edit {
         /// The ID of the event being edited.
-        event_id: String,
+        event_id: OwnedEventId,
     },
 }
 
@@ -973,12 +971,12 @@ impl StateStoreDataValue {
     }
 
     /// Get this value if it is a user avatar url.
-    pub fn into_user_avatar_url(self) -> Option<String> {
+    pub fn into_user_avatar_url(self) -> Option<OwnedMxcUri> {
         as_variant!(self, Self::UserAvatarUrl)
     }
 
     /// Get this value if it is a list of recently visited rooms.
-    pub fn into_recently_visited_rooms(self) -> Option<Vec<String>> {
+    pub fn into_recently_visited_rooms(self) -> Option<Vec<OwnedRoomId>> {
         as_variant!(self, Self::RecentlyVisitedRooms)
     }
 

crates/matrix-sdk-indexeddb/src/state_store/mod.rs

@@ -44,8 +44,8 @@ use ruma::{
         GlobalAccountDataEventType, RoomAccountDataEventType, StateEventType, SyncStateEvent,
     },
     serde::Raw,
-    CanonicalJsonObject, EventId, MxcUri, OwnedEventId, OwnedRoomId, OwnedTransactionId,
-    OwnedUserId, RoomId, RoomVersionId, TransactionId, UserId,
+    CanonicalJsonObject, EventId, MxcUri, OwnedEventId, OwnedMxcUri, OwnedRoomId,
+    OwnedTransactionId, OwnedUserId, RoomId, RoomVersionId, TransactionId, UserId,
 };
 use serde::{de::DeserializeOwned, Deserialize, Serialize};
 use tracing::{debug, warn};
@@ -471,11 +471,11 @@ impl_state_store!({
                 .transpose()?
                 .map(StateStoreDataValue::Filter),
             StateStoreDataKey::UserAvatarUrl(_) => value
-                .map(|f| self.deserialize_value::<String>(&f))
+                .map(|f| self.deserialize_value::<OwnedMxcUri>(&f))
                 .transpose()?
                 .map(StateStoreDataValue::UserAvatarUrl),
             StateStoreDataKey::RecentlyVisitedRooms(_) => value
-                .map(|f| self.deserialize_value::<Vec<String>>(&f))
+                .map(|f| self.deserialize_value::<Vec<OwnedRoomId>>(&f))
                 .transpose()?
                 .map(StateStoreDataValue::RecentlyVisitedRooms),
             StateStoreDataKey::UtdHookManagerData => value

crates/matrix-sdk/src/account.rs

@@ -44,7 +44,7 @@ use ruma::{
     push::Ruleset,
     serde::Raw,
     thirdparty::Medium,
-    ClientSecret, MxcUri, OwnedMxcUri, OwnedUserId, RoomId, SessionId, UInt, UserId,
+    ClientSecret, MxcUri, OwnedMxcUri, OwnedRoomId, OwnedUserId, RoomId, SessionId, UInt, UserId,
 };
 use serde::Deserialize;
 use tracing::error;
@@ -155,7 +155,7 @@ impl Account {
                 .store()
                 .set_kv_data(
                     StateStoreDataKey::UserAvatarUrl(user_id),
-                    StateStoreDataValue::UserAvatarUrl(url.to_string()),
+                    StateStoreDataValue::UserAvatarUrl(url),
                 )
                 .await;
         } else {
@@ -167,7 +167,7 @@ impl Account {
     }
 
     /// Get the URL of the account's avatar, if is stored in cache.
-    pub async fn get_cached_avatar_url(&self) -> Result<Option<String>> {
+    pub async fn get_cached_avatar_url(&self) -> Result<Option<OwnedMxcUri>> {
         let user_id = self.client.user_id().ok_or(Error::AuthenticationRequired)?;
         let data =
             self.client.store().get_kv_data(StateStoreDataKey::UserAvatarUrl(user_id)).await?;
@@ -927,7 +927,7 @@ impl Account {
     }
 
     /// Retrieves the user's recently visited room list
-    pub async fn get_recently_visited_rooms(&self) -> Result<Vec<String>> {
+    pub async fn get_recently_visited_rooms(&self) -> Result<Vec<OwnedRoomId>> {
         let user_id = self.client.user_id().ok_or(Error::AuthenticationRequired)?;
         let data = self
             .client
@@ -944,14 +944,9 @@ impl Account {
     }
 
     /// Moves/inserts the given room to the front of the recently visited list
-    pub async fn track_recently_visited_room(&self, room_id: String) -> Result<(), Error> {
+    pub async fn track_recently_visited_room(&self, room_id: OwnedRoomId) -> Result<(), Error> {
         let user_id = self.client.user_id().ok_or(Error::AuthenticationRequired)?;
 
-        if let Err(error) = RoomId::parse(&room_id) {
-            error!("Invalid room id: {}", error);
-            return Err(Error::Identifier(error));
-        }
-
         // Get the previously stored recently visited rooms
         let mut recently_visited_rooms = self.get_recently_visited_rooms().await?;