feat: Contact::lookup_id_by_addr_ex: Prefer returning key-contact

If an address-contact and a key-contact were seen at exactly the same time, that doesn't necessarily
mean that it's a random event, it might occur because some code updates contacts this way in some
scenario. While this is unlikely, prefer to look up the key-contact.

src/contact.rs

@@ -808,7 +808,7 @@ impl Contact {
                 "SELECT id FROM contacts
                  WHERE addr=?1 COLLATE NOCASE
                  AND id>?2 AND origin>=?3 AND (? OR blocked=?)
-                 ORDER BY last_seen DESC LIMIT 1",
+                 ORDER BY last_seen DESC, fingerprint DESC LIMIT 1",
                 (
                     &addr_normalized,
                     ContactId::LAST_SPECIAL,

src/contact/contact_tests.rs

@@ -1035,6 +1035,37 @@ async fn test_was_seen_recently_event() -> Result<()> {
     Ok(())
 }
 
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_lookup_id_by_addr_recent() -> Result<()> {
+    let mut tcm = TestContextManager::new();
+    let bob = &tcm.bob().await;
+
+    let raw = include_bytes!("../../test-data/message/thunderbird_with_autocrypt.eml");
+    assert!(std::str::from_utf8(raw)?.contains("Date: Thu, 24 Nov 2022 20:05:57 +0100"));
+    let received_msg = receive_imf(bob, raw, false).await?.unwrap();
+    received_msg.chat_id.accept(bob).await?;
+
+    let raw = r#"From: Alice <alice@example.org>
+To: bob@example.net
+Message-ID: message$TIME@example.org
+Content-Type: text/plain; charset=utf-8; format=flowed; delsp=no
+Date: Thu, 24 Nov 2022 $TIME +0100
+
+Hi"#
+    .to_string();
+    for (time, is_key_contact) in [("20:05:57", true), ("20:05:58", false)] {
+        let raw = raw.replace("$TIME", time);
+        let received_msg = receive_imf(bob, raw.as_bytes(), false).await?.unwrap();
+        received_msg.chat_id.accept(bob).await?;
+        let contact_id = Contact::lookup_id_by_addr(bob, "alice@example.org", Origin::Unknown)
+            .await?
+            .unwrap();
+        let contact = Contact::get_by_id(bob, contact_id).await?;
+        assert_eq!(contact.is_key_contact(), is_key_contact);
+    }
+    Ok(())
+}
+
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
 async fn test_verified_by_none() -> Result<()> {
     let mut tcm = TestContextManager::new();

src/sql/migrations/migrations_tests.rs

@@ -2,6 +2,7 @@ use super::*;
 use crate::chat;
 use crate::chat::ChatId;
 use crate::config::Config;
+use crate::constants;
 use crate::contact::Contact;
 use crate::contact::ContactId;
 use crate::contact::Origin;
@@ -43,19 +44,11 @@ async fn test_key_contacts_migration_autocrypt() -> Result<()> {
     t.sql.run_migrations(&t).await?;
 
     //std::thread::sleep(std::time::Duration::from_secs(1000));
-    let email_bob_id = Contact::lookup_id_by_addr(&t, "bob@example.net", Origin::Hidden)
+    let pgp_bob_id = Contact::lookup_id_by_addr(&t, "bob@example.net", Origin::Hidden)
         .await?
         .unwrap();
-    let email_bob = Contact::get_by_id(&t, email_bob_id).await?;
-    assert_eq!(email_bob.is_key_contact(), false);
-    assert_eq!(email_bob.origin, Origin::Hidden); // Email bob is in no chats, so, contact is hidden
-    assert_eq!(email_bob.e2ee_avail(&t).await?, false);
-    assert_eq!(email_bob.fingerprint(), None);
-    assert_eq!(email_bob.get_verifier_id(&t).await?, None);
-
-    let bob_chat_contacts = chat::get_chat_contacts(&t, ChatId::new(10)).await?;
-    let pgp_bob_id = tools::single_value(bob_chat_contacts).unwrap();
     let pgp_bob = Contact::get_by_id(&t, pgp_bob_id).await?;
+    assert_eq!(pgp_bob.is_key_contact(), true);
     assert_eq!(pgp_bob.origin, Origin::OutgoingTo);
     assert_eq!(pgp_bob.e2ee_avail(&t).await?, true);
     assert_eq!(
@@ -64,6 +57,16 @@ async fn test_key_contacts_migration_autocrypt() -> Result<()> {
     );
     assert_eq!(pgp_bob.get_verifier_id(&t).await?, None);
 
+    // Hidden address-contact can't be looked up.
+    assert!(
+        Contact::get_all(&t, constants::DC_GCL_ADDRESS, Some("bob@example.net"))
+            .await?
+            .is_empty()
+    );
+
+    let bob_chat_contacts = chat::get_chat_contacts(&t, ChatId::new(10)).await?;
+    assert_eq!(tools::single_value(bob_chat_contacts).unwrap(), pgp_bob_id);
+
     Ok(())
 }
 
@@ -82,8 +85,9 @@ async fn test_key_contacts_migration_email1() -> Result<()> {
     )?)).await?;
     t.sql.run_migrations(&t).await?;
 
-    let email_bob_id = Contact::lookup_id_by_addr(&t, "bob@example.net", Origin::Hidden)
+    let email_bob_id = *Contact::get_all(&t, constants::DC_GCL_ADDRESS, Some("bob@example.net"))
         .await?
+        .first()
         .unwrap();
     let email_bob = Contact::get_by_id(&t, email_bob_id).await?;
     assert_eq!(email_bob.is_key_contact(), false);
@@ -112,12 +116,19 @@ async fn test_key_contacts_migration_email2() -> Result<()> {
     )?)).await?;
     t.sql.run_migrations(&t).await?;
 
