From b518cc98ce5f063dd3a38c7632607e12031937c1 Mon Sep 17 00:00:00 2001 From: Naomi Plasterer Date: Wed, 7 Aug 2024 16:01:12 -0600 Subject: [PATCH 01/11] create the database migration for the private preference work --- .../down.sql | 2 ++ .../up.sql | 11 +++++++++++ 2 files changed, 13 insertions(+) create mode 100644 xmtp_mls/migrations/2024-08-07-213816_create-private-preference-store/down.sql create mode 100644 xmtp_mls/migrations/2024-08-07-213816_create-private-preference-store/up.sql diff --git a/xmtp_mls/migrations/2024-08-07-213816_create-private-preference-store/down.sql b/xmtp_mls/migrations/2024-08-07-213816_create-private-preference-store/down.sql new file mode 100644 index 000000000..60df1eda3 --- /dev/null +++ b/xmtp_mls/migrations/2024-08-07-213816_create-private-preference-store/down.sql @@ -0,0 +1,2 @@ +-- This file should undo anything in `up.sql` +DROP TABLE IF EXISTS "private_preferences"; diff --git a/xmtp_mls/migrations/2024-08-07-213816_create-private-preference-store/up.sql b/xmtp_mls/migrations/2024-08-07-213816_create-private-preference-store/up.sql new file mode 100644 index 000000000..b20640cfd --- /dev/null +++ b/xmtp_mls/migrations/2024-08-07-213816_create-private-preference-store/up.sql @@ -0,0 +1,11 @@ +CREATE TABLE "private_preferences"( + -- The inbox_id the private preferences are owned by + "inbox_id" text NOT NULL, + -- Enum of the PRIVATE_PREFERENCE_TYPE (GROUP, INBOX, etc..) + "valueType" int NOT NULL, + -- Enum of PREFERENCE_STATE (ALLOWED, DENIED, etc..) + "state" int NOT NULL, + -- The value of what has a preference + "value" text NOT NULL, + PRIMARY KEY (valueType, value) +); \ No newline at end of file From 07ac8d43cd3ad171af48efbeb4dcf1e91514ead9 Mon Sep 17 00:00:00 2001 From: Naomi Plasterer Date: Thu, 5 Sep 2024 21:28:19 -0600 Subject: [PATCH 02/11] update the table to be focused on consent --- Cargo.lock | 13 ++----------- .../down.sql | 2 +- .../up.sql | 16 +++++++--------- 3 files changed, 10 insertions(+), 21 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dcfbe4341..59a3775b8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2646,15 +2646,6 @@ dependencies = [ "either", ] -[[package]] -name = "itertools" -version = "0.12.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ba291022dbbd398a455acf126c1e341954079855bc60dfdda641363bd6922569" -dependencies = [ - "either", -] - [[package]] name = "itertools" version = "0.13.0" @@ -3902,7 +3893,7 @@ checksum = "5bb182580f71dd070f88d01ce3de9f4da5021db7115d2e1c3605a754153b77c1" dependencies = [ "bytes", "heck", - "itertools 0.12.1", + "itertools 0.13.0", "log", "multimap", "once_cell", @@ -3922,7 +3913,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "18bec9b0adc4eba778b33684b7ba3e7137789434769ee3ce3930463ef904cfca" dependencies = [ "anyhow", - "itertools 0.12.1", + "itertools 0.13.0", "proc-macro2", "quote", "syn 2.0.72", diff --git a/xmtp_mls/migrations/2024-08-07-213816_create-private-preference-store/down.sql b/xmtp_mls/migrations/2024-08-07-213816_create-private-preference-store/down.sql index 60df1eda3..dd3043403 100644 --- a/xmtp_mls/migrations/2024-08-07-213816_create-private-preference-store/down.sql +++ b/xmtp_mls/migrations/2024-08-07-213816_create-private-preference-store/down.sql @@ -1,2 +1,2 @@ -- This file should undo anything in `up.sql` -DROP TABLE IF EXISTS "private_preferences"; +DROP TABLE IF EXISTS "consent_records"; diff --git a/xmtp_mls/migrations/2024-08-07-213816_create-private-preference-store/up.sql b/xmtp_mls/migrations/2024-08-07-213816_create-private-preference-store/up.sql index b20640cfd..2973605da 100644 --- a/xmtp_mls/migrations/2024-08-07-213816_create-private-preference-store/up.sql +++ b/xmtp_mls/migrations/2024-08-07-213816_create-private-preference-store/up.sql @@ -1,11 +1,9 @@ -CREATE TABLE "private_preferences"( - -- The inbox_id the private preferences are owned by - "inbox_id" text NOT NULL, - -- Enum of the PRIVATE_PREFERENCE_TYPE (GROUP, INBOX, etc..) - "valueType" int NOT NULL, - -- Enum of PREFERENCE_STATE (ALLOWED, DENIED, etc..) +CREATE TABLE "consent_records"( + -- Enum of the CONSENT_TYPE (GROUP_ID, INBOX_ID, etc..) + "entityType" int NOT NULL, + -- Enum of CONSENT_STATE (ALLOWED, DENIED, etc..) "state" int NOT NULL, - -- The value of what has a preference - "value" text NOT NULL, - PRIMARY KEY (valueType, value) + -- The entity of what has consent (0x00 etc..) + "entity" text NOT NULL, + PRIMARY KEY (entityType, entity) ); \ No newline at end of file From 53775f3a08094e69df0391dcfc7134ffca756417 Mon Sep 17 00:00:00 2001 From: Naomi Plasterer Date: Thu, 5 Sep 2024 21:42:08 -0600 Subject: [PATCH 03/11] first pass at database storage structure --- .../storage/encrypted_store/consent_record.rs | 28 +++++++++++++++++++ xmtp_mls/src/storage/encrypted_store/mod.rs | 1 + .../src/storage/encrypted_store/schema.rs | 9 ++++++ 3 files changed, 38 insertions(+) create mode 100644 xmtp_mls/src/storage/encrypted_store/consent_record.rs diff --git a/xmtp_mls/src/storage/encrypted_store/consent_record.rs b/xmtp_mls/src/storage/encrypted_store/consent_record.rs new file mode 100644 index 000000000..d215c1349 --- /dev/null +++ b/xmtp_mls/src/storage/encrypted_store/consent_record.rs @@ -0,0 +1,28 @@ +// use diesel::{ +// backend::Backend, +// deserialize::{self, FromSql, FromSqlRow}, +// expression::AsExpression, +// prelude::*, +// serialize::{self, IsNull, Output, ToSql}, +// sql_types::Integer, +// sqlite::Sqlite, +// }; +// use serde::{Deserialize, Serialize}; + +// use super::{ +// db_connection::DbConnection, +// schema::{groups, groups::dsl}, +// }; +// use crate::{impl_fetch, impl_store, StorageError}; + +// #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Insertable, Identifiable, Queryable)] +// #[diesel(table_name = consent_record)] +// #[diesel(primary_key(id))] +// pub struct StoredConsentRecord { +// /// Enum, [`ConsentType`] representing access to the group +// pub entity_type: ConsentType, +// /// Enum, [`ConsentType`] representing access to the group +// pub state: ConsentState, +// /// The inbox_id of who added the user to a group. +// pub entity: String, +// } diff --git a/xmtp_mls/src/storage/encrypted_store/mod.rs b/xmtp_mls/src/storage/encrypted_store/mod.rs index f5d2f843d..4a45e6c04 100644 --- a/xmtp_mls/src/storage/encrypted_store/mod.rs +++ b/xmtp_mls/src/storage/encrypted_store/mod.rs @@ -12,6 +12,7 @@ pub mod association_state; pub mod db_connection; +pub mod consent_record; pub mod group; pub mod group_intent; pub mod group_message; diff --git a/xmtp_mls/src/storage/encrypted_store/schema.rs b/xmtp_mls/src/storage/encrypted_store/schema.rs index 84d3c69ec..96a7c8dca 100644 --- a/xmtp_mls/src/storage/encrypted_store/schema.rs +++ b/xmtp_mls/src/storage/encrypted_store/schema.rs @@ -8,6 +8,14 @@ diesel::table! { } } +diesel::table! { + consent_records (entityType, entity) { + entityType -> Integer, + state -> Integer, + entity -> Text, + } +} + diesel::table! { group_intents (id) { id -> Integer, @@ -94,6 +102,7 @@ diesel::joinable!(group_messages -> groups (group_id)); diesel::allow_tables_to_appear_in_same_query!( association_state, + consent_records, group_intents, group_messages, groups, From 56d0da2369d06d38df7cb98fe81bd4df9a9aac5b Mon Sep 17 00:00:00 2001 From: Naomi Plasterer Date: Fri, 6 Sep 2024 14:38:19 -0600 Subject: [PATCH 04/11] update the get method for consent records --- .../up.sql | 4 +- .../storage/encrypted_store/consent_record.rs | 239 ++++++++++++++++-- .../src/storage/encrypted_store/schema.rs | 4 +- 3 files changed, 215 insertions(+), 32 deletions(-) diff --git a/xmtp_mls/migrations/2024-08-07-213816_create-private-preference-store/up.sql b/xmtp_mls/migrations/2024-08-07-213816_create-private-preference-store/up.sql index 2973605da..76e7f7f27 100644 --- a/xmtp_mls/migrations/2024-08-07-213816_create-private-preference-store/up.sql +++ b/xmtp_mls/migrations/2024-08-07-213816_create-private-preference-store/up.sql @@ -1,9 +1,9 @@ CREATE TABLE "consent_records"( -- Enum of the CONSENT_TYPE (GROUP_ID, INBOX_ID, etc..) - "entityType" int NOT NULL, + "entity_type" int NOT NULL, -- Enum of CONSENT_STATE (ALLOWED, DENIED, etc..) "state" int NOT NULL, -- The entity of what has consent (0x00 etc..) "entity" text NOT NULL, - PRIMARY KEY (entityType, entity) + PRIMARY KEY (entity_type, entity) ); \ No newline at end of file diff --git a/xmtp_mls/src/storage/encrypted_store/consent_record.rs b/xmtp_mls/src/storage/encrypted_store/consent_record.rs index d215c1349..c4cfaf2c7 100644 --- a/xmtp_mls/src/storage/encrypted_store/consent_record.rs +++ b/xmtp_mls/src/storage/encrypted_store/consent_record.rs @@ -1,28 +1,211 @@ -// use diesel::{ -// backend::Backend, -// deserialize::{self, FromSql, FromSqlRow}, -// expression::AsExpression, -// prelude::*, -// serialize::{self, IsNull, Output, ToSql}, -// sql_types::Integer, -// sqlite::Sqlite, -// }; -// use serde::{Deserialize, Serialize}; - -// use super::{ -// db_connection::DbConnection, -// schema::{groups, groups::dsl}, -// }; -// use crate::{impl_fetch, impl_store, StorageError}; - -// #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Insertable, Identifiable, Queryable)] -// #[diesel(table_name = consent_record)] -// #[diesel(primary_key(id))] -// pub struct StoredConsentRecord { -// /// Enum, [`ConsentType`] representing access to the group -// pub entity_type: ConsentType, -// /// Enum, [`ConsentType`] representing access to the group -// pub state: ConsentState, -// /// The inbox_id of who added the user to a group. -// pub entity: String, -// } +use crate::{impl_store, storage::StorageError}; + +use super::{ + db_connection::DbConnection, + schema::consent_records::{self, dsl}, +}; +use diesel::{ + backend::Backend, + deserialize::{self, FromSql, FromSqlRow}, + expression::AsExpression, + prelude::*, + serialize::{self, IsNull, Output, ToSql}, + sql_types::Integer, + sqlite::Sqlite, +}; +use serde::{Deserialize, Serialize}; + +/// StoredConsentRecord holds a serialized ConsentRecord +#[derive(Insertable, Queryable, Debug, Clone, PartialEq, Eq)] +#[diesel(table_name = consent_records)] +#[diesel(primary_key(entity_type, entity))] +pub struct StoredConsentRecord { + /// Enum, [`ConsentType`] representing the type of consent (group_id inbox_id, etc..) + pub entity_type: ConsentType, + /// Enum, [`ConsentState`] representing the state of consent (allowed, denied, etc..) + pub state: ConsentState, + /// The entity of what was consented (0x00 etc..) + pub entity: String, +} + +impl StoredConsentRecord { + pub fn new( + entity_type: ConsentType, + state: ConsentState, + entity: String, + ) -> Self { + Self { + entity_type, + state, + entity, + } + } +} + +impl_store!(StoredConsentRecord, consent_records); + +impl DbConnection { + /// Returns the consent_records for the given entity up + pub fn get_consent_record( + &self, + entity: String, + entity_type: ConsentType, + ) -> Result, StorageError> { + Ok(self.raw_query(|conn| { + dsl::consent_records + .filter(dsl::entity.eq(entity)) + .filter(dsl::entity_type.eq(entity_type)) + .first(conn) + .optional() + })?) + } + + /// Batch insert consent_records, ignoring duplicates. + pub fn insert_or_ignore_consent_records( + &self, + updates: &[StoredConsentRecord], + ) -> Result<(), StorageError> { + Ok(self.raw_query(|conn| { + diesel::insert_or_ignore_into(dsl::consent_records) + .values(updates) + .execute(conn)?; + + Ok(()) + })?) + } +} + +#[repr(i32)] +#[derive(Debug, Copy, Clone, Serialize, Deserialize, Eq, PartialEq, AsExpression, FromSqlRow)] +#[diesel(sql_type = Integer)] +/// Type of consent record stored +pub enum ConsentType { + /// Consent is for a group + GroupId = 1, + /// Consent is for an inbox + InboxId = 2, + /// Consent is for an address + Address = 3, +} + +impl ToSql for ConsentType +where + i32: ToSql, +{ + fn to_sql<'b>(&'b self, out: &mut Output<'b, '_, Sqlite>) -> serialize::Result { + out.set_value(*self as i32); + Ok(IsNull::No) + } +} + +impl FromSql for ConsentType +where + i32: FromSql, +{ + fn from_sql(bytes: ::RawValue<'_>) -> deserialize::Result { + match i32::from_sql(bytes)? { + 1 => Ok(ConsentType::GroupId), + 2 => Ok(ConsentType::InboxId), + 3 => Ok(ConsentType::Address), + x => Err(format!("Unrecognized variant {}", x).into()), + } + } +} + +#[repr(i32)] +#[derive(Debug, Copy, Clone, Serialize, Deserialize, Eq, PartialEq, AsExpression, FromSqlRow)] +#[diesel(sql_type = Integer)] +/// The state of the consent +pub enum ConsentState { + /// Consent is allowed + Allowed = 1, + /// Consent is denied + Denied = 2 +} + +impl ToSql for ConsentState +where + i32: ToSql, +{ + fn to_sql<'b>(&'b self, out: &mut Output<'b, '_, Sqlite>) -> serialize::Result { + out.set_value(*self as i32); + Ok(IsNull::No) + } +} + +impl FromSql for ConsentState +where + i32: FromSql, +{ + fn from_sql(bytes: ::RawValue<'_>) -> deserialize::Result { + match i32::from_sql(bytes)? { + 1 => Ok(ConsentState::Allowed), + 2 => Ok(ConsentState::Denied), + x => Err(format!("Unrecognized variant {}", x).into()), + } + } +} + +#[cfg(test)] +mod tests { + use crate::{ + storage::encrypted_store::tests::with_connection, + utils::test::{rand_time, rand_vec}, + Store, + }; + + use super::*; + + // fn build_update(inbox_id: &str, sequence_id: i64) -> StoredConsentRecord { + // StoredConsentRecord::new(inbox_id.to_string(), sequence_id, rand_time(), rand_vec()) + // } + + // #[test] + // fn insert_and_read() { + // with_connection(|conn| { + // let inbox_id = "inbox_1"; + // let update_1 = build_update(inbox_id, 1); + // let update_1_payload = update_1.payload.clone(); + // let update_2 = build_update(inbox_id, 2); + // let update_2_payload = update_2.payload.clone(); + + // update_1.store(&conn).expect("should store without error"); + // update_2.store(&conn).expect("should store without error"); + + // let all_updates = conn + // .get_identity_updates(inbox_id, None) + // .expect("query should work"); + + // assert_eq!(all_updates.len(), 2); + // let first_update = all_updates.first().unwrap(); + // assert_eq!(first_update.payload, update_1_payload); + // let second_update = all_updates.last().unwrap(); + // assert_eq!(second_update.payload, update_2_payload); + // }); + // } + + // #[test] + // fn test_filter() { + // with_connection(|conn| { + // let inbox_id = "inbox_1"; + // let update_1 = build_update(inbox_id, 1); + // let update_2 = build_update(inbox_id, 2); + // let update_3 = build_update(inbox_id, 3); + + // conn.insert_or_ignore_identity_updates(&[update_1, update_2, update_3]) + // .expect("insert should succeed"); + + // let update_1_and_2 = conn + // .get_identity_updates(inbox_id, Some(2)) + // .expect("query should work"); + + // assert_eq!(update_1_and_2.len(), 2); + + // let all_updates = conn + // .get_identity_updates(inbox_id, None) + // .expect("query should work"); + + // assert_eq!(all_updates.len(), 3); + // }) + // } +} diff --git a/xmtp_mls/src/storage/encrypted_store/schema.rs b/xmtp_mls/src/storage/encrypted_store/schema.rs index 96a7c8dca..026969582 100644 --- a/xmtp_mls/src/storage/encrypted_store/schema.rs +++ b/xmtp_mls/src/storage/encrypted_store/schema.rs @@ -9,8 +9,8 @@ diesel::table! { } diesel::table! { - consent_records (entityType, entity) { - entityType -> Integer, + consent_records (entity_type, entity) { + entity_type -> Integer, state -> Integer, entity -> Text, } From af31a14fce52d828f65709d89d2fdfb1388f6232 Mon Sep 17 00:00:00 2001 From: Naomi Plasterer Date: Fri, 6 Sep 2024 15:28:40 -0600 Subject: [PATCH 05/11] fix up the set method --- .../storage/encrypted_store/consent_record.rs | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/xmtp_mls/src/storage/encrypted_store/consent_record.rs b/xmtp_mls/src/storage/encrypted_store/consent_record.rs index c4cfaf2c7..b795dfc8c 100644 --- a/xmtp_mls/src/storage/encrypted_store/consent_record.rs +++ b/xmtp_mls/src/storage/encrypted_store/consent_record.rs @@ -11,7 +11,7 @@ use diesel::{ prelude::*, serialize::{self, IsNull, Output, ToSql}, sql_types::Integer, - sqlite::Sqlite, + sqlite::Sqlite, upsert::excluded, }; use serde::{Deserialize, Serialize}; @@ -60,18 +60,24 @@ impl DbConnection { })?) } - /// Batch insert consent_records, ignoring duplicates. - pub fn insert_or_ignore_consent_records( + /// Batch insert consent_records, and replace existing entries + pub fn insert_or_replace_consent_record( &self, - updates: &[StoredConsentRecord], + record: StoredConsentRecord, ) -> Result<(), StorageError> { - Ok(self.raw_query(|conn| { - diesel::insert_or_ignore_into(dsl::consent_records) - .values(updates) - .execute(conn)?; + self.raw_query(|conn| { + let _ = diesel::insert_into(dsl::consent_records) + .values(&record) + .on_conflict((dsl::entity_type, dsl::entity)) + .do_update() + .set(dsl::state.eq(excluded(dsl::state))) + .execute(conn) + .map_err(StorageError::from); Ok(()) - })?) + })?; + + Ok(()) } } From ce5c562fcf1f9cac19259fc6ed2bc27893363b25 Mon Sep 17 00:00:00 2001 From: Naomi Plasterer Date: Mon, 9 Sep 2024 21:04:04 -0600 Subject: [PATCH 06/11] add a test --- .../storage/encrypted_store/consent_record.rs | 110 +++++++----------- xmtp_mls/src/storage/encrypted_store/mod.rs | 2 +- 2 files changed, 42 insertions(+), 70 deletions(-) diff --git a/xmtp_mls/src/storage/encrypted_store/consent_record.rs b/xmtp_mls/src/storage/encrypted_store/consent_record.rs index b795dfc8c..d2ff161d3 100644 --- a/xmtp_mls/src/storage/encrypted_store/consent_record.rs +++ b/xmtp_mls/src/storage/encrypted_store/consent_record.rs @@ -11,7 +11,8 @@ use diesel::{ prelude::*, serialize::{self, IsNull, Output, ToSql}, sql_types::Integer, - sqlite::Sqlite, upsert::excluded, + sqlite::Sqlite, + upsert::excluded, }; use serde::{Deserialize, Serialize}; @@ -29,11 +30,7 @@ pub struct StoredConsentRecord { } impl StoredConsentRecord { - pub fn new( - entity_type: ConsentType, - state: ConsentState, - entity: String, - ) -> Self { + pub fn new(entity_type: ConsentType, state: ConsentState, entity: String) -> Self { Self { entity_type, state, @@ -60,20 +57,18 @@ impl DbConnection { })?) } - /// Batch insert consent_records, and replace existing entries + /// Insert consent_record, and replace existing entries pub fn insert_or_replace_consent_record( &self, record: StoredConsentRecord, ) -> Result<(), StorageError> { self.raw_query(|conn| { - let _ = diesel::insert_into(dsl::consent_records) + diesel::insert_into(dsl::consent_records) .values(&record) .on_conflict((dsl::entity_type, dsl::entity)) .do_update() .set(dsl::state.eq(excluded(dsl::state))) - .execute(conn) - .map_err(StorageError::from); - + .execute(conn)?; Ok(()) })?; @@ -126,7 +121,7 @@ pub enum ConsentState { /// Consent is allowed Allowed = 1, /// Consent is denied - Denied = 2 + Denied = 2, } impl ToSql for ConsentState @@ -154,64 +149,41 @@ where #[cfg(test)] mod tests { - use crate::{ - storage::encrypted_store::tests::with_connection, - utils::test::{rand_time, rand_vec}, - Store, - }; + use crate::storage::encrypted_store::tests::with_connection; use super::*; - // fn build_update(inbox_id: &str, sequence_id: i64) -> StoredConsentRecord { - // StoredConsentRecord::new(inbox_id.to_string(), sequence_id, rand_time(), rand_vec()) - // } - - // #[test] - // fn insert_and_read() { - // with_connection(|conn| { - // let inbox_id = "inbox_1"; - // let update_1 = build_update(inbox_id, 1); - // let update_1_payload = update_1.payload.clone(); - // let update_2 = build_update(inbox_id, 2); - // let update_2_payload = update_2.payload.clone(); - - // update_1.store(&conn).expect("should store without error"); - // update_2.store(&conn).expect("should store without error"); - - // let all_updates = conn - // .get_identity_updates(inbox_id, None) - // .expect("query should work"); - - // assert_eq!(all_updates.len(), 2); - // let first_update = all_updates.first().unwrap(); - // assert_eq!(first_update.payload, update_1_payload); - // let second_update = all_updates.last().unwrap(); - // assert_eq!(second_update.payload, update_2_payload); - // }); - // } - - // #[test] - // fn test_filter() { - // with_connection(|conn| { - // let inbox_id = "inbox_1"; - // let update_1 = build_update(inbox_id, 1); - // let update_2 = build_update(inbox_id, 2); - // let update_3 = build_update(inbox_id, 3); - - // conn.insert_or_ignore_identity_updates(&[update_1, update_2, update_3]) - // .expect("insert should succeed"); - - // let update_1_and_2 = conn - // .get_identity_updates(inbox_id, Some(2)) - // .expect("query should work"); - - // assert_eq!(update_1_and_2.len(), 2); - - // let all_updates = conn - // .get_identity_updates(inbox_id, None) - // .expect("query should work"); - - // assert_eq!(all_updates.len(), 3); - // }) - // } + fn generate_consent_record( + entity_type: Option, + state: Option, + entity: Option, + ) -> StoredConsentRecord { + StoredConsentRecord { + entity_type: entity_type.unwrap_or(ConsentType::GroupId), + state: state.unwrap_or(ConsentState::Allowed), + entity: entity.unwrap_or_default(), + } + } + + #[test] + fn insert_and_read() { + with_connection(|conn| { + let inbox_id = "inbox_1"; + let consent_record = generate_consent_record( + Some(ConsentType::InboxId), + Some(ConsentState::Denied), + inbox_id, + ); + let consent_record_entity = consent_record.entity.clone(); + + conn.insert_or_replace_consent_record(consent_record) + .expect("should store without error"); + + let consent_record = conn + .get_consent_record(inbox_id.to_owned(), ConsentType::InboxId) + .expect("query should work"); + + assert_eq!(consent_record.unwrap().entity, consent_record_entity); + }); + } } diff --git a/xmtp_mls/src/storage/encrypted_store/mod.rs b/xmtp_mls/src/storage/encrypted_store/mod.rs index 4a45e6c04..371f060c6 100644 --- a/xmtp_mls/src/storage/encrypted_store/mod.rs +++ b/xmtp_mls/src/storage/encrypted_store/mod.rs @@ -11,8 +11,8 @@ //! `diesel print-schema` or use `cargo run update-schema` which will update the files for you. pub mod association_state; -pub mod db_connection; pub mod consent_record; +pub mod db_connection; pub mod group; pub mod group_intent; pub mod group_message; From cc7035412e7710861fb84c055228173af9ae159c Mon Sep 17 00:00:00 2001 From: Naomi Plasterer Date: Mon, 9 Sep 2024 21:10:33 -0600 Subject: [PATCH 07/11] fix up the test --- .../storage/encrypted_store/consent_record.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/xmtp_mls/src/storage/encrypted_store/consent_record.rs b/xmtp_mls/src/storage/encrypted_store/consent_record.rs index d2ff161d3..a454387d0 100644 --- a/xmtp_mls/src/storage/encrypted_store/consent_record.rs +++ b/xmtp_mls/src/storage/encrypted_store/consent_record.rs @@ -154,14 +154,14 @@ mod tests { use super::*; fn generate_consent_record( - entity_type: Option, - state: Option, - entity: Option, + entity_type: ConsentType, + state: ConsentState, + entity: String, ) -> StoredConsentRecord { StoredConsentRecord { - entity_type: entity_type.unwrap_or(ConsentType::GroupId), - state: state.unwrap_or(ConsentState::Allowed), - entity: entity.unwrap_or_default(), + entity_type: entity_type, + state: state, + entity: entity, } } @@ -170,9 +170,9 @@ mod tests { with_connection(|conn| { let inbox_id = "inbox_1"; let consent_record = generate_consent_record( - Some(ConsentType::InboxId), - Some(ConsentState::Denied), - inbox_id, + ConsentType::InboxId, + ConsentState::Denied, + inbox_id.to_string(), ); let consent_record_entity = consent_record.entity.clone(); From d0c149c3413125ae1abf57117c1386c588b76098 Mon Sep 17 00:00:00 2001 From: Naomi Plasterer Date: Mon, 9 Sep 2024 21:20:34 -0600 Subject: [PATCH 08/11] fix up the clippy error with consent record --- xmtp_mls/src/storage/encrypted_store/consent_record.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/xmtp_mls/src/storage/encrypted_store/consent_record.rs b/xmtp_mls/src/storage/encrypted_store/consent_record.rs index a454387d0..d124c3490 100644 --- a/xmtp_mls/src/storage/encrypted_store/consent_record.rs +++ b/xmtp_mls/src/storage/encrypted_store/consent_record.rs @@ -159,9 +159,9 @@ mod tests { entity: String, ) -> StoredConsentRecord { StoredConsentRecord { - entity_type: entity_type, - state: state, - entity: entity, + entity_type, + state, + entity, } } From 59572738b972d9122320c00ad8e011a9ccd8b1cf Mon Sep 17 00:00:00 2001 From: Naomi Plasterer Date: Mon, 9 Sep 2024 21:42:17 -0600 Subject: [PATCH 09/11] fix up the clippy error with consent record --- xmtp_mls/src/groups/group_metadata.rs | 20 ++++++++++++-------- xmtp_mls/src/groups/group_permissions.rs | 19 ++++++++++--------- xmtp_mls/src/groups/mod.rs | 8 ++++---- 3 files changed, 26 insertions(+), 21 deletions(-) diff --git a/xmtp_mls/src/groups/group_metadata.rs b/xmtp_mls/src/groups/group_metadata.rs index 781ab0f49..3d8b0a72a 100644 --- a/xmtp_mls/src/groups/group_metadata.rs +++ b/xmtp_mls/src/groups/group_metadata.rs @@ -17,6 +17,8 @@ pub enum GroupMetadataError { InvalidConversationType, #[error("missing extension")] MissingExtension, + #[error("missing a dm member")] + MissingDmMember, } #[derive(Debug, Clone, PartialEq)] @@ -72,14 +74,12 @@ impl TryFrom for GroupMetadata { type Error = GroupMetadataError; fn try_from(value: GroupMetadataProto) -> Result { - let dm_members = if value.dm_members.is_some() { - Some(DmMembers::try_from(value.dm_members.unwrap())?) - } else { - None - }; + let dm_members = value.dm_members + .map(|dm| DmMembers::try_from(dm)) + .transpose()?; Ok(Self::new( value.conversation_type.try_into()?, - value.creator_inbox_id.clone(), + value.creator_inbox_id, dm_members, )) } @@ -150,8 +150,12 @@ impl TryFrom for DmMembers { fn try_from(value: DmMembersProto) -> Result { Ok(Self { - member_one_inbox_id: value.dm_member_one.unwrap().inbox_id.clone(), - member_two_inbox_id: value.dm_member_two.unwrap().inbox_id.clone(), + member_one_inbox_id: value.dm_member_one + .ok_or(GroupMetadataError::MissingDmMember)? + .inbox_id, + member_two_inbox_id: value.dm_member_two + .ok_or(GroupMetadataError::MissingDmMember)? + .inbox_id, }) } } diff --git a/xmtp_mls/src/groups/group_permissions.rs b/xmtp_mls/src/groups/group_permissions.rs index 43c331ba4..870f6b588 100644 --- a/xmtp_mls/src/groups/group_permissions.rs +++ b/xmtp_mls/src/groups/group_permissions.rs @@ -863,15 +863,16 @@ impl PolicySet { ); // We can always add DM member's inboxId to a DM - if commit.dm_members.is_some() - && commit.added_inboxes.len() == 1 - && (commit.added_inboxes[0].inbox_id - == commit.dm_members.as_ref().unwrap().member_one_inbox_id - || commit.added_inboxes[0].inbox_id - == commit.dm_members.as_ref().unwrap().member_two_inbox_id) - && commit.added_inboxes[0].inbox_id != commit.actor_inbox_id() - { - added_inboxes_valid = true; + if let Some(dm_members) = &commit.dm_members { + if commit.added_inboxes.len() == 1 { + let added_inbox_id = &commit.added_inboxes[0].inbox_id; + if (added_inbox_id == &dm_members.member_one_inbox_id + || added_inbox_id == &dm_members.member_two_inbox_id) + && added_inbox_id != &commit.actor_inbox_id() + { + added_inboxes_valid = true; + } + } } // Verify remove member policy was not violated diff --git a/xmtp_mls/src/groups/mod.rs b/xmtp_mls/src/groups/mod.rs index f7727a49c..957b597d3 100644 --- a/xmtp_mls/src/groups/mod.rs +++ b/xmtp_mls/src/groups/mod.rs @@ -3068,16 +3068,16 @@ mod tests { amal_dm.sync(&amal).await.unwrap(); bola_dm.sync(&bola).await.unwrap(); let is_amal_admin = amal_dm - .is_admin(amal.inbox_id(), &amal.mls_provider().unwrap()) + .is_admin(amal.inbox_id(), amal.mls_provider().unwrap()) .unwrap(); let is_bola_admin = amal_dm - .is_admin(bola.inbox_id(), &bola.mls_provider().unwrap()) + .is_admin(bola.inbox_id(), bola.mls_provider().unwrap()) .unwrap(); let is_amal_super_admin = amal_dm - .is_super_admin(amal.inbox_id(), &amal.mls_provider().unwrap()) + .is_super_admin(amal.inbox_id(), amal.mls_provider().unwrap()) .unwrap(); let is_bola_super_admin = amal_dm - .is_super_admin(bola.inbox_id(), &bola.mls_provider().unwrap()) + .is_super_admin(bola.inbox_id(), bola.mls_provider().unwrap()) .unwrap(); assert!(!is_amal_admin); assert!(!is_bola_admin); From 714aa8d8216148c91009a4a463ae6a5f377cf73e Mon Sep 17 00:00:00 2001 From: Naomi Plasterer Date: Mon, 9 Sep 2024 21:43:46 -0600 Subject: [PATCH 10/11] fix up all clippy issues --- xmtp_mls/src/groups/group_metadata.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xmtp_mls/src/groups/group_metadata.rs b/xmtp_mls/src/groups/group_metadata.rs index 3d8b0a72a..abf4359b5 100644 --- a/xmtp_mls/src/groups/group_metadata.rs +++ b/xmtp_mls/src/groups/group_metadata.rs @@ -75,7 +75,7 @@ impl TryFrom for GroupMetadata { fn try_from(value: GroupMetadataProto) -> Result { let dm_members = value.dm_members - .map(|dm| DmMembers::try_from(dm)) + .map(DmMembers::try_from) .transpose()?; Ok(Self::new( value.conversation_type.try_into()?, From c0380012fe598188368c545c78b942298eff5c80 Mon Sep 17 00:00:00 2001 From: Naomi Plasterer Date: Mon, 9 Sep 2024 21:45:12 -0600 Subject: [PATCH 11/11] cargo fmt --- xmtp_mls/src/groups/group_metadata.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/xmtp_mls/src/groups/group_metadata.rs b/xmtp_mls/src/groups/group_metadata.rs index abf4359b5..b274df3f1 100644 --- a/xmtp_mls/src/groups/group_metadata.rs +++ b/xmtp_mls/src/groups/group_metadata.rs @@ -74,9 +74,7 @@ impl TryFrom for GroupMetadata { type Error = GroupMetadataError; fn try_from(value: GroupMetadataProto) -> Result { - let dm_members = value.dm_members - .map(DmMembers::try_from) - .transpose()?; + let dm_members = value.dm_members.map(DmMembers::try_from).transpose()?; Ok(Self::new( value.conversation_type.try_into()?, value.creator_inbox_id, @@ -150,10 +148,12 @@ impl TryFrom for DmMembers { fn try_from(value: DmMembersProto) -> Result { Ok(Self { - member_one_inbox_id: value.dm_member_one + member_one_inbox_id: value + .dm_member_one .ok_or(GroupMetadataError::MissingDmMember)? .inbox_id, - member_two_inbox_id: value.dm_member_two + member_two_inbox_id: value + .dm_member_two .ok_or(GroupMetadataError::MissingDmMember)? .inbox_id, })