diff --git a/src/chat.rs b/src/chat.rs index 8ac946c87..5e9bc6580 100644 --- a/src/chat.rs +++ b/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 { + 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, 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 { + 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); diff --git a/src/dc_receive_imf.rs b/src/dc_receive_imf.rs index ad06afed5..26bde39ef 100644 --- a/src/dc_receive_imf.rs +++ b/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; diff --git a/src/log.rs b/src/log.rs index ce76fc9f6..46b56f60c 100644 --- a/src/log.rs +++ b/src/log.rs @@ -126,7 +126,7 @@ where } } -impl LogExt for Result { +impl LogExt for Result { #[track_caller] fn log_err_inner(self, context: &Context, msg: Option<&str>) -> Result { if let Err(e) = &self { diff --git a/src/message.rs b/src/message.rs index ad6f708da..ed267cf89 100644 --- a/src/message.rs +++ b/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) diff --git a/src/peerstate.rs b/src/peerstate.rs index 17148c81e..2931064a0 100644 --- a/src/peerstate.rs +++ b/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); } diff --git a/src/qr.rs b/src/qr.rs index 6952086ca..bd070cde2 100644 --- a/src/qr.rs +++ b/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 { diff --git a/src/securejoin.rs b/src/securejoin.rs index 6a668440d..dc3c0091e 100644 --- a/src/securejoin.rs +++ b/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() {