-    let email_bob_id = Contact::lookup_id_by_addr(&t, "bob@example.net", Origin::Hidden)
+    let pgp_bob_id = Contact::lookup_id_by_addr(&t, "bob@example.net", Origin::Hidden)
         .await?
         .unwrap();
+    let pgp_bob = Contact::get_by_id(&t, pgp_bob_id).await?;
+    assert_eq!(pgp_bob.is_key_contact(), true);
+
+    let email_bob_id = *Contact::get_all(&t, constants::DC_GCL_ADDRESS, Some("bob@example.net"))
+        .await?
+        .first()
+        .unwrap();
     let email_bob = Contact::get_by_id(&t, email_bob_id).await?;
     assert_eq!(email_bob.is_key_contact(), false);
-    assert_eq!(email_bob.origin, Origin::OutgoingTo); // Email bob is in no chats, so, contact is hidden
+    assert_eq!(email_bob.origin, Origin::OutgoingTo);
     assert_eq!(email_bob.e2ee_avail(&t).await?, false);
     assert_eq!(email_bob.fingerprint(), None);
     assert_eq!(email_bob.get_verifier_id(&t).await?, None);
@@ -146,16 +157,12 @@ async fn test_key_contacts_migration_verified() -> Result<()> {
     )?)).await?;
     t.sql.run_migrations(&t).await?;
 
-    let email_bob_id = Contact::lookup_id_by_addr(&t, "bob@example.net", Origin::Hidden)
+    // Hidden address-contact can't be looked up.
-        .await?
+    assert!(
-        .unwrap();
+        Contact::get_all(&t, constants::DC_GCL_ADDRESS, Some("bob@example.net"))
-    let email_bob = Contact::get_by_id(&t, email_bob_id).await?;
+            .await?
-    dbg!(&email_bob);
+            .is_empty()
-    assert_eq!(email_bob.is_key_contact(), false);
+    );
-    assert_eq!(email_bob.origin, Origin::Hidden); // Email bob is in no chats, so, contact is hidden
-    assert_eq!(email_bob.e2ee_avail(&t).await?, false);
-    assert_eq!(email_bob.fingerprint(), None);
-    assert_eq!(email_bob.get_verifier_id(&t).await?, None);
 
     let mut bob_chat_contacts = chat::get_chat_contacts(&t, ChatId::new(10)).await?;
     assert_eq!(bob_chat_contacts.len(), 2);