diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index d1b29a0cc..8633986fb 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -1539,6 +1539,9 @@ pub unsafe extern "C" fn dc_lookup_contact_id_by_addr( to_string_lossy(addr), Origin::IncomingReplyTo, )) + .ok() + .flatten() + .unwrap_or_default() } #[no_mangle] diff --git a/src/contact.rs b/src/contact.rs index 6fefa7d36..d743e65d3 100644 --- a/src/contact.rs +++ b/src/contact.rs @@ -1,6 +1,6 @@ //! Contacts module -use anyhow::{bail, ensure, format_err, Result}; +use anyhow::{bail, ensure, format_err, Context as _, Error, Result}; use async_std::path::PathBuf; use deltachat_derive::{FromSql, ToSql}; use itertools::Itertools; @@ -283,6 +283,7 @@ impl Contact { } /// Check if an e-mail address belongs to a known and unblocked contact. + /// /// Known and unblocked contacts will be returned by `dc_get_contacts()`. /// /// To validate an e-mail address independently of the contact database @@ -291,29 +292,28 @@ impl Contact { context: &Context, addr: impl AsRef, min_origin: Origin, - ) -> u32 { + ) -> Result> { if addr.as_ref().is_empty() { - return 0; + return Err(Error::msg("lookup_id_by_addr: empty address")); } let addr_normalized = addr_normalize(addr.as_ref()); - let addr_self = context - .get_config(Config::ConfiguredAddr) - .await - .unwrap_or_default(); - if addr_cmp(addr_normalized, addr_self) { - return DC_CONTACT_ID_SELF; + if let Some(addr_self) = context.get_config(Config::ConfiguredAddr).await { + if addr_cmp(addr_normalized, addr_self) { + return Ok(Some(DC_CONTACT_ID_SELF)); + } } - context.sql.query_get_value( - context, + context.sql.query_get_value_result( "SELECT id FROM contacts WHERE addr=?1 COLLATE NOCASE AND id>?2 AND origin>=?3 AND blocked=0;", paramsv![ addr_normalized, DC_CONTACT_ID_LAST_SPECIAL as i32, min_origin as u32, ], - ).await.unwrap_or_default() + ) + .await + .context("lookup_id_by_addr: SQL query failed") } /// Lookup a contact and create it if it does not exist yet. @@ -1589,4 +1589,29 @@ mod tests { .await .is_err()); } + + #[async_std::test] + async fn test_lookup_id_by_addr() { + let t = TestContext::new().await; + + let id = Contact::lookup_id_by_addr(&t.ctx, "the.other@example.net", Origin::Unknown) + .await + .unwrap(); + assert!(id.is_none()); + + let other_id = Contact::create(&t.ctx, "The Other", "the.other@example.net") + .await + .unwrap(); + let id = Contact::lookup_id_by_addr(&t.ctx, "the.other@example.net", Origin::Unknown) + .await + .unwrap(); + assert_eq!(id, Some(other_id)); + + let alice = TestContext::new_alice().await; + + let id = Contact::lookup_id_by_addr(&alice.ctx, "alice@example.com", Origin::Unknown) + .await + .unwrap(); + assert_eq!(id, Some(DC_CONTACT_ID_SELF)); + } } diff --git a/src/dc_receive_imf.rs b/src/dc_receive_imf.rs index 1380b9fa0..b8764fba7 100644 --- a/src/dc_receive_imf.rs +++ b/src/dc_receive_imf.rs @@ -1184,26 +1184,27 @@ async fn create_or_lookup_group( // but we might not know about this group let grpname = mime_parser.get(HeaderDef::ChatGroupName).cloned(); - let mut removed_id = 0; + let mut removed_id = None; if let Some(removed_addr) = mime_parser.get(HeaderDef::ChatGroupMemberRemoved).cloned() { - removed_id = Contact::lookup_id_by_addr(context, &removed_addr, Origin::Unknown).await; - if removed_id == 0 { - warn!(context, "removed {:?} has no contact_id", removed_addr); - } else { - mime_parser.is_system_message = SystemMessage::MemberRemovedFromGroup; - better_msg = context - .stock_system_msg( - if removed_id == from_id as u32 { - StockMessage::MsgGroupLeft - } else { - StockMessage::MsgDelMember - }, - &removed_addr, - "", - from_id as u32, - ) - .await; + removed_id = Contact::lookup_id_by_addr(context, &removed_addr, Origin::Unknown).await?; + match removed_id { + Some(contact_id) => { + mime_parser.is_system_message = SystemMessage::MemberRemovedFromGroup; + better_msg = context + .stock_system_msg( + if contact_id == from_id as u32 { + StockMessage::MsgGroupLeft + } else { + StockMessage::MsgDelMember + }, + &removed_addr, + "", + from_id as u32, + ) + .await; + } + None => warn!(context, "removed {:?} has no contact_id", removed_addr), } } else { let field = mime_parser.get(HeaderDef::ChatGroupMemberAdded).cloned(); @@ -1285,7 +1286,7 @@ async fn create_or_lookup_group( && !grpid.is_empty() && grpname.is_some() // otherwise, a pending "quit" message may pop up - && removed_id == 0 + && removed_id.is_none() // re-create explicitly left groups only if ourself is re-added && (!group_explicitly_left || X_MrAddToGrp.is_some() && addr_cmp(&self_addr, X_MrAddToGrp.as_ref().unwrap())) @@ -1434,8 +1435,8 @@ async fn create_or_lookup_group( } } send_EVENT_CHAT_MODIFIED = true; - } else if removed_id > 0 { - chat::remove_from_chat_contacts_table(context, chat_id, removed_id).await; + } else if let Some(contact_id) = removed_id { + chat::remove_from_chat_contacts_table(context, chat_id, contact_id).await; send_EVENT_CHAT_MODIFIED = true; } diff --git a/src/message.rs b/src/message.rs index c5def69d7..1380fea67 100644 --- a/src/message.rs +++ b/src/message.rs @@ -1730,8 +1730,9 @@ async fn ndn_maybe_add_info_msg( ) -> anyhow::Result<()> { if chat_type == Chattype::Group { if let Some(failed_recipient) = &failed.failed_recipient { - let contact_id = - Contact::lookup_id_by_addr(context, failed_recipient, Origin::Unknown).await; + let contact_id = Contact::lookup_id_by_addr(context, failed_recipient, Origin::Unknown) + .await? + .ok_or_else(|| Error::msg("ndn_maybe_add_info_msg: Contact ID not found"))?; let contact = Contact::load_from_db(context, contact_id).await?; // Tell the user which of the recipients failed if we know that (because in a group, this might otherwise be unclear) let text = context diff --git a/src/securejoin.rs b/src/securejoin.rs index bb179b8d1..7ae7ec956 100644 --- a/src/securejoin.rs +++ b/src/securejoin.rs @@ -1170,7 +1170,9 @@ mod tests { } => { let alice_contact_id = Contact::lookup_id_by_addr(&bob.ctx, "alice@example.com", Origin::Unknown) - .await; + .await + .expect("Error looking up contact") + .expect("Contact not found"); assert_eq!(contact_id, alice_contact_id); assert_eq!(progress, 400); } @@ -1198,7 +1200,10 @@ mod tests { // Alice should not yet have Bob verified let contact_bob_id = - Contact::lookup_id_by_addr(&alice.ctx, "bob@example.net", Origin::Unknown).await; + Contact::lookup_id_by_addr(&alice.ctx, "bob@example.net", Origin::Unknown) + .await + .expect("Error looking up contact") + .expect("Contact not found"); let contact_bob = Contact::load_from_db(&alice.ctx, contact_bob_id) .await .unwrap(); @@ -1243,7 +1248,10 @@ mod tests { // Bob should not yet have Alice verified let contact_alice_id = - Contact::lookup_id_by_addr(&bob.ctx, "alice@example.com", Origin::Unknown).await; + Contact::lookup_id_by_addr(&bob.ctx, "alice@example.com", Origin::Unknown) + .await + .expect("Error looking up contact") + .expect("Contact not found"); let contact_alice = Contact::load_from_db(&bob.ctx, contact_alice_id) .await .unwrap(); @@ -1354,7 +1362,9 @@ mod tests { } => { let alice_contact_id = Contact::lookup_id_by_addr(&bob.ctx, "alice@example.com", Origin::Unknown) - .await; + .await + .expect("Error looking up contact") + .expect("Contact not found"); assert_eq!(contact_id, alice_contact_id); assert_eq!(progress, 400); } @@ -1414,7 +1424,10 @@ mod tests { // Bob should not yet have Alice verified let contact_alice_id = - Contact::lookup_id_by_addr(&bob.ctx, "alice@example.com", Origin::Unknown).await; + Contact::lookup_id_by_addr(&bob.ctx, "alice@example.com", Origin::Unknown) + .await + .expect("Error looking up contact") + .expect("Contact not found"); let contact_alice = Contact::load_from_db(&bob.ctx, contact_alice_id) .await .unwrap(); @@ -1504,7 +1517,9 @@ mod tests { } => { let alice_contact_id = Contact::lookup_id_by_addr(&bob.ctx, "alice@example.com", Origin::Unknown) - .await; + .await + .expect("Error looking up contact") + .expect("Contact not found"); assert_eq!(contact_id, alice_contact_id); assert_eq!(progress, 400); } @@ -1531,7 +1546,10 @@ mod tests { // Alice should not yet have Bob verified let contact_bob_id = - Contact::lookup_id_by_addr(&alice.ctx, "bob@example.net", Origin::Unknown).await; + Contact::lookup_id_by_addr(&alice.ctx, "bob@example.net", Origin::Unknown) + .await + .expect("Error looking up contact") + .expect("Contact not found"); let contact_bob = Contact::load_from_db(&alice.ctx, contact_bob_id) .await .unwrap(); @@ -1554,7 +1572,10 @@ mod tests { // Bob should not yet have Alice verified let contact_alice_id = - Contact::lookup_id_by_addr(&bob.ctx, "alice@example.com", Origin::Unknown).await; + Contact::lookup_id_by_addr(&bob.ctx, "alice@example.com", Origin::Unknown) + .await + .expect("Error looking up contact") + .expect("Contact not found"); let contact_alice = Contact::load_from_db(&bob.ctx, contact_alice_id) .await .unwrap(); diff --git a/src/stock.rs b/src/stock.rs index 9dc6e9be8..072e743ec 100644 --- a/src/stock.rs +++ b/src/stock.rs @@ -383,16 +383,13 @@ impl Context { param2: impl AsRef, from_id: u32, ) -> String { - let insert1 = if id == StockMessage::MsgAddMember || id == StockMessage::MsgDelMember { - let contact_id = - Contact::lookup_id_by_addr(self, param1.as_ref(), Origin::Unknown).await; - if contact_id != 0 { - Contact::get_by_id(self, contact_id) + let insert1 = if matches!(id, StockMessage::MsgAddMember | StockMessage::MsgDelMember) { + match Contact::lookup_id_by_addr(self, param1.as_ref(), Origin::Unknown).await { + Ok(Some(contact_id)) => Contact::get_by_id(self, contact_id) .await .map(|contact| contact.get_name_n_addr()) - .unwrap_or_default() - } else { - param1.as_ref().to_string() + .unwrap_or_else(|_| param1.as_ref().to_string()), + _ => param1.as_ref().to_string(), } } else { param1.as_ref().to_string()