From f6bb1a9b57f7e41273a0b29b312230c25590a3fc Mon Sep 17 00:00:00 2001 From: James Magahern Date: Thu, 26 Jun 2025 18:23:15 -0700 Subject: [PATCH] Don't overwrite already resolved participants, better naming of keys --- .../up.sql | 34 ----- .../2025-04-25-223015_add_settings/down.sql | 7 - .../2025-04-25-223015_add_settings/up.sql | 11 -- .../down.sql | 2 - .../up.sql | 2 - .../down.sql | 2 - .../up.sql | 2 - .../down.sql | 14 -- .../up.sql | 2 - .../down.sql | 5 +- .../2025-06-26-233940_create_schema/up.sql | 46 +++++++ kordophone-db/src/models/conversation.rs | 2 +- kordophone-db/src/models/db/message.rs | 13 +- kordophone-db/src/models/db/participant.rs | 29 ++-- kordophone-db/src/models/message.rs | 3 +- kordophone-db/src/models/mod.rs | 2 +- kordophone-db/src/models/participant.rs | 32 ++--- kordophone-db/src/repository.rs | 114 +++++++--------- kordophone-db/src/schema.rs | 25 ++-- kordophone-db/src/tests/mod.rs | 35 ++--- .../src/daemon/contact_resolver/eds.rs | 127 ++++++++++-------- .../src/daemon/contact_resolver/mod.rs | 1 + kordophoned/src/daemon/mod.rs | 10 +- kordophoned/src/daemon/models/message.rs | 32 +++-- kordophoned/src/dbus/agent.rs | 17 ++- 25 files changed, 263 insertions(+), 306 deletions(-) delete mode 100644 kordophone-db/migrations/2025-01-21-051154_create_conversations/up.sql delete mode 100644 kordophone-db/migrations/2025-04-25-223015_add_settings/down.sql delete mode 100644 kordophone-db/migrations/2025-04-25-223015_add_settings/up.sql delete mode 100644 kordophone-db/migrations/2025-05-26-232206_add_attachment_metadata_to_messages/down.sql delete mode 100644 kordophone-db/migrations/2025-05-26-232206_add_attachment_metadata_to_messages/up.sql delete mode 100644 kordophone-db/migrations/2025-05-26-234230_add_file_transfer_guids_to_messages/down.sql delete mode 100644 kordophone-db/migrations/2025-05-26-234230_add_file_transfer_guids_to_messages/up.sql delete mode 100644 kordophone-db/migrations/2025-06-01-000000_add_contact_id_to_participants/down.sql delete mode 100644 kordophone-db/migrations/2025-06-01-000000_add_contact_id_to_participants/up.sql rename kordophone-db/migrations/{2025-01-21-051154_create_conversations => 2025-06-26-233940_create_schema}/down.sql (88%) create mode 100644 kordophone-db/migrations/2025-06-26-233940_create_schema/up.sql diff --git a/kordophone-db/migrations/2025-01-21-051154_create_conversations/up.sql b/kordophone-db/migrations/2025-01-21-051154_create_conversations/up.sql deleted file mode 100644 index 69c3b8b..0000000 --- a/kordophone-db/migrations/2025-01-21-051154_create_conversations/up.sql +++ /dev/null @@ -1,34 +0,0 @@ --- Your SQL goes here -CREATE TABLE `conversation_participants`( - `conversation_id` TEXT NOT NULL, - `participant_id` INTEGER NOT NULL, - PRIMARY KEY(`conversation_id`, `participant_id`) -); - -CREATE TABLE `messages`( - `id` TEXT NOT NULL PRIMARY KEY, - `text` TEXT NOT NULL, - `sender_participant_id` INTEGER, - `date` TIMESTAMP NOT NULL -); - -CREATE TABLE `conversation_messages`( - `conversation_id` TEXT NOT NULL, - `message_id` TEXT NOT NULL, - PRIMARY KEY(`conversation_id`, `message_id`) -); - -CREATE TABLE `participants`( - `id` INTEGER NOT NULL PRIMARY KEY, - `display_name` TEXT, - `is_me` BOOL NOT NULL -); - -CREATE TABLE `conversations`( - `id` TEXT NOT NULL PRIMARY KEY, - `unread_count` BIGINT NOT NULL, - `display_name` TEXT, - `last_message_preview` TEXT, - `date` TIMESTAMP NOT NULL -); - diff --git a/kordophone-db/migrations/2025-04-25-223015_add_settings/down.sql b/kordophone-db/migrations/2025-04-25-223015_add_settings/down.sql deleted file mode 100644 index 0445954..0000000 --- a/kordophone-db/migrations/2025-04-25-223015_add_settings/down.sql +++ /dev/null @@ -1,7 +0,0 @@ --- This file should undo anything in `up.sql` - - - - - -DROP TABLE IF EXISTS `settings`; diff --git a/kordophone-db/migrations/2025-04-25-223015_add_settings/up.sql b/kordophone-db/migrations/2025-04-25-223015_add_settings/up.sql deleted file mode 100644 index 56e9cc6..0000000 --- a/kordophone-db/migrations/2025-04-25-223015_add_settings/up.sql +++ /dev/null @@ -1,11 +0,0 @@ --- Your SQL goes here - - - - - -CREATE TABLE `settings`( - `key` TEXT NOT NULL PRIMARY KEY, - `value` BINARY NOT NULL -); - diff --git a/kordophone-db/migrations/2025-05-26-232206_add_attachment_metadata_to_messages/down.sql b/kordophone-db/migrations/2025-05-26-232206_add_attachment_metadata_to_messages/down.sql deleted file mode 100644 index 722f7e8..0000000 --- a/kordophone-db/migrations/2025-05-26-232206_add_attachment_metadata_to_messages/down.sql +++ /dev/null @@ -1,2 +0,0 @@ --- Remove attachment_metadata column from messages table -ALTER TABLE messages DROP COLUMN attachment_metadata; \ No newline at end of file diff --git a/kordophone-db/migrations/2025-05-26-232206_add_attachment_metadata_to_messages/up.sql b/kordophone-db/migrations/2025-05-26-232206_add_attachment_metadata_to_messages/up.sql deleted file mode 100644 index 50d4826..0000000 --- a/kordophone-db/migrations/2025-05-26-232206_add_attachment_metadata_to_messages/up.sql +++ /dev/null @@ -1,2 +0,0 @@ --- Add attachment_metadata column to messages table -ALTER TABLE messages ADD COLUMN attachment_metadata TEXT; \ No newline at end of file diff --git a/kordophone-db/migrations/2025-05-26-234230_add_file_transfer_guids_to_messages/down.sql b/kordophone-db/migrations/2025-05-26-234230_add_file_transfer_guids_to_messages/down.sql deleted file mode 100644 index d58fd05..0000000 --- a/kordophone-db/migrations/2025-05-26-234230_add_file_transfer_guids_to_messages/down.sql +++ /dev/null @@ -1,2 +0,0 @@ --- Remove file_transfer_guids column from messages table -ALTER TABLE messages DROP COLUMN file_transfer_guids; \ No newline at end of file diff --git a/kordophone-db/migrations/2025-05-26-234230_add_file_transfer_guids_to_messages/up.sql b/kordophone-db/migrations/2025-05-26-234230_add_file_transfer_guids_to_messages/up.sql deleted file mode 100644 index 6a244b7..0000000 --- a/kordophone-db/migrations/2025-05-26-234230_add_file_transfer_guids_to_messages/up.sql +++ /dev/null @@ -1,2 +0,0 @@ --- Add file_transfer_guids column to messages table -ALTER TABLE messages ADD COLUMN file_transfer_guids TEXT; \ No newline at end of file diff --git a/kordophone-db/migrations/2025-06-01-000000_add_contact_id_to_participants/down.sql b/kordophone-db/migrations/2025-06-01-000000_add_contact_id_to_participants/down.sql deleted file mode 100644 index a549445..0000000 --- a/kordophone-db/migrations/2025-06-01-000000_add_contact_id_to_participants/down.sql +++ /dev/null @@ -1,14 +0,0 @@ --- Revert participants table to remove contact_id column --- SQLite does not support DROP COLUMN directly, so we recreate the table without contact_id -PRAGMA foreign_keys=off; -CREATE TABLE participants_backup ( - id INTEGER NOT NULL PRIMARY KEY, - display_name TEXT, - is_me BOOL NOT NULL -); -INSERT INTO participants_backup (id, display_name, is_me) - SELECT id, display_name, is_me - FROM participants; -DROP TABLE participants; -ALTER TABLE participants_backup RENAME TO participants; -PRAGMA foreign_keys=on; \ No newline at end of file diff --git a/kordophone-db/migrations/2025-06-01-000000_add_contact_id_to_participants/up.sql b/kordophone-db/migrations/2025-06-01-000000_add_contact_id_to_participants/up.sql deleted file mode 100644 index c72b96b..0000000 --- a/kordophone-db/migrations/2025-06-01-000000_add_contact_id_to_participants/up.sql +++ /dev/null @@ -1,2 +0,0 @@ --- Add contact_id column to participants to store an external contact identifier (e.g., Folks ID) -ALTER TABLE participants ADD COLUMN contact_id TEXT; \ No newline at end of file diff --git a/kordophone-db/migrations/2025-01-21-051154_create_conversations/down.sql b/kordophone-db/migrations/2025-06-26-233940_create_schema/down.sql similarity index 88% rename from kordophone-db/migrations/2025-01-21-051154_create_conversations/down.sql rename to kordophone-db/migrations/2025-06-26-233940_create_schema/down.sql index 34cefce..4740fab 100644 --- a/kordophone-db/migrations/2025-01-21-051154_create_conversations/down.sql +++ b/kordophone-db/migrations/2025-06-26-233940_create_schema/down.sql @@ -1,6 +1,7 @@ -- This file should undo anything in `up.sql` -DROP TABLE IF EXISTS `conversation_participants`; DROP TABLE IF EXISTS `messages`; DROP TABLE IF EXISTS `conversation_messages`; -DROP TABLE IF EXISTS `participants`; +DROP TABLE IF EXISTS `settings`; DROP TABLE IF EXISTS `conversations`; +DROP TABLE IF EXISTS `participants`; +DROP TABLE IF EXISTS `conversation_participants`; diff --git a/kordophone-db/migrations/2025-06-26-233940_create_schema/up.sql b/kordophone-db/migrations/2025-06-26-233940_create_schema/up.sql new file mode 100644 index 0000000..d1ba160 --- /dev/null +++ b/kordophone-db/migrations/2025-06-26-233940_create_schema/up.sql @@ -0,0 +1,46 @@ +-- Your SQL goes here +CREATE TABLE `messages`( + `id` TEXT NOT NULL PRIMARY KEY, + `text` TEXT NOT NULL, + `sender_participant_handle` TEXT, + `date` TIMESTAMP NOT NULL, + `file_transfer_guids` TEXT, + `attachment_metadata` TEXT, + FOREIGN KEY (`sender_participant_handle`) REFERENCES `participants`(`handle`) +); + +CREATE TABLE `conversation_messages`( + `conversation_id` TEXT NOT NULL, + `message_id` TEXT NOT NULL, + PRIMARY KEY(`conversation_id`, `message_id`), + FOREIGN KEY (`conversation_id`) REFERENCES `conversations`(`id`), + FOREIGN KEY (`message_id`) REFERENCES `messages`(`id`) +); + +CREATE TABLE `settings`( + `key` TEXT NOT NULL PRIMARY KEY, + `value` BINARY NOT NULL +); + +CREATE TABLE `conversations`( + `id` TEXT NOT NULL PRIMARY KEY, + `unread_count` BIGINT NOT NULL, + `display_name` TEXT, + `last_message_preview` TEXT, + `date` TIMESTAMP NOT NULL +); + +CREATE TABLE `participants`( + `handle` TEXT NOT NULL PRIMARY KEY, + `is_me` BOOL NOT NULL, + `contact_id` TEXT +); + +CREATE TABLE `conversation_participants`( + `conversation_id` TEXT NOT NULL, + `participant_handle` TEXT NOT NULL, + PRIMARY KEY(`conversation_id`, `participant_handle`), + FOREIGN KEY (`conversation_id`) REFERENCES `conversations`(`id`), + FOREIGN KEY (`participant_handle`) REFERENCES `participants`(`handle`) +); + diff --git a/kordophone-db/src/models/conversation.rs b/kordophone-db/src/models/conversation.rs index 2df10a7..4b06f75 100644 --- a/kordophone-db/src/models/conversation.rs +++ b/kordophone-db/src/models/conversation.rs @@ -75,7 +75,7 @@ impl From for Conversation { participants: value .participant_display_names .into_iter() - .map(|p| p.into()) + .map(|p| Participant::Remote { handle: p, contact_id: None }) // todo: this is wrong .collect(), } } diff --git a/kordophone-db/src/models/db/message.rs b/kordophone-db/src/models/db/message.rs index 9ca6e72..7821183 100644 --- a/kordophone-db/src/models/db/message.rs +++ b/kordophone-db/src/models/db/message.rs @@ -7,7 +7,7 @@ use diesel::prelude::*; #[diesel(check_for_backend(diesel::sqlite::Sqlite))] pub struct Record { pub id: String, - pub sender_participant_id: Option, + pub sender_participant_handle: Option, pub text: String, pub date: NaiveDateTime, pub file_transfer_guids: Option, // JSON array @@ -28,9 +28,9 @@ impl From for Record { Self { id: message.id, - sender_participant_id: match message.sender { + sender_participant_handle: match message.sender { Participant::Me => None, - Participant::Remote { id, .. } => id, + Participant::Remote { handle, .. } => Some(handle), }, text: message.text, date: message.date, @@ -51,10 +51,13 @@ impl From for Message { .attachment_metadata .and_then(|json| serde_json::from_str(&json).ok()); + let message_sender = match record.sender_participant_handle { + Some(handle) => Participant::Remote { handle, contact_id: None }, + None => Participant::Me, + }; Self { id: record.id, - // We'll set the proper sender later when loading participant info - sender: Participant::Me, + sender: message_sender, text: record.text, date: record.date, file_transfer_guids, diff --git a/kordophone-db/src/models/db/participant.rs b/kordophone-db/src/models/db/participant.rs index 959f8a9..6b22917 100644 --- a/kordophone-db/src/models/db/participant.rs +++ b/kordophone-db/src/models/db/participant.rs @@ -4,9 +4,9 @@ use diesel::prelude::*; #[derive(Queryable, Selectable, AsChangeset, Identifiable)] #[diesel(table_name = crate::schema::participants)] +#[diesel(primary_key(handle))] pub struct Record { - pub id: i32, - pub display_name: Option, + pub handle: String, pub is_me: bool, pub contact_id: Option, } @@ -14,7 +14,7 @@ pub struct Record { #[derive(Insertable)] #[diesel(table_name = crate::schema::participants)] pub struct InsertableRecord { - pub display_name: Option, + pub handle: String, pub is_me: bool, pub contact_id: Option, } @@ -23,12 +23,12 @@ impl From for InsertableRecord { fn from(participant: Participant) -> Self { match participant { Participant::Me => InsertableRecord { - display_name: None, + handle: "me".to_string(), is_me: true, contact_id: None, }, - Participant::Remote { display_name, contact_id, .. } => InsertableRecord { - display_name: Some(display_name), + Participant::Remote { handle, contact_id, .. } => InsertableRecord { + handle, is_me: false, contact_id, }, @@ -38,12 +38,12 @@ impl From for InsertableRecord { #[derive(Identifiable, Selectable, Queryable, Associations, Debug)] #[diesel(belongs_to(super::conversation::Record, foreign_key = conversation_id))] -#[diesel(belongs_to(Record, foreign_key = participant_id))] +#[diesel(belongs_to(Record, foreign_key = participant_handle))] #[diesel(table_name = conversation_participants)] -#[diesel(primary_key(conversation_id, participant_id))] +#[diesel(primary_key(conversation_id, participant_handle))] pub struct ConversationParticipant { pub conversation_id: String, - pub participant_id: i32, + pub participant_handle: String, } impl From for Participant { @@ -52,8 +52,7 @@ impl From for Participant { Participant::Me } else { Participant::Remote { - id: Some(record.id), - display_name: record.display_name.unwrap_or_default(), + handle: record.handle.clone(), contact_id: record.contact_id, } } @@ -64,14 +63,12 @@ impl From for Record { fn from(participant: Participant) -> Self { match participant { Participant::Me => Record { - id: 0, // This will be set by the database - display_name: None, + handle: "me".to_string(), is_me: true, contact_id: None, }, - Participant::Remote { display_name, contact_id, .. } => Record { - id: 0, // This will be set by the database - display_name: Some(display_name), + Participant::Remote { handle, contact_id, .. } => Record { + handle, is_me: false, contact_id, }, diff --git a/kordophone-db/src/models/message.rs b/kordophone-db/src/models/message.rs index 75d0cab..92ffba7 100644 --- a/kordophone-db/src/models/message.rs +++ b/kordophone-db/src/models/message.rs @@ -27,8 +27,7 @@ impl From for Message { id: value.guid, sender: match value.sender { Some(sender) => Participant::Remote { - id: None, - display_name: sender, + handle: sender, contact_id: None, }, None => Participant::Me, diff --git a/kordophone-db/src/models/mod.rs b/kordophone-db/src/models/mod.rs index 13571fc..a8c7e94 100644 --- a/kordophone-db/src/models/mod.rs +++ b/kordophone-db/src/models/mod.rs @@ -5,4 +5,4 @@ pub mod participant; pub use conversation::Conversation; pub use message::Message; -pub use participant::Participant; +pub use participant::Participant; \ No newline at end of file diff --git a/kordophone-db/src/models/participant.rs b/kordophone-db/src/models/participant.rs index ce9d37d..1024a5b 100644 --- a/kordophone-db/src/models/participant.rs +++ b/kordophone-db/src/models/participant.rs @@ -2,37 +2,21 @@ pub enum Participant { Me, Remote { - id: Option, - display_name: String, + handle: String, contact_id: Option, }, } -impl From for Participant { - fn from(display_name: String) -> Self { - Participant::Remote { - id: None, - display_name, - contact_id: None, - } - } -} - -impl From<&str> for Participant { - fn from(display_name: &str) -> Self { - Participant::Remote { - id: None, - display_name: display_name.to_string(), - contact_id: None, - } - } -} - impl Participant { - pub fn display_name(&self) -> String { + pub fn handle(&self) -> String { match self { Participant::Me => "(Me)".to_string(), - Participant::Remote { display_name, .. } => display_name.clone(), + Participant::Remote { handle, .. } => handle.clone(), } } + + // Temporary alias for backward compatibility + pub fn display_name(&self) -> String { + self.handle() + } } diff --git a/kordophone-db/src/repository.rs b/kordophone-db/src/repository.rs index b6787cb..9c52a99 100644 --- a/kordophone-db/src/repository.rs +++ b/kordophone-db/src/repository.rs @@ -36,21 +36,19 @@ impl<'a> Repository<'a> { .values(&db_conversation) .execute(self.connection)?; - diesel::replace_into(participants) - .values(&db_participants) - .execute(self.connection)?; + for participant in &db_participants { + diesel::insert_into(participants) + .values(participant) + .on_conflict_do_nothing() + .execute(self.connection)?; + } // Sqlite backend doesn't support batch insert, so we have to do this manually - for participant in db_participants { - let pid = participants - .select(schema::participants::id) - .filter(schema::participants::display_name.eq(&participant.display_name)) - .first::(self.connection)?; - + for participant in &db_participants { diesel::replace_into(conversation_participants) .values(( conversation_id.eq(&db_conversation.id), - participant_id.eq(pid), + participant_handle.eq(&participant.handle), )) .execute(self.connection)?; } @@ -117,7 +115,7 @@ impl<'a> Repository<'a> { // Handle participant if message has a remote sender let sender = message.sender.clone(); let mut db_message: MessageRecord = message.into(); - db_message.sender_participant_id = self.get_or_create_participant(&sender); + db_message.sender_participant_handle = self.get_or_create_participant(&sender); diesel::replace_into(messages) .values(&db_message) @@ -161,11 +159,11 @@ impl<'a> Repository<'a> { // individual queries. self.connection .transaction::<_, diesel::result::Error, _>(|conn| { - // Cache participant ids we have already looked up / created – in a + // Cache participant handles we have already looked up / created – in a // typical conversation we only have a handful of participants, but we // might be processing hundreds of messages. Avoiding an extra SELECT per // message saves a lot of round-trips to SQLite. - let mut participant_cache: HashMap = HashMap::new(); + let mut participant_cache: HashMap = HashMap::new(); // Prepare collections for the batch inserts. let mut db_messages: Vec = Vec::with_capacity(in_messages.len()); @@ -174,50 +172,40 @@ impl<'a> Repository<'a> { for message in in_messages { // Resolve/insert the sender participant only once per display name. - let sender_id = match &message.sender { + let sender_handle_opt = match &message.sender { Participant::Me => None, - Participant::Remote { display_name, .. } => { - if let Some(cached_participant_id) = participant_cache.get(display_name) - { - Some(*cached_participant_id) + Participant::Remote { handle, contact_id } => { + if participant_cache.contains_key(handle) { + Some(handle.clone()) } else { - // Try to load from DB first - let existing: Option = participants_dsl::participants - .filter(participants_dsl::display_name.eq(display_name)) - .select(participants_dsl::id) - .first::(conn) + // Ensure participant exists in DB + let exists: Option = participants_dsl::participants + .filter(participants_dsl::handle.eq(handle)) + .select(participants_dsl::handle) + .first::(conn) .optional()?; - let participant_id = if let Some(pid) = existing { - pid - } else { + if exists.is_none() { let new_participant = InsertableParticipantRecord { - display_name: Some(display_name.clone()), + handle: handle.clone(), is_me: false, - contact_id: None, + contact_id: contact_id.clone(), }; diesel::insert_into(participants_dsl::participants) .values(&new_participant) .execute(conn)?; + } - // last_insert_rowid() is connection-wide, but we are the only - // writer inside this transaction. - diesel::select(diesel::dsl::sql::( - "last_insert_rowid()", - )) - .get_result::(conn)? - }; - - participant_cache.insert(display_name.clone(), participant_id); - Some(participant_id) + participant_cache.insert(handle.clone(), handle.clone()); + Some(handle.clone()) } } }; // Convert the message into its DB form. let mut db_message: MessageRecord = message.into(); - db_message.sender_participant_id = sender_id; + db_message.sender_participant_handle = sender_handle_opt.clone(); conv_msg_records.push(InsertableConversationMessage { conversation_id: conversation_guid.to_string(), @@ -280,10 +268,10 @@ impl<'a> Repository<'a> { for message_record in message_records { let mut message: Message = message_record.clone().into(); - // If there's a sender_participant_id, load the participant info - if let Some(pid) = message_record.sender_participant_id { + // If the message references a sender participant, load the participant info + if let Some(sender_handle) = message_record.sender_participant_handle { let participant = participants - .find(pid) + .find(sender_handle) .first::(self.connection)?; message.sender = participant.into(); } @@ -374,50 +362,44 @@ impl<'a> Repository<'a> { /// Update the contact_id for an existing participant record. pub fn update_participant_contact( &mut self, - participant_db_id: i32, + participant_handle: &str, new_contact_id: &str, ) -> Result<()> { use crate::schema::participants::dsl::*; - log::debug!(target: target::REPOSITORY, "Updating participant contact id {} => {}", participant_db_id, new_contact_id); - diesel::update(participants.filter(id.eq(participant_db_id))) + log::debug!(target: target::REPOSITORY, "Updating participant contact {} => {}", participant_handle, new_contact_id); + diesel::update(participants.filter(handle.eq(participant_handle))) .set(contact_id.eq(Some(new_contact_id.to_string()))) .execute(self.connection)?; Ok(()) } - fn get_or_create_participant(&mut self, participant: &Participant) -> Option { + fn get_or_create_participant(&mut self, participant: &Participant) -> Option { match participant { Participant::Me => None, - Participant::Remote { - display_name: p_name, - contact_id: c_id, - .. - } => { + Participant::Remote { handle: p_handle, contact_id: c_id, .. } => { use crate::schema::participants::dsl::*; let existing_participant = participants - .filter(display_name.eq(p_name)) + .filter(handle.eq(p_handle)) .first::(self.connection) .optional() .unwrap(); - if let Some(participant) = existing_participant { - return Some(participant.id); + if existing_participant.is_none() { + let participant_record = InsertableParticipantRecord { + handle: p_handle.clone(), + is_me: false, + contact_id: c_id.clone(), + }; + + diesel::insert_into(participants) + .values(&participant_record) + .execute(self.connection) + .unwrap(); } - let participant_record = InsertableParticipantRecord { - display_name: Some(participant.display_name()), - is_me: false, - contact_id: c_id.clone(), - }; - - diesel::insert_into(participants) - .values(&participant_record) - .execute(self.connection) - .unwrap(); - - self.last_insert_id().ok() + Some(p_handle.clone()) } } } diff --git a/kordophone-db/src/schema.rs b/kordophone-db/src/schema.rs index 8f1f554..c2b41e5 100644 --- a/kordophone-db/src/schema.rs +++ b/kordophone-db/src/schema.rs @@ -12,18 +12,17 @@ diesel::table! { } diesel::table! { - participants (id) { - id -> Integer, - display_name -> Nullable, + participants (handle) { + handle -> Text, is_me -> Bool, contact_id -> Nullable, } } diesel::table! { - conversation_participants (conversation_id, participant_id) { + conversation_participants (conversation_id, participant_handle) { conversation_id -> Text, - participant_id -> Integer, + participant_handle -> Text, } } @@ -31,7 +30,7 @@ diesel::table! { messages (id) { id -> Text, // guid text -> Text, - sender_participant_id -> Nullable, + sender_participant_handle -> Nullable, date -> Timestamp, file_transfer_guids -> Nullable, // JSON array of file transfer GUIDs attachment_metadata -> Nullable, // JSON string of attachment metadata @@ -53,13 +52,15 @@ diesel::table! { } diesel::joinable!(conversation_participants -> conversations (conversation_id)); -diesel::joinable!(conversation_participants -> participants (participant_id)); +diesel::joinable!(conversation_participants -> participants (participant_handle)); +diesel::joinable!(messages -> participants (sender_participant_handle)); +diesel::joinable!(conversation_messages -> conversations (conversation_id)); +diesel::joinable!(conversation_messages -> messages (message_id)); diesel::allow_tables_to_appear_in_same_query!( conversations, participants, - conversation_participants + conversation_participants, + messages, + conversation_messages, + settings, ); - -diesel::joinable!(conversation_messages -> conversations (conversation_id)); -diesel::joinable!(conversation_messages -> messages (message_id)); -diesel::allow_tables_to_appear_in_same_query!(conversations, messages, conversation_messages); diff --git a/kordophone-db/src/tests/mod.rs b/kordophone-db/src/tests/mod.rs index 1ab6e86..f631fd3 100644 --- a/kordophone-db/src/tests/mod.rs +++ b/kordophone-db/src/tests/mod.rs @@ -12,14 +12,8 @@ fn participants_equal_ignoring_id(a: &Participant, b: &Participant) -> bool { match (a, b) { (Participant::Me, Participant::Me) => true, ( - Participant::Remote { - display_name: name_a, - .. - }, - Participant::Remote { - display_name: name_b, - .. - }, + Participant::Remote { handle: name_a, .. }, + Participant::Remote { handle: name_b, .. }, ) => name_a == name_b, _ => false, } @@ -29,9 +23,14 @@ fn participants_vec_equal_ignoring_id(a: &[Participant], b: &[Participant]) -> b if a.len() != b.len() { return false; } - a.iter() - .zip(b.iter()) - .all(|(a, b)| participants_equal_ignoring_id(a, b)) + // For each participant in a, check if there is a matching participant in b + a.iter().all(|a_participant| { + b.iter().any(|b_participant| participants_equal_ignoring_id(a_participant, b_participant)) + }) && + // Also check the reverse to ensure no duplicates + b.iter().all(|b_participant| { + a.iter().any(|a_participant| participants_equal_ignoring_id(b_participant, a_participant)) + }) } #[tokio::test] @@ -214,8 +213,8 @@ async fn test_messages() { // Verify second message (from Alice) let retrieved_message2 = messages.iter().find(|m| m.id == message2.id).unwrap(); assert_eq!(retrieved_message2.text, "Hi there!"); - if let Participant::Remote { display_name, .. } = &retrieved_message2.sender { - assert_eq!(display_name, "Alice"); + if let Participant::Remote { handle, .. } = &retrieved_message2.sender { + assert_eq!(handle, "Alice"); } else { panic!( "Expected Remote participant. Got: {:?}", @@ -345,14 +344,8 @@ async fn test_insert_messages_batch() { match (&original.sender, &retrieved.sender) { (Participant::Me, Participant::Me) => {} ( - Participant::Remote { - display_name: o_name, - .. - }, - Participant::Remote { - display_name: r_name, - .. - }, + Participant::Remote { handle: o_name, .. }, + Participant::Remote { handle: r_name, .. }, ) => assert_eq!(o_name, r_name), _ => panic!( "Sender mismatch: original {:?}, retrieved {:?}", diff --git a/kordophoned/src/daemon/contact_resolver/eds.rs b/kordophoned/src/daemon/contact_resolver/eds.rs index a913cf4..424bacb 100644 --- a/kordophoned/src/daemon/contact_resolver/eds.rs +++ b/kordophoned/src/daemon/contact_resolver/eds.rs @@ -4,7 +4,10 @@ use dbus::arg::{RefArg, Variant}; use once_cell::sync::OnceCell; use std::collections::HashMap; use std::time::Duration; +use std::sync::Mutex; +use std::thread; +#[derive(Clone)] pub struct EDSContactResolverBackend; // Cache the UID of the default local address book so we do not have to scan @@ -12,6 +15,42 @@ pub struct EDSContactResolverBackend; // D-Bus round-trip that we would rather avoid on every lookup. static ADDRESS_BOOK_SOURCE_UID: OnceCell = OnceCell::new(); +/// Holds a D-Bus connection and the identifiers needed to create an address-book proxy. +struct AddressBookHandle { + connection: Connection, + object_path: String, + bus_name: String, +} + +impl AddressBookHandle { + fn new() -> anyhow::Result { + let connection = new_session_connection()?; + let source_uid = ensure_address_book_uid(&connection)?; + let (object_path, bus_name) = open_address_book(&connection, &source_uid)?; + + Ok(Self { + connection, + object_path, + bus_name, + }) + } +} + +/// Obtain the global address-book handle, initialising it on the first call. +static ADDRESS_BOOK_HANDLE: OnceCell> = OnceCell::new(); + +fn get_address_book_handle() -> Option<&'static Mutex> { + ADDRESS_BOOK_HANDLE + .get_or_try_init(|| AddressBookHandle::new().map(Mutex::new)) + .map_err(|e| { + log::debug!( + "EDS resolver: failed to initialise address book handle: {}", + e + ); + }) + .ok() +} + /// Helper that returns a blocking D-Bus session connection. Creating the /// connection is cheap (<1 ms) but we still keep it around because the /// underlying socket is re-used by the dbus crate. @@ -109,51 +148,41 @@ impl ContactResolverBackend for EDSContactResolverBackend { type ContactID = String; fn resolve_contact_id(&self, address: &str) -> Option { - // Only email addresses are supported for now. We fall back to NONE on - // any error to keep the resolver infallible for callers. - let conn = match new_session_connection() { - Ok(c) => c, - Err(e) => { - log::debug!("EDS resolver: failed to open session D-Bus: {}", e); - return None; - } + let handle_mutex = match get_address_book_handle() { + Some(h) => h, + None => return None, }; - let source_uid = match ensure_address_book_uid(&conn) { - Ok(u) => u, - Err(e) => { - log::debug!("EDS resolver: could not determine address-book UID: {}", e); - return None; - } - }; - - let (object_path, bus_name) = match open_address_book(&conn, &source_uid) { - Ok(v) => v, - Err(e) => { - log::debug!("EDS resolver: failed to open address book: {}", e); - return None; - } - }; - - let address_book_proxy = conn.with_proxy(bus_name, object_path, Duration::from_secs(60)); + let handle = handle_mutex.lock().unwrap(); + let address_book_proxy = handle.connection.with_proxy( + &handle.bus_name, + &handle.object_path, + Duration::from_secs(60), + ); let filter = if address.contains('@') { format!("(is \"email\" \"{}\")", address) } else { - // Remove country code, if present - let address = address.replace("+", "") + let normalized_address = address + .replace('+', "") .chars() .skip_while(|c| c.is_numeric() || *c == '(' || *c == ')') - .collect::(); - - // Remove any remaining non-numeric characters - let address = address.chars() + .collect::() + .chars() .filter(|c| c.is_numeric()) .collect::(); - - format!("(is \"phone\" \"{}\")", address) + format!( + "(or (is \"phone\" \"{}\") (is \"phone\" \"{}\") )", + address, normalized_address + ) }; + log::trace!( + "EDS resolver: GetContactListUids filter: {}, address: {}", + filter, + address + ); + let uids_result: Result<(Vec,), _> = address_book_proxy.method_call( "org.gnome.evolution.dataserver.AddressBook", "GetContactListUids", @@ -172,31 +201,17 @@ impl ContactResolverBackend for EDSContactResolverBackend { } fn get_contact_display_name(&self, contact_id: &Self::ContactID) -> Option { - let conn = match new_session_connection() { - Ok(c) => c, - Err(e) => { - log::debug!("EDS resolver: failed to open session D-Bus: {}", e); - return None; - } + let handle_mutex = match get_address_book_handle() { + Some(h) => h, + None => return None, }; - let source_uid = match ensure_address_book_uid(&conn) { - Ok(u) => u, - Err(e) => { - log::debug!("EDS resolver: could not determine address-book UID: {}", e); - return None; - } - }; - - let (object_path, bus_name) = match open_address_book(&conn, &source_uid) { - Ok(v) => v, - Err(e) => { - log::debug!("EDS resolver: failed to open address book: {}", e); - return None; - } - }; - - let address_book_proxy = conn.with_proxy(bus_name, object_path, Duration::from_secs(60)); + let handle = handle_mutex.lock().unwrap(); + let address_book_proxy = handle.connection.with_proxy( + &handle.bus_name, + &handle.object_path, + Duration::from_secs(60), + ); let vcard_result: Result<(String,), _> = address_book_proxy.method_call( "org.gnome.evolution.dataserver.AddressBook", diff --git a/kordophoned/src/daemon/contact_resolver/mod.rs b/kordophoned/src/daemon/contact_resolver/mod.rs index 2271fae..f8dd1b8 100644 --- a/kordophoned/src/daemon/contact_resolver/mod.rs +++ b/kordophoned/src/daemon/contact_resolver/mod.rs @@ -10,6 +10,7 @@ pub trait ContactResolverBackend { pub type AnyContactID = String; +#[derive(Clone)] pub struct ContactResolver { backend: T, } diff --git a/kordophoned/src/daemon/mod.rs b/kordophoned/src/daemon/mod.rs index 546424f..d9567b8 100644 --- a/kordophoned/src/daemon/mod.rs +++ b/kordophoned/src/daemon/mod.rs @@ -522,15 +522,15 @@ impl Daemon { .await? { for p in &saved.participants { - if let DbParticipant::Remote { id: Some(pid), display_name, contact_id: None } = p { - log::trace!(target: target::SYNC, "Resolving contact id for participant: {}", display_name); - if let Some(contact) = contact_resolver.resolve_contact_id(display_name) { + if let DbParticipant::Remote { handle, contact_id: None } = p { + log::trace!(target: target::SYNC, "Resolving contact id for participant: {}", handle); + if let Some(contact) = contact_resolver.resolve_contact_id(handle) { log::trace!(target: target::SYNC, "Resolved contact id for participant: {}", contact); let _ = database - .with_repository(|r| r.update_participant_contact(*pid, &contact)) + .with_repository(|r| r.update_participant_contact(&handle, &contact)) .await; } else { - log::trace!(target: target::SYNC, "No contact id found for participant: {}", display_name); + log::trace!(target: target::SYNC, "No contact id found for participant: {}", handle); } } } diff --git a/kordophoned/src/daemon/models/message.rs b/kordophoned/src/daemon/models/message.rs index 7b68ae8..ead4f2a 100644 --- a/kordophoned/src/daemon/models/message.rs +++ b/kordophoned/src/daemon/models/message.rs @@ -5,14 +5,14 @@ use crate::daemon::attachment_store::AttachmentStore; use crate::daemon::models::Attachment; use kordophone::model::message::AttachmentMetadata; use kordophone::model::outgoing_message::OutgoingMessage; +use kordophone_db::models::participant::Participant as DbParticipant; use std::collections::HashMap; #[derive(Clone, Debug)] pub enum Participant { Me, Remote { - id: Option, - display_name: String, + handle: String, contact_id: Option, }, } @@ -20,8 +20,7 @@ pub enum Participant { impl From for Participant { fn from(display_name: String) -> Self { Participant::Remote { - id: None, - display_name, + handle: display_name, contact_id: None, } } @@ -30,8 +29,7 @@ impl From for Participant { impl From<&str> for Participant { fn from(display_name: &str) -> Self { Participant::Remote { - id: None, - display_name: display_name.to_string(), + handle: display_name.to_string(), contact_id: None, } } @@ -41,8 +39,8 @@ impl From for Participant { fn from(participant: kordophone_db::models::Participant) -> Self { match participant { kordophone_db::models::Participant::Me => Participant::Me, - kordophone_db::models::Participant::Remote { id, display_name, contact_id } => { - Participant::Remote { id, display_name, contact_id } + kordophone_db::models::Participant::Remote { handle, contact_id } => { + Participant::Remote { handle, contact_id } } } } @@ -52,7 +50,7 @@ impl Participant { pub fn display_name(&self) -> String { match self { Participant::Me => "(Me)".to_string(), - Participant::Remote { display_name, .. } => display_name.clone(), + Participant::Remote { handle, .. } => handle.clone(), } } } @@ -110,8 +108,8 @@ impl From for kordophone_db::models::Message { id: message.id, sender: match message.sender { Participant::Me => kordophone_db::models::Participant::Me, - Participant::Remote { id, display_name, contact_id } => { - kordophone_db::models::Participant::Remote { id, display_name, contact_id } + Participant::Remote { handle, contact_id } => { + kordophone_db::models::Participant::Remote { handle, contact_id } } }, text: message.text, @@ -146,8 +144,7 @@ impl From for Message { id: message.guid, sender: match message.sender { Some(sender) => Participant::Remote { - id: None, - display_name: sender, + handle: sender, contact_id: None, }, None => Participant::Me, @@ -175,3 +172,12 @@ impl From<&OutgoingMessage> for Message { } } } + +impl From for DbParticipant { + fn from(participant: Participant) -> Self { + match participant { + Participant::Me => DbParticipant::Me, + Participant::Remote { handle, contact_id } => DbParticipant::Remote { handle, contact_id: contact_id.clone() }, + } + } +} \ No newline at end of file diff --git a/kordophoned/src/dbus/agent.rs b/kordophoned/src/dbus/agent.rs index e8ecc60..755dbfd 100644 --- a/kordophoned/src/dbus/agent.rs +++ b/kordophoned/src/dbus/agent.rs @@ -23,6 +23,7 @@ use dbus_tokio::connection; pub struct DBusAgent { event_sink: mpsc::Sender, signal_receiver: Arc>>>, + contact_resolver: ContactResolver, } impl DBusAgent { @@ -30,6 +31,7 @@ impl DBusAgent { Self { event_sink, signal_receiver: Arc::new(Mutex::new(Some(signal_receiver))), + contact_resolver: ContactResolver::new(EDSContactResolverBackend::default()), } } @@ -172,19 +174,18 @@ impl DBusAgent { } fn resolve_participant_display_name(&self, participant: &Participant) -> String { - let resolver = ContactResolver::new(EDSContactResolverBackend::default()); match participant { // Me (we should use a special string here...) Participant::Me => "(Me)".to_string(), // Remote participant with a resolved contact_id - Participant::Remote { display_name, contact_id: Some(contact_id), .. } => { - resolver.get_contact_display_name(contact_id).unwrap_or_else(|| display_name.clone()) + Participant::Remote { handle, contact_id: Some(contact_id), .. } => { + self.contact_resolver.get_contact_display_name(contact_id).unwrap_or_else(|| handle.clone()) } // Remote participant without a resolved contact_id - Participant::Remote { display_name, .. } => { - display_name.clone() + Participant::Remote { handle, .. } => { + handle.clone() } } } @@ -278,6 +279,8 @@ impl DbusRepository for DBusAgent { // Remove the attachment placeholder here. let text = msg.text.replace("\u{FFFC}", ""); + log::debug!("sender: {:?}", msg.sender.clone()); + map.insert("text".into(), arg::Variant(Box::new(text))); map.insert( "date".into(), @@ -285,9 +288,11 @@ impl DbusRepository for DBusAgent { ); map.insert( "sender".into(), - arg::Variant(Box::new(msg.sender.display_name())), + arg::Variant(Box::new(self.resolve_participant_display_name(&msg.sender.into()))), ); + + // Attachments array let attachments: Vec = msg .attachments