From 843faee0d9489754827ebcbde2808daacb6e6424 Mon Sep 17 00:00:00 2001 From: Mojtaba Chenani Date: Tue, 14 Jan 2025 18:25:12 +0100 Subject: [PATCH 01/20] wip --- bindings_ffi/src/mls.rs | 186 +++++++++++++++++- xmtp_mls/src/groups/device_sync.rs | 119 +++++++++++ xmtp_mls/src/groups/group_mutable_metadata.rs | 30 ++- xmtp_mls/src/groups/intents.rs | 28 ++- xmtp_mls/src/groups/mod.rs | 179 ++++++++++++++++- .../storage/encrypted_store/group_message.rs | 61 +++++- 6 files changed, 572 insertions(+), 31 deletions(-) diff --git a/bindings_ffi/src/mls.rs b/bindings_ffi/src/mls.rs index 5deae3dac..37d5e5bec 100644 --- a/bindings_ffi/src/mls.rs +++ b/bindings_ffi/src/mls.rs @@ -19,6 +19,7 @@ use xmtp_id::{ InboxId, }; use xmtp_mls::groups::device_sync::preference_sync::UserPreferenceUpdate; +use xmtp_mls::groups::group_mutable_metadata::GroupMessageExpirationSettings; use xmtp_mls::groups::scoped_client::LocalScopedGroupClient; use xmtp_mls::groups::HmacKey; use xmtp_mls::storage::group::ConversationType; @@ -1223,6 +1224,21 @@ impl FfiConversationListItem { } } +#[derive(uniffi::Record, Debug)] +pub struct FfiGroupMessageExpirationSettings { + pub expire_from_ms: i64, + pub expire_in_ms: i64, +} + +impl FfiGroupMessageExpirationSettings { + fn new(expire_from_ms: i64, expire_in_ms: i64) -> Self { + Self { + expire_from_ms, + expire_in_ms, + } + } +} + impl From> for FfiConversation { fn from(mls_group: MlsGroup) -> FfiConversation { FfiConversation { inner: mls_group } @@ -1333,6 +1349,12 @@ impl From for SortDirection { } } +impl From for GroupMessageExpirationSettings { + fn from(settings: FfiGroupMessageExpirationSettings) -> Self { + GroupMessageExpirationSettings::new(settings.expire_from_ms, settings.expire_in_ms) + } +} + #[derive(uniffi::Record, Clone, Default)] pub struct FfiListMessagesOptions { pub sent_before_ns: Option, @@ -1388,13 +1410,23 @@ pub struct FfiCreateGroupOptions { impl FfiCreateGroupOptions { pub fn into_group_metadata_options(self) -> GroupMetadataOptions { + let message_retention_settings: Option = + if let (Some(message_expiration_from_ms), Some(message_expiration_ms)) = + (self.message_expiration_from_ms, self.message_expiration_ms) + { + Some(GroupMessageExpirationSettings::new( + message_expiration_from_ms, + message_expiration_ms, + )) + } else { + None + }; GroupMetadataOptions { name: self.group_name, image_url_square: self.group_image_url_square, description: self.group_description, pinned_frame_url: self.group_pinned_frame_url, - message_expiration_from_ms: self.message_expiration_from_ms, - message_expiration_ms: self.message_expiration_ms, + message_retention_settings, } } } @@ -1625,6 +1657,32 @@ impl FfiConversation { .map_err(Into::into) } + pub async fn update_group_message_expiration_settings( + &self, + settings: FfiGroupMessageExpirationSettings, + ) -> Result<(), GenericError> { + self.inner + .update_group_message_expiration_settings(GroupMessageExpirationSettings::from( + settings, + )) + .await?; + + Ok(()) + } + + pub fn group_message_expiration_settings( + &self, + ) -> Result { + let provider = self.inner.mls_provider()?; + let group_message_expiration_settings = + self.inner.group_message_expiration_settings(&provider)?; // Use `?` to handle the Result + + Ok(FfiGroupMessageExpirationSettings::new( + group_message_expiration_settings.expire_from_ms, + group_message_expiration_settings.expire_in_ms, + )) + } + pub fn admin_list(&self) -> Result, GenericError> { let provider = self.inner.mls_provider()?; self.inner.admin_list(&provider).map_err(Into::into) @@ -2161,10 +2219,11 @@ mod tests { connect_to_backend, decode_reaction, encode_reaction, get_inbox_id_for_address, inbox_owner::SigningError, FfiConsent, FfiConsentEntityType, FfiConsentState, FfiContentType, FfiConversation, FfiConversationCallback, FfiConversationMessageKind, - FfiCreateGroupOptions, FfiDirection, FfiGroupPermissionsOptions, FfiInboxOwner, - FfiListConversationsOptions, FfiListMessagesOptions, FfiMessageWithReactions, - FfiMetadataField, FfiPermissionPolicy, FfiPermissionPolicySet, FfiPermissionUpdateType, - FfiReaction, FfiReactionAction, FfiReactionSchema, FfiSubscribeError, + FfiCreateGroupOptions, FfiDirection, FfiGroupMessageExpirationSettings, + FfiGroupPermissionsOptions, FfiInboxOwner, FfiListConversationsOptions, + FfiListMessagesOptions, FfiMessageWithReactions, FfiMetadataField, FfiPermissionPolicy, + FfiPermissionPolicySet, FfiPermissionUpdateType, FfiReaction, FfiReactionAction, + FfiReactionSchema, FfiSubscribeError, }; use ethers::utils::hex; use prost::Message; @@ -2584,6 +2643,8 @@ mod tests { } use xmtp_cryptography::utils::generate_local_wallet; + use xmtp_mls::groups::group_mutable_metadata::{GroupMessageExpirationSettings, MetadataField}; + use xmtp_mls::groups::{GroupMetadataOptions, PreconfiguredPolicies}; #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn test_can_add_wallet_to_inbox() { @@ -2856,6 +2917,8 @@ mod tests { let amal = new_test_client().await; let bola = new_test_client().await; + let group_message_expiration_settings = FfiGroupMessageExpirationSettings::new(10, 100); + let group = amal .conversations() .create_group( @@ -2867,8 +2930,10 @@ mod tests { group_description: Some("group description".to_string()), group_pinned_frame_url: Some("pinned frame".to_string()), custom_permission_policy_set: None, - message_expiration_from_ms: None, - message_expiration_ms: None, + message_expiration_from_ms: Some( + group_message_expiration_settings.expire_from_ms, + ), + message_expiration_ms: Some(group_message_expiration_settings.expire_in_ms), }, ) .await @@ -2880,8 +2945,26 @@ mod tests { assert_eq!(group.group_image_url_square().unwrap(), "url"); assert_eq!(group.group_description().unwrap(), "group description"); assert_eq!(group.group_pinned_frame_url().unwrap(), "pinned frame"); + assert_eq!(group.group_pinned_frame_url().unwrap(), "pinned frame"); + assert_eq!( + group + .group_message_expiration_settings() + .unwrap() + .expire_from_ms, + group_message_expiration_settings.expire_from_ms + ); + assert_eq!( + group + .group_message_expiration_settings() + .unwrap() + .expire_in_ms, + group_message_expiration_settings.expire_in_ms + ); } + //test to stream the metadata + //test the policy if someone can change the settings + // Looks like this test might be a separate issue #[tokio::test(flavor = "multi_thread", worker_threads = 5)] async fn test_can_stream_group_messages_for_updates() { @@ -4692,6 +4775,93 @@ mod tests { assert_eq!(bola_conversations.len(), 1); } + // #[tokio::test(flavor = "multi_thread", worker_threads = 5)] + // async fn test_groups_with_expiration_settings() { + // // Step 1: Setup test clients + // let amal = new_test_client().await; + // + // // Step 2: Create policy set and groups + // let policy_set = Some(PreconfiguredPolicies::AdminsOnly.to_policy_set()); + // let group1 = amal + // .inner_client + // .create_group(policy_set.clone(), GroupMetadataOptions::default()) + // .expect("Failed to create group1"); + // let group2 = amal + // .inner_client + // .create_group(policy_set.clone(), GroupMetadataOptions::default()) + // .expect("Failed to create group2"); + // let group3 = amal + // .inner_client + // .create_group(policy_set, GroupMetadataOptions::default()) + // .expect("Failed to create group3"); + // + // // Sync groups + // group1.sync().await.expect("Failed to sync group1"); + // group2.sync().await.expect("Failed to sync group2"); + // group3.sync().await.expect("Failed to sync group3"); + // + // // Step 3: Verify metadata and set expiration for group1 + // let group1_metadata = group1 + // .mutable_metadata(&group1.mls_provider().expect("Missing MLS provider")) + // .expect("Failed to fetch metadata for group1"); + // assert_eq!(group1_metadata.attributes.len(), 4); + // assert!(group1_metadata + // .attributes + // .get(&MetadataField::GroupName.to_string()) + // .expect("Missing group name field") + // .is_empty()); + // + // // Enable expiration settings for group1 + // let expiration_settings = GroupMessageExpirationSettings { + // expire_from_ms: 1000, + // expire_in_ms: 1000, + // }; + // group1 + // .update_group_message_expiration_settings(expiration_settings) + // .await + // .expect("Failed to update expiration settings for group1"); + // + // // Verify only group1 is returned + // let groups_with_expiration = amal + // .conversations() + // .get_groups_with_expiration_enabled() + // .expect("Failed to fetch groups with expiration enabled"); + // assert_eq!(groups_with_expiration.len(), 1); + // assert!(groups_with_expiration.contains(&group1.group_id)); + // + // // Step 4: Remove expiration settings from group1 + // group1 + // .remove_group_message_expiration_settings() + // .await + // .expect("Failed to remove expiration settings for group1"); + // + // // Verify no groups are returned + // let groups_with_expiration = amal + // .conversations() + // .get_groups_with_expiration_enabled() + // .expect("Failed to fetch groups with expiration enabled"); + // assert_eq!(groups_with_expiration.len(), 0); + // + // // Step 5: Enable expiration settings for group1 and group2 + // group1 + // .update_group_message_expiration_settings(expiration_settings) + // .await + // .expect("Failed to update expiration settings for group1"); + // group2 + // .update_group_message_expiration_settings(expiration_settings) + // .await + // .expect("Failed to update expiration settings for group2"); + // + // // Verify group1 and group2 are returned + // let groups_with_expiration = amal + // .conversations() + // .get_groups_with_expiration_enabled() + // .expect("Failed to fetch groups with expiration enabled"); + // assert_eq!(groups_with_expiration.len(), 2); + // assert!(groups_with_expiration.contains(&group1.group_id)); + // assert!(groups_with_expiration.contains(&group2.group_id)); + // } + #[tokio::test(flavor = "multi_thread", worker_threads = 5)] async fn test_dm_streaming() { let alix = new_test_client().await; diff --git a/xmtp_mls/src/groups/device_sync.rs b/xmtp_mls/src/groups/device_sync.rs index 734aad132..58c00c609 100644 --- a/xmtp_mls/src/groups/device_sync.rs +++ b/xmtp_mls/src/groups/device_sync.rs @@ -1,4 +1,5 @@ use super::{GroupError, MlsGroup}; +use crate::groups::group_mutable_metadata::GroupMessageExpirationSettings; #[cfg(any(test, feature = "test-utils"))] pub use crate::utils::WorkerHandle; use crate::{ @@ -107,6 +108,8 @@ pub enum DeviceSyncError { Bincode(#[from] bincode::Error), } +#[derive(Debug, Error)] +pub enum ExpirationWorkerError {} impl RetryableError for DeviceSyncError { fn is_retryable(&self) -> bool { true @@ -138,6 +141,19 @@ where self.set_sync_worker_handle(worker.handle.clone()); worker.spawn_worker(); } + + #[instrument(level = "trace", skip_all)] + pub fn start_expired_messages_cleaner_worker(&self) { + let client = self.clone(); + tracing::debug!( + inbox_id = client.inbox_id(), + installation_id = hex::encode(client.installation_public_key()), + "starting expired messages cleaners worker" + ); + + let worker = MessageExpirationWorker::new(client); + worker.spawn_worker(); + } } pub struct SyncWorker { @@ -353,6 +369,109 @@ where } } +pub struct MessageExpirationWorker { + client: Client, + init: OnceCell<()>, +} +impl MessageExpirationWorker +where + ApiClient: XmtpApi + Send + Sync + 'static, + V: SmartContractSignatureVerifier + Send + Sync + 'static, +{ + fn new(client: Client) -> Self { + Self { + client, + init: OnceCell::new(), + } + } + fn spawn_worker(mut self) { + crate::spawn(None, async move { + let inbox_id = self.client.inbox_id().to_string(); + let installation_id = hex::encode(self.client.installation_public_key()); + while let Err(err) = self.run().await { + tracing::info!("Running worker.."); + match err { + DeviceSyncError::Client(ClientError::Storage( + StorageError::PoolNeedsConnection, + )) => { + tracing::warn!( + inbox_id, + installation_id, + "Pool disconnected. task will restart on reconnect" + ); + break; + } + _ => { + tracing::error!(inbox_id, installation_id, "sync worker error {err}"); + // Wait 2 seconds before restarting. + xmtp_common::time::sleep(Duration::from_secs(2)).await; + } + } + } + }); + } +} + +impl MessageExpirationWorker +where + ApiClient: XmtpApi + Send + Sync + 'static, + V: SmartContractSignatureVerifier + Send + Sync + 'static, +{ + /// Get a list of groups with expiration_settings enabled + pub fn get_groups_with_expiration_enabled( + &mut self, + ) -> Result, GroupMessageExpirationSettings)>, DeviceSyncError> { + let inner = self.client.clone(); + + let conversations = inner.find_groups(GroupQueryArgs { + include_duplicate_dms: true, + ..GroupQueryArgs::default() + })?; + + let mut group_ids_with_expiration = Vec::new(); + + for group in conversations.iter() { + if let Ok(provider) = group.mls_provider() { + if let Some(settings) = + group.get_group_message_expiration_settings_if_valid(&provider) + { + //todo: I would insert the expiration settings into the db instead of getting it always from the group! probably: more info of the group can go into db! + group_ids_with_expiration.push((group.group_id.clone(), settings)); + } + } + } + + Ok(group_ids_with_expiration) + } + + /// Iterate on the list of groups and delete expired messages + async fn delete_expired_messages(&mut self) -> Result<(), DeviceSyncError> { + let groups = self.get_groups_with_expiration_enabled()?; + let provider = self.client.mls_provider()?; + for (group_id, settings) in groups { + if let Err(e) = provider + .conn_ref() + .delete_expired_messages(group_id.clone(), settings) + { + tracing::error!( + "Failed to delete expired messages for group: {:?}, error: {:?}", + group_id, + e + ); + } + } + Ok(()) + } + + async fn run(&mut self) -> Result<(), DeviceSyncError> { + // Call delete_expired_messages on every iteration + if let Err(err) = self.delete_expired_messages().await { + tracing::error!("Error during deletion of expired messages: {:?}", err); + } + Ok(()) + } +} + impl Client where ApiClient: XmtpApi, diff --git a/xmtp_mls/src/groups/group_mutable_metadata.rs b/xmtp_mls/src/groups/group_mutable_metadata.rs index e1285a2aa..4c2ebe1bb 100644 --- a/xmtp_mls/src/groups/group_mutable_metadata.rs +++ b/xmtp_mls/src/groups/group_mutable_metadata.rs @@ -11,13 +11,12 @@ use xmtp_proto::xmtp::mls::message_contents::{ GroupMutableMetadataV1 as GroupMutableMetadataProto, Inboxes as InboxesProto, }; +use super::GroupMetadataOptions; use crate::configuration::{ DEFAULT_GROUP_DESCRIPTION, DEFAULT_GROUP_IMAGE_URL_SQUARE, DEFAULT_GROUP_NAME, DEFAULT_GROUP_PINNED_FRAME_URL, MUTABLE_METADATA_EXTENSION_ID, }; -use super::GroupMetadataOptions; - /// Errors that can occur when working with GroupMutableMetadata. #[derive(Debug, Error)] pub enum GroupMutableMetadataError { @@ -71,6 +70,25 @@ impl fmt::Display for MetadataField { } } +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub struct GroupMessageExpirationSettings { + pub expire_from_ms: i64, + pub expire_in_ms: i64, +} + +impl GroupMessageExpirationSettings { + pub fn new(expire_from_ms: i64, expire_in_ms: i64) -> Self { + Self { + expire_from_ms, + expire_in_ms, + } + } +} + +impl Default for GroupMessageExpirationSettings { + fn default() -> Self {Self::new(0, 0)} +} + /// Represents the mutable metadata for a group. /// /// This struct is stored as an MLS Unknown Group Context Extension. @@ -126,16 +144,14 @@ impl GroupMutableMetadata { .unwrap_or_else(|| DEFAULT_GROUP_PINNED_FRAME_URL.to_string()), ); - if let Some(message_expiration_from_ms) = opts.message_expiration_from_ms { + if let Some(message_retention_settings) = opts.message_retention_settings { attributes.insert( MetadataField::MessageExpirationFromMillis.to_string(), - message_expiration_from_ms.to_string(), + message_retention_settings.expire_from_ms.to_string(), ); - } - if let Some(message_expiration_ms) = opts.message_expiration_ms { attributes.insert( MetadataField::MessageExpirationMillis.to_string(), - message_expiration_ms.to_string(), + message_retention_settings.expire_in_ms.to_string(), ); } diff --git a/xmtp_mls/src/groups/intents.rs b/xmtp_mls/src/groups/intents.rs index 5c9863496..1f24949ba 100644 --- a/xmtp_mls/src/groups/intents.rs +++ b/xmtp_mls/src/groups/intents.rs @@ -25,6 +25,13 @@ use xmtp_proto::xmtp::mls::database::{ UpdateAdminListsData, UpdateGroupMembershipData, UpdateMetadataData, UpdatePermissionData, }; +use super::{ + group_membership::GroupMembership, + group_mutable_metadata::MetadataField, + group_permissions::{MembershipPolicies, MetadataPolicies, PermissionsPolicies}, + scoped_client::ScopedGroupClient, + GroupError, MlsGroup, +}; use crate::{ configuration::GROUP_KEY_ROTATION_INTERVAL_NS, storage::{ @@ -36,14 +43,6 @@ use crate::{ XmtpOpenMlsProvider, }; -use super::{ - group_membership::GroupMembership, - group_mutable_metadata::MetadataField, - group_permissions::{MembershipPolicies, MetadataPolicies, PermissionsPolicies}, - scoped_client::ScopedGroupClient, - GroupError, MlsGroup, -}; - #[derive(Debug, Error)] pub enum IntentError { #[error("decode error: {0}")] @@ -240,6 +239,19 @@ impl UpdateMetadataIntentData { field_value: pinned_frame_url, } } + + pub fn new_update_group_message_expiration_from_ms(expire_from_ms: i64) -> Self { + Self { + field_name: MetadataField::MessageExpirationFromMillis.to_string(), + field_value: expire_from_ms.to_string(), + } + } + pub fn new_update_group_message_expiration_in_ms(expire_in_ms: i64) -> Self { + Self { + field_name: MetadataField::MessageExpirationMillis.to_string(), + field_value: expire_in_ms.to_string(), + } + } } impl From for Vec { diff --git a/xmtp_mls/src/groups/mod.rs b/xmtp_mls/src/groups/mod.rs index d8c4487f8..e1154afc2 100644 --- a/xmtp_mls/src/groups/mod.rs +++ b/xmtp_mls/src/groups/mod.rs @@ -107,6 +107,7 @@ use std::{collections::HashSet, sync::Arc}; use xmtp_cryptography::signature::{sanitize_evm_addresses, AddressValidationError}; use xmtp_id::{InboxId, InboxIdRef}; +use crate::groups::group_mutable_metadata::GroupMessageExpirationSettings; use xmtp_common::retry::RetryableError; #[derive(Debug, Error)] @@ -293,8 +294,7 @@ pub struct GroupMetadataOptions { pub image_url_square: Option, pub description: Option, pub pinned_frame_url: Option, - pub message_expiration_from_ms: Option, - pub message_expiration_ms: Option, + pub message_retention_settings: Option, } impl Clone for MlsGroup { @@ -1138,6 +1138,91 @@ impl MlsGroup { } } + pub async fn update_group_message_expiration_settings( + &self, + settings: GroupMessageExpirationSettings, + ) -> Result<(), GroupError> { + let provider = self.client.mls_provider()?; + + self.update_group_message_expiration_from_ms(&provider, settings.expire_from_ms) + .await?; + self.update_group_message_expire_in_ms(&provider, settings.expire_in_ms) + .await + } + + pub async fn remove_group_message_expiration_settings(&self) -> Result<(), GroupError> { + self.update_group_message_expiration_settings(GroupMessageExpirationSettings::default()) + .await + } + + async fn update_group_message_expiration_from_ms( + &self, + provider: &XmtpOpenMlsProvider, + expire_from_ms: i64, + ) -> Result<(), GroupError> { + let intent_data: Vec = + UpdateMetadataIntentData::new_update_group_message_expiration_from_ms(expire_from_ms) + .into(); + let intent = self.queue_intent(&provider, IntentKind::MetadataUpdate, intent_data)?; + self.sync_until_intent_resolved(&provider, intent.id).await + } + + async fn update_group_message_expire_in_ms( + &self, + provider: &XmtpOpenMlsProvider, + expire_in_ms: i64, + ) -> Result<(), GroupError> { + let intent_data: Vec = + UpdateMetadataIntentData::new_update_group_message_expiration_in_ms(expire_in_ms) + .into(); + let intent = self.queue_intent(&provider, IntentKind::MetadataUpdate, intent_data)?; + self.sync_until_intent_resolved(&provider, intent.id).await + } + + pub fn group_message_expiration_settings( + &self, + provider: &XmtpOpenMlsProvider, + ) -> Result { + let mutable_metadata = self.mutable_metadata(provider)?; + let expire_from_ms = mutable_metadata + .attributes + .get(&MetadataField::MessageExpirationFromMillis.to_string()); + let expire_in_ms = mutable_metadata + .attributes + .get(&MetadataField::MessageExpirationMillis.to_string()); + + if let (Some(Ok(message_expiration_from_ms)), Some(Ok(message_expiration_ms))) = ( + expire_from_ms.map(|s| s.parse::()), + expire_in_ms.map(|s| s.parse::()), + ) { + Ok(GroupMessageExpirationSettings::new( + message_expiration_from_ms, + message_expiration_ms, + )) + } else { + Err(GroupError::GroupMetadata( + GroupMetadataError::MissingExtension, + )) + } + } + + /// Check the group expiration settings, and if both are gt then 0 will return the settings + pub fn get_group_message_expiration_settings_if_valid( + &self, + provider: &XmtpOpenMlsProvider, + ) -> Option { + match self.group_message_expiration_settings(provider) { + Ok(expiration_settings) => { + if expiration_settings.expire_from_ms > 0 && expiration_settings.expire_in_ms > 0 { + Some(expiration_settings) + } else { + None + } + } + Err(_) => None, + } + } + /// Retrieves the admin list of the group from the group's mutable metadata extension. pub fn admin_list(&self, provider: &XmtpOpenMlsProvider) -> Result, GroupError> { let mutable_metadata = self.mutable_metadata(provider)?; @@ -1771,6 +1856,8 @@ pub(crate) mod tests { use xmtp_proto::xmtp::mls::api::v1::group_message::Version; use xmtp_proto::xmtp::mls::message_contents::EncodedContent; + use super::{group_permissions::PolicySet, MlsGroup}; + use crate::groups::group_mutable_metadata::GroupMessageExpirationSettings; use crate::storage::group::StoredGroup; use crate::storage::schema::groups; use crate::{ @@ -1797,8 +1884,6 @@ pub(crate) mod tests { InboxOwner, StreamHandle as _, }; - use super::{group_permissions::PolicySet, MlsGroup}; - async fn receive_group_invite(client: &FullXmtpClient) -> MlsGroup { client .sync_welcomes(&client.mls_provider().unwrap()) @@ -2583,6 +2668,9 @@ pub(crate) mod tests { #[wasm_bindgen_test(unsupported = tokio::test(flavor = "current_thread"))] async fn test_group_options() { + let expected_group_message_expiration_settings = + GroupMessageExpirationSettings::new(100, 200); + let amal = ClientBuilder::new_test_client(&generate_local_wallet()).await; let amal_group = amal @@ -2593,8 +2681,7 @@ pub(crate) mod tests { image_url_square: Some("url".to_string()), description: Some("group description".to_string()), pinned_frame_url: Some("pinned frame".to_string()), - message_expiration_from_ms: None, - message_expiration_ms: None, + message_retention_settings: Some(expected_group_message_expiration_settings), }, ) .unwrap(); @@ -2618,11 +2705,30 @@ pub(crate) mod tests { .attributes .get(&MetadataField::GroupPinnedFrameUrl.to_string()) .unwrap(); - + let amal_group_message_expiration_from_ms = binding + .attributes + .get(&MetadataField::MessageExpirationFromMillis.to_string()) + .unwrap(); + let amal_group_message_expire_in_ms = binding + .attributes + .get(&MetadataField::MessageExpirationMillis.to_string()) + .unwrap(); assert_eq!(amal_group_name, "Group Name"); assert_eq!(amal_group_image_url, "url"); assert_eq!(amal_group_description, "group description"); assert_eq!(amal_group_pinned_frame_url, "pinned frame"); + assert_eq!( + amal_group_message_expiration_from_ms.clone(), + expected_group_message_expiration_settings + .expire_from_ms + .to_string() + ); + assert_eq!( + amal_group_message_expire_in_ms.clone(), + expected_group_message_expiration_settings + .expire_in_ms + .to_string() + ); } #[wasm_bindgen_test(unsupported = tokio::test(flavor = "current_thread"))] @@ -2818,6 +2924,65 @@ pub(crate) mod tests { assert_eq!(amal_group_pinned_frame_url, "a frame url"); } + #[wasm_bindgen_test(unsupported = tokio::test(flavor = "current_thread"))] + async fn test_update_group_message_expiration_settings() { + let amal = ClientBuilder::new_test_client(&generate_local_wallet()).await; + + // Create a group and verify it has the default group name + let policy_set = Some(PreconfiguredPolicies::AdminsOnly.to_policy_set()); + let amal_group = amal + .create_group(policy_set, GroupMetadataOptions::default()) + .unwrap(); + amal_group.sync().await.unwrap(); + + let group_mutable_metadata = amal_group + .mutable_metadata(&amal_group.mls_provider().unwrap()) + .unwrap(); + assert!(group_mutable_metadata + .attributes + .get(&MetadataField::MessageExpirationMillis.to_string()) + .is_none()); + assert!(group_mutable_metadata + .attributes + .get(&MetadataField::MessageExpirationFromMillis.to_string()) + .is_none()); + + // Update group name + let expected_group_message_expiration_settings = + GroupMessageExpirationSettings::new(100, 200); + + amal_group + .update_group_message_expiration_settings(expected_group_message_expiration_settings) + .await + .unwrap(); + + // Verify amal group sees update + amal_group.sync().await.unwrap(); + let binding = amal_group + .mutable_metadata(&amal_group.mls_provider().unwrap()) + .expect("msg"); + let amal_message_expiration_from_ms: &String = binding + .attributes + .get(&MetadataField::MessageExpirationFromMillis.to_string()) + .unwrap(); + let amal_message_expiration_ms: &String = binding + .attributes + .get(&MetadataField::MessageExpirationMillis.to_string()) + .unwrap(); + assert_eq!( + amal_message_expiration_from_ms.clone(), + expected_group_message_expiration_settings + .expire_from_ms + .to_string() + ); + assert_eq!( + amal_message_expiration_ms.clone(), + expected_group_message_expiration_settings + .expire_in_ms + .to_string() + ); + } + #[wasm_bindgen_test(unsupported = tokio::test(flavor = "current_thread"))] async fn test_group_mutable_data_group_permissions() { let amal = ClientBuilder::new_test_client(&generate_local_wallet()).await; diff --git a/xmtp_mls/src/storage/encrypted_store/group_message.rs b/xmtp_mls/src/storage/encrypted_store/group_message.rs index 20335e88c..60e74f993 100644 --- a/xmtp_mls/src/storage/encrypted_store/group_message.rs +++ b/xmtp_mls/src/storage/encrypted_store/group_message.rs @@ -23,6 +23,7 @@ use super::{ }, Sqlite, }; +use crate::groups::group_mutable_metadata::GroupMessageExpirationSettings; use crate::{impl_fetch, impl_store, impl_store_or_ignore, StorageError}; #[derive( @@ -426,6 +427,23 @@ impl DbConnection { .execute(conn) })?) } + + pub fn delete_expired_messages>( + &self, + group_id: GroupId, + settings: GroupMessageExpirationSettings, + ) -> Result { + Ok(self.raw_query(|conn| { + diesel::delete(dsl::group_messages) + .filter(dsl::group_id.eq(group_id.as_ref())) + .filter(dsl::delivery_status.eq(DeliveryStatus::Published)) + .filter(dsl::sent_at_ns.between( + settings.expire_from_ms * 1000, // convert to ns -- keep in ms to reduce the sensitivity + (settings.expire_from_ms + settings.expire_in_ms) * 1000, // convert to ns + )) + .execute(conn) + })?) + } } #[cfg(test)] @@ -455,7 +473,7 @@ pub(crate) mod tests { sender_installation_id: rand_vec::<24>(), sender_inbox_id: "0x0".to_string(), kind: kind.unwrap_or(GroupMessageKind::Application), - delivery_status: DeliveryStatus::Unpublished, + delivery_status: DeliveryStatus::Published, content_type: content_type.unwrap_or(ContentType::Unknown), version_major: 0, version_minor: 0, @@ -589,6 +607,47 @@ pub(crate) mod tests { .await } + #[wasm_bindgen_test(unsupported = tokio::test)] + async fn it_deletes_middle_message_by_expiration_time() { + with_connection(|conn| { + let group = generate_group(None); + group.store(conn).unwrap(); + + let messages = vec![ + generate_message(None, Some(&group.id), Some(1_000_000_000), None), + generate_message(None, Some(&group.id), Some(1_001_000_000), None), + generate_message(None, Some(&group.id), Some(1_002_000_000), None), + ]; + assert_ok!(messages.store(conn)); + + let expired_from_ms = 1_000_500; // After Message 1 + let expired_in_ms = 500; // Before Message 3 + let result = conn + .delete_expired_messages(&group.id, expired_from_ms, expired_in_ms) + .unwrap(); + assert_eq!(result, 1); // Ensure exactly 1 message is deleted + + let remaining_messages = conn + .get_group_messages( + &group.id, + &MsgQueryArgs { + ..Default::default() + }, + ) + .unwrap(); + + // Verify the count and content of the remaining messages + assert_eq!(remaining_messages.len(), 2); + assert!(remaining_messages + .iter() + .any(|msg| msg.sent_at_ns == 1_000_000_000)); // Message 1 + assert!(remaining_messages + .iter() + .any(|msg| msg.sent_at_ns == 1_002_000_000)); // Message 3 + }) + .await + } + #[wasm_bindgen_test(unsupported = tokio::test)] async fn it_gets_messages_by_kind() { with_connection(|conn| { From b9be121aa20fd728abd502ca530d9e6bb99580de Mon Sep 17 00:00:00 2001 From: Mojtaba Chenani Date: Thu, 16 Jan 2025 19:02:19 +0100 Subject: [PATCH 02/20] add group message expiration setting to db --- .../down.sql | 2 + .../up.sql | 2 + xmtp_mls/src/groups/device_sync.rs | 46 +++---------------- xmtp_mls/src/storage/encrypted_store/group.rs | 10 ++++ .../storage/encrypted_store/group_message.rs | 45 ++++++++++-------- .../src/storage/encrypted_store/schema_gen.rs | 2 + 6 files changed, 48 insertions(+), 59 deletions(-) create mode 100644 xmtp_mls/migrations/2025-01-16-143131_add_message_expiration_to_groups/down.sql create mode 100644 xmtp_mls/migrations/2025-01-16-143131_add_message_expiration_to_groups/up.sql diff --git a/xmtp_mls/migrations/2025-01-16-143131_add_message_expiration_to_groups/down.sql b/xmtp_mls/migrations/2025-01-16-143131_add_message_expiration_to_groups/down.sql new file mode 100644 index 000000000..43acba66d --- /dev/null +++ b/xmtp_mls/migrations/2025-01-16-143131_add_message_expiration_to_groups/down.sql @@ -0,0 +1,2 @@ +ALTER TABLE GROUPS DROP COLUMN message_expire_from_ms; +ALTER TABLE GROUPS DROP COLUMN message_expire_in_ms; \ No newline at end of file diff --git a/xmtp_mls/migrations/2025-01-16-143131_add_message_expiration_to_groups/up.sql b/xmtp_mls/migrations/2025-01-16-143131_add_message_expiration_to_groups/up.sql new file mode 100644 index 000000000..8406a0de0 --- /dev/null +++ b/xmtp_mls/migrations/2025-01-16-143131_add_message_expiration_to_groups/up.sql @@ -0,0 +1,2 @@ +ALTER TABLE GROUPS ADD COLUMN message_expire_from_ms BIGINT NOT NULL DEFAULT 0; +ALTER TABLE GROUPS ADD COLUMN message_expire_in_ms BIGINT NOT NULL DEFAULT 0; diff --git a/xmtp_mls/src/groups/device_sync.rs b/xmtp_mls/src/groups/device_sync.rs index af51fd59f..f5dffb3de 100644 --- a/xmtp_mls/src/groups/device_sync.rs +++ b/xmtp_mls/src/groups/device_sync.rs @@ -148,7 +148,7 @@ where tracing::debug!( inbox_id = client.inbox_id(), installation_id = hex::encode(client.installation_public_key()), - "starting expired messages cleaners worker" + "starting expired messages cleaner worker" ); let worker = MessageExpirationWorker::new(client); @@ -425,48 +425,14 @@ where ApiClient: XmtpApi + Send + Sync + 'static, V: SmartContractSignatureVerifier + Send + Sync + 'static, { - /// Get a list of groups with expiration_settings enabled - pub fn get_groups_with_expiration_enabled( - &mut self, - ) -> Result, GroupMessageExpirationSettings)>, DeviceSyncError> { - let inner = self.client.clone(); - - let conversations = inner.find_groups(GroupQueryArgs { - include_duplicate_dms: true, - ..GroupQueryArgs::default() - })?; - - let mut group_ids_with_expiration = Vec::new(); - - for group in conversations.iter() { - if let Ok(provider) = group.mls_provider() { - if let Some(settings) = - group.get_group_message_expiration_settings_if_valid(&provider) - { - //todo: I would insert the expiration settings into the db instead of getting it always from the group! probably: more info of the group can go into db! - group_ids_with_expiration.push((group.group_id.clone(), settings)); - } - } - } - - Ok(group_ids_with_expiration) - } - /// Iterate on the list of groups and delete expired messages async fn delete_expired_messages(&mut self) -> Result<(), DeviceSyncError> { - let groups = self.get_groups_with_expiration_enabled()?; let provider = self.client.mls_provider()?; - for (group_id, settings) in groups { - if let Err(e) = provider - .conn_ref() - .delete_expired_messages(group_id.clone(), settings) - { - tracing::error!( - "Failed to delete expired messages for group: {:?}, error: {:?}", - group_id, - e - ); - } + if let Err(e) = provider.conn_ref().delete_expired_messages() { + tracing::error!( + "Failed to delete expired messages for group: {:?}, error: {:?}", + e + ); } Ok(()) } diff --git a/xmtp_mls/src/storage/encrypted_store/group.rs b/xmtp_mls/src/storage/encrypted_store/group.rs index e597ad062..8569cac32 100644 --- a/xmtp_mls/src/storage/encrypted_store/group.rs +++ b/xmtp_mls/src/storage/encrypted_store/group.rs @@ -51,6 +51,10 @@ pub struct StoredGroup { pub dm_id: Option, /// Timestamp of when the last message was sent for this group (updated automatically in a trigger) pub last_message_ns: Option, + /// The Time in MS when the message expiration starts for the group + pub message_expire_from_ms: i64, + /// How long a message in the group can live in MS + pub message_expire_in_ms: i64, } impl_fetch!(StoredGroup, groups, Vec); @@ -78,6 +82,8 @@ impl StoredGroup { rotated_at_ns: 0, dm_id: dm_members.map(String::from), last_message_ns: Some(now_ns()), + message_expire_from_ms: 0, + message_expire_in_ms: 0, } } @@ -103,6 +109,8 @@ impl StoredGroup { rotated_at_ns: 0, dm_id: dm_members.map(String::from), last_message_ns: Some(now_ns()), + message_expire_from_ms: 0, + message_expire_in_ms: 0, } } @@ -124,6 +132,8 @@ impl StoredGroup { rotated_at_ns: 0, dm_id: None, last_message_ns: Some(now_ns()), + message_expire_from_ms: 0, + message_expire_in_ms: 0, } } } diff --git a/xmtp_mls/src/storage/encrypted_store/group_message.rs b/xmtp_mls/src/storage/encrypted_store/group_message.rs index 60e74f993..e6bc4a526 100644 --- a/xmtp_mls/src/storage/encrypted_store/group_message.rs +++ b/xmtp_mls/src/storage/encrypted_store/group_message.rs @@ -1,5 +1,4 @@ -use std::collections::HashMap; - +use diesel::dsl::sql; use diesel::{ backend::Backend, deserialize::{self, FromSql, FromSqlRow}, @@ -8,8 +7,8 @@ use diesel::{ serialize::{self, IsNull, Output, ToSql}, sql_types::Integer, }; - use serde::{Deserialize, Serialize}; +use std::collections::HashMap; use xmtp_content_types::{ attachment, group_updated, membership_change, reaction, read_receipt, remote_attachment, reply, text, transaction_reference, @@ -23,7 +22,6 @@ use super::{ }, Sqlite, }; -use crate::groups::group_mutable_metadata::GroupMessageExpirationSettings; use crate::{impl_fetch, impl_store, impl_store_or_ignore, StorageError}; #[derive( @@ -428,19 +426,26 @@ impl DbConnection { })?) } - pub fn delete_expired_messages>( - &self, - group_id: GroupId, - settings: GroupMessageExpirationSettings, - ) -> Result { + pub fn delete_expired_messages(&self) -> Result { Ok(self.raw_query(|conn| { - diesel::delete(dsl::group_messages) - .filter(dsl::group_id.eq(group_id.as_ref())) + use diesel::prelude::*; + let expire_messages = dsl::group_messages + .left_join( + groups_dsl::groups.on(sql::( + "lower(hex(group_messages.group_id))", + ) + .eq(sql::("lower(hex(groups.id))"))), + ) .filter(dsl::delivery_status.eq(DeliveryStatus::Published)) .filter(dsl::sent_at_ns.between( - settings.expire_from_ms * 1000, // convert to ns -- keep in ms to reduce the sensitivity - (settings.expire_from_ms + settings.expire_in_ms) * 1000, // convert to ns + groups_dsl::message_expire_from_ms * 1_000, // Convert ms to ns + (groups_dsl::message_expire_from_ms + groups_dsl::message_expire_in_ms) * 1_000, // Add duration and convert )) + .select(dsl::id); + let expired_message_ids = expire_messages.load::>(conn)?; + + // Then delete the rows by their IDs + diesel::delete(dsl::group_messages.filter(dsl::id.eq_any(expired_message_ids))) .execute(conn) })?) } @@ -610,7 +615,13 @@ pub(crate) mod tests { #[wasm_bindgen_test(unsupported = tokio::test)] async fn it_deletes_middle_message_by_expiration_time() { with_connection(|conn| { - let group = generate_group(None); + let mut group = generate_group(None); + + let expired_from_ms = 1_000_500; // After Message 1 + let expired_in_ms = 500; // Before Message 3 + group.message_expire_from_ms = expired_from_ms; + group.message_expire_in_ms = expired_in_ms; + group.store(conn).unwrap(); let messages = vec![ @@ -620,11 +631,7 @@ pub(crate) mod tests { ]; assert_ok!(messages.store(conn)); - let expired_from_ms = 1_000_500; // After Message 1 - let expired_in_ms = 500; // Before Message 3 - let result = conn - .delete_expired_messages(&group.id, expired_from_ms, expired_in_ms) - .unwrap(); + let result = conn.delete_expired_messages().unwrap(); assert_eq!(result, 1); // Ensure exactly 1 message is deleted let remaining_messages = conn diff --git a/xmtp_mls/src/storage/encrypted_store/schema_gen.rs b/xmtp_mls/src/storage/encrypted_store/schema_gen.rs index f687f4fb9..d7f20b5fa 100644 --- a/xmtp_mls/src/storage/encrypted_store/schema_gen.rs +++ b/xmtp_mls/src/storage/encrypted_store/schema_gen.rs @@ -63,6 +63,8 @@ diesel::table! { conversation_type -> Integer, dm_id -> Nullable, last_message_ns -> Nullable, + message_expire_from_ms -> BigInt, + message_expire_in_ms -> BigInt, } } From 1dcb8989bede625d0321552d2432bf182f515ee9 Mon Sep 17 00:00:00 2001 From: Mojtaba Chenani Date: Mon, 20 Jan 2025 16:03:05 +0100 Subject: [PATCH 03/20] rename to ConversationMessageDisappearingSettings --- bindings_ffi/src/mls.rs | 26 ++++---- bindings_node/src/conversation.rs | 20 +++++- bindings_node/src/conversations.rs | 10 ++- bindings_node/src/permissions.rs | 4 +- bindings_wasm/src/conversation.rs | 7 ++ bindings_wasm/src/conversations.rs | 19 ++---- bindings_wasm/src/permissions.rs | 8 ++- .../down.sql | 4 +- .../up.sql | 4 +- xmtp_mls/src/groups/device_sync.rs | 6 +- xmtp_mls/src/groups/group_mutable_metadata.rs | 41 ++++++------ xmtp_mls/src/groups/group_permissions.rs | 10 +-- xmtp_mls/src/groups/intents.rs | 4 +- xmtp_mls/src/groups/mod.rs | 66 +++++++++---------- xmtp_mls/src/storage/encrypted_store/group.rs | 20 +++--- .../storage/encrypted_store/group_message.rs | 29 +++++--- .../src/storage/encrypted_store/schema_gen.rs | 4 +- 17 files changed, 158 insertions(+), 124 deletions(-) diff --git a/bindings_ffi/src/mls.rs b/bindings_ffi/src/mls.rs index 9731d2cd1..7493b5485 100644 --- a/bindings_ffi/src/mls.rs +++ b/bindings_ffi/src/mls.rs @@ -19,7 +19,7 @@ use xmtp_id::{ InboxId, }; use xmtp_mls::groups::device_sync::preference_sync::UserPreferenceUpdate; -use xmtp_mls::groups::group_mutable_metadata::GroupMessageExpirationSettings; +use xmtp_mls::groups::group_mutable_metadata::ConversationMessageDisappearingSettings; use xmtp_mls::groups::scoped_client::LocalScopedGroupClient; use xmtp_mls::groups::HmacKey; use xmtp_mls::storage::group::ConversationType; @@ -824,14 +824,14 @@ impl TryFrom for PolicySet { ); // MessageExpirationFromMillis follows the same policy as MessageExpirationMillis metadata_permissions_map.insert( - MetadataField::MessageExpirationFromMillis.to_string(), + MetadataField::MessageDisappearFromNS.to_string(), policy_set .update_message_expiration_ms_policy .clone() .try_into()?, ); metadata_permissions_map.insert( - MetadataField::MessageExpirationMillis.to_string(), + MetadataField::MessageDisappearInNS.to_string(), policy_set.update_message_expiration_ms_policy.try_into()?, ); @@ -1423,9 +1423,9 @@ impl From for SortDirection { } } -impl From for GroupMessageExpirationSettings { +impl From for ConversationMessageDisappearingSettings { fn from(settings: FfiGroupMessageExpirationSettings) -> Self { - GroupMessageExpirationSettings::new(settings.expire_from_ms, settings.expire_in_ms) + ConversationMessageDisappearingSettings::new(settings.expire_from_ms, settings.expire_in_ms) } } @@ -1484,11 +1484,11 @@ pub struct FfiCreateGroupOptions { impl FfiCreateGroupOptions { pub fn into_group_metadata_options(self) -> GroupMetadataOptions { - let message_retention_settings: Option = + let message_retention_settings: Option = if let (Some(message_expiration_from_ms), Some(message_expiration_ms)) = (self.message_expiration_from_ms, self.message_expiration_ms) { - Some(GroupMessageExpirationSettings::new( + Some(ConversationMessageDisappearingSettings::new( message_expiration_from_ms, message_expiration_ms, )) @@ -1500,7 +1500,7 @@ impl FfiCreateGroupOptions { image_url_square: self.group_image_url_square, description: self.group_description, pinned_frame_url: self.group_pinned_frame_url, - message_retention_settings, + message_disappearing_settings: None//todo: fix mapping, } } } @@ -1736,7 +1736,7 @@ impl FfiConversation { settings: FfiGroupMessageExpirationSettings, ) -> Result<(), GenericError> { self.inner - .update_group_message_expiration_settings(GroupMessageExpirationSettings::from( + .update_group_message_expiration_settings(ConversationMessageDisappearingSettings::from( settings, )) .await?; @@ -1752,8 +1752,8 @@ impl FfiConversation { self.inner.group_message_expiration_settings(&provider)?; // Use `?` to handle the Result Ok(FfiGroupMessageExpirationSettings::new( - group_message_expiration_settings.expire_from_ms, - group_message_expiration_settings.expire_in_ms, + group_message_expiration_settings.from_ns, + group_message_expiration_settings.in_ns, )) } @@ -2277,7 +2277,7 @@ impl FfiGroupPermissions { MetadataField::GroupPinnedFrameUrl.as_str(), ), update_message_expiration_ms_policy: get_policy( - MetadataField::MessageExpirationMillis.as_str(), + MetadataField::MessageDisappearInNS.as_str(), ), }) } @@ -2723,7 +2723,7 @@ mod tests { } use xmtp_cryptography::utils::generate_local_wallet; - use xmtp_mls::groups::group_mutable_metadata::{GroupMessageExpirationSettings, MetadataField}; + use xmtp_mls::groups::group_mutable_metadata::{ConversationMessageDisappearingSettings, MetadataField}; use xmtp_mls::groups::{GroupMetadataOptions, PreconfiguredPolicies}; #[tokio::test(flavor = "multi_thread", worker_threads = 1)] diff --git a/bindings_node/src/conversation.rs b/bindings_node/src/conversation.rs index 98c27179b..7ce441b5c 100644 --- a/bindings_node/src/conversation.rs +++ b/bindings_node/src/conversation.rs @@ -28,7 +28,7 @@ use crate::{ streams::StreamCloser, ErrorWrapper, }; - +use xmtp_mls::groups::group_mutable_metadata::ConversationMessageDisappearingSettings as XmtpConversationMessageDisappearingSettings; use prost::Message as ProstMessage; use napi_derive::napi; @@ -38,6 +38,24 @@ pub struct GroupMetadata { inner: XmtpGroupMetadata, } +#[napi] +#[derive(Clone)] +pub struct ConversationMessageDisappearingSettings { + inner: XmtpConversationMessageDisappearingSettings, +} + +#[napi] +impl ConversationMessageDisappearingSettings { + #[napi] + pub fn new(from_ns: i64, in_ns: i64) -> Self { + let inner = XmtpConversationMessageDisappearingSettings { + from_ns, + in_ns, + }; + Self { inner } + } +} + #[napi] impl GroupMetadata { #[napi] diff --git a/bindings_node/src/conversations.rs b/bindings_node/src/conversations.rs index 7111e7fcc..ec5ec0aab 100644 --- a/bindings_node/src/conversations.rs +++ b/bindings_node/src/conversations.rs @@ -16,6 +16,7 @@ use crate::message::Message; use crate::permissions::{GroupPermissionsOptions, PermissionPolicySet}; use crate::ErrorWrapper; use crate::{client::RustXmtpClient, conversation::Conversation, streams::StreamCloser}; +use crate::conversation::ConversationMessageDisappearingSettings; #[napi] #[derive(Debug)] @@ -122,8 +123,7 @@ pub struct CreateGroupOptions { pub group_description: Option, pub group_pinned_frame_url: Option, pub custom_permission_policy_set: Option, - pub message_expiration_from_ms: Option, - pub message_expiration_ms: Option, + pub message_disappearing_settings: Option, } impl CreateGroupOptions { @@ -133,8 +133,7 @@ impl CreateGroupOptions { image_url_square: self.group_image_url_square, description: self.group_description, pinned_frame_url: self.group_pinned_frame_url, - message_expiration_from_ms: self.message_expiration_from_ms, - message_expiration_ms: self.message_expiration_ms, + message_disappearing_settings: None //todo: fix mapping, } } } @@ -163,8 +162,7 @@ impl Conversations { group_description: None, group_pinned_frame_url: None, custom_permission_policy_set: None, - message_expiration_from_ms: None, - message_expiration_ms: None, + message_disappearing_settings: None, }); if let Some(GroupPermissionsOptions::CustomPolicy) = options.permissions { diff --git a/bindings_node/src/permissions.rs b/bindings_node/src/permissions.rs index 1eb667f64..94009f765 100644 --- a/bindings_node/src/permissions.rs +++ b/bindings_node/src/permissions.rs @@ -217,7 +217,7 @@ impl GroupPermissions { XmtpMetadataField::GroupPinnedFrameUrl.as_str(), ), update_message_expiration_ms_policy: get_policy( - XmtpMetadataField::MessageExpirationMillis.as_str(), + XmtpMetadataField::MessageDisappearInNS.as_str(), ), }) } @@ -246,7 +246,7 @@ impl TryFrom for PolicySet { policy_set.update_group_pinned_frame_url_policy.try_into()?, ); metadata_permissions_map.insert( - XmtpMetadataField::MessageExpirationMillis.to_string(), + XmtpMetadataField::MessageDisappearInNS.to_string(), policy_set.update_message_expiration_ms_policy.try_into()?, ); diff --git a/bindings_wasm/src/conversation.rs b/bindings_wasm/src/conversation.rs index 6509c5873..0c0ca2876 100644 --- a/bindings_wasm/src/conversation.rs +++ b/bindings_wasm/src/conversation.rs @@ -18,12 +18,19 @@ use xmtp_mls::storage::group_message::{GroupMessageKind as XmtpGroupMessageKind, use xmtp_proto::xmtp::mls::message_contents::EncodedContent as XmtpEncodedContent; use prost::Message as ProstMessage; +use xmtp_mls::groups::group_mutable_metadata::ConversationMessageDisappearingSettings as XmtpGroupMessageDisappearingSettings; #[wasm_bindgen] pub struct GroupMetadata { inner: XmtpGroupMetadata, } +#[wasm_bindgen] +#[derive(Clone)] +pub struct ConversationMessageDisappearingSettings { + inner: XmtpGroupMessageDisappearingSettings, +} + #[wasm_bindgen] impl GroupMetadata { #[wasm_bindgen(js_name = creatorInboxId)] diff --git a/bindings_wasm/src/conversations.rs b/bindings_wasm/src/conversations.rs index 6eeb0f426..0d958937d 100644 --- a/bindings_wasm/src/conversations.rs +++ b/bindings_wasm/src/conversations.rs @@ -7,6 +7,7 @@ use xmtp_mls::storage::group::ConversationType as XmtpConversationType; use xmtp_mls::storage::group::GroupMembershipState as XmtpGroupMembershipState; use xmtp_mls::storage::group::GroupQueryArgs; +use crate::conversation::ConversationMessageDisappearingSettings; use crate::messages::Message; use crate::permissions::{GroupPermissionsOptions, PermissionPolicySet}; use crate::{client::RustXmtpClient, conversation::Conversation}; @@ -130,10 +131,8 @@ pub struct CreateGroupOptions { pub group_pinned_frame_url: Option, #[wasm_bindgen(js_name = customPermissionPolicySet)] pub custom_permission_policy_set: Option, - #[wasm_bindgen(js_name = messageExpirationFromMillis)] - pub message_expiration_from_ms: Option, - #[wasm_bindgen(js_name = messageExpirationMillis)] - pub message_expiration_ms: Option, + #[wasm_bindgen(js_name = messageDisappearingSettings)] + pub message_disappearing_settings: Option, } #[wasm_bindgen] @@ -147,8 +146,7 @@ impl CreateGroupOptions { group_description: Option, group_pinned_frame_url: Option, custom_permission_policy_set: Option, - message_expiration_from_ms: Option, - message_expiration_ms: Option, + message_disappearing_settings: Option, ) -> Self { Self { permissions, @@ -157,8 +155,7 @@ impl CreateGroupOptions { group_description, group_pinned_frame_url, custom_permission_policy_set, - message_expiration_from_ms, - message_expiration_ms, + message_disappearing_settings, } } } @@ -170,8 +167,7 @@ impl CreateGroupOptions { image_url_square: self.group_image_url_square, description: self.group_description, pinned_frame_url: self.group_pinned_frame_url, - message_expiration_from_ms: self.message_expiration_from_ms, - message_expiration_ms: self.message_expiration_ms, + message_disappearing_settings: None// todo: fix mapping, } } } @@ -218,8 +214,7 @@ impl Conversations { group_description: None, group_pinned_frame_url: None, custom_permission_policy_set: None, - message_expiration_from_ms: None, - message_expiration_ms: None, + message_disappearing_settings: None, }); if let Some(GroupPermissionsOptions::CustomPolicy) = options.permissions { diff --git a/bindings_wasm/src/permissions.rs b/bindings_wasm/src/permissions.rs index 2baa7ca74..46222f0be 100644 --- a/bindings_wasm/src/permissions.rs +++ b/bindings_wasm/src/permissions.rs @@ -258,7 +258,7 @@ impl GroupPermissions { XmtpMetadataField::GroupPinnedFrameUrl.as_str(), ), update_message_expiration_ms_policy: get_policy( - XmtpMetadataField::MessageExpirationMillis.as_str(), + XmtpMetadataField::MessageDisappearInNS.as_str(), ), }) } @@ -285,7 +285,7 @@ impl TryFrom for PolicySet { policy_set.update_group_pinned_frame_url_policy.try_into()?, ); metadata_permissions_map.insert( - XmtpMetadataField::MessageExpirationMillis.to_string(), + XmtpMetadataField::MessageDisappearInNS.to_string(), policy_set.update_message_expiration_ms_policy.try_into()?, ); @@ -306,6 +306,8 @@ pub enum MetadataField { Description, ImageUrlSquare, PinnedFrameUrl, + MessageExpirationFromMS, + MessageExpirationMS, } impl From<&MetadataField> for XmtpMetadataField { @@ -315,6 +317,8 @@ impl From<&MetadataField> for XmtpMetadataField { MetadataField::Description => XmtpMetadataField::Description, MetadataField::ImageUrlSquare => XmtpMetadataField::GroupImageUrlSquare, MetadataField::PinnedFrameUrl => XmtpMetadataField::GroupPinnedFrameUrl, + MetadataField::MessageExpirationFromMS => XmtpMetadataField::MessageDisappearFromNS, + MetadataField::MessageExpirationMS => XmtpMetadataField::MessageDisappearInNS, } } } diff --git a/xmtp_mls/migrations/2025-01-16-143131_add_message_expiration_to_groups/down.sql b/xmtp_mls/migrations/2025-01-16-143131_add_message_expiration_to_groups/down.sql index 43acba66d..c5f2a3b25 100644 --- a/xmtp_mls/migrations/2025-01-16-143131_add_message_expiration_to_groups/down.sql +++ b/xmtp_mls/migrations/2025-01-16-143131_add_message_expiration_to_groups/down.sql @@ -1,2 +1,2 @@ -ALTER TABLE GROUPS DROP COLUMN message_expire_from_ms; -ALTER TABLE GROUPS DROP COLUMN message_expire_in_ms; \ No newline at end of file +ALTER TABLE GROUPS DROP COLUMN message_disappear_from_ns; +ALTER TABLE GROUPS DROP COLUMN message_disappear_in_ns; \ No newline at end of file diff --git a/xmtp_mls/migrations/2025-01-16-143131_add_message_expiration_to_groups/up.sql b/xmtp_mls/migrations/2025-01-16-143131_add_message_expiration_to_groups/up.sql index 8406a0de0..9a7c21984 100644 --- a/xmtp_mls/migrations/2025-01-16-143131_add_message_expiration_to_groups/up.sql +++ b/xmtp_mls/migrations/2025-01-16-143131_add_message_expiration_to_groups/up.sql @@ -1,2 +1,2 @@ -ALTER TABLE GROUPS ADD COLUMN message_expire_from_ms BIGINT NOT NULL DEFAULT 0; -ALTER TABLE GROUPS ADD COLUMN message_expire_in_ms BIGINT NOT NULL DEFAULT 0; +ALTER TABLE GROUPS ADD COLUMN message_disappear_from_ns BIGINT; +ALTER TABLE GROUPS ADD COLUMN message_disappear_in_ns BIGINT; diff --git a/xmtp_mls/src/groups/device_sync.rs b/xmtp_mls/src/groups/device_sync.rs index f5dffb3de..3b164a70a 100644 --- a/xmtp_mls/src/groups/device_sync.rs +++ b/xmtp_mls/src/groups/device_sync.rs @@ -1,5 +1,4 @@ use super::{GroupError, MlsGroup}; -use crate::groups::group_mutable_metadata::GroupMessageExpirationSettings; #[cfg(any(test, feature = "test-utils"))] pub use crate::utils::WorkerHandle; use crate::{ @@ -429,10 +428,7 @@ where async fn delete_expired_messages(&mut self) -> Result<(), DeviceSyncError> { let provider = self.client.mls_provider()?; if let Err(e) = provider.conn_ref().delete_expired_messages() { - tracing::error!( - "Failed to delete expired messages for group: {:?}, error: {:?}", - e - ); + tracing::error!("Failed to delete expired messages, error: {:?}", e); } Ok(()) } diff --git a/xmtp_mls/src/groups/group_mutable_metadata.rs b/xmtp_mls/src/groups/group_mutable_metadata.rs index 4c2ebe1bb..617793e08 100644 --- a/xmtp_mls/src/groups/group_mutable_metadata.rs +++ b/xmtp_mls/src/groups/group_mutable_metadata.rs @@ -46,8 +46,8 @@ pub enum MetadataField { Description, GroupImageUrlSquare, GroupPinnedFrameUrl, - MessageExpirationFromMillis, - MessageExpirationMillis, + MessageDisappearFromNS, + MessageDisappearInNS, } impl MetadataField { @@ -58,8 +58,8 @@ impl MetadataField { MetadataField::Description => "description", MetadataField::GroupImageUrlSquare => "group_image_url_square", MetadataField::GroupPinnedFrameUrl => "group_pinned_frame_url", - MetadataField::MessageExpirationFromMillis => "message_expiration_from_ms", - MetadataField::MessageExpirationMillis => "message_expiration_ms", + MetadataField::MessageDisappearFromNS => "message_expiration_from_ms", + MetadataField::MessageDisappearInNS => "message_expiration_ms", } } } @@ -71,22 +71,24 @@ impl fmt::Display for MetadataField { } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] -pub struct GroupMessageExpirationSettings { - pub expire_from_ms: i64, - pub expire_in_ms: i64, +pub struct ConversationMessageDisappearingSettings { + pub from_ns: i64, + pub in_ns: i64, } -impl GroupMessageExpirationSettings { - pub fn new(expire_from_ms: i64, expire_in_ms: i64) -> Self { +impl ConversationMessageDisappearingSettings { + pub fn new(from_ns: i64, in_ns: i64) -> Self { Self { - expire_from_ms, - expire_in_ms, + from_ns, + in_ns, } } } -impl Default for GroupMessageExpirationSettings { - fn default() -> Self {Self::new(0, 0)} +impl Default for ConversationMessageDisappearingSettings { + fn default() -> Self { + Self::new(0, 0) + } } /// Represents the mutable metadata for a group. @@ -144,14 +146,14 @@ impl GroupMutableMetadata { .unwrap_or_else(|| DEFAULT_GROUP_PINNED_FRAME_URL.to_string()), ); - if let Some(message_retention_settings) = opts.message_retention_settings { + if let Some(message_retention_settings) = opts.message_disappearing_settings { attributes.insert( - MetadataField::MessageExpirationFromMillis.to_string(), - message_retention_settings.expire_from_ms.to_string(), + MetadataField::MessageDisappearFromNS.to_string(), + message_retention_settings.from_ns.to_string(), ); attributes.insert( - MetadataField::MessageExpirationMillis.to_string(), - message_retention_settings.expire_in_ms.to_string(), + MetadataField::MessageDisappearInNS.to_string(), + message_retention_settings.in_ns.to_string(), ); } @@ -202,7 +204,8 @@ impl GroupMutableMetadata { MetadataField::Description, MetadataField::GroupImageUrlSquare, MetadataField::GroupPinnedFrameUrl, - MetadataField::MessageExpirationMillis, + MetadataField::MessageDisappearFromNS, + MetadataField::MessageDisappearInNS, ] } diff --git a/xmtp_mls/src/groups/group_permissions.rs b/xmtp_mls/src/groups/group_permissions.rs index f15f4c6b6..1bb092db0 100644 --- a/xmtp_mls/src/groups/group_permissions.rs +++ b/xmtp_mls/src/groups/group_permissions.rs @@ -31,7 +31,7 @@ use super::{ }; use crate::configuration::{GROUP_PERMISSIONS_EXTENSION_ID, SUPER_ADMIN_METADATA_PREFIX}; use crate::groups::group_mutable_metadata::MetadataField; -use crate::groups::group_mutable_metadata::MetadataField::MessageExpirationMillis; +use crate::groups::group_mutable_metadata::MetadataField::MessageDisappearInNS; /// Errors that can occur when working with GroupMutablePermissions. #[derive(Debug, Error)] @@ -228,7 +228,7 @@ impl MetadataPolicies { pub fn default_map(policies: MetadataPolicies) -> HashMap { let mut map: HashMap = HashMap::new(); for field in GroupMutableMetadata::supported_fields() { - if field == MessageExpirationMillis { + if field == MessageDisappearInNS { map.insert(field.to_string(), MetadataPolicies::allow_if_actor_admin()); } else { map.insert(field.to_string(), policies.clone()); @@ -1158,7 +1158,7 @@ pub fn is_policy_default(policy: &PolicySet) -> Result { name: field_name.to_string(), }, )?; - if field_name == MessageExpirationMillis.as_str() { + if field_name == MessageDisappearInNS.as_str() { metadata_policies_equal = metadata_policies_equal && metadata_policy.eq(&MetadataPolicies::allow_if_actor_admin()); } else { @@ -1208,7 +1208,7 @@ pub(crate) fn default_policy() -> PolicySet { metadata_policies_map.insert(field.to_string(), MetadataPolicies::allow()); } metadata_policies_map.insert( - MessageExpirationMillis.to_string(), + MessageDisappearInNS.to_string(), MetadataPolicies::allow_if_actor_admin(), ); @@ -1231,7 +1231,7 @@ pub(crate) fn policy_admin_only() -> PolicySet { metadata_policies_map.insert(field.to_string(), MetadataPolicies::allow_if_actor_admin()); } metadata_policies_map.insert( - MetadataField::MessageExpirationMillis.to_string(), + MetadataField::MessageDisappearInNS.to_string(), MetadataPolicies::allow_if_actor_admin(), ); diff --git a/xmtp_mls/src/groups/intents.rs b/xmtp_mls/src/groups/intents.rs index c080373fc..495b59e5c 100644 --- a/xmtp_mls/src/groups/intents.rs +++ b/xmtp_mls/src/groups/intents.rs @@ -243,13 +243,13 @@ impl UpdateMetadataIntentData { pub fn new_update_group_message_expiration_from_ms(expire_from_ms: i64) -> Self { Self { - field_name: MetadataField::MessageExpirationFromMillis.to_string(), + field_name: MetadataField::MessageDisappearFromNS.to_string(), field_value: expire_from_ms.to_string(), } } pub fn new_update_group_message_expiration_in_ms(expire_in_ms: i64) -> Self { Self { - field_name: MetadataField::MessageExpirationMillis.to_string(), + field_name: MetadataField::MessageDisappearInNS.to_string(), field_value: expire_in_ms.to_string(), } } diff --git a/xmtp_mls/src/groups/mod.rs b/xmtp_mls/src/groups/mod.rs index 2e743a0df..92fd73f7d 100644 --- a/xmtp_mls/src/groups/mod.rs +++ b/xmtp_mls/src/groups/mod.rs @@ -107,7 +107,7 @@ use std::{collections::HashSet, sync::Arc}; use xmtp_cryptography::signature::{sanitize_evm_addresses, AddressValidationError}; use xmtp_id::{InboxId, InboxIdRef}; -use crate::groups::group_mutable_metadata::GroupMessageExpirationSettings; +use crate::groups::group_mutable_metadata::ConversationMessageDisappearingSettings; use xmtp_common::retry::RetryableError; #[derive(Debug, Error)] @@ -294,7 +294,7 @@ pub struct GroupMetadataOptions { pub image_url_square: Option, pub description: Option, pub pinned_frame_url: Option, - pub message_retention_settings: Option, + pub message_disappearing_settings: Option, } impl Clone for MlsGroup { @@ -1140,18 +1140,18 @@ impl MlsGroup { pub async fn update_group_message_expiration_settings( &self, - settings: GroupMessageExpirationSettings, + settings: ConversationMessageDisappearingSettings, ) -> Result<(), GroupError> { let provider = self.client.mls_provider()?; - self.update_group_message_expiration_from_ms(&provider, settings.expire_from_ms) + self.update_group_message_expiration_from_ms(&provider, settings.from_ns) .await?; - self.update_group_message_expire_in_ms(&provider, settings.expire_in_ms) + self.update_group_message_expire_in_ms(&provider, settings.in_ns) .await } pub async fn remove_group_message_expiration_settings(&self) -> Result<(), GroupError> { - self.update_group_message_expiration_settings(GroupMessageExpirationSettings::default()) + self.update_group_message_expiration_settings(ConversationMessageDisappearingSettings::default()) .await } @@ -1182,20 +1182,20 @@ impl MlsGroup { pub fn group_message_expiration_settings( &self, provider: &XmtpOpenMlsProvider, - ) -> Result { + ) -> Result { let mutable_metadata = self.mutable_metadata(provider)?; let expire_from_ms = mutable_metadata .attributes - .get(&MetadataField::MessageExpirationFromMillis.to_string()); + .get(&MetadataField::MessageDisappearFromNS.to_string()); let expire_in_ms = mutable_metadata .attributes - .get(&MetadataField::MessageExpirationMillis.to_string()); + .get(&MetadataField::MessageDisappearInNS.to_string()); if let (Some(Ok(message_expiration_from_ms)), Some(Ok(message_expiration_ms))) = ( expire_from_ms.map(|s| s.parse::()), expire_in_ms.map(|s| s.parse::()), ) { - Ok(GroupMessageExpirationSettings::new( + Ok(ConversationMessageDisappearingSettings::new( message_expiration_from_ms, message_expiration_ms, )) @@ -1210,10 +1210,10 @@ impl MlsGroup { pub fn get_group_message_expiration_settings_if_valid( &self, provider: &XmtpOpenMlsProvider, - ) -> Option { + ) -> Option { match self.group_message_expiration_settings(provider) { Ok(expiration_settings) => { - if expiration_settings.expire_from_ms > 0 && expiration_settings.expire_in_ms > 0 { + if expiration_settings.from_ns > 0 && expiration_settings.in_ns > 0 { Some(expiration_settings) } else { None @@ -1877,7 +1877,7 @@ pub(crate) mod tests { use xmtp_proto::xmtp::mls::message_contents::EncodedContent; use super::{group_permissions::PolicySet, MlsGroup}; - use crate::groups::group_mutable_metadata::GroupMessageExpirationSettings; + use crate::groups::group_mutable_metadata::ConversationMessageDisappearingSettings; use crate::storage::group::StoredGroup; use crate::storage::schema::groups; use crate::{ @@ -2687,8 +2687,8 @@ pub(crate) mod tests { #[wasm_bindgen_test(unsupported = tokio::test(flavor = "current_thread"))] async fn test_group_options() { - let expected_group_message_expiration_settings = - GroupMessageExpirationSettings::new(100, 200); + let expected_group_message_disappearing_settings = + ConversationMessageDisappearingSettings::new(100, 200); let amal = ClientBuilder::new_test_client(&generate_local_wallet()).await; @@ -2700,7 +2700,7 @@ pub(crate) mod tests { image_url_square: Some("url".to_string()), description: Some("group description".to_string()), pinned_frame_url: Some("pinned frame".to_string()), - message_retention_settings: Some(expected_group_message_expiration_settings), + message_disappearing_settings: Some(expected_group_message_disappearing_settings), }, ) .unwrap(); @@ -2724,28 +2724,28 @@ pub(crate) mod tests { .attributes .get(&MetadataField::GroupPinnedFrameUrl.to_string()) .unwrap(); - let amal_group_message_expiration_from_ms = binding + let amal_group_message_disappear_from_ns = binding .attributes - .get(&MetadataField::MessageExpirationFromMillis.to_string()) + .get(&MetadataField::MessageDisappearFromNS.to_string()) .unwrap(); - let amal_group_message_expire_in_ms = binding + let amal_group_message_disappear_in_ns = binding .attributes - .get(&MetadataField::MessageExpirationMillis.to_string()) + .get(&MetadataField::MessageDisappearInNS.to_string()) .unwrap(); assert_eq!(amal_group_name, "Group Name"); assert_eq!(amal_group_image_url, "url"); assert_eq!(amal_group_description, "group description"); assert_eq!(amal_group_pinned_frame_url, "pinned frame"); assert_eq!( - amal_group_message_expiration_from_ms.clone(), - expected_group_message_expiration_settings - .expire_from_ms + amal_group_message_disappear_from_ns.clone(), + expected_group_message_disappearing_settings + .from_ns .to_string() ); assert_eq!( - amal_group_message_expire_in_ms.clone(), - expected_group_message_expiration_settings - .expire_in_ms + amal_group_message_disappear_in_ns.clone(), + expected_group_message_disappearing_settings + .in_ns .to_string() ); } @@ -3026,16 +3026,16 @@ pub(crate) mod tests { .unwrap(); assert!(group_mutable_metadata .attributes - .get(&MetadataField::MessageExpirationMillis.to_string()) + .get(&MetadataField::MessageDisappearInNS.to_string()) .is_none()); assert!(group_mutable_metadata .attributes - .get(&MetadataField::MessageExpirationFromMillis.to_string()) + .get(&MetadataField::MessageDisappearFromNS.to_string()) .is_none()); // Update group name let expected_group_message_expiration_settings = - GroupMessageExpirationSettings::new(100, 200); + ConversationMessageDisappearingSettings::new(100, 200); amal_group .update_group_message_expiration_settings(expected_group_message_expiration_settings) @@ -3049,22 +3049,22 @@ pub(crate) mod tests { .expect("msg"); let amal_message_expiration_from_ms: &String = binding .attributes - .get(&MetadataField::MessageExpirationFromMillis.to_string()) + .get(&MetadataField::MessageDisappearFromNS.to_string()) .unwrap(); let amal_message_expiration_ms: &String = binding .attributes - .get(&MetadataField::MessageExpirationMillis.to_string()) + .get(&MetadataField::MessageDisappearInNS.to_string()) .unwrap(); assert_eq!( amal_message_expiration_from_ms.clone(), expected_group_message_expiration_settings - .expire_from_ms + .from_ns .to_string() ); assert_eq!( amal_message_expiration_ms.clone(), expected_group_message_expiration_settings - .expire_in_ms + .in_ns .to_string() ); } diff --git a/xmtp_mls/src/storage/encrypted_store/group.rs b/xmtp_mls/src/storage/encrypted_store/group.rs index ad709aeaa..a726e3b5b 100644 --- a/xmtp_mls/src/storage/encrypted_store/group.rs +++ b/xmtp_mls/src/storage/encrypted_store/group.rs @@ -51,10 +51,10 @@ pub struct StoredGroup { pub dm_id: Option, /// Timestamp of when the last message was sent for this group (updated automatically in a trigger) pub last_message_ns: Option, - /// The Time in MS when the message expiration starts for the group - pub message_expire_from_ms: i64, - /// How long a message in the group can live in MS - pub message_expire_in_ms: i64, + /// The Time in NS when the messages should be deleted + pub message_disappear_from_ns: Option, + /// How long a message in the group can live in NS + pub message_disappear_in_ns: Option, } impl_fetch!(StoredGroup, groups, Vec); @@ -82,8 +82,8 @@ impl StoredGroup { rotated_at_ns: 0, dm_id: dm_members.map(String::from), last_message_ns: Some(now_ns()), - message_expire_from_ms: 0, - message_expire_in_ms: 0, + message_disappear_from_ns: None, + message_disappear_in_ns: None, } } @@ -109,8 +109,8 @@ impl StoredGroup { rotated_at_ns: 0, dm_id: dm_members.map(String::from), last_message_ns: Some(now_ns()), - message_expire_from_ms: 0, - message_expire_in_ms: 0, + message_disappear_from_ns: None, + message_disappear_in_ns: None, } } @@ -132,8 +132,8 @@ impl StoredGroup { rotated_at_ns: 0, dm_id: None, last_message_ns: Some(now_ns()), - message_expire_from_ms: 0, - message_expire_in_ms: 0, + message_disappear_from_ns: None, + message_disappear_in_ns: None, } } } diff --git a/xmtp_mls/src/storage/encrypted_store/group_message.rs b/xmtp_mls/src/storage/encrypted_store/group_message.rs index e6bc4a526..2670dfcde 100644 --- a/xmtp_mls/src/storage/encrypted_store/group_message.rs +++ b/xmtp_mls/src/storage/encrypted_store/group_message.rs @@ -1,4 +1,5 @@ use diesel::dsl::sql; +use diesel::sql_types::BigInt; use diesel::{ backend::Backend, deserialize::{self, FromSql, FromSqlRow}, @@ -429,6 +430,13 @@ impl DbConnection { pub fn delete_expired_messages(&self) -> Result { Ok(self.raw_query(|conn| { use diesel::prelude::*; + let disappear_from_ns = groups_dsl::message_disappear_from_ns + .assume_not_null() + .into_sql::(); + let disappear_duration_ns = groups_dsl::message_disappear_in_ns + .assume_not_null() + .into_sql::(); + let expire_messages = dsl::group_messages .left_join( groups_dsl::groups.on(sql::( @@ -437,10 +445,15 @@ impl DbConnection { .eq(sql::("lower(hex(groups.id))"))), ) .filter(dsl::delivery_status.eq(DeliveryStatus::Published)) - .filter(dsl::sent_at_ns.between( - groups_dsl::message_expire_from_ms * 1_000, // Convert ms to ns - (groups_dsl::message_expire_from_ms + groups_dsl::message_expire_in_ms) * 1_000, // Add duration and convert - )) + .filter( + groups_dsl::message_disappear_from_ns + .is_not_null() + .and(groups_dsl::message_disappear_in_ns.is_not_null()), + ) + .filter( + dsl::sent_at_ns + .between(disappear_from_ns, disappear_from_ns + disappear_duration_ns), + ) .select(dsl::id); let expired_message_ids = expire_messages.load::>(conn)?; @@ -617,10 +630,10 @@ pub(crate) mod tests { with_connection(|conn| { let mut group = generate_group(None); - let expired_from_ms = 1_000_500; // After Message 1 - let expired_in_ms = 500; // Before Message 3 - group.message_expire_from_ms = expired_from_ms; - group.message_expire_in_ms = expired_in_ms; + let disappear_from_ns = Some(1_000_500_000); // After Message 1 + let disappear_in_ns = Some(500_000); // Before Message 3 + group.message_disappear_from_ns = disappear_from_ns; + group.message_disappear_in_ns = disappear_in_ns; group.store(conn).unwrap(); diff --git a/xmtp_mls/src/storage/encrypted_store/schema_gen.rs b/xmtp_mls/src/storage/encrypted_store/schema_gen.rs index d7f20b5fa..2022d2441 100644 --- a/xmtp_mls/src/storage/encrypted_store/schema_gen.rs +++ b/xmtp_mls/src/storage/encrypted_store/schema_gen.rs @@ -63,8 +63,8 @@ diesel::table! { conversation_type -> Integer, dm_id -> Nullable, last_message_ns -> Nullable, - message_expire_from_ms -> BigInt, - message_expire_in_ms -> BigInt, + message_disappear_from_ns -> Nullable, + message_disappear_in_ns -> Nullable, } } From 354b7e62428150bd8b1a2423e19dd6eac6e63321 Mon Sep 17 00:00:00 2001 From: Mojtaba Chenani Date: Tue, 21 Jan 2025 19:28:34 +0100 Subject: [PATCH 04/20] wip --- bindings_ffi/src/mls.rs | 205 +++++------------- bindings_node/src/conversation.rs | 7 +- bindings_node/src/conversations.rs | 4 +- bindings_wasm/src/conversations.rs | 2 +- xmtp_mls/src/groups/group_mutable_metadata.rs | 5 +- xmtp_mls/src/groups/intents.rs | 8 +- xmtp_mls/src/groups/mls_sync.rs | 4 +- xmtp_mls/src/groups/mod.rs | 46 ++-- 8 files changed, 96 insertions(+), 185 deletions(-) diff --git a/bindings_ffi/src/mls.rs b/bindings_ffi/src/mls.rs index 7493b5485..725ab5acb 100644 --- a/bindings_ffi/src/mls.rs +++ b/bindings_ffi/src/mls.rs @@ -1299,17 +1299,14 @@ impl FfiConversationListItem { } #[derive(uniffi::Record, Debug)] -pub struct FfiGroupMessageExpirationSettings { - pub expire_from_ms: i64, - pub expire_in_ms: i64, +pub struct FfiConversationMessageDisappearingSettings { + pub from_ns: i64, + pub in_ns: i64, } -impl FfiGroupMessageExpirationSettings { - fn new(expire_from_ms: i64, expire_in_ms: i64) -> Self { - Self { - expire_from_ms, - expire_in_ms, - } +impl FfiConversationMessageDisappearingSettings { + fn new(from_ns: i64, in_ns: i64) -> Self { + Self { from_ns, in_ns } } } @@ -1423,9 +1420,9 @@ impl From for SortDirection { } } -impl From for ConversationMessageDisappearingSettings { - fn from(settings: FfiGroupMessageExpirationSettings) -> Self { - ConversationMessageDisappearingSettings::new(settings.expire_from_ms, settings.expire_in_ms) +impl From for ConversationMessageDisappearingSettings { + fn from(settings: FfiConversationMessageDisappearingSettings) -> Self { + ConversationMessageDisappearingSettings::new(settings.from_ns, settings.from_ns) } } @@ -1478,29 +1475,18 @@ pub struct FfiCreateGroupOptions { pub group_description: Option, pub group_pinned_frame_url: Option, pub custom_permission_policy_set: Option, - pub message_expiration_from_ms: Option, - pub message_expiration_ms: Option, + pub message_disappear_from_ns: Option, + pub message_disappear_in_ns: Option, } impl FfiCreateGroupOptions { pub fn into_group_metadata_options(self) -> GroupMetadataOptions { - let message_retention_settings: Option = - if let (Some(message_expiration_from_ms), Some(message_expiration_ms)) = - (self.message_expiration_from_ms, self.message_expiration_ms) - { - Some(ConversationMessageDisappearingSettings::new( - message_expiration_from_ms, - message_expiration_ms, - )) - } else { - None - }; GroupMetadataOptions { name: self.group_name, image_url_square: self.group_image_url_square, description: self.group_description, pinned_frame_url: self.group_pinned_frame_url, - message_disappearing_settings: None//todo: fix mapping, + message_disappearing_settings: None, } } } @@ -1731,27 +1717,28 @@ impl FfiConversation { .map_err(Into::into) } - pub async fn update_group_message_expiration_settings( + pub async fn update_conversation_message_disappearing_settings( &self, - settings: FfiGroupMessageExpirationSettings, + settings: FfiConversationMessageDisappearingSettings, ) -> Result<(), GenericError> { self.inner - .update_group_message_expiration_settings(ConversationMessageDisappearingSettings::from( - settings, - )) + .update_conversation_message_disappearing_settings( + ConversationMessageDisappearingSettings::from(settings), + ) .await?; Ok(()) } - pub fn group_message_expiration_settings( + pub fn conversation_message_disappearing_settings( &self, - ) -> Result { + ) -> Result { let provider = self.inner.mls_provider()?; - let group_message_expiration_settings = - self.inner.group_message_expiration_settings(&provider)?; // Use `?` to handle the Result + let group_message_expiration_settings = self + .inner + .conversation_message_disappearing_settings(&provider)?; // Use `?` to handle the Result - Ok(FfiGroupMessageExpirationSettings::new( + Ok(FfiConversationMessageDisappearingSettings::new( group_message_expiration_settings.from_ns, group_message_expiration_settings.in_ns, )) @@ -2292,12 +2279,12 @@ mod tests { use crate::{ connect_to_backend, decode_reaction, encode_reaction, get_inbox_id_for_address, inbox_owner::SigningError, FfiConsent, FfiConsentEntityType, FfiConsentState, - FfiContentType, FfiConversation, FfiConversationCallback, FfiConversationMessageKind, - FfiCreateGroupOptions, FfiDirection, FfiGroupMessageExpirationSettings, - FfiGroupPermissionsOptions, FfiInboxOwner, FfiListConversationsOptions, - FfiListMessagesOptions, FfiMessageWithReactions, FfiMetadataField, FfiPermissionPolicy, - FfiPermissionPolicySet, FfiPermissionUpdateType, FfiReaction, FfiReactionAction, - FfiReactionSchema, FfiSubscribeError, + FfiContentType, FfiConversation, FfiConversationCallback, + FfiConversationMessageDisappearingSettings, FfiConversationMessageKind, + FfiCreateGroupOptions, FfiDirection, FfiGroupPermissionsOptions, FfiInboxOwner, + FfiListConversationsOptions, FfiListMessagesOptions, FfiMessageWithReactions, + FfiMetadataField, FfiPermissionPolicy, FfiPermissionPolicySet, FfiPermissionUpdateType, + FfiReaction, FfiReactionAction, FfiReactionSchema, FfiSubscribeError, }; use ethers::utils::hex; use prost::Message; @@ -2723,7 +2710,9 @@ mod tests { } use xmtp_cryptography::utils::generate_local_wallet; - use xmtp_mls::groups::group_mutable_metadata::{ConversationMessageDisappearingSettings, MetadataField}; + use xmtp_mls::groups::group_mutable_metadata::{ + ConversationMessageDisappearingSettings, MetadataField, + }; use xmtp_mls::groups::{GroupMetadataOptions, PreconfiguredPolicies}; #[tokio::test(flavor = "multi_thread", worker_threads = 1)] @@ -2997,7 +2986,8 @@ mod tests { let amal = new_test_client().await; let bola = new_test_client().await; - let group_message_expiration_settings = FfiGroupMessageExpirationSettings::new(10, 100); + let conversation_message_disappearing_settings = + FfiConversationMessageDisappearingSettings::new(10, 100); let group = amal .conversations() @@ -3010,10 +3000,12 @@ mod tests { group_description: Some("group description".to_string()), group_pinned_frame_url: Some("pinned frame".to_string()), custom_permission_policy_set: None, - message_expiration_from_ms: Some( - group_message_expiration_settings.expire_from_ms, + message_disappear_from_ns: Some( + conversation_message_disappearing_settings.from_ns, + ), + message_disappear_in_ns: Some( + conversation_message_disappearing_settings.from_ns, ), - message_expiration_ms: Some(group_message_expiration_settings.expire_in_ms), }, ) .await @@ -3028,17 +3020,17 @@ mod tests { assert_eq!(group.group_pinned_frame_url().unwrap(), "pinned frame"); assert_eq!( group - .group_message_expiration_settings() + .conversation_message_disappearing_settings() .unwrap() - .expire_from_ms, - group_message_expiration_settings.expire_from_ms + .from_ns, + conversation_message_disappearing_settings.from_ns ); assert_eq!( group - .group_message_expiration_settings() + .conversation_message_disappearing_settings() .unwrap() - .expire_in_ms, - group_message_expiration_settings.expire_in_ms + .from_ns, + conversation_message_disappearing_settings.from_ns ); } @@ -4759,8 +4751,8 @@ mod tests { group_description: Some("A test group".to_string()), group_pinned_frame_url: Some("https://example.com/frame.png".to_string()), custom_permission_policy_set: Some(custom_permissions), - message_expiration_from_ms: None, - message_expiration_ms: None, + message_disappear_from_ns: None, + message_disappear_in_ns: None, }; let alix_group = alix @@ -4888,8 +4880,8 @@ mod tests { group_description: Some("A test group".to_string()), group_pinned_frame_url: Some("https://example.com/frame.png".to_string()), custom_permission_policy_set: Some(custom_permissions_invalid_1), - message_expiration_from_ms: None, - message_expiration_ms: None, + message_disappear_from_ns: None, + message_disappear_in_ns: None, }; let results_1 = alix @@ -4909,8 +4901,8 @@ mod tests { group_description: Some("A test group".to_string()), group_pinned_frame_url: Some("https://example.com/frame.png".to_string()), custom_permission_policy_set: Some(custom_permissions_valid.clone()), - message_expiration_from_ms: None, - message_expiration_ms: None, + message_disappear_from_ns: None, + message_disappear_in_ns: None, }; let results_2 = alix @@ -4930,8 +4922,8 @@ mod tests { group_description: Some("A test group".to_string()), group_pinned_frame_url: Some("https://example.com/frame.png".to_string()), custom_permission_policy_set: Some(custom_permissions_valid.clone()), - message_expiration_from_ms: None, - message_expiration_ms: None, + message_disappear_from_ns: None, + message_disappear_in_ns: None, }; let results_3 = alix @@ -4951,8 +4943,8 @@ mod tests { group_description: Some("A test group".to_string()), group_pinned_frame_url: Some("https://example.com/frame.png".to_string()), custom_permission_policy_set: Some(custom_permissions_valid), - message_expiration_from_ms: None, - message_expiration_ms: None, + message_disappear_from_ns: None, + message_disappear_in_ns: None, }; let results_4 = alix @@ -5126,93 +5118,6 @@ mod tests { assert_eq!(bola_conversations.len(), 1); } - // #[tokio::test(flavor = "multi_thread", worker_threads = 5)] - // async fn test_groups_with_expiration_settings() { - // // Step 1: Setup test clients - // let amal = new_test_client().await; - // - // // Step 2: Create policy set and groups - // let policy_set = Some(PreconfiguredPolicies::AdminsOnly.to_policy_set()); - // let group1 = amal - // .inner_client - // .create_group(policy_set.clone(), GroupMetadataOptions::default()) - // .expect("Failed to create group1"); - // let group2 = amal - // .inner_client - // .create_group(policy_set.clone(), GroupMetadataOptions::default()) - // .expect("Failed to create group2"); - // let group3 = amal - // .inner_client - // .create_group(policy_set, GroupMetadataOptions::default()) - // .expect("Failed to create group3"); - // - // // Sync groups - // group1.sync().await.expect("Failed to sync group1"); - // group2.sync().await.expect("Failed to sync group2"); - // group3.sync().await.expect("Failed to sync group3"); - // - // // Step 3: Verify metadata and set expiration for group1 - // let group1_metadata = group1 - // .mutable_metadata(&group1.mls_provider().expect("Missing MLS provider")) - // .expect("Failed to fetch metadata for group1"); - // assert_eq!(group1_metadata.attributes.len(), 4); - // assert!(group1_metadata - // .attributes - // .get(&MetadataField::GroupName.to_string()) - // .expect("Missing group name field") - // .is_empty()); - // - // // Enable expiration settings for group1 - // let expiration_settings = GroupMessageExpirationSettings { - // expire_from_ms: 1000, - // expire_in_ms: 1000, - // }; - // group1 - // .update_group_message_expiration_settings(expiration_settings) - // .await - // .expect("Failed to update expiration settings for group1"); - // - // // Verify only group1 is returned - // let groups_with_expiration = amal - // .conversations() - // .get_groups_with_expiration_enabled() - // .expect("Failed to fetch groups with expiration enabled"); - // assert_eq!(groups_with_expiration.len(), 1); - // assert!(groups_with_expiration.contains(&group1.group_id)); - // - // // Step 4: Remove expiration settings from group1 - // group1 - // .remove_group_message_expiration_settings() - // .await - // .expect("Failed to remove expiration settings for group1"); - // - // // Verify no groups are returned - // let groups_with_expiration = amal - // .conversations() - // .get_groups_with_expiration_enabled() - // .expect("Failed to fetch groups with expiration enabled"); - // assert_eq!(groups_with_expiration.len(), 0); - // - // // Step 5: Enable expiration settings for group1 and group2 - // group1 - // .update_group_message_expiration_settings(expiration_settings) - // .await - // .expect("Failed to update expiration settings for group1"); - // group2 - // .update_group_message_expiration_settings(expiration_settings) - // .await - // .expect("Failed to update expiration settings for group2"); - // - // // Verify group1 and group2 are returned - // let groups_with_expiration = amal - // .conversations() - // .get_groups_with_expiration_enabled() - // .expect("Failed to fetch groups with expiration enabled"); - // assert_eq!(groups_with_expiration.len(), 2); - // assert!(groups_with_expiration.contains(&group1.group_id)); - // assert!(groups_with_expiration.contains(&group2.group_id)); - // } - #[tokio::test(flavor = "multi_thread", worker_threads = 5)] async fn test_dm_streaming() { let alix = new_test_client().await; diff --git a/bindings_node/src/conversation.rs b/bindings_node/src/conversation.rs index 7ce441b5c..7d9b0c554 100644 --- a/bindings_node/src/conversation.rs +++ b/bindings_node/src/conversation.rs @@ -28,8 +28,8 @@ use crate::{ streams::StreamCloser, ErrorWrapper, }; -use xmtp_mls::groups::group_mutable_metadata::ConversationMessageDisappearingSettings as XmtpConversationMessageDisappearingSettings; use prost::Message as ProstMessage; +use xmtp_mls::groups::group_mutable_metadata::ConversationMessageDisappearingSettings as XmtpConversationMessageDisappearingSettings; use napi_derive::napi; @@ -48,10 +48,7 @@ pub struct ConversationMessageDisappearingSettings { impl ConversationMessageDisappearingSettings { #[napi] pub fn new(from_ns: i64, in_ns: i64) -> Self { - let inner = XmtpConversationMessageDisappearingSettings { - from_ns, - in_ns, - }; + let inner = XmtpConversationMessageDisappearingSettings { from_ns, in_ns }; Self { inner } } } diff --git a/bindings_node/src/conversations.rs b/bindings_node/src/conversations.rs index ec5ec0aab..69085b607 100644 --- a/bindings_node/src/conversations.rs +++ b/bindings_node/src/conversations.rs @@ -12,11 +12,11 @@ use xmtp_mls::storage::group::ConversationType as XmtpConversationType; use xmtp_mls::storage::group::GroupMembershipState as XmtpGroupMembershipState; use xmtp_mls::storage::group::GroupQueryArgs; +use crate::conversation::ConversationMessageDisappearingSettings; use crate::message::Message; use crate::permissions::{GroupPermissionsOptions, PermissionPolicySet}; use crate::ErrorWrapper; use crate::{client::RustXmtpClient, conversation::Conversation, streams::StreamCloser}; -use crate::conversation::ConversationMessageDisappearingSettings; #[napi] #[derive(Debug)] @@ -133,7 +133,7 @@ impl CreateGroupOptions { image_url_square: self.group_image_url_square, description: self.group_description, pinned_frame_url: self.group_pinned_frame_url, - message_disappearing_settings: None //todo: fix mapping, + message_disappearing_settings: None, //todo: fix mapping, } } } diff --git a/bindings_wasm/src/conversations.rs b/bindings_wasm/src/conversations.rs index 0d958937d..6cb376d46 100644 --- a/bindings_wasm/src/conversations.rs +++ b/bindings_wasm/src/conversations.rs @@ -167,7 +167,7 @@ impl CreateGroupOptions { image_url_square: self.group_image_url_square, description: self.group_description, pinned_frame_url: self.group_pinned_frame_url, - message_disappearing_settings: None// todo: fix mapping, + message_disappearing_settings: None, // todo: fix mapping, } } } diff --git a/xmtp_mls/src/groups/group_mutable_metadata.rs b/xmtp_mls/src/groups/group_mutable_metadata.rs index 617793e08..7bd5f70e1 100644 --- a/xmtp_mls/src/groups/group_mutable_metadata.rs +++ b/xmtp_mls/src/groups/group_mutable_metadata.rs @@ -78,10 +78,7 @@ pub struct ConversationMessageDisappearingSettings { impl ConversationMessageDisappearingSettings { pub fn new(from_ns: i64, in_ns: i64) -> Self { - Self { - from_ns, - in_ns, - } + Self { from_ns, in_ns } } } diff --git a/xmtp_mls/src/groups/intents.rs b/xmtp_mls/src/groups/intents.rs index 495b59e5c..00e28fd86 100644 --- a/xmtp_mls/src/groups/intents.rs +++ b/xmtp_mls/src/groups/intents.rs @@ -241,16 +241,16 @@ impl UpdateMetadataIntentData { } } - pub fn new_update_group_message_expiration_from_ms(expire_from_ms: i64) -> Self { + pub fn new_update_conversation_message_disappear_from_ns(from_ns: i64) -> Self { Self { field_name: MetadataField::MessageDisappearFromNS.to_string(), - field_value: expire_from_ms.to_string(), + field_value: from_ns.to_string(), } } - pub fn new_update_group_message_expiration_in_ms(expire_in_ms: i64) -> Self { + pub fn new_update_conversation_message_disappear_in_ns(in_ns: i64) -> Self { Self { field_name: MetadataField::MessageDisappearInNS.to_string(), - field_value: expire_in_ms.to_string(), + field_value: in_ns.to_string(), } } } diff --git a/xmtp_mls/src/groups/mls_sync.rs b/xmtp_mls/src/groups/mls_sync.rs index 9c22bc492..e3147b199 100644 --- a/xmtp_mls/src/groups/mls_sync.rs +++ b/xmtp_mls/src/groups/mls_sync.rs @@ -699,6 +699,7 @@ where "[{}] staged commit is valid, will attempt to merge", self.context().inbox_id() ); + mls_group.merge_staged_commit(provider, sc)?; self.save_transcript_message( provider.conn_ref(), @@ -768,6 +769,7 @@ where } IntentState::Committed => { Ok(provider.conn_ref().set_group_intent_committed(intent_id)?) + //todo: update the group based on the intent here? } IntentState::Published => { tracing::error!("Unexpected behaviour: returned intent state published from process_own_message"); @@ -976,7 +978,7 @@ where authority_id: content_type.authority_id.to_string(), reference_id: None, }; - + //todo: update the db? conversation disappearing settings msg.store_or_ignore(conn)?; Ok(Some(msg)) } diff --git a/xmtp_mls/src/groups/mod.rs b/xmtp_mls/src/groups/mod.rs index 92fd73f7d..829b4b631 100644 --- a/xmtp_mls/src/groups/mod.rs +++ b/xmtp_mls/src/groups/mod.rs @@ -1138,48 +1138,56 @@ impl MlsGroup { } } - pub async fn update_group_message_expiration_settings( + pub async fn update_conversation_message_disappearing_settings( &self, settings: ConversationMessageDisappearingSettings, ) -> Result<(), GroupError> { let provider = self.client.mls_provider()?; - self.update_group_message_expiration_from_ms(&provider, settings.from_ns) + self.update_conversation_message_disappear_from_ns(&provider, settings.from_ns) .await?; - self.update_group_message_expire_in_ms(&provider, settings.in_ns) + self.update_conversation_message_disappear_in_ns(&provider, settings.in_ns) .await + + //todo: update db here for the current user } - pub async fn remove_group_message_expiration_settings(&self) -> Result<(), GroupError> { - self.update_group_message_expiration_settings(ConversationMessageDisappearingSettings::default()) - .await + pub async fn remove_conversation_message_disappearing_settings( + &self, + ) -> Result<(), GroupError> { + self.update_conversation_message_disappearing_settings( + ConversationMessageDisappearingSettings::default(), + ) + .await } - async fn update_group_message_expiration_from_ms( + async fn update_conversation_message_disappear_from_ns( &self, provider: &XmtpOpenMlsProvider, expire_from_ms: i64, ) -> Result<(), GroupError> { let intent_data: Vec = - UpdateMetadataIntentData::new_update_group_message_expiration_from_ms(expire_from_ms) - .into(); + UpdateMetadataIntentData::new_update_conversation_message_disappear_from_ns( + expire_from_ms, + ) + .into(); let intent = self.queue_intent(&provider, IntentKind::MetadataUpdate, intent_data)?; self.sync_until_intent_resolved(&provider, intent.id).await } - async fn update_group_message_expire_in_ms( + async fn update_conversation_message_disappear_in_ns( &self, provider: &XmtpOpenMlsProvider, expire_in_ms: i64, ) -> Result<(), GroupError> { let intent_data: Vec = - UpdateMetadataIntentData::new_update_group_message_expiration_in_ms(expire_in_ms) + UpdateMetadataIntentData::new_update_conversation_message_disappear_in_ns(expire_in_ms) .into(); let intent = self.queue_intent(&provider, IntentKind::MetadataUpdate, intent_data)?; self.sync_until_intent_resolved(&provider, intent.id).await } - pub fn group_message_expiration_settings( + pub fn conversation_message_disappearing_settings( &self, provider: &XmtpOpenMlsProvider, ) -> Result { @@ -1211,7 +1219,7 @@ impl MlsGroup { &self, provider: &XmtpOpenMlsProvider, ) -> Option { - match self.group_message_expiration_settings(provider) { + match self.conversation_message_disappearing_settings(provider) { Ok(expiration_settings) => { if expiration_settings.from_ns > 0 && expiration_settings.in_ns > 0 { Some(expiration_settings) @@ -2700,7 +2708,9 @@ pub(crate) mod tests { image_url_square: Some("url".to_string()), description: Some("group description".to_string()), pinned_frame_url: Some("pinned frame".to_string()), - message_disappearing_settings: Some(expected_group_message_disappearing_settings), + message_disappearing_settings: Some( + expected_group_message_disappearing_settings, + ), }, ) .unwrap(); @@ -3038,7 +3048,9 @@ pub(crate) mod tests { ConversationMessageDisappearingSettings::new(100, 200); amal_group - .update_group_message_expiration_settings(expected_group_message_expiration_settings) + .update_conversation_message_disappearing_settings( + expected_group_message_expiration_settings, + ) .await .unwrap(); @@ -3063,9 +3075,7 @@ pub(crate) mod tests { ); assert_eq!( amal_message_expiration_ms.clone(), - expected_group_message_expiration_settings - .in_ns - .to_string() + expected_group_message_expiration_settings.in_ns.to_string() ); } From 1b1101640e6c798547b1b08def542e143536b2c7 Mon Sep 17 00:00:00 2001 From: Mojtaba Chenani Date: Tue, 21 Jan 2025 19:42:11 +0100 Subject: [PATCH 05/20] extract disappearing messages worker to a separate file --- xmtp_mls/src/groups/device_sync.rs | 73 +----------------- xmtp_mls/src/groups/disappearing_messages.rs | 81 ++++++++++++++++++++ xmtp_mls/src/groups/mod.rs | 1 + 3 files changed, 85 insertions(+), 70 deletions(-) create mode 100644 xmtp_mls/src/groups/disappearing_messages.rs diff --git a/xmtp_mls/src/groups/device_sync.rs b/xmtp_mls/src/groups/device_sync.rs index 3b164a70a..7c2d775df 100644 --- a/xmtp_mls/src/groups/device_sync.rs +++ b/xmtp_mls/src/groups/device_sync.rs @@ -1,4 +1,5 @@ use super::{GroupError, MlsGroup}; +use crate::groups::disappearing_messages::DisappearingMessagesCleanerWorker; #[cfg(any(test, feature = "test-utils"))] pub use crate::utils::WorkerHandle; use crate::{ @@ -107,8 +108,6 @@ pub enum DeviceSyncError { Bincode(#[from] bincode::Error), } -#[derive(Debug, Error)] -pub enum ExpirationWorkerError {} impl RetryableError for DeviceSyncError { fn is_retryable(&self) -> bool { true @@ -142,7 +141,7 @@ where } #[instrument(level = "trace", skip_all)] - pub fn start_expired_messages_cleaner_worker(&self) { + pub fn start_disappearing_messages_cleaner_worker(&self) { let client = self.clone(); tracing::debug!( inbox_id = client.inbox_id(), @@ -150,7 +149,7 @@ where "starting expired messages cleaner worker" ); - let worker = MessageExpirationWorker::new(client); + let worker = DisappearingMessagesCleanerWorker::new(client); worker.spawn_worker(); } } @@ -376,72 +375,6 @@ where } } -pub struct MessageExpirationWorker { - client: Client, - init: OnceCell<()>, -} -impl MessageExpirationWorker -where - ApiClient: XmtpApi + Send + Sync + 'static, - V: SmartContractSignatureVerifier + Send + Sync + 'static, -{ - fn new(client: Client) -> Self { - Self { - client, - init: OnceCell::new(), - } - } - fn spawn_worker(mut self) { - crate::spawn(None, async move { - let inbox_id = self.client.inbox_id().to_string(); - let installation_id = hex::encode(self.client.installation_public_key()); - while let Err(err) = self.run().await { - tracing::info!("Running worker.."); - match err { - DeviceSyncError::Client(ClientError::Storage( - StorageError::PoolNeedsConnection, - )) => { - tracing::warn!( - inbox_id, - installation_id, - "Pool disconnected. task will restart on reconnect" - ); - break; - } - _ => { - tracing::error!(inbox_id, installation_id, "sync worker error {err}"); - // Wait 2 seconds before restarting. - xmtp_common::time::sleep(Duration::from_secs(2)).await; - } - } - } - }); - } -} - -impl MessageExpirationWorker -where - ApiClient: XmtpApi + Send + Sync + 'static, - V: SmartContractSignatureVerifier + Send + Sync + 'static, -{ - /// Iterate on the list of groups and delete expired messages - async fn delete_expired_messages(&mut self) -> Result<(), DeviceSyncError> { - let provider = self.client.mls_provider()?; - if let Err(e) = provider.conn_ref().delete_expired_messages() { - tracing::error!("Failed to delete expired messages, error: {:?}", e); - } - Ok(()) - } - - async fn run(&mut self) -> Result<(), DeviceSyncError> { - // Call delete_expired_messages on every iteration - if let Err(err) = self.delete_expired_messages().await { - tracing::error!("Error during deletion of expired messages: {:?}", err); - } - Ok(()) - } -} - impl Client where ApiClient: XmtpApi, diff --git a/xmtp_mls/src/groups/disappearing_messages.rs b/xmtp_mls/src/groups/disappearing_messages.rs new file mode 100644 index 000000000..9c09497a7 --- /dev/null +++ b/xmtp_mls/src/groups/disappearing_messages.rs @@ -0,0 +1,81 @@ +use crate::client::ClientError; +use crate::storage::StorageError; +use crate::Client; +use std::time::Duration; +use thiserror::Error; +use tokio::sync::OnceCell; +use xmtp_id::scw_verifier::SmartContractSignatureVerifier; +use xmtp_proto::api_client::trait_impls::XmtpApi; + +#[derive(Debug, Error)] +pub enum DisappearingMessagesCleanerError { + #[error("storage error: {0}")] + Storage(#[from] StorageError), + #[error("client error: {0}")] + Client(#[from] ClientError), +} + +pub struct DisappearingMessagesCleanerWorker { + client: Client, + init: OnceCell<()>, +} +impl DisappearingMessagesCleanerWorker +where + ApiClient: XmtpApi + Send + Sync + 'static, + V: SmartContractSignatureVerifier + Send + Sync + 'static, +{ + pub fn new(client: Client) -> Self { + Self { + client, + init: OnceCell::new(), + } + } + pub(crate) fn spawn_worker(mut self) { + crate::spawn(None, async move { + let inbox_id = self.client.inbox_id().to_string(); + let installation_id = hex::encode(self.client.installation_public_key()); + while let Err(err) = self.run().await { + tracing::info!("Running worker.."); + match err { + DisappearingMessagesCleanerError::Client(ClientError::Storage( + StorageError::PoolNeedsConnection, + )) => { + tracing::warn!( + inbox_id, + installation_id, + "Pool disconnected. task will restart on reconnect" + ); + break; + } + _ => { + tracing::error!(inbox_id, installation_id, "sync worker error {err}"); + // Wait 2 seconds before restarting. + xmtp_common::time::sleep(Duration::from_secs(2)).await; + } + } + } + }); + } +} + +impl DisappearingMessagesCleanerWorker +where + ApiClient: XmtpApi + Send + Sync + 'static, + V: SmartContractSignatureVerifier + Send + Sync + 'static, +{ + /// Iterate on the list of groups and delete expired messages + async fn delete_expired_messages(&mut self) -> Result<(), DisappearingMessagesCleanerError> { + let provider = self.client.mls_provider()?; + if let Err(e) = provider.conn_ref().delete_expired_messages() { + tracing::error!("Failed to delete expired messages, error: {:?}", e); + } + Ok(()) + } + + async fn run(&mut self) -> Result<(), DisappearingMessagesCleanerError> { + if let Err(err) = self.delete_expired_messages().await { + tracing::error!("Error during deletion of expired messages: {:?}", err); + } + Ok(()) + } +} diff --git a/xmtp_mls/src/groups/mod.rs b/xmtp_mls/src/groups/mod.rs index 829b4b631..cc2448d56 100644 --- a/xmtp_mls/src/groups/mod.rs +++ b/xmtp_mls/src/groups/mod.rs @@ -7,6 +7,7 @@ pub mod intents; pub mod members; pub mod scoped_client; +mod disappearing_messages; pub(super) mod mls_sync; pub(super) mod subscriptions; pub mod validated_commit; From 8951e87ef9eed58682fe9db0093fdc79aab0fe7b Mon Sep 17 00:00:00 2001 From: Mojtaba Chenani Date: Thu, 23 Jan 2025 15:34:51 +0100 Subject: [PATCH 06/20] store metadatas into db --- bindings_ffi/src/mls.rs | 28 +++++++-------- bindings_node/src/permissions.rs | 6 ++-- bindings_wasm/src/permissions.rs | 12 +++---- xmtp_mls/src/groups/group_mutable_metadata.rs | 15 ++++---- xmtp_mls/src/groups/mls_sync.rs | 35 +++++++++++++++++-- xmtp_mls/src/groups/mod.rs | 18 +++++----- xmtp_mls/src/storage/encrypted_store/group.rs | 28 +++++++++++++++ 7 files changed, 102 insertions(+), 40 deletions(-) diff --git a/bindings_ffi/src/mls.rs b/bindings_ffi/src/mls.rs index 725ab5acb..445019122 100644 --- a/bindings_ffi/src/mls.rs +++ b/bindings_ffi/src/mls.rs @@ -790,7 +790,7 @@ pub struct FfiPermissionPolicySet { pub update_group_description_policy: FfiPermissionPolicy, pub update_group_image_url_square_policy: FfiPermissionPolicy, pub update_group_pinned_frame_url_policy: FfiPermissionPolicy, - pub update_message_expiration_ms_policy: FfiPermissionPolicy, + pub update_message_disappearing_policy: FfiPermissionPolicy, } impl From for FfiGroupPermissionsOptions { @@ -826,13 +826,13 @@ impl TryFrom for PolicySet { metadata_permissions_map.insert( MetadataField::MessageDisappearFromNS.to_string(), policy_set - .update_message_expiration_ms_policy + .update_message_disappearing_policy .clone() .try_into()?, ); metadata_permissions_map.insert( MetadataField::MessageDisappearInNS.to_string(), - policy_set.update_message_expiration_ms_policy.try_into()?, + policy_set.update_message_disappearing_policy.try_into()?, ); Ok(PolicySet { @@ -2263,7 +2263,7 @@ impl FfiGroupPermissions { update_group_pinned_frame_url_policy: get_policy( MetadataField::GroupPinnedFrameUrl.as_str(), ), - update_message_expiration_ms_policy: get_policy( + update_message_disappearing_policy: get_policy( MetadataField::MessageDisappearInNS.as_str(), ), }) @@ -4534,7 +4534,7 @@ mod tests { update_group_description_policy: FfiPermissionPolicy::Admin, update_group_image_url_square_policy: FfiPermissionPolicy::Admin, update_group_pinned_frame_url_policy: FfiPermissionPolicy::Admin, - update_message_expiration_ms_policy: FfiPermissionPolicy::Admin, + update_message_disappearing_policy: FfiPermissionPolicy::Admin, }; assert_eq!(alix_permission_policy_set, expected_permission_policy_set); @@ -4564,7 +4564,7 @@ mod tests { update_group_description_policy: FfiPermissionPolicy::Allow, update_group_image_url_square_policy: FfiPermissionPolicy::Allow, update_group_pinned_frame_url_policy: FfiPermissionPolicy::Allow, - update_message_expiration_ms_policy: FfiPermissionPolicy::Admin, + update_message_disappearing_policy: FfiPermissionPolicy::Admin, }; assert_eq!(alix_permission_policy_set, expected_permission_policy_set); } @@ -4595,7 +4595,7 @@ mod tests { update_group_description_policy: FfiPermissionPolicy::Allow, update_group_image_url_square_policy: FfiPermissionPolicy::Allow, update_group_pinned_frame_url_policy: FfiPermissionPolicy::Allow, - update_message_expiration_ms_policy: FfiPermissionPolicy::Allow, + update_message_disappearing_policy: FfiPermissionPolicy::Allow, }; assert_eq!(alix_permission_policy_set, expected_permission_policy_set); @@ -4625,7 +4625,7 @@ mod tests { update_group_description_policy: FfiPermissionPolicy::Allow, update_group_image_url_square_policy: FfiPermissionPolicy::Allow, update_group_pinned_frame_url_policy: FfiPermissionPolicy::Allow, - update_message_expiration_ms_policy: FfiPermissionPolicy::Admin, + update_message_disappearing_policy: FfiPermissionPolicy::Admin, }; assert_eq!(alix_permission_policy_set, expected_permission_policy_set); } @@ -4659,7 +4659,7 @@ mod tests { update_group_description_policy: FfiPermissionPolicy::Admin, update_group_image_url_square_policy: FfiPermissionPolicy::Admin, update_group_pinned_frame_url_policy: FfiPermissionPolicy::Admin, - update_message_expiration_ms_policy: FfiPermissionPolicy::Admin, + update_message_disappearing_policy: FfiPermissionPolicy::Admin, }; assert_eq!(alix_group_permissions, expected_permission_policy_set); @@ -4687,7 +4687,7 @@ mod tests { update_group_description_policy: FfiPermissionPolicy::Admin, update_group_image_url_square_policy: FfiPermissionPolicy::Allow, update_group_pinned_frame_url_policy: FfiPermissionPolicy::Admin, - update_message_expiration_ms_policy: FfiPermissionPolicy::Admin, + update_message_disappearing_policy: FfiPermissionPolicy::Admin, }; assert_eq!(alix_group_permissions, new_expected_permission_policy_set); @@ -4741,7 +4741,7 @@ mod tests { update_group_pinned_frame_url_policy: FfiPermissionPolicy::Admin, add_member_policy: FfiPermissionPolicy::Allow, remove_member_policy: FfiPermissionPolicy::Deny, - update_message_expiration_ms_policy: FfiPermissionPolicy::Admin, + update_message_disappearing_policy: FfiPermissionPolicy::Admin, }; let create_group_options = FfiCreateGroupOptions { @@ -4792,7 +4792,7 @@ mod tests { FfiPermissionPolicy::Admin ); assert_eq!( - group_permissions_policy_set.update_message_expiration_ms_policy, + group_permissions_policy_set.update_message_disappearing_policy, FfiPermissionPolicy::Admin ); assert_eq!( @@ -4858,7 +4858,7 @@ mod tests { update_group_pinned_frame_url_policy: FfiPermissionPolicy::Admin, add_member_policy: FfiPermissionPolicy::Allow, remove_member_policy: FfiPermissionPolicy::Deny, - update_message_expiration_ms_policy: FfiPermissionPolicy::Admin, + update_message_disappearing_policy: FfiPermissionPolicy::Admin, }; let custom_permissions_valid = FfiPermissionPolicySet { @@ -4870,7 +4870,7 @@ mod tests { update_group_pinned_frame_url_policy: FfiPermissionPolicy::Admin, add_member_policy: FfiPermissionPolicy::Allow, remove_member_policy: FfiPermissionPolicy::Deny, - update_message_expiration_ms_policy: FfiPermissionPolicy::Admin, + update_message_disappearing_policy: FfiPermissionPolicy::Admin, }; let create_group_options_invalid_1 = FfiCreateGroupOptions { diff --git a/bindings_node/src/permissions.rs b/bindings_node/src/permissions.rs index 94009f765..e3077dd81 100644 --- a/bindings_node/src/permissions.rs +++ b/bindings_node/src/permissions.rs @@ -161,7 +161,7 @@ pub struct PermissionPolicySet { pub update_group_description_policy: PermissionPolicy, pub update_group_image_url_square_policy: PermissionPolicy, pub update_group_pinned_frame_url_policy: PermissionPolicy, - pub update_message_expiration_ms_policy: PermissionPolicy, + pub update_message_disappearing_policy: PermissionPolicy, } impl From for GroupPermissionsOptions { @@ -216,7 +216,7 @@ impl GroupPermissions { update_group_pinned_frame_url_policy: get_policy( XmtpMetadataField::GroupPinnedFrameUrl.as_str(), ), - update_message_expiration_ms_policy: get_policy( + update_message_disappearing_policy: get_policy( XmtpMetadataField::MessageDisappearInNS.as_str(), ), }) @@ -247,7 +247,7 @@ impl TryFrom for PolicySet { ); metadata_permissions_map.insert( XmtpMetadataField::MessageDisappearInNS.to_string(), - policy_set.update_message_expiration_ms_policy.try_into()?, + policy_set.update_message_disappearing_policy.try_into()?, ); Ok(PolicySet { diff --git a/bindings_wasm/src/permissions.rs b/bindings_wasm/src/permissions.rs index 46222f0be..3256b9eeb 100644 --- a/bindings_wasm/src/permissions.rs +++ b/bindings_wasm/src/permissions.rs @@ -170,8 +170,8 @@ pub struct PermissionPolicySet { pub update_group_image_url_square_policy: PermissionPolicy, #[wasm_bindgen(js_name = updateGroupPinnedFrameUrlPolicy)] pub update_group_pinned_frame_url_policy: PermissionPolicy, - #[wasm_bindgen(js_name = updateMessageExpirationPolicy)] - pub update_message_expiration_ms_policy: PermissionPolicy, + #[wasm_bindgen(js_name = updateMessageDisappearingPolicy)] + pub update_message_disappearing_policy: PermissionPolicy, } #[wasm_bindgen] @@ -187,7 +187,7 @@ impl PermissionPolicySet { update_group_description_policy: PermissionPolicy, update_group_image_url_square_policy: PermissionPolicy, update_group_pinned_frame_url_policy: PermissionPolicy, - update_message_expiration_ms_policy: PermissionPolicy, + update_message_disappearing_policy: PermissionPolicy, ) -> Self { Self { add_member_policy, @@ -198,7 +198,7 @@ impl PermissionPolicySet { update_group_description_policy, update_group_image_url_square_policy, update_group_pinned_frame_url_policy, - update_message_expiration_ms_policy, + update_message_disappearing_policy, } } } @@ -257,7 +257,7 @@ impl GroupPermissions { update_group_pinned_frame_url_policy: get_policy( XmtpMetadataField::GroupPinnedFrameUrl.as_str(), ), - update_message_expiration_ms_policy: get_policy( + update_message_disappearing_policy: get_policy( XmtpMetadataField::MessageDisappearInNS.as_str(), ), }) @@ -286,7 +286,7 @@ impl TryFrom for PolicySet { ); metadata_permissions_map.insert( XmtpMetadataField::MessageDisappearInNS.to_string(), - policy_set.update_message_expiration_ms_policy.try_into()?, + policy_set.update_message_disappearing_policy.try_into()?, ); Ok(PolicySet { diff --git a/xmtp_mls/src/groups/group_mutable_metadata.rs b/xmtp_mls/src/groups/group_mutable_metadata.rs index 7bd5f70e1..b0631087c 100644 --- a/xmtp_mls/src/groups/group_mutable_metadata.rs +++ b/xmtp_mls/src/groups/group_mutable_metadata.rs @@ -6,7 +6,6 @@ use openmls::{ }; use prost::Message; use thiserror::Error; - use xmtp_proto::xmtp::mls::message_contents::{ GroupMutableMetadataV1 as GroupMutableMetadataProto, Inboxes as InboxesProto, }; @@ -58,8 +57,8 @@ impl MetadataField { MetadataField::Description => "description", MetadataField::GroupImageUrlSquare => "group_image_url_square", MetadataField::GroupPinnedFrameUrl => "group_pinned_frame_url", - MetadataField::MessageDisappearFromNS => "message_expiration_from_ms", - MetadataField::MessageDisappearInNS => "message_expiration_ms", + MetadataField::MessageDisappearFromNS => "message_disappear_from_ns", + MetadataField::MessageDisappearInNS => "message_disappear_in_ns", } } } @@ -143,14 +142,18 @@ impl GroupMutableMetadata { .unwrap_or_else(|| DEFAULT_GROUP_PINNED_FRAME_URL.to_string()), ); - if let Some(message_retention_settings) = opts.message_disappearing_settings { + if let Some(message_disappearing_settings) = opts.message_disappearing_settings { + println!( + "message_disappearing_setting:{:?}", + message_disappearing_settings.clone() + ); attributes.insert( MetadataField::MessageDisappearFromNS.to_string(), - message_retention_settings.from_ns.to_string(), + message_disappearing_settings.from_ns.to_string(), ); attributes.insert( MetadataField::MessageDisappearInNS.to_string(), - message_retention_settings.in_ns.to_string(), + message_disappearing_settings.in_ns.to_string(), ); } diff --git a/xmtp_mls/src/groups/mls_sync.rs b/xmtp_mls/src/groups/mls_sync.rs index e3147b199..0d59da597 100644 --- a/xmtp_mls/src/groups/mls_sync.rs +++ b/xmtp_mls/src/groups/mls_sync.rs @@ -8,6 +8,8 @@ use super::{ validated_commit::{extract_group_membership, CommitValidationError}, GroupError, HmacKey, MlsGroup, ScopedGroupClient, }; +use crate::groups::group_mutable_metadata::MetadataField; +use crate::storage::group_intent::IntentKind::MetadataUpdate; use crate::{ configuration::{ GRPC_DATA_LIMIT, HMAC_SALT, MAX_GROUP_SIZE, MAX_INTENT_PUBLISH_ATTEMPTS, MAX_PAST_EPOCHS, @@ -761,15 +763,15 @@ where intent_id ); match self - .process_own_message(intent, provider, message.into(), envelope) + .process_own_message(intent.clone(), provider, message.into(), envelope) .await? { IntentState::ToPublish => { Ok(provider.conn_ref().set_group_intent_to_publish(intent_id)?) } IntentState::Committed => { + self.handle_metadata_update(&provider, &intent)?; Ok(provider.conn_ref().set_group_intent_committed(intent_id)?) - //todo: update the group based on the intent here? } IntentState::Published => { tracing::error!("Unexpected behaviour: returned intent state published from process_own_message"); @@ -799,6 +801,35 @@ where } } + /// In case of metadataUpdate will extract the updated fields and store them to the db + fn handle_metadata_update( + &self, + provider: &XmtpOpenMlsProvider, + intent: &StoredGroupIntent, + ) -> Result<(), StorageError> { + if intent.kind == MetadataUpdate { + let data = UpdateMetadataIntentData::try_from(intent.data.clone())?; + + match data.field_name.as_str() { + field_name if field_name == MetadataField::MessageDisappearFromNS.as_str() => { + provider.conn_ref().update_message_disappearing_from_ns( + self.group_id.clone(), + data.field_value.parse::().ok(), + )? + } + field_name if field_name == MetadataField::MessageDisappearInNS.as_str() => { + provider.conn_ref().update_message_disappearing_in_ns( + self.group_id.clone(), + data.field_value.parse::().ok(), + )? + } + _ => {} // handle other metadata updates + } + } + + Ok(()) + } + #[tracing::instrument(level = "trace", skip_all)] async fn consume_message( &self, diff --git a/xmtp_mls/src/groups/mod.rs b/xmtp_mls/src/groups/mod.rs index cc2448d56..d8cf07f29 100644 --- a/xmtp_mls/src/groups/mod.rs +++ b/xmtp_mls/src/groups/mod.rs @@ -1193,20 +1193,20 @@ impl MlsGroup { provider: &XmtpOpenMlsProvider, ) -> Result { let mutable_metadata = self.mutable_metadata(provider)?; - let expire_from_ms = mutable_metadata + let disappear_from_ns = mutable_metadata .attributes .get(&MetadataField::MessageDisappearFromNS.to_string()); - let expire_in_ms = mutable_metadata + let disappear_in_ns = mutable_metadata .attributes .get(&MetadataField::MessageDisappearInNS.to_string()); - if let (Some(Ok(message_expiration_from_ms)), Some(Ok(message_expiration_ms))) = ( - expire_from_ms.map(|s| s.parse::()), - expire_in_ms.map(|s| s.parse::()), + if let (Some(Ok(message_disappear_from_ns)), Some(Ok(message_disappear_in_ns))) = ( + disappear_from_ns.map(|s| s.parse::()), + disappear_in_ns.map(|s| s.parse::()), ) { Ok(ConversationMessageDisappearingSettings::new( - message_expiration_from_ms, - message_expiration_ms, + message_disappear_from_ns, + message_disappear_in_ns, )) } else { Err(GroupError::GroupMetadata( @@ -3064,7 +3064,7 @@ pub(crate) mod tests { .attributes .get(&MetadataField::MessageDisappearFromNS.to_string()) .unwrap(); - let amal_message_expiration_ms: &String = binding + let amal_message_disappear_in_ns: &String = binding .attributes .get(&MetadataField::MessageDisappearInNS.to_string()) .unwrap(); @@ -3075,7 +3075,7 @@ pub(crate) mod tests { .to_string() ); assert_eq!( - amal_message_expiration_ms.clone(), + amal_message_disappear_in_ns.clone(), expected_group_message_expiration_settings.in_ns.to_string() ); } diff --git a/xmtp_mls/src/storage/encrypted_store/group.rs b/xmtp_mls/src/storage/encrypted_store/group.rs index a726e3b5b..c2ab12ce5 100644 --- a/xmtp_mls/src/storage/encrypted_store/group.rs +++ b/xmtp_mls/src/storage/encrypted_store/group.rs @@ -468,6 +468,34 @@ impl DbConnection { Ok(()) } + pub fn update_message_disappearing_from_ns( + &self, + group_id: Vec, + from_ns: Option, + ) -> Result<(), StorageError> { + self.raw_query(|conn| { + diesel::update(dsl::groups.find(&group_id)) + .set(dsl::message_disappear_from_ns.eq(from_ns)) + .execute(conn) + })?; + + Ok(()) + } + + pub fn update_message_disappearing_in_ns( + &self, + group_id: Vec, + in_ns: Option, + ) -> Result<(), StorageError> { + self.raw_query(|conn| { + diesel::update(dsl::groups.find(&group_id)) + .set(dsl::message_disappear_in_ns.eq(in_ns)) + .execute(conn) + })?; + + Ok(()) + } + pub fn insert_or_replace_group(&self, group: StoredGroup) -> Result { tracing::info!("Trying to insert group"); let stored_group = self.raw_query(|conn| { From de91ebff8e9dc48ee714df5289b5e4cc46015120 Mon Sep 17 00:00:00 2001 From: Mojtaba Chenani Date: Thu, 23 Jan 2025 15:40:13 +0100 Subject: [PATCH 07/20] remove comments --- bindings_ffi/src/mls.rs | 5 +---- xmtp_mls/src/groups/mls_sync.rs | 1 - xmtp_mls/src/groups/mod.rs | 2 -- 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/bindings_ffi/src/mls.rs b/bindings_ffi/src/mls.rs index 445019122..77fdad7cd 100644 --- a/bindings_ffi/src/mls.rs +++ b/bindings_ffi/src/mls.rs @@ -1736,7 +1736,7 @@ impl FfiConversation { let provider = self.inner.mls_provider()?; let group_message_expiration_settings = self .inner - .conversation_message_disappearing_settings(&provider)?; // Use `?` to handle the Result + .conversation_message_disappearing_settings(&provider)?; Ok(FfiConversationMessageDisappearingSettings::new( group_message_expiration_settings.from_ns, @@ -3034,9 +3034,6 @@ mod tests { ); } - //test to stream the metadata - //test the policy if someone can change the settings - // Looks like this test might be a separate issue #[tokio::test(flavor = "multi_thread", worker_threads = 5)] async fn test_can_stream_group_messages_for_updates() { diff --git a/xmtp_mls/src/groups/mls_sync.rs b/xmtp_mls/src/groups/mls_sync.rs index 0d59da597..dd05ed370 100644 --- a/xmtp_mls/src/groups/mls_sync.rs +++ b/xmtp_mls/src/groups/mls_sync.rs @@ -1009,7 +1009,6 @@ where authority_id: content_type.authority_id.to_string(), reference_id: None, }; - //todo: update the db? conversation disappearing settings msg.store_or_ignore(conn)?; Ok(Some(msg)) } diff --git a/xmtp_mls/src/groups/mod.rs b/xmtp_mls/src/groups/mod.rs index d8cf07f29..ccad35466 100644 --- a/xmtp_mls/src/groups/mod.rs +++ b/xmtp_mls/src/groups/mod.rs @@ -1149,8 +1149,6 @@ impl MlsGroup { .await?; self.update_conversation_message_disappear_in_ns(&provider, settings.in_ns) .await - - //todo: update db here for the current user } pub async fn remove_conversation_message_disappearing_settings( From 62e2aaeec83064fb9fd313c8fccdfe8ca443da51 Mon Sep 17 00:00:00 2001 From: Mojtaba Chenani Date: Thu, 23 Jan 2025 20:19:47 +0100 Subject: [PATCH 08/20] wip --- bindings_ffi/src/mls.rs | 17 ++++++++++++----- bindings_node/src/conversation.rs | 11 +++++++++-- bindings_node/src/conversations.rs | 4 +++- xmtp_mls/src/groups/group_mutable_metadata.rs | 2 +- 4 files changed, 25 insertions(+), 9 deletions(-) diff --git a/bindings_ffi/src/mls.rs b/bindings_ffi/src/mls.rs index 77fdad7cd..ee50ca673 100644 --- a/bindings_ffi/src/mls.rs +++ b/bindings_ffi/src/mls.rs @@ -822,7 +822,7 @@ impl TryFrom for PolicySet { MetadataField::GroupPinnedFrameUrl.to_string(), policy_set.update_group_pinned_frame_url_policy.try_into()?, ); - // MessageExpirationFromMillis follows the same policy as MessageExpirationMillis + // MessageDisappearFromNS follows the same policy as MessageDisappearInNS metadata_permissions_map.insert( MetadataField::MessageDisappearFromNS.to_string(), policy_set @@ -1298,7 +1298,7 @@ impl FfiConversationListItem { } } -#[derive(uniffi::Record, Debug)] +#[derive(uniffi::Record, Clone, Debug)] pub struct FfiConversationMessageDisappearingSettings { pub from_ns: i64, pub in_ns: i64, @@ -1310,6 +1310,12 @@ impl FfiConversationMessageDisappearingSettings { } } +impl From for FfiConversationMessageDisappearingSettings { + fn from(value: ConversationMessageDisappearingSettings) -> Self { + FfiConversationMessageDisappearingSettings::new(value.from_ns, value.in_ns) + } +} + impl From> for FfiConversation { fn from(mls_group: MlsGroup) -> FfiConversation { FfiConversation { inner: mls_group } @@ -1475,8 +1481,7 @@ pub struct FfiCreateGroupOptions { pub group_description: Option, pub group_pinned_frame_url: Option, pub custom_permission_policy_set: Option, - pub message_disappear_from_ns: Option, - pub message_disappear_in_ns: Option, + pub message_disappearing_settings: Option, } impl FfiCreateGroupOptions { @@ -1486,7 +1491,9 @@ impl FfiCreateGroupOptions { image_url_square: self.group_image_url_square, description: self.group_description, pinned_frame_url: self.group_pinned_frame_url, - message_disappearing_settings: None, + message_disappearing_settings: self + .message_disappearing_settings + .map(|settings| settings.into()), } } } diff --git a/bindings_node/src/conversation.rs b/bindings_node/src/conversation.rs index 7d9b0c554..7ffd0881d 100644 --- a/bindings_node/src/conversation.rs +++ b/bindings_node/src/conversation.rs @@ -38,10 +38,11 @@ pub struct GroupMetadata { inner: XmtpGroupMetadata, } -#[napi] +#[napi(object)] #[derive(Clone)] pub struct ConversationMessageDisappearingSettings { - inner: XmtpConversationMessageDisappearingSettings, + #[napi] + pub inner: XmtpConversationMessageDisappearingSettings, } #[napi] @@ -53,6 +54,12 @@ impl ConversationMessageDisappearingSettings { } } +impl From for XmtpConversationMessageDisappearingSettings { + fn from(value: ConversationMessageDisappearingSettings) -> Self { + XmtpConversationMessageDisappearingSettings::new(value.inner.from_ns, value.inner.in_ns) + } +} + #[napi] impl GroupMetadata { #[napi] diff --git a/bindings_node/src/conversations.rs b/bindings_node/src/conversations.rs index 69085b607..6df70a160 100644 --- a/bindings_node/src/conversations.rs +++ b/bindings_node/src/conversations.rs @@ -133,7 +133,9 @@ impl CreateGroupOptions { image_url_square: self.group_image_url_square, description: self.group_description, pinned_frame_url: self.group_pinned_frame_url, - message_disappearing_settings: None, //todo: fix mapping, + message_disappearing_settings: self + .message_disappearing_settings + .map(|settings| settings.into()), } } } diff --git a/xmtp_mls/src/groups/group_mutable_metadata.rs b/xmtp_mls/src/groups/group_mutable_metadata.rs index b0631087c..135247c4a 100644 --- a/xmtp_mls/src/groups/group_mutable_metadata.rs +++ b/xmtp_mls/src/groups/group_mutable_metadata.rs @@ -69,7 +69,7 @@ impl fmt::Display for MetadataField { } } -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[derive(Debug, Clone, PartialEq)] pub struct ConversationMessageDisappearingSettings { pub from_ns: i64, pub in_ns: i64, From c3a3f9177e454c9a122149f7a6a81630e45cad03 Mon Sep 17 00:00:00 2001 From: Mojtaba Chenani Date: Fri, 24 Jan 2025 16:38:11 +0100 Subject: [PATCH 09/20] add test --- bindings_ffi/src/mls.rs | 155 +++++++++++++++--- xmtp_mls/src/groups/device_sync.rs | 2 +- xmtp_mls/src/groups/disappearing_messages.rs | 10 +- xmtp_mls/src/groups/mls_sync.rs | 2 +- xmtp_mls/src/groups/mod.rs | 8 +- .../storage/encrypted_store/group_message.rs | 16 +- 6 files changed, 159 insertions(+), 34 deletions(-) diff --git a/bindings_ffi/src/mls.rs b/bindings_ffi/src/mls.rs index ee50ca673..ba9086d5b 100644 --- a/bindings_ffi/src/mls.rs +++ b/bindings_ffi/src/mls.rs @@ -1428,7 +1428,7 @@ impl From for SortDirection { impl From for ConversationMessageDisappearingSettings { fn from(settings: FfiConversationMessageDisappearingSettings) -> Self { - ConversationMessageDisappearingSettings::new(settings.from_ns, settings.from_ns) + ConversationMessageDisappearingSettings::new(settings.from_ns, settings.in_ns) } } @@ -1737,6 +1737,16 @@ impl FfiConversation { Ok(()) } + pub async fn remove_conversation_message_disappearing_settings( + &self, + ) -> Result<(), GenericError> { + self.inner + .remove_conversation_message_disappearing_settings() + .await?; + + Ok(()) + } + pub fn conversation_message_disappearing_settings( &self, ) -> Result { @@ -2717,10 +2727,6 @@ mod tests { } use xmtp_cryptography::utils::generate_local_wallet; - use xmtp_mls::groups::group_mutable_metadata::{ - ConversationMessageDisappearingSettings, MetadataField, - }; - use xmtp_mls::groups::{GroupMetadataOptions, PreconfiguredPolicies}; #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn test_can_add_wallet_to_inbox() { @@ -3007,11 +3013,8 @@ mod tests { group_description: Some("group description".to_string()), group_pinned_frame_url: Some("pinned frame".to_string()), custom_permission_policy_set: None, - message_disappear_from_ns: Some( - conversation_message_disappearing_settings.from_ns, - ), - message_disappear_in_ns: Some( - conversation_message_disappearing_settings.from_ns, + message_disappearing_settings: Some( + conversation_message_disappearing_settings.clone(), ), }, ) @@ -3030,14 +3033,14 @@ mod tests { .conversation_message_disappearing_settings() .unwrap() .from_ns, - conversation_message_disappearing_settings.from_ns + conversation_message_disappearing_settings.clone().from_ns ); assert_eq!( group .conversation_message_disappearing_settings() .unwrap() - .from_ns, - conversation_message_disappearing_settings.from_ns + .in_ns, + conversation_message_disappearing_settings.in_ns ); } @@ -4731,6 +4734,117 @@ mod tests { assert_eq!(alix_group.group_name().unwrap(), ""); } + #[tokio::test(flavor = "multi_thread", worker_threads = 5)] + async fn test_disappearing_messages_deletion() { + let alix = new_test_client().await; + let alix_provider = alix.inner_client.mls_provider().unwrap(); + + // Step 1: Create a group + let alix_group = alix + .conversations() + .create_group(vec![], FfiCreateGroupOptions::default()) + .await + .unwrap(); + + // Step 2: Send a message and sync + alix_group + .send("Msg 1 from group".as_bytes().to_vec()) + .await + .unwrap(); + alix_group.sync().await.unwrap(); + + // Step 3: Verify initial messages + let mut alix_messages = alix_group + .find_messages(FfiListMessagesOptions::default()) + .await + .unwrap(); + assert_eq!(alix_messages.len(), 1); + + // Step 4: Set disappearing settings to 5ns after the latest message + let latest_message_sent_at_ns = alix_messages.last().unwrap().sent_at_ns; + let disappearing_settings = + FfiConversationMessageDisappearingSettings::new(latest_message_sent_at_ns, 5); + alix_group + .update_conversation_message_disappearing_settings(disappearing_settings.clone()) + .await + .unwrap(); + alix_group.sync().await.unwrap(); + + // Verify the settings were applied + let group_from_db = alix_provider + .conn_ref() + .find_group(alix_group.id()) + .unwrap(); + assert_eq!( + group_from_db + .clone() + .unwrap() + .message_disappear_from_ns + .unwrap(), + disappearing_settings.from_ns + ); + assert_eq!( + group_from_db.unwrap().message_disappear_in_ns.unwrap(), + disappearing_settings.in_ns + ); + + // Step 5: Send additional messages + for msg in &["Msg 2 from group", "Msg 3 from group", "Msg 4 from group"] { + alix_group.send(msg.as_bytes().to_vec()).await.unwrap(); + } + alix_group.sync().await.unwrap(); + + // Step 6: Verify total message count before cleanup + alix_messages = alix_group + .find_messages(FfiListMessagesOptions::default()) + .await + .unwrap(); + let msg_counts_before_cleanup = alix_messages.len(); + + // Step 7: Start cleanup worker and delete expired messages + alix.inner_client + .start_disappearing_messages_cleaner_worker(); + + // Wait for cleanup to complete + tokio::time::sleep(std::time::Duration::from_secs(2)).await; + + // Step 8: Disable disappearing messages + alix_group + .remove_conversation_message_disappearing_settings() + .await + .unwrap(); + alix_group.sync().await.unwrap(); + + // Verify disappearing settings are disabled + let group_from_db = alix_provider + .conn_ref() + .find_group(alix_group.id()) + .unwrap(); + assert_eq!( + group_from_db + .clone() + .unwrap() + .message_disappear_from_ns + .unwrap(), + 0 + ); + assert_eq!(group_from_db.unwrap().message_disappear_in_ns.unwrap(), 0); + + // Step 9: Send another message + alix_group + .send("Msg 5 from group".as_bytes().to_vec()) + .await + .unwrap(); + + // Step 10: Verify messages after cleanup + alix_messages = alix_group + .find_messages(FfiListMessagesOptions::default()) + .await + .unwrap(); + assert_eq!(msg_counts_before_cleanup, alix_messages.len()); + // 3 messages got deleted, then two messages got added for metadataUpdate and one normal messaged added later + } + #[tokio::test(flavor = "multi_thread", worker_threads = 5)] async fn test_group_creation_custom_permissions() { let alix = new_test_client().await; @@ -4755,8 +4869,7 @@ mod tests { group_description: Some("A test group".to_string()), group_pinned_frame_url: Some("https://example.com/frame.png".to_string()), custom_permission_policy_set: Some(custom_permissions), - message_disappear_from_ns: None, - message_disappear_in_ns: None, + message_disappearing_settings: None, }; let alix_group = alix @@ -4884,8 +4997,7 @@ mod tests { group_description: Some("A test group".to_string()), group_pinned_frame_url: Some("https://example.com/frame.png".to_string()), custom_permission_policy_set: Some(custom_permissions_invalid_1), - message_disappear_from_ns: None, - message_disappear_in_ns: None, + message_disappearing_settings: None, }; let results_1 = alix @@ -4905,8 +5017,7 @@ mod tests { group_description: Some("A test group".to_string()), group_pinned_frame_url: Some("https://example.com/frame.png".to_string()), custom_permission_policy_set: Some(custom_permissions_valid.clone()), - message_disappear_from_ns: None, - message_disappear_in_ns: None, + message_disappearing_settings: None, }; let results_2 = alix @@ -4926,8 +5037,7 @@ mod tests { group_description: Some("A test group".to_string()), group_pinned_frame_url: Some("https://example.com/frame.png".to_string()), custom_permission_policy_set: Some(custom_permissions_valid.clone()), - message_disappear_from_ns: None, - message_disappear_in_ns: None, + message_disappearing_settings: None, }; let results_3 = alix @@ -4947,8 +5057,7 @@ mod tests { group_description: Some("A test group".to_string()), group_pinned_frame_url: Some("https://example.com/frame.png".to_string()), custom_permission_policy_set: Some(custom_permissions_valid), - message_disappear_from_ns: None, - message_disappear_in_ns: None, + message_disappearing_settings: None, }; let results_4 = alix diff --git a/xmtp_mls/src/groups/device_sync.rs b/xmtp_mls/src/groups/device_sync.rs index 7c2d775df..586265469 100644 --- a/xmtp_mls/src/groups/device_sync.rs +++ b/xmtp_mls/src/groups/device_sync.rs @@ -143,7 +143,7 @@ where #[instrument(level = "trace", skip_all)] pub fn start_disappearing_messages_cleaner_worker(&self) { let client = self.clone(); - tracing::debug!( + tracing::trace!( inbox_id = client.inbox_id(), installation_id = hex::encode(client.installation_public_key()), "starting expired messages cleaner worker" diff --git a/xmtp_mls/src/groups/disappearing_messages.rs b/xmtp_mls/src/groups/disappearing_messages.rs index 9c09497a7..03260c563 100644 --- a/xmtp_mls/src/groups/disappearing_messages.rs +++ b/xmtp_mls/src/groups/disappearing_messages.rs @@ -66,12 +66,16 @@ where /// Iterate on the list of groups and delete expired messages async fn delete_expired_messages(&mut self) -> Result<(), DisappearingMessagesCleanerError> { let provider = self.client.mls_provider()?; - if let Err(e) = provider.conn_ref().delete_expired_messages() { - tracing::error!("Failed to delete expired messages, error: {:?}", e); + match provider.conn_ref().delete_expired_messages() { + Ok(deleted_count) => { + tracing::info!("Successfully deleted {} expired messages", deleted_count); + } + Err(e) => { + tracing::error!("Failed to delete expired messages, error: {:?}", e); + } } Ok(()) } - async fn run(&mut self) -> Result<(), DisappearingMessagesCleanerError> { if let Err(err) = self.delete_expired_messages().await { tracing::error!("Error during deletion of expired messages: {:?}", err); diff --git a/xmtp_mls/src/groups/mls_sync.rs b/xmtp_mls/src/groups/mls_sync.rs index dd05ed370..b9bdf2f5f 100644 --- a/xmtp_mls/src/groups/mls_sync.rs +++ b/xmtp_mls/src/groups/mls_sync.rs @@ -770,7 +770,7 @@ where Ok(provider.conn_ref().set_group_intent_to_publish(intent_id)?) } IntentState::Committed => { - self.handle_metadata_update(&provider, &intent)?; + self.handle_metadata_update(provider, &intent)?; Ok(provider.conn_ref().set_group_intent_committed(intent_id)?) } IntentState::Published => { diff --git a/xmtp_mls/src/groups/mod.rs b/xmtp_mls/src/groups/mod.rs index ccad35466..3d8172322 100644 --- a/xmtp_mls/src/groups/mod.rs +++ b/xmtp_mls/src/groups/mod.rs @@ -1170,8 +1170,8 @@ impl MlsGroup { expire_from_ms, ) .into(); - let intent = self.queue_intent(&provider, IntentKind::MetadataUpdate, intent_data)?; - self.sync_until_intent_resolved(&provider, intent.id).await + let intent = self.queue_intent(provider, IntentKind::MetadataUpdate, intent_data)?; + self.sync_until_intent_resolved(provider, intent.id).await } async fn update_conversation_message_disappear_in_ns( @@ -1182,8 +1182,8 @@ impl MlsGroup { let intent_data: Vec = UpdateMetadataIntentData::new_update_conversation_message_disappear_in_ns(expire_in_ms) .into(); - let intent = self.queue_intent(&provider, IntentKind::MetadataUpdate, intent_data)?; - self.sync_until_intent_resolved(&provider, intent.id).await + let intent = self.queue_intent(provider, IntentKind::MetadataUpdate, intent_data)?; + self.sync_until_intent_resolved(provider, intent.id).await } pub fn conversation_message_disappearing_settings( diff --git a/xmtp_mls/src/storage/encrypted_store/group_message.rs b/xmtp_mls/src/storage/encrypted_store/group_message.rs index 2670dfcde..bf7116c7e 100644 --- a/xmtp_mls/src/storage/encrypted_store/group_message.rs +++ b/xmtp_mls/src/storage/encrypted_store/group_message.rs @@ -10,6 +10,8 @@ use diesel::{ }; use serde::{Deserialize, Serialize}; use std::collections::HashMap; +use std::ops::Sub; +use xmtp_common::time::now_ns; use xmtp_content_types::{ attachment, group_updated, membership_change, reaction, read_receipt, remote_attachment, reply, text, transaction_reference, @@ -436,6 +438,7 @@ impl DbConnection { let disappear_duration_ns = groups_dsl::message_disappear_in_ns .assume_not_null() .into_sql::(); + let now = now_ns(); let expire_messages = dsl::group_messages .left_join( @@ -445,14 +448,23 @@ impl DbConnection { .eq(sql::("lower(hex(groups.id))"))), ) .filter(dsl::delivery_status.eq(DeliveryStatus::Published)) + .filter(dsl::kind.eq(GroupMessageKind::Application)) .filter( groups_dsl::message_disappear_from_ns .is_not_null() .and(groups_dsl::message_disappear_in_ns.is_not_null()), ) .filter( - dsl::sent_at_ns - .between(disappear_from_ns, disappear_from_ns + disappear_duration_ns), + disappear_from_ns + .gt(0) // to make sure the settings are correct + .and( + dsl::sent_at_ns.gt(disappear_from_ns).and( + dsl::sent_at_ns.lt(sql::("") + .bind::(now) + .assume_not_null() + .sub(disappear_duration_ns)), + ), + ), ) .select(dsl::id); let expired_message_ids = expire_messages.load::>(conn)?; From 6f5473390d2a6760c89da2aed236ecd744d6f96b Mon Sep 17 00:00:00 2001 From: Mojtaba Chenani Date: Fri, 24 Jan 2025 17:09:14 +0100 Subject: [PATCH 10/20] fix tests --- xmtp_mls/src/groups/mod.rs | 21 ++------------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/xmtp_mls/src/groups/mod.rs b/xmtp_mls/src/groups/mod.rs index 3d8172322..0571fcd20 100644 --- a/xmtp_mls/src/groups/mod.rs +++ b/xmtp_mls/src/groups/mod.rs @@ -1213,23 +1213,6 @@ impl MlsGroup { } } - /// Check the group expiration settings, and if both are gt then 0 will return the settings - pub fn get_group_message_expiration_settings_if_valid( - &self, - provider: &XmtpOpenMlsProvider, - ) -> Option { - match self.conversation_message_disappearing_settings(provider) { - Ok(expiration_settings) => { - if expiration_settings.from_ns > 0 && expiration_settings.in_ns > 0 { - Some(expiration_settings) - } else { - None - } - } - Err(_) => None, - } - } - /// Retrieves the admin list of the group from the group's mutable metadata extension. pub fn admin_list(&self, provider: &XmtpOpenMlsProvider) -> Result, GroupError> { let mutable_metadata = self.mutable_metadata(provider)?; @@ -2708,7 +2691,7 @@ pub(crate) mod tests { description: Some("group description".to_string()), pinned_frame_url: Some("pinned frame".to_string()), message_disappearing_settings: Some( - expected_group_message_disappearing_settings, + expected_group_message_disappearing_settings.clone(), ), }, ) @@ -3048,7 +3031,7 @@ pub(crate) mod tests { amal_group .update_conversation_message_disappearing_settings( - expected_group_message_expiration_settings, + expected_group_message_expiration_settings.clone(), ) .await .unwrap(); From 0825f6ba0b24b39edd2648d0bc1eab44b62a4308 Mon Sep 17 00:00:00 2001 From: Mojtaba Chenani Date: Fri, 24 Jan 2025 18:12:05 +0100 Subject: [PATCH 11/20] fix tests --- xmtp_mls/src/storage/encrypted_store/group_message.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xmtp_mls/src/storage/encrypted_store/group_message.rs b/xmtp_mls/src/storage/encrypted_store/group_message.rs index bf7116c7e..1dcda5674 100644 --- a/xmtp_mls/src/storage/encrypted_store/group_message.rs +++ b/xmtp_mls/src/storage/encrypted_store/group_message.rs @@ -652,7 +652,7 @@ pub(crate) mod tests { let messages = vec![ generate_message(None, Some(&group.id), Some(1_000_000_000), None), generate_message(None, Some(&group.id), Some(1_001_000_000), None), - generate_message(None, Some(&group.id), Some(1_002_000_000), None), + generate_message(None, Some(&group.id), Some(2_000_000_000_000_000_000), None), ]; assert_ok!(messages.store(conn)); @@ -675,7 +675,7 @@ pub(crate) mod tests { .any(|msg| msg.sent_at_ns == 1_000_000_000)); // Message 1 assert!(remaining_messages .iter() - .any(|msg| msg.sent_at_ns == 1_002_000_000)); // Message 3 + .any(|msg| msg.sent_at_ns == 2_000_000_000_000_000_000)); // Message 3 }) .await } From 3911e8f2d3af4ea635cd376ac5953ff14a593e9e Mon Sep 17 00:00:00 2001 From: Mojtaba Chenani Date: Fri, 24 Jan 2025 18:14:41 +0100 Subject: [PATCH 12/20] address comments --- xmtp_mls/src/groups/group_mutable_metadata.rs | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/xmtp_mls/src/groups/group_mutable_metadata.rs b/xmtp_mls/src/groups/group_mutable_metadata.rs index 135247c4a..b5307ec66 100644 --- a/xmtp_mls/src/groups/group_mutable_metadata.rs +++ b/xmtp_mls/src/groups/group_mutable_metadata.rs @@ -69,7 +69,7 @@ impl fmt::Display for MetadataField { } } -#[derive(Debug, Clone, PartialEq)] +#[derive(Default, Debug, Clone, PartialEq)] pub struct ConversationMessageDisappearingSettings { pub from_ns: i64, pub in_ns: i64, @@ -81,12 +81,6 @@ impl ConversationMessageDisappearingSettings { } } -impl Default for ConversationMessageDisappearingSettings { - fn default() -> Self { - Self::new(0, 0) - } -} - /// Represents the mutable metadata for a group. /// /// This struct is stored as an MLS Unknown Group Context Extension. @@ -143,10 +137,6 @@ impl GroupMutableMetadata { ); if let Some(message_disappearing_settings) = opts.message_disappearing_settings { - println!( - "message_disappearing_setting:{:?}", - message_disappearing_settings.clone() - ); attributes.insert( MetadataField::MessageDisappearFromNS.to_string(), message_disappearing_settings.from_ns.to_string(), From 6bf97ed267fe9e1023a55416252590b1c65c64a6 Mon Sep 17 00:00:00 2001 From: Mojtaba Chenani Date: Mon, 27 Jan 2025 15:25:40 +0100 Subject: [PATCH 13/20] fix clippy --- xmtp_mls/src/groups/mod.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/xmtp_mls/src/groups/mod.rs b/xmtp_mls/src/groups/mod.rs index 0571fcd20..75d19db4c 100644 --- a/xmtp_mls/src/groups/mod.rs +++ b/xmtp_mls/src/groups/mod.rs @@ -3022,8 +3022,7 @@ pub(crate) mod tests { .is_none()); assert!(group_mutable_metadata .attributes - .get(&MetadataField::MessageDisappearFromNS.to_string()) - .is_none()); + .get(&MetadataField::MessageDisappearFromNS.to_string())); // Update group name let expected_group_message_expiration_settings = From bf2b8945437a6f9ea36832cd00f3a3532371513f Mon Sep 17 00:00:00 2001 From: Mojtaba Chenani Date: Mon, 27 Jan 2025 15:34:56 +0100 Subject: [PATCH 14/20] fix clippy --- xmtp_mls/src/groups/mod.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/xmtp_mls/src/groups/mod.rs b/xmtp_mls/src/groups/mod.rs index 75d19db4c..51c58d057 100644 --- a/xmtp_mls/src/groups/mod.rs +++ b/xmtp_mls/src/groups/mod.rs @@ -3016,13 +3016,12 @@ pub(crate) mod tests { let group_mutable_metadata = amal_group .mutable_metadata(&amal_group.mls_provider().unwrap()) .unwrap(); - assert!(group_mutable_metadata + assert_eq!(group_mutable_metadata .attributes - .get(&MetadataField::MessageDisappearInNS.to_string()) - .is_none()); - assert!(group_mutable_metadata + .get(&MetadataField::MessageDisappearInNS.to_string()), None); + assert_eq!(group_mutable_metadata .attributes - .get(&MetadataField::MessageDisappearFromNS.to_string())); + .get(&MetadataField::MessageDisappearFromNS.to_string()), None); // Update group name let expected_group_message_expiration_settings = From e6136cb738ddaed6db0eb84313c3275162a6bae3 Mon Sep 17 00:00:00 2001 From: Mojtaba Chenani Date: Mon, 27 Jan 2025 15:35:08 +0100 Subject: [PATCH 15/20] fix clippy --- xmtp_mls/src/groups/mod.rs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/xmtp_mls/src/groups/mod.rs b/xmtp_mls/src/groups/mod.rs index 51c58d057..5bca7ace6 100644 --- a/xmtp_mls/src/groups/mod.rs +++ b/xmtp_mls/src/groups/mod.rs @@ -3016,12 +3016,18 @@ pub(crate) mod tests { let group_mutable_metadata = amal_group .mutable_metadata(&amal_group.mls_provider().unwrap()) .unwrap(); - assert_eq!(group_mutable_metadata - .attributes - .get(&MetadataField::MessageDisappearInNS.to_string()), None); - assert_eq!(group_mutable_metadata - .attributes - .get(&MetadataField::MessageDisappearFromNS.to_string()), None); + assert_eq!( + group_mutable_metadata + .attributes + .get(&MetadataField::MessageDisappearInNS.to_string()), + None + ); + assert_eq!( + group_mutable_metadata + .attributes + .get(&MetadataField::MessageDisappearFromNS.to_string()), + None + ); // Update group name let expected_group_message_expiration_settings = From 1a527a9293c982c0b0e569db15aa3d24b6b8e85e Mon Sep 17 00:00:00 2001 From: Mojtaba Chenani Date: Mon, 27 Jan 2025 18:45:29 +0100 Subject: [PATCH 16/20] fix napi and address comments --- bindings_ffi/src/mls.rs | 46 +++++++++---------- bindings_node/src/conversation.rs | 19 ++++---- bindings_node/src/conversations.rs | 4 +- bindings_wasm/src/conversation.rs | 5 +- xmtp_mls/src/groups/disappearing_messages.rs | 7 ++- xmtp_mls/src/groups/group_mutable_metadata.rs | 4 +- xmtp_mls/src/groups/mod.rs | 19 ++++---- 7 files changed, 53 insertions(+), 51 deletions(-) diff --git a/bindings_ffi/src/mls.rs b/bindings_ffi/src/mls.rs index ba9086d5b..1a25afad2 100644 --- a/bindings_ffi/src/mls.rs +++ b/bindings_ffi/src/mls.rs @@ -19,7 +19,7 @@ use xmtp_id::{ InboxId, }; use xmtp_mls::groups::device_sync::preference_sync::UserPreferenceUpdate; -use xmtp_mls::groups::group_mutable_metadata::ConversationMessageDisappearingSettings; +use xmtp_mls::groups::group_mutable_metadata::MessageDisappearingSettings; use xmtp_mls::groups::scoped_client::LocalScopedGroupClient; use xmtp_mls::groups::HmacKey; use xmtp_mls::storage::group::ConversationType; @@ -1299,20 +1299,20 @@ impl FfiConversationListItem { } #[derive(uniffi::Record, Clone, Debug)] -pub struct FfiConversationMessageDisappearingSettings { +pub struct FfiMessageDisappearingSettings { pub from_ns: i64, pub in_ns: i64, } -impl FfiConversationMessageDisappearingSettings { +impl FfiMessageDisappearingSettings { fn new(from_ns: i64, in_ns: i64) -> Self { Self { from_ns, in_ns } } } -impl From for FfiConversationMessageDisappearingSettings { - fn from(value: ConversationMessageDisappearingSettings) -> Self { - FfiConversationMessageDisappearingSettings::new(value.from_ns, value.in_ns) +impl From for FfiMessageDisappearingSettings { + fn from(value: MessageDisappearingSettings) -> Self { + FfiMessageDisappearingSettings::new(value.from_ns, value.in_ns) } } @@ -1426,9 +1426,9 @@ impl From for SortDirection { } } -impl From for ConversationMessageDisappearingSettings { - fn from(settings: FfiConversationMessageDisappearingSettings) -> Self { - ConversationMessageDisappearingSettings::new(settings.from_ns, settings.in_ns) +impl From for MessageDisappearingSettings { + fn from(settings: FfiMessageDisappearingSettings) -> Self { + MessageDisappearingSettings::new(settings.from_ns, settings.in_ns) } } @@ -1481,7 +1481,7 @@ pub struct FfiCreateGroupOptions { pub group_description: Option, pub group_pinned_frame_url: Option, pub custom_permission_policy_set: Option, - pub message_disappearing_settings: Option, + pub message_disappearing_settings: Option, } impl FfiCreateGroupOptions { @@ -1726,12 +1726,12 @@ impl FfiConversation { pub async fn update_conversation_message_disappearing_settings( &self, - settings: FfiConversationMessageDisappearingSettings, + settings: FfiMessageDisappearingSettings, ) -> Result<(), GenericError> { self.inner - .update_conversation_message_disappearing_settings( - ConversationMessageDisappearingSettings::from(settings), - ) + .update_conversation_message_disappearing_settings(MessageDisappearingSettings::from( + settings, + )) .await?; Ok(()) @@ -1749,13 +1749,13 @@ impl FfiConversation { pub fn conversation_message_disappearing_settings( &self, - ) -> Result { + ) -> Result { let provider = self.inner.mls_provider()?; let group_message_expiration_settings = self .inner .conversation_message_disappearing_settings(&provider)?; - Ok(FfiConversationMessageDisappearingSettings::new( + Ok(FfiMessageDisappearingSettings::new( group_message_expiration_settings.from_ns, group_message_expiration_settings.in_ns, )) @@ -2296,12 +2296,12 @@ mod tests { use crate::{ connect_to_backend, decode_reaction, encode_reaction, get_inbox_id_for_address, inbox_owner::SigningError, FfiConsent, FfiConsentEntityType, FfiConsentState, - FfiContentType, FfiConversation, FfiConversationCallback, - FfiConversationMessageDisappearingSettings, FfiConversationMessageKind, + FfiContentType, FfiConversation, FfiConversationCallback, FfiConversationMessageKind, FfiCreateGroupOptions, FfiDirection, FfiGroupPermissionsOptions, FfiInboxOwner, - FfiListConversationsOptions, FfiListMessagesOptions, FfiMessageWithReactions, - FfiMetadataField, FfiPermissionPolicy, FfiPermissionPolicySet, FfiPermissionUpdateType, - FfiReaction, FfiReactionAction, FfiReactionSchema, FfiSubscribeError, + FfiListConversationsOptions, FfiListMessagesOptions, FfiMessageDisappearingSettings, + FfiMessageWithReactions, FfiMetadataField, FfiPermissionPolicy, FfiPermissionPolicySet, + FfiPermissionUpdateType, FfiReaction, FfiReactionAction, FfiReactionSchema, + FfiSubscribeError, }; use ethers::utils::hex; use prost::Message; @@ -3000,7 +3000,7 @@ mod tests { let bola = new_test_client().await; let conversation_message_disappearing_settings = - FfiConversationMessageDisappearingSettings::new(10, 100); + FfiMessageDisappearingSettings::new(10, 100); let group = amal .conversations() @@ -4763,7 +4763,7 @@ mod tests { // Step 4: Set disappearing settings to 5ns after the latest message let latest_message_sent_at_ns = alix_messages.last().unwrap().sent_at_ns; let disappearing_settings = - FfiConversationMessageDisappearingSettings::new(latest_message_sent_at_ns, 5); + FfiMessageDisappearingSettings::new(latest_message_sent_at_ns, 5); alix_group .update_conversation_message_disappearing_settings(disappearing_settings.clone()) .await diff --git a/bindings_node/src/conversation.rs b/bindings_node/src/conversation.rs index 7ffd0881d..960b9a76f 100644 --- a/bindings_node/src/conversation.rs +++ b/bindings_node/src/conversation.rs @@ -29,7 +29,7 @@ use crate::{ ErrorWrapper, }; use prost::Message as ProstMessage; -use xmtp_mls::groups::group_mutable_metadata::ConversationMessageDisappearingSettings as XmtpConversationMessageDisappearingSettings; +use xmtp_mls::groups::group_mutable_metadata::MessageDisappearingSettings as XmtpConversationMessageDisappearingSettings; use napi_derive::napi; @@ -40,23 +40,22 @@ pub struct GroupMetadata { #[napi(object)] #[derive(Clone)] -pub struct ConversationMessageDisappearingSettings { - #[napi] - pub inner: XmtpConversationMessageDisappearingSettings, +pub struct MessageDisappearingSettings { + pub from_ns: i64, + pub in_ns: i64, } #[napi] -impl ConversationMessageDisappearingSettings { +impl MessageDisappearingSettings { #[napi] pub fn new(from_ns: i64, in_ns: i64) -> Self { - let inner = XmtpConversationMessageDisappearingSettings { from_ns, in_ns }; - Self { inner } + Self { from_ns, in_ns } } } -impl From for XmtpConversationMessageDisappearingSettings { - fn from(value: ConversationMessageDisappearingSettings) -> Self { - XmtpConversationMessageDisappearingSettings::new(value.inner.from_ns, value.inner.in_ns) +impl From for XmtpConversationMessageDisappearingSettings { + fn from(value: MessageDisappearingSettings) -> Self { + XmtpConversationMessageDisappearingSettings::new(value.from_ns, value.in_ns) } } diff --git a/bindings_node/src/conversations.rs b/bindings_node/src/conversations.rs index 6df70a160..70f5fd170 100644 --- a/bindings_node/src/conversations.rs +++ b/bindings_node/src/conversations.rs @@ -12,7 +12,7 @@ use xmtp_mls::storage::group::ConversationType as XmtpConversationType; use xmtp_mls::storage::group::GroupMembershipState as XmtpGroupMembershipState; use xmtp_mls::storage::group::GroupQueryArgs; -use crate::conversation::ConversationMessageDisappearingSettings; +use crate::conversation::MessageDisappearingSettings; use crate::message::Message; use crate::permissions::{GroupPermissionsOptions, PermissionPolicySet}; use crate::ErrorWrapper; @@ -123,7 +123,7 @@ pub struct CreateGroupOptions { pub group_description: Option, pub group_pinned_frame_url: Option, pub custom_permission_policy_set: Option, - pub message_disappearing_settings: Option, + pub message_disappearing_settings: Option, } impl CreateGroupOptions { diff --git a/bindings_wasm/src/conversation.rs b/bindings_wasm/src/conversation.rs index 0c0ca2876..1bac2528a 100644 --- a/bindings_wasm/src/conversation.rs +++ b/bindings_wasm/src/conversation.rs @@ -18,7 +18,7 @@ use xmtp_mls::storage::group_message::{GroupMessageKind as XmtpGroupMessageKind, use xmtp_proto::xmtp::mls::message_contents::EncodedContent as XmtpEncodedContent; use prost::Message as ProstMessage; -use xmtp_mls::groups::group_mutable_metadata::ConversationMessageDisappearingSettings as XmtpGroupMessageDisappearingSettings; +use xmtp_mls::groups::group_mutable_metadata::MessageDisappearingSettings as XmtpMessageDisappearingSettings; #[wasm_bindgen] pub struct GroupMetadata { @@ -28,7 +28,8 @@ pub struct GroupMetadata { #[wasm_bindgen] #[derive(Clone)] pub struct ConversationMessageDisappearingSettings { - inner: XmtpGroupMessageDisappearingSettings, + #[allow(dead_code)] + inner: XmtpMessageDisappearingSettings, } #[wasm_bindgen] diff --git a/xmtp_mls/src/groups/disappearing_messages.rs b/xmtp_mls/src/groups/disappearing_messages.rs index 03260c563..2a7110bb8 100644 --- a/xmtp_mls/src/groups/disappearing_messages.rs +++ b/xmtp_mls/src/groups/disappearing_messages.rs @@ -7,6 +7,9 @@ use tokio::sync::OnceCell; use xmtp_id::scw_verifier::SmartContractSignatureVerifier; use xmtp_proto::api_client::trait_impls::XmtpApi; +/// Restart the DisappearingMessagesCleanerWorker every 1 sec to delete the expired messages +pub const WORKER_RESTART_DELAY: Duration = Duration::from_secs(1); + #[derive(Debug, Error)] pub enum DisappearingMessagesCleanerError { #[error("storage error: {0}")] @@ -17,6 +20,7 @@ pub enum DisappearingMessagesCleanerError { pub struct DisappearingMessagesCleanerWorker { client: Client, + #[allow(dead_code)] init: OnceCell<()>, } impl DisappearingMessagesCleanerWorker @@ -49,8 +53,7 @@ where } _ => { tracing::error!(inbox_id, installation_id, "sync worker error {err}"); - // Wait 2 seconds before restarting. - xmtp_common::time::sleep(Duration::from_secs(2)).await; + xmtp_common::time::sleep(WORKER_RESTART_DELAY).await; } } } diff --git a/xmtp_mls/src/groups/group_mutable_metadata.rs b/xmtp_mls/src/groups/group_mutable_metadata.rs index b5307ec66..48fcc8345 100644 --- a/xmtp_mls/src/groups/group_mutable_metadata.rs +++ b/xmtp_mls/src/groups/group_mutable_metadata.rs @@ -70,12 +70,12 @@ impl fmt::Display for MetadataField { } #[derive(Default, Debug, Clone, PartialEq)] -pub struct ConversationMessageDisappearingSettings { +pub struct MessageDisappearingSettings { pub from_ns: i64, pub in_ns: i64, } -impl ConversationMessageDisappearingSettings { +impl MessageDisappearingSettings { pub fn new(from_ns: i64, in_ns: i64) -> Self { Self { from_ns, in_ns } } diff --git a/xmtp_mls/src/groups/mod.rs b/xmtp_mls/src/groups/mod.rs index 5bca7ace6..a75391123 100644 --- a/xmtp_mls/src/groups/mod.rs +++ b/xmtp_mls/src/groups/mod.rs @@ -108,7 +108,7 @@ use std::{collections::HashSet, sync::Arc}; use xmtp_cryptography::signature::{sanitize_evm_addresses, AddressValidationError}; use xmtp_id::{InboxId, InboxIdRef}; -use crate::groups::group_mutable_metadata::ConversationMessageDisappearingSettings; +use crate::groups::group_mutable_metadata::MessageDisappearingSettings; use xmtp_common::retry::RetryableError; #[derive(Debug, Error)] @@ -295,7 +295,7 @@ pub struct GroupMetadataOptions { pub image_url_square: Option, pub description: Option, pub pinned_frame_url: Option, - pub message_disappearing_settings: Option, + pub message_disappearing_settings: Option, } impl Clone for MlsGroup { @@ -1141,7 +1141,7 @@ impl MlsGroup { pub async fn update_conversation_message_disappearing_settings( &self, - settings: ConversationMessageDisappearingSettings, + settings: MessageDisappearingSettings, ) -> Result<(), GroupError> { let provider = self.client.mls_provider()?; @@ -1155,7 +1155,7 @@ impl MlsGroup { &self, ) -> Result<(), GroupError> { self.update_conversation_message_disappearing_settings( - ConversationMessageDisappearingSettings::default(), + MessageDisappearingSettings::default(), ) .await } @@ -1189,7 +1189,7 @@ impl MlsGroup { pub fn conversation_message_disappearing_settings( &self, provider: &XmtpOpenMlsProvider, - ) -> Result { + ) -> Result { let mutable_metadata = self.mutable_metadata(provider)?; let disappear_from_ns = mutable_metadata .attributes @@ -1202,7 +1202,7 @@ impl MlsGroup { disappear_from_ns.map(|s| s.parse::()), disappear_in_ns.map(|s| s.parse::()), ) { - Ok(ConversationMessageDisappearingSettings::new( + Ok(MessageDisappearingSettings::new( message_disappear_from_ns, message_disappear_in_ns, )) @@ -1867,7 +1867,7 @@ pub(crate) mod tests { use xmtp_proto::xmtp::mls::message_contents::EncodedContent; use super::{group_permissions::PolicySet, MlsGroup}; - use crate::groups::group_mutable_metadata::ConversationMessageDisappearingSettings; + use crate::groups::group_mutable_metadata::MessageDisappearingSettings; use crate::storage::group::StoredGroup; use crate::storage::schema::groups; use crate::{ @@ -2678,7 +2678,7 @@ pub(crate) mod tests { #[wasm_bindgen_test(unsupported = tokio::test(flavor = "current_thread"))] async fn test_group_options() { let expected_group_message_disappearing_settings = - ConversationMessageDisappearingSettings::new(100, 200); + MessageDisappearingSettings::new(100, 200); let amal = ClientBuilder::new_test_client(&generate_local_wallet()).await; @@ -3030,8 +3030,7 @@ pub(crate) mod tests { ); // Update group name - let expected_group_message_expiration_settings = - ConversationMessageDisappearingSettings::new(100, 200); + let expected_group_message_expiration_settings = MessageDisappearingSettings::new(100, 200); amal_group .update_conversation_message_disappearing_settings( From fbdd11f2ed4c6a391eaef8bd9539913018b816c5 Mon Sep 17 00:00:00 2001 From: Mojtaba Chenani Date: Mon, 27 Jan 2025 18:49:05 +0100 Subject: [PATCH 17/20] add comments to messageDisappearingSettings --- bindings_ffi/src/mls.rs | 6 ++++++ bindings_node/src/conversation.rs | 6 ++++++ xmtp_mls/src/groups/group_mutable_metadata.rs | 6 ++++++ 3 files changed, 18 insertions(+) diff --git a/bindings_ffi/src/mls.rs b/bindings_ffi/src/mls.rs index 1a25afad2..3312b9be7 100644 --- a/bindings_ffi/src/mls.rs +++ b/bindings_ffi/src/mls.rs @@ -1298,6 +1298,12 @@ impl FfiConversationListItem { } } +/// Settings for disappearing messages in a conversation. +/// +/// # Fields +/// +/// * `from_ns` - The timestamp (in nanoseconds) from when messages should be tracked for deletion. +/// * `in_ns` - The duration (in nanoseconds) after which tracked messages will be deleted. #[derive(uniffi::Record, Clone, Debug)] pub struct FfiMessageDisappearingSettings { pub from_ns: i64, diff --git a/bindings_node/src/conversation.rs b/bindings_node/src/conversation.rs index 960b9a76f..1100727d2 100644 --- a/bindings_node/src/conversation.rs +++ b/bindings_node/src/conversation.rs @@ -38,6 +38,12 @@ pub struct GroupMetadata { inner: XmtpGroupMetadata, } +/// Settings for disappearing messages in a conversation. +/// +/// # Fields +/// +/// * `from_ns` - The timestamp (in nanoseconds) from when messages should be tracked for deletion. +/// * `in_ns` - The duration (in nanoseconds) after which tracked messages will be deleted. #[napi(object)] #[derive(Clone)] pub struct MessageDisappearingSettings { diff --git a/xmtp_mls/src/groups/group_mutable_metadata.rs b/xmtp_mls/src/groups/group_mutable_metadata.rs index 48fcc8345..d9f51d396 100644 --- a/xmtp_mls/src/groups/group_mutable_metadata.rs +++ b/xmtp_mls/src/groups/group_mutable_metadata.rs @@ -69,6 +69,12 @@ impl fmt::Display for MetadataField { } } +/// Settings for disappearing messages in a conversation. +/// +/// # Fields +/// +/// * `from_ns` - The timestamp (in nanoseconds) from when messages should be tracked for deletion. +/// * `in_ns` - The duration (in nanoseconds) after which tracked messages will be deleted. #[derive(Default, Debug, Clone, PartialEq)] pub struct MessageDisappearingSettings { pub from_ns: i64, From aeccd04c57c580e05e91f5e81a0e09c1f08e20d0 Mon Sep 17 00:00:00 2001 From: Mojtaba Chenani Date: Mon, 27 Jan 2025 19:05:41 +0100 Subject: [PATCH 18/20] fix missing mappings --- bindings_wasm/src/conversation.rs | 11 ++++++++++- bindings_wasm/src/conversations.rs | 10 ++++++---- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/bindings_wasm/src/conversation.rs b/bindings_wasm/src/conversation.rs index 1bac2528a..d8874b066 100644 --- a/bindings_wasm/src/conversation.rs +++ b/bindings_wasm/src/conversation.rs @@ -27,11 +27,20 @@ pub struct GroupMetadata { #[wasm_bindgen] #[derive(Clone)] -pub struct ConversationMessageDisappearingSettings { +pub struct MessageDisappearingSettings { #[allow(dead_code)] inner: XmtpMessageDisappearingSettings, } +impl From for XmtpMessageDisappearingSettings { + fn from(value: MessageDisappearingSettings) -> Self { + Self { + from_ns: value.inner.from_ns, + in_ns: value.inner.in_ns, + } + } +} + #[wasm_bindgen] impl GroupMetadata { #[wasm_bindgen(js_name = creatorInboxId)] diff --git a/bindings_wasm/src/conversations.rs b/bindings_wasm/src/conversations.rs index 6cb376d46..1daa849bb 100644 --- a/bindings_wasm/src/conversations.rs +++ b/bindings_wasm/src/conversations.rs @@ -7,7 +7,7 @@ use xmtp_mls::storage::group::ConversationType as XmtpConversationType; use xmtp_mls::storage::group::GroupMembershipState as XmtpGroupMembershipState; use xmtp_mls::storage::group::GroupQueryArgs; -use crate::conversation::ConversationMessageDisappearingSettings; +use crate::conversation::MessageDisappearingSettings; use crate::messages::Message; use crate::permissions::{GroupPermissionsOptions, PermissionPolicySet}; use crate::{client::RustXmtpClient, conversation::Conversation}; @@ -132,7 +132,7 @@ pub struct CreateGroupOptions { #[wasm_bindgen(js_name = customPermissionPolicySet)] pub custom_permission_policy_set: Option, #[wasm_bindgen(js_name = messageDisappearingSettings)] - pub message_disappearing_settings: Option, + pub message_disappearing_settings: Option, } #[wasm_bindgen] @@ -146,7 +146,7 @@ impl CreateGroupOptions { group_description: Option, group_pinned_frame_url: Option, custom_permission_policy_set: Option, - message_disappearing_settings: Option, + message_disappearing_settings: Option, ) -> Self { Self { permissions, @@ -167,7 +167,9 @@ impl CreateGroupOptions { image_url_square: self.group_image_url_square, description: self.group_description, pinned_frame_url: self.group_pinned_frame_url, - message_disappearing_settings: None, // todo: fix mapping, + message_disappearing_settings: self + .message_disappearing_settings + .map(|settings| settings.into()), } } } From 9ec0034310d78aaae870542caba540ba41f2704e Mon Sep 17 00:00:00 2001 From: Mojtaba Chenani Date: Mon, 27 Jan 2025 19:15:50 +0100 Subject: [PATCH 19/20] fix bindings_node --- bindings_node/test/Conversations.test.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/bindings_node/test/Conversations.test.ts b/bindings_node/test/Conversations.test.ts index ad280521b..0ec31af4c 100644 --- a/bindings_node/test/Conversations.test.ts +++ b/bindings_node/test/Conversations.test.ts @@ -52,7 +52,7 @@ describe('Conversations', () => { updateGroupDescriptionPolicy: 0, updateGroupImageUrlSquarePolicy: 0, updateGroupPinnedFrameUrlPolicy: 0, - updateMessageExpirationMsPolicy: 2, + updateMessageDisappearingPolicy: 2, }) expect(group.addedByInboxId()).toBe(client1.inboxId()) expect((await group.findMessages()).length).toBe(1) @@ -104,7 +104,7 @@ describe('Conversations', () => { updateGroupDescriptionPolicy: 1, updateGroupImageUrlSquarePolicy: 0, updateGroupPinnedFrameUrlPolicy: 3, - updateMessageExpirationMsPolicy: 2, + updateMessageDisappearingPolicy: 2, }, }) expect(group).toBeDefined() @@ -120,7 +120,7 @@ describe('Conversations', () => { updateGroupDescriptionPolicy: 1, updateGroupImageUrlSquarePolicy: 0, updateGroupPinnedFrameUrlPolicy: 3, - updateMessageExpirationMsPolicy: 2, + updateMessageDisappearingPolicy: 2, }) }) @@ -142,7 +142,7 @@ describe('Conversations', () => { updateGroupDescriptionPolicy: 0, updateGroupImageUrlSquarePolicy: 0, updateGroupPinnedFrameUrlPolicy: 0, - updateMessageExpirationMsPolicy: 2, + updateMessageDisappearingPolicy: 2, }) await group.updatePermissionPolicy( @@ -159,7 +159,7 @@ describe('Conversations', () => { updateGroupDescriptionPolicy: 0, updateGroupImageUrlSquarePolicy: 0, updateGroupPinnedFrameUrlPolicy: 0, - updateMessageExpirationMsPolicy: 2, + updateMessageDisappearingPolicy: 2, }) await group.updatePermissionPolicy( @@ -177,7 +177,7 @@ describe('Conversations', () => { updateGroupDescriptionPolicy: 0, updateGroupImageUrlSquarePolicy: 0, updateGroupPinnedFrameUrlPolicy: 0, - updateMessageExpirationMsPolicy: 2, + updateMessageDisappearingPolicy: 2, }) }) @@ -342,7 +342,7 @@ describe('Conversations', () => { updateGroupDescriptionPolicy: 2, updateGroupImageUrlSquarePolicy: 2, updateGroupPinnedFrameUrlPolicy: 2, - updateMessageExpirationMsPolicy: 2, + updateMessageDisappearingPolicy: 2, }) const groupWithDescription = await client1 From 94f5b342988973783dbf31a68e89f71a382a743e Mon Sep 17 00:00:00 2001 From: Mojtaba Chenani Date: Mon, 27 Jan 2025 19:22:48 +0100 Subject: [PATCH 20/20] fix bindings_node --- bindings_node/test/Conversations.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bindings_node/test/Conversations.test.ts b/bindings_node/test/Conversations.test.ts index 0ec31af4c..0c8ed0292 100644 --- a/bindings_node/test/Conversations.test.ts +++ b/bindings_node/test/Conversations.test.ts @@ -204,7 +204,7 @@ describe('Conversations', () => { updateGroupImageUrlSquarePolicy: 0, updateGroupNamePolicy: 0, updateGroupPinnedFrameUrlPolicy: 0, - updateMessageExpirationMsPolicy: 0, + updateMessageDisappearingPolicy: 0, }) expect(group.addedByInboxId()).toBe(client1.inboxId()) expect((await group.findMessages()).length).toBe(0)