Explicit API for creating chats with blocked status

This introduces the explicit ChatIdBlocked struct to more explicitly
create a chat with a blocked status.  It also adds a common shortcut
to ChatId itself which is more natural to use in many cases.

src/chat.rs

@@ -4,8 +4,7 @@ use std::convert::{TryFrom, TryInto};
 use std::str::FromStr;
 use std::time::{Duration, SystemTime};
 
-use anyhow::Context as _;
-use anyhow::{bail, ensure, format_err, Result};
+use anyhow::{bail, ensure, format_err, Context as _, Result};
 use async_std::path::{Path, PathBuf};
 use deltachat_derive::{FromSql, ToSql};
 use itertools::Itertools;
@@ -156,6 +155,15 @@ impl ChatId {
         self == DC_CHAT_ID_ALLDONE_HINT
     }
 
+    /// Returns the [`ChatId`] for the 1:1 chat with `contact_id`.
+    ///
+    /// If the chat does not yet exist an unblocked chat ([`Blocked::Not`]) is created.
+    pub async fn get_for_contact(context: &Context, contact_id: u32) -> Result<Self> {
+        ChatIdBlocked::get_for_contact(context, contact_id, Blocked::Not)
+            .await
+            .map(|chat| chat.id)
+    }
+
     pub async fn set_selfavatar_timestamp(self, context: &Context, timestamp: i64) -> Result<()> {
         context
             .sql
@@ -1330,8 +1338,7 @@ pub async fn create_by_contact_id(context: &Context, contact_id: u32) -> Result<
                 );
                 return Err(err);
             } else {
-                let (chat_id, _) =
-                    create_or_lookup_by_contact_id(context, contact_id, Blocked::Not).await?;
+                let chat_id = ChatId::get_for_contact(context, contact_id).await?;
                 Contact::scaleup_origin_by_id(context, contact_id, Origin::CreateChat).await;
                 chat_id
             }
@@ -1408,60 +1415,122 @@ pub(crate) async fn update_special_chat_names(context: &Context) -> Result<()> {
     Ok(())
 }
 
