Resultify Contact::lookup_id_by_addr

Using a Contact ID as an error type is risky and a C leftover.

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]

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<str>,
         min_origin: Origin,
-    ) -> u32 {
+    ) -> Result<Option<u32>> {
         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 let Some(addr_self) = context.get_config(Config::ConfiguredAddr).await {
             if addr_cmp(addr_normalized, addr_self) {
-            return DC_CONTACT_ID_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));
+    }
 }

src/dc_receive_imf.rs

@@ -1184,17 +1184,16 @@ 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;
+        removed_id = Contact::lookup_id_by_addr(context, &removed_addr, Origin::Unknown).await?;
-        if removed_id == 0 {
+        match removed_id {
-            warn!(context, "removed {:?} has no contact_id", removed_addr);
+            Some(contact_id) => {
-        } else {
                 mime_parser.is_system_message = SystemMessage::MemberRemovedFromGroup;
                 better_msg = context
                     .stock_system_msg(
-                    if removed_id == from_id as u32 {
+                        if contact_id == from_id as u32 {
                             StockMessage::MsgGroupLeft
                         } else {
                             StockMessage::MsgDelMember
@@ -1205,6 +1204,8 @@ async fn create_or_lookup_group(
                     )
                     .await;
             }
+            None => warn!(context, "removed {:?} has no contact_id", removed_addr),
+        }
     } else {
         let field = mime_parser.get(HeaderDef::ChatGroupMemberAdded).cloned();
         if let Some(optional_field) = field {
@@ -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 {
+    } else if let Some(contact_id) = removed_id {
-        chat::remove_from_chat_contacts_table(context, chat_id, removed_id).await;
+        chat::remove_from_chat_contacts_table(context, chat_id, contact_id).await;
         send_EVENT_CHAT_MODIFIED = true;
     }
 

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 =
+            let contact_id = Contact::lookup_id_by_addr(context, failed_recipient, Origin::Unknown)
-                Contact::lookup_id_by_addr(context, failed_recipient, Origin::Unknown).await;
+                .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

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();

src/stock.rs

@@ -383,16 +383,13 @@ impl Context {
         param2: impl AsRef<str>,
         from_id: u32,
     ) -> String {
-        let insert1 = if id == StockMessage::MsgAddMember || id == StockMessage::MsgDelMember {
+        let insert1 = if matches!(id, StockMessage::MsgAddMember | StockMessage::MsgDelMember) {
-            let contact_id =
+            match Contact::lookup_id_by_addr(self, param1.as_ref(), Origin::Unknown).await {
-                Contact::lookup_id_by_addr(self, param1.as_ref(), Origin::Unknown).await;
+                Ok(Some(contact_id)) => Contact::get_by_id(self, contact_id)
-            if contact_id != 0 {
-                Contact::get_by_id(self, contact_id)
                     .await
                     .map(|contact| contact.get_name_n_addr())
-                    .unwrap_or_default()
+                    .unwrap_or_else(|_| param1.as_ref().to_string()),
-            } else {
+                _ => param1.as_ref().to_string(),
-                param1.as_ref().to_string()
             }
         } else {
             param1.as_ref().to_string()