Skip to content

Commit

Permalink
Remove Purpose, Add Conversation Type to database. Sync sync groups w…
Browse files Browse the repository at this point in the history
…hen syncing all groups too. (#1261)

* logs

* wip

* cleanup

* remove comment

* syncable groups fix

* revert this file

* invert the approach

* add down
  • Loading branch information
codabrink authored Nov 14, 2024
1 parent 42e3db6 commit 213dd16
Show file tree
Hide file tree
Showing 16 changed files with 113 additions and 85 deletions.
5 changes: 3 additions & 2 deletions bindings_ffi/src/mls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,15 @@ use xmtp_id::{
InboxId,
};
use xmtp_mls::groups::scoped_client::LocalScopedGroupClient;
use xmtp_mls::storage::group::ConversationType;
use xmtp_mls::storage::group_message::MsgQueryArgs;
use xmtp_mls::storage::group_message::SortDirection;
use xmtp_mls::{
api::ApiClientWrapper,
builder::ClientBuilder,
client::{Client as MlsClient, ClientError},
groups::{
group_metadata::{ConversationType, GroupMetadata},
group_metadata::GroupMetadata,
group_mutable_metadata::MetadataField,
group_permissions::{
BasePolicies, GroupMutablePermissions, GroupMutablePermissionsError,
Expand Down Expand Up @@ -857,7 +858,7 @@ impl FfiConversations {

pub async fn sync_all_conversations(&self) -> Result<u32, GenericError> {
let inner = self.inner_client.as_ref();
let groups = inner.find_groups(GroupQueryArgs::default())?;
let groups = inner.find_groups(GroupQueryArgs::default().include_sync_groups())?;

log::info!(
"groups for client inbox id {:?}: {:?}",
Expand Down
7 changes: 3 additions & 4 deletions bindings_node/src/conversation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,10 @@ use napi::{
use xmtp_cryptography::signature::ed25519_public_key_to_address;
use xmtp_mls::{
groups::{
group_metadata::{ConversationType, GroupMetadata as XmtpGroupMetadata},
members::PermissionLevel as XmtpPermissionLevel,
MlsGroup, UpdateAdminListType,
group_metadata::GroupMetadata as XmtpGroupMetadata,
members::PermissionLevel as XmtpPermissionLevel, MlsGroup, UpdateAdminListType,
},
storage::group_message::MsgQueryArgs,
storage::{group::ConversationType, group_message::MsgQueryArgs},
};
use xmtp_proto::xmtp::mls::message_contents::EncodedContent as XmtpEncodedContent;

Expand Down
2 changes: 1 addition & 1 deletion bindings_node/src/conversations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use napi::bindgen_prelude::{Error, Result, Uint8Array};
use napi::threadsafe_function::{ErrorStrategy, ThreadsafeFunction, ThreadsafeFunctionCallMode};
use napi::JsFunction;
use napi_derive::napi;
use xmtp_mls::groups::group_metadata::ConversationType as XmtpConversationType;
use xmtp_mls::groups::{GroupMetadataOptions, PreconfiguredPolicies};
use xmtp_mls::storage::group::ConversationType as XmtpConversationType;
use xmtp_mls::storage::group::GroupMembershipState as XmtpGroupMembershipState;
use xmtp_mls::storage::group::GroupQueryArgs;

Expand Down
6 changes: 3 additions & 3 deletions bindings_wasm/src/conversation.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
use std::sync::Arc;
use wasm_bindgen::JsValue;
use wasm_bindgen::{prelude::wasm_bindgen, JsError};
use xmtp_mls::storage::group::ConversationType;

use crate::client::RustXmtpClient;
use crate::encoded_content::EncodedContent;
use crate::messages::{ListMessagesOptions, Message};
use crate::{consent_state::ConsentState, permissions::GroupPermissions};
use xmtp_cryptography::signature::ed25519_public_key_to_address;
use xmtp_mls::groups::{
group_metadata::{ConversationType, GroupMetadata as XmtpGroupMetadata},
members::PermissionLevel as XmtpPermissionLevel,
MlsGroup, UpdateAdminListType,
group_metadata::GroupMetadata as XmtpGroupMetadata,
members::PermissionLevel as XmtpPermissionLevel, MlsGroup, UpdateAdminListType,
};
use xmtp_proto::xmtp::mls::message_contents::EncodedContent as XmtpEncodedContent;

Expand Down
2 changes: 1 addition & 1 deletion bindings_wasm/src/conversations.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use std::sync::Arc;
use wasm_bindgen::prelude::wasm_bindgen;
use wasm_bindgen::{JsError, JsValue};
use xmtp_mls::groups::group_metadata::ConversationType as XmtpConversationType;
use xmtp_mls::groups::{GroupMetadataOptions, PreconfiguredPolicies};
use xmtp_mls::storage::group::ConversationType as XmtpConversationType;
use xmtp_mls::storage::group::GroupMembershipState as XmtpGroupMembershipState;
use xmtp_mls::storage::group::GroupQueryArgs;

Expand Down
9 changes: 3 additions & 6 deletions bindings_wasm/tests/web.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
use bindings_wasm::client::Level;
use bindings_wasm::{
client::{create_client, LogOptions},
inbox_id::get_inbox_id_for_address,
};
use bindings_wasm::client::LogLevel;
use bindings_wasm::client::{create_client, LogOptions};
use wasm_bindgen::prelude::*;
use wasm_bindgen_test::*;
use xmtp_api_http::constants::ApiUrls;
Expand Down Expand Up @@ -35,7 +32,7 @@ pub async fn test_create_client() {
Some(LogOptions {
structured: false,
performance: false,
level: Some(Level::Info),
level: Some(LogLevel::Info),
}),
)
.await;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
ALTER TABLE groups
ADD COLUMN purpose INTEGER NOT NULL;

UPDATE groups
SET purpose = CASE
WHEN conversation_type = 3 THEN 2
ELSE 1
END;

ALTER TABLE groups DROP COLUMN conversation_type;
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
ALTER TABLE groups
ADD COLUMN conversation_type INTEGER NOT NULL;

UPDATE groups
SET conversation_type = CASE
-- Purpose is conversation and is not a DM
-- Then set to 1 (ConversationType::Group)
WHEN purpose = 1 AND dm_inbox_id IS NULL THEN 1
-- Otherwise dm_inbox_id is not null
-- Then set to 2 (ConversationType::Dm)
WHEN purpose = 1 THEN 2
-- Otherwise this is a Sync Group
ELSE 3
END;

ALTER TABLE groups DROP COLUMN purpose;
9 changes: 7 additions & 2 deletions xmtp_mls/src/groups/device_sync.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use super::group_metadata::ConversationType;
use super::{GroupError, MlsGroup};
use crate::configuration::NS_IN_HOUR;
use crate::retry::{RetryBuilder, RetryableError};
use crate::storage::group::GroupQueryArgs;
use crate::storage::group::{ConversationType, GroupQueryArgs};
use crate::storage::group_message::MsgQueryArgs;
use crate::storage::DbConnection;
use crate::subscriptions::{StreamMessages, SubscribeError, SyncMessage};
Expand Down Expand Up @@ -219,6 +218,10 @@ where
* Will auto-send a sync request if sync group is created.
*/
pub async fn sync_init(&self, provider: &XmtpOpenMlsProvider) -> Result<(), DeviceSyncError> {
tracing::info!(
"Initializing device sync... url: {:?}",
self.history_sync_url
);
if self.get_sync_group().is_err() {
self.ensure_sync_group(provider).await?;

Expand All @@ -227,6 +230,7 @@ where
self.send_sync_request(provider, DeviceSyncKind::MessageHistory)
.await?;
}
tracing::info!("Device sync initialized.");

Ok(())
}
Expand All @@ -252,6 +256,7 @@ where
provider: &XmtpOpenMlsProvider,
kind: DeviceSyncKind,
) -> Result<DeviceSyncRequestProto, DeviceSyncError> {
tracing::info!("Sending a sync request for {kind:?}");
let request = DeviceSyncRequest::new(kind);

// find the sync group
Expand Down
3 changes: 2 additions & 1 deletion xmtp_mls/src/groups/device_sync/message_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@ where
conn: &DbConnection,
) -> Result<Vec<Syncable>, DeviceSyncError> {
let groups = conn
.find_groups(GroupQueryArgs::default().conversation_type(ConversationType::Group))?
.find_groups(GroupQueryArgs::default())?
.into_iter()
.map(Syncable::Group)
.collect();

Ok(groups)
}

Expand Down
8 changes: 2 additions & 6 deletions xmtp_mls/src/groups/group_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ use xmtp_proto::xmtp::mls::message_contents::{
GroupMetadataV1 as GroupMetadataProto, Inbox as InboxProto,
};

use crate::storage::group::ConversationType;

#[derive(Debug, Error)]
pub enum GroupMetadataError {
#[error("serialization: {0}")]
Expand Down Expand Up @@ -104,12 +106,6 @@ impl TryFrom<&Extensions> for GroupMetadata {
* *DM*: A conversation between 2 members with simplified permissions
* *Sync*: A conversation between all the devices of a single member with simplified permissions
*/
#[derive(Debug, Copy, Clone, PartialEq)]
pub enum ConversationType {
Group,
Dm,
Sync,
}

impl From<ConversationType> for ConversationTypeProto {
fn from(value: ConversationType) -> Self {
Expand Down
2 changes: 2 additions & 0 deletions xmtp_mls/src/groups/mls_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,7 @@ where

// Ignore this installation's sync messages
if !sent_from_this_installation {
tracing::info!("Received a history request.");
let _ = self.client.local_events().send(LocalEvents::SyncMessage(
SyncMessage::Request { message_id },
));
Expand Down Expand Up @@ -531,6 +532,7 @@ where

// Ignore this installation's sync messages
if !sent_from_this_installation {
tracing::info!("Received a history reply.");
let _ = self.client.local_events().send(LocalEvents::SyncMessage(
SyncMessage::Reply { message_id },
));
Expand Down
34 changes: 14 additions & 20 deletions xmtp_mls/src/groups/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ use self::{
validated_commit::extract_group_membership,
};
use self::{
group_metadata::{ConversationType, GroupMetadata, GroupMetadataError},
group_metadata::{GroupMetadata, GroupMetadataError},
group_permissions::PolicySet,
validated_commit::CommitValidationError,
};
Expand Down Expand Up @@ -85,7 +85,7 @@ use crate::{
storage::{
consent_record::{ConsentState, ConsentType, StoredConsentRecord},
db_connection::DbConnection,
group::{GroupMembershipState, Purpose, StoredGroup},
group::{ConversationType, GroupMembershipState, StoredGroup},
group_intent::IntentKind,
group_message::{DeliveryStatus, GroupMessageKind, MsgQueryArgs, StoredGroupMessage},
sql_key_store,
Expand Down Expand Up @@ -313,7 +313,7 @@ impl<ScopedClient: ScopedGroupClient> MlsGroup<ScopedClient> {
let provider = XmtpOpenMlsProvider::new(conn);
let creator_inbox_id = context.inbox_id();
let protected_metadata =
build_protected_metadata_extension(creator_inbox_id, Purpose::Conversation)?;
build_protected_metadata_extension(creator_inbox_id, ConversationType::Group)?;
let mutable_metadata = build_mutable_metadata_extension_default(creator_inbox_id, opts)?;
let group_membership = build_starting_group_membership_extension(creator_inbox_id, 0);
let mutable_permissions = build_mutable_permissions_extension(permissions_policy_set)?;
Expand Down Expand Up @@ -427,16 +427,16 @@ impl<ScopedClient: ScopedGroupClient> MlsGroup<ScopedClient> {
} else {
None
};
let group_type = metadata.conversation_type;
let conversation_type = metadata.conversation_type;

let to_store = match group_type {
let to_store = match conversation_type {
ConversationType::Group => StoredGroup::new_from_welcome(
group_id.clone(),
now_ns(),
GroupMembershipState::Pending,
added_by_inbox,
welcome_id,
Purpose::Conversation,
conversation_type,
dm_inbox_id,
),
ConversationType::Dm => {
Expand All @@ -447,7 +447,7 @@ impl<ScopedClient: ScopedGroupClient> MlsGroup<ScopedClient> {
GroupMembershipState::Pending,
added_by_inbox,
welcome_id,
Purpose::Conversation,
conversation_type,
dm_inbox_id,
)
}
Expand All @@ -457,7 +457,7 @@ impl<ScopedClient: ScopedGroupClient> MlsGroup<ScopedClient> {
GroupMembershipState::Allowed,
added_by_inbox,
welcome_id,
Purpose::Sync,
conversation_type,
dm_inbox_id,
),
};
Expand Down Expand Up @@ -517,7 +517,7 @@ impl<ScopedClient: ScopedGroupClient> MlsGroup<ScopedClient> {
let provider = client.mls_provider()?;

let protected_metadata =
build_protected_metadata_extension(creator_inbox_id, Purpose::Sync)?;
build_protected_metadata_extension(creator_inbox_id, ConversationType::Sync)?;
let mutable_metadata = build_mutable_metadata_extension_default(
creator_inbox_id,
GroupMetadataOptions::default(),
Expand Down Expand Up @@ -1156,14 +1156,9 @@ pub fn extract_group_id(message: &GroupMessage) -> Result<Vec<u8>, MessageProces

fn build_protected_metadata_extension(
creator_inbox_id: &str,
group_purpose: Purpose,
conversation_type: ConversationType,
) -> Result<Extension, GroupError> {
let group_type = match group_purpose {
Purpose::Conversation => ConversationType::Group,
Purpose::Sync => ConversationType::Sync,
};

let metadata = GroupMetadata::new(group_type, creator_inbox_id.to_string(), None);
let metadata = GroupMetadata::new(conversation_type, creator_inbox_id.to_string(), None);
let protected_metadata = Metadata::new(metadata.try_into()?);

Ok(Extension::ImmutableMetadata(protected_metadata))
Expand Down Expand Up @@ -1541,7 +1536,7 @@ pub(crate) mod tests {
groups::{
build_dm_protected_metadata_extension, build_mutable_metadata_extension_default,
build_protected_metadata_extension,
group_metadata::{ConversationType, GroupMetadata},
group_metadata::GroupMetadata,
group_mutable_metadata::MetadataField,
intents::{PermissionPolicyOption, PermissionUpdateType},
members::{GroupMember, PermissionLevel},
Expand All @@ -1550,8 +1545,7 @@ pub(crate) mod tests {
},
storage::{
consent_record::ConsentState,
group::GroupQueryArgs,
group::Purpose,
group::{ConversationType, GroupQueryArgs},
group_intent::{IntentKind, IntentState},
group_message::{GroupMessageKind, MsgQueryArgs, StoredGroupMessage},
},
Expand Down Expand Up @@ -3721,7 +3715,7 @@ pub(crate) mod tests {

// Test case 2: Invalid conversation type
let invalid_protected_metadata =
build_protected_metadata_extension(creator_inbox_id, Purpose::Conversation).unwrap();
build_protected_metadata_extension(creator_inbox_id, ConversationType::Group).unwrap();
let invalid_type_group = MlsGroup::<FullXmtpClient>::create_test_dm_group(
client.clone().into(),
dm_target_inbox_id.clone(),
Expand Down
Loading

0 comments on commit 213dd16

Please sign in to comment.