-pub(crate) async fn create_or_lookup_by_contact_id(
-    context: &Context,
-    contact_id: u32,
-    create_blocked: Blocked,
-) -> Result<(ChatId, Blocked)> {
-    ensure!(context.sql.is_open().await, "Database not available");
-    ensure!(contact_id > 0, "Invalid contact id requested");
+/// Handle a [`ChatId`] and its [`Blocked`] status at once.
+///
+/// This struct is an optimisation to read a [`ChatId`] and its [`Blocked`] status at once
+/// from the database.  It [`Deref`]s to [`ChatId`] so it can be used as an extension to
+/// [`ChatId`].
+///
+/// [`Deref`]: std::ops::Deref
+#[derive(Debug)]
+pub(crate) struct ChatIdBlocked {
+    pub id: ChatId,
+    pub blocked: Blocked,
+}
 
-    if let Ok((chat_id, chat_blocked)) = lookup_by_contact_id(context, contact_id).await {
-        // Already exists, no need to create.
-        return Ok((chat_id, chat_blocked));
+impl ChatIdBlocked {
+    /// Searches the database for the 1:1 chat with this contact.
+    ///
+    /// If no chat is found `None` is returned.
+    async fn lookup_by_contact(context: &Context, contact_id: u32) -> Result<Option<Self>, Error> {
+        ensure!(context.sql.is_open().await, "Database not available");
+        ensure!(contact_id > 0, "Invalid contact id requested");
+
+        context
+            .sql
+            .query_row_optional(
+                "SELECT c.id, c.blocked
+                   FROM chats c
+                  INNER JOIN chats_contacts j
+                          ON c.id=j.chat_id
+                  WHERE c.type=100  -- 100 = Chattype::Single
+                    AND c.id>9      -- 9 = DC_CHAT_ID_LAST_SPECIAL
+                    AND j.contact_id=?;",
+                paramsv![contact_id],
+                |row| {
+                    let id: ChatId = row.get(0)?;
+                    let blocked: Blocked = row.get(1)?;
+                    Ok(ChatIdBlocked { id, blocked })
+                },
+            )
+            .await
+            .map_err(Into::into)
     }
 
-    let contact = Contact::load_from_db(context, contact_id).await?;
-    let chat_name = contact.get_display_name().to_string();
+    /// Returns the chat for the 1:1 chat with this contact.
+    ///
+    /// I the chat does not yet exist a new one is created, using the provided [`Blocked`]
+    /// state.
+    pub async fn get_for_contact(
+        context: &Context,
+        contact_id: u32,
+        create_blocked: Blocked,
+    ) -> Result<Self, Error> {
+        ensure!(context.sql.is_open().await, "Database not available");
+        ensure!(contact_id > 0, "Invalid contact id requested");
 
-    context
-        .sql
-        .transaction(move |transaction| {
-            transaction.execute(
-                "INSERT INTO chats
-                 (type, name, param, blocked, created_timestamp)
-                 VALUES(?, ?, ?, ?, ?)",
-                params![
-                    Chattype::Single,
-                    chat_name,
-                    match contact_id {
-                        DC_CONTACT_ID_SELF => "K=1".to_string(), // K = Param::Selftalk
-                        DC_CONTACT_ID_DEVICE => "D=1".to_string(), // D = Param::Devicetalk
-                        _ => "".to_string(),
-                    },
-                    create_blocked as u8,
-                    time(),
-                ],
-            )?;
+        if let Some(res) = Self::lookup_by_contact(context, contact_id).await? {
+            // Already exists, no need to create.
+            return Ok(res);
+        }
 
-            transaction.execute(
-                "INSERT INTO chats_contacts
+        let contact = Contact::load_from_db(context, contact_id).await?;
+        let chat_name = contact.get_display_name().to_string();
+        let mut params = Params::new();
+        match contact_id {
+            DC_CONTACT_ID_SELF => {
+                params.set_int(Param::Selftalk, 1);
+            }
+            DC_CONTACT_ID_DEVICE => {
+                params.set_int(Param::Devicetalk, 1);
+            }
+            _ => (),
+        }
+
+        let chat_id = context
+            .sql
+            .transaction(move |transaction| {
+                transaction.execute(
+                    "INSERT INTO chats
+                     (type, name, param, blocked, created_timestamp)
+                     VALUES(?, ?, ?, ?, ?)",
+                    params![
+                        Chattype::Single,
+                        chat_name,
+                        params.to_string(),
+                        create_blocked as u8,
+                        time(),
+                    ],
+                )?;
+                let chat_id = ChatId::new(
+                    transaction
+                        .last_insert_rowid()
+                        .try_into()
+                        .context("chat table rowid overflows u32")?,
+                );
+
+                transaction.execute(
+                    "INSERT INTO chats_contacts
                  (chat_id, contact_id)
                  VALUES((SELECT last_insert_rowid()), ?)",
-                params![contact_id],
-            )?;
+                    params![contact_id],
+                )?;
 
-            Ok(())
+                Ok(chat_id)
+            })
+            .await?;
+
+        match contact_id {
+            DC_CONTACT_ID_SELF => update_saved_messages_icon(context).await?,
+            DC_CONTACT_ID_DEVICE => update_device_icon(context).await?,
+            _ => (),
+        }
+
+        Ok(Self {
+            id: chat_id,
+            blocked: create_blocked,
         })
-        .await?;
-
-    if contact_id == DC_CONTACT_ID_SELF {
-        update_saved_messages_icon(context).await?;
-    } else if contact_id == DC_CONTACT_ID_DEVICE {
-        update_device_icon(context).await?;
     }
-
-    lookup_by_contact_id(context, contact_id).await
 }
 
 pub(crate) async fn lookup_by_contact_id(
@@ -2851,9 +2920,7 @@ pub async fn add_device_msg_with_importance(
     }
 
     if let Some(msg) = msg {
-        chat_id = create_or_lookup_by_contact_id(context, DC_CONTACT_ID_DEVICE, Blocked::Not)
-            .await?
-            .0;
+        chat_id = ChatId::get_for_contact(context, DC_CONTACT_ID_DEVICE).await?;
 
         let rfc724_mid = dc_create_outgoing_rfc724_mid(None, "@device");
         msg.try_calc_and_set_dimensions(context).await.ok();
@@ -3314,10 +3381,9 @@ mod tests {
     async fn test_device_chat_cannot_sent() {
         let t = TestContext::new().await;
         t.update_device_chats().await.unwrap();
-        let (device_chat_id, _) =
-            create_or_lookup_by_contact_id(&t, DC_CONTACT_ID_DEVICE, Blocked::Not)
-                .await
-                .unwrap();
+        let device_chat_id = ChatId::get_for_contact(&t, DC_CONTACT_ID_DEVICE)
+            .await
+            .unwrap();
 
         let mut msg = Message::new(Viewtype::Text);
         msg.text = Some("message text".to_string());
@@ -3767,9 +3833,10 @@ mod tests {
 
         // create contact, then blocked chat
         let contact_id = Contact::create(&ctx, "", "claire@foo.de").await.unwrap();
-        let (chat_id, _) = create_or_lookup_by_contact_id(&ctx, contact_id, Blocked::Manually)
+        let chat_id = ChatIdBlocked::get_for_contact(&ctx, contact_id, Blocked::Manually)
             .await
-            .unwrap();
+            .unwrap()
+            .id;
         let (chat_id2, blocked) = lookup_by_contact_id(&ctx, contact_id).await.unwrap();
         assert_eq!(chat_id, chat_id2);
         assert_eq!(blocked, Blocked::Manually);

src/dc_receive_imf.rs

@@ -8,7 +8,7 @@ use once_cell::sync::Lazy;
 use regex::Regex;
 use sha2::{Digest, Sha256};
 
-use crate::chat::{self, Chat, ChatId, ProtectionStatus};
+use crate::chat::{self, Chat, ChatId, ChatIdBlocked, ProtectionStatus};
 use crate::config::Config;
 use crate::constants::{
     Blocked, Chattype, ShowEmails, Viewtype, DC_CHAT_ID_TRASH, DC_CONTACT_ID_LAST_SPECIAL,
@@ -604,12 +604,13 @@ async fn add_parts(
                 *chat_id = test_normal_chat_id;
                 chat_id_blocked = test_normal_chat_id_blocked;
             } else if allow_creation {
-                let (id, bl) =
-                    chat::create_or_lookup_by_contact_id(context, from_id, create_blocked)
-                        .await
-                        .unwrap_or_default();
-                *chat_id = id;
-                chat_id_blocked = bl;
+                if let Ok(chat) = ChatIdBlocked::get_for_contact(context, from_id, create_blocked)
+                    .await
+                    .log_err(context, "Failed to get (new) chat for contact")
+                {
+                    *chat_id = chat.id;
+                    chat_id_blocked = chat.blocked;
+                }
             }
             if !chat_id.is_unset() && Blocked::Not != chat_id_blocked {
                 if Blocked::Not == create_blocked {
@@ -725,11 +726,12 @@ async fn add_parts(
                 } else {
                     Blocked::Deaddrop
                 };
-                let (id, bl) = chat::create_or_lookup_by_contact_id(context, to_id, create_blocked)
-                    .await
-                    .unwrap_or_default();
-                *chat_id = id;
-                chat_id_blocked = bl;
+                if let Ok(chat) =
+                    ChatIdBlocked::get_for_contact(context, to_id, create_blocked).await
+                {
+                    *chat_id = chat.id;
+                    chat_id_blocked = chat.blocked;
+                }
 
                 if !chat_id.is_unset()
                     && Blocked::Not != chat_id_blocked
@@ -747,12 +749,14 @@ async fn add_parts(
         if chat_id.is_unset() && self_sent {
             // from_id==to_id==DC_CONTACT_ID_SELF - this is a self-sent messages,
             // maybe an Autocrypt Setup Message
-            let (id, bl) =
-                chat::create_or_lookup_by_contact_id(context, DC_CONTACT_ID_SELF, Blocked::Not)
+            if let Ok(chat) =
+                ChatIdBlocked::get_for_contact(context, DC_CONTACT_ID_SELF, Blocked::Not)
                     .await
-                    .unwrap_or_default();
-            *chat_id = id;
-            chat_id_blocked = bl;
+                    .log_err(context, "Failed to get (new) chat for contact")
+            {
+                *chat_id = chat.id;
+                chat_id_blocked = chat.blocked;
+            }
 
             if !chat_id.is_unset() && Blocked::Not != chat_id_blocked {
                 chat_id.unblock(context).await;

src/log.rs

@@ -126,7 +126,7 @@ where
     }
 }
 
-impl<T: Default, E: std::fmt::Display> LogExt<T, E> for Result<T, E> {
+impl<T, E: std::fmt::Display> LogExt<T, E> for Result<T, E> {
     #[track_caller]
     fn log_err_inner(self, context: &Context, msg: Option<&str>) -> Result<T, E> {
         if let Err(e) = &self {

src/message.rs

@@ -2592,10 +2592,9 @@ mod tests {
         // test that get_width() and get_height() are returning some dimensions for images;
         // (as the device-chat contains a welcome-images, we check that)
         t.update_device_chats().await.ok();
-        let (device_chat_id, _) =
-            chat::create_or_lookup_by_contact_id(&t, DC_CONTACT_ID_DEVICE, Blocked::Not)
-                .await
-                .unwrap();
+        let device_chat_id = ChatId::get_for_contact(&t, DC_CONTACT_ID_DEVICE)
+            .await
+            .unwrap();
 
         let mut has_image = false;
         let chatitems = chat::get_chat_msgs(&t, device_chat_id, 0, None)

src/peerstate.rs

@@ -4,7 +4,7 @@ use std::collections::HashSet;
 use std::fmt;
 
 use crate::aheader::{Aheader, EncryptPreference};
-use crate::chat;
+use crate::chat::{self, ChatIdBlocked};
 use crate::constants::Blocked;
 use crate::context::Context;
 use crate::events::EventType;
@@ -267,15 +267,15 @@ impl Peerstate {
                 .query_get_value("SELECT id FROM contacts WHERE addr=?;", paramsv![self.addr])
                 .await?
             {
-                let (contact_chat_id, _) =
-                    chat::create_or_lookup_by_contact_id(context, contact_id, Blocked::Deaddrop)
-                        .await
-                        .unwrap_or_default();
+                let chat_id =
+                    ChatIdBlocked::get_for_contact(context, contact_id, Blocked::Deaddrop)
+                        .await?
+                        .id;
 
                 let msg = stock_str::contact_setup_changed(context, self.addr.clone()).await;
 
-                chat::add_info_msg(context, contact_chat_id, msg).await;
-                emit_event!(context, EventType::ChatModified(contact_chat_id));
+                chat::add_info_msg(context, chat_id, msg).await;
+                emit_event!(context, EventType::ChatModified(chat_id));
             } else {
                 bail!("contact with peerstate.addr {:?} not found", &self.addr);
             }

src/qr.rs

@@ -6,12 +6,13 @@ use percent_encoding::percent_decode_str;
 use serde::Deserialize;
 use std::collections::BTreeMap;
 
-use crate::chat;
+use crate::chat::{self, ChatIdBlocked};
 use crate::config::Config;
 use crate::constants::Blocked;
 use crate::contact::{addr_normalize, may_be_valid_addr, Contact, Origin};
 use crate::context::Context;
 use crate::key::Fingerprint;
+use crate::log::LogExt;
 use crate::lot::{Lot, LotState};
 use crate::message::Message;
 use crate::peerstate::Peerstate;
@@ -165,11 +166,12 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Lot {
             .map(|(id, _)| id)
             .unwrap_or_default();
 
-            let (id, _) = chat::create_or_lookup_by_contact_id(context, lot.id, Blocked::Deaddrop)
+            if let Ok(chat) = ChatIdBlocked::get_for_contact(context, lot.id, Blocked::Deaddrop)
                 .await
-                .unwrap_or_default();
-
-            chat::add_info_msg(context, id, format!("{} verified.", peerstate.addr)).await;
+                .log_err(context, "Failed to create (new) chat for contact")
+            {
+                chat::add_info_msg(context, chat.id, format!("{} verified.", peerstate.addr)).await;
+            }
         } else if let Some(addr) = addr {
             lot.state = LotState::QrFprMismatch;
             lot.id = match Contact::lookup_id_by_addr(context, &addr, Origin::Unknown).await {

src/securejoin.rs

@@ -9,7 +9,7 @@ use async_std::sync::Mutex;
 use percent_encoding::{utf8_percent_encode, AsciiSet, NON_ALPHANUMERIC};
 
 use crate::aheader::EncryptPreference;
-use crate::chat::{self, Chat, ChatId};
+use crate::chat::{self, Chat, ChatId, ChatIdBlocked};
 use crate::config::Config;
 use crate::constants::{Blocked, Viewtype, DC_CONTACT_ID_LAST_SPECIAL};
 use crate::contact::{Contact, Origin, VerifiedStatus};
@@ -498,19 +498,18 @@ pub(crate) async fn handle_securejoin_handshake(
     );
 
     let contact_chat_id = {
-        let (chat_id, blocked) =
-            chat::create_or_lookup_by_contact_id(context, contact_id, Blocked::Not)
-                .await
-                .with_context(|| {
-                    format!(
-                        "Failed to look up or create chat for contact {}",
-                        contact_id
-                    )
-                })?;
-        if blocked != Blocked::Not {
-            chat_id.unblock(context).await;
+        let chat = ChatIdBlocked::get_for_contact(context, contact_id, Blocked::Not)
+            .await
+            .with_context(|| {
+                format!(
+                    "Failed to look up or create chat for contact {}",
+                    contact_id
+                )
+            })?;
+        if chat.blocked != Blocked::Not {
+            chat.id.unblock(context).await;
         }
-        chat_id
+        chat.id
     };
 
     let join_vg = step.starts_with("vg-");
@@ -794,19 +793,18 @@ pub(crate) async fn observe_securejoin_on_other_device(
     info!(context, "observing secure-join message \'{}\'", step);
 
     let contact_chat_id = {
-        let (chat_id, blocked) =
-            chat::create_or_lookup_by_contact_id(context, contact_id, Blocked::Not)
-                .await
-                .with_context(|| {
-                    format!(
-                        "Failed to look up or create chat for contact {}",
-                        contact_id
-                    )
-                })?;
-        if blocked != Blocked::Not {
-            chat_id.unblock(context).await;
+        let chat = ChatIdBlocked::get_for_contact(context, contact_id, Blocked::Not)
+            .await
+            .with_context(|| {
+                format!(
+                    "Failed to look up or create chat for contact {}",
+                    contact_id
+                )
+            })?;
+        if chat.blocked != Blocked::Not {
+            chat.id.unblock(context).await;
         }
-        chat_id
+        chat.id
     };
 
     match step.as_str() {