Resultify get_chat_id_by_grpid and create_or_lookup_mailinglist

Use `Option` instead of `Error` to indicate that no chat ID is found.
This commit is contained in:
link2xt
2021-08-07 09:20:30 +00:00
parent ac245a6cb2
commit 5a5b80c960
7 changed files with 42 additions and 58 deletions

View File

@@ -2808,10 +2808,10 @@ pub(crate) async fn get_chat_cnt(context: &Context) -> Result<usize> {
pub(crate) async fn get_chat_id_by_grpid( pub(crate) async fn get_chat_id_by_grpid(
context: &Context, context: &Context,
grpid: impl AsRef<str>, grpid: impl AsRef<str>,
) -> Result<(ChatId, bool, Blocked)> { ) -> Result<Option<(ChatId, bool, Blocked)>> {
context context
.sql .sql
.query_row( .query_row_optional(
"SELECT id, blocked, protected FROM chats WHERE grpid=?;", "SELECT id, blocked, protected FROM chats WHERE grpid=?;",
paramsv![grpid.as_ref()], paramsv![grpid.as_ref()],
|row| { |row| {

View File

@@ -1202,7 +1202,8 @@ WHERE type=? AND id IN (
// also unblock mailinglist // also unblock mailinglist
// if the contact is a mailinglist address explicitly created to allow unblocking // if the contact is a mailinglist address explicitly created to allow unblocking
if !new_blocking && contact.origin == Origin::MailinglistAddress { if !new_blocking && contact.origin == Origin::MailinglistAddress {
if let Ok((chat_id, _, _)) = chat::get_chat_id_by_grpid(context, contact.addr).await { if let Some((chat_id, _, _)) = chat::get_chat_id_by_grpid(context, contact.addr).await?
{
chat_id.unblock(context).await?; chat_id.unblock(context).await?;
} }
} }

View File

@@ -532,7 +532,7 @@ async fn add_parts(
list_id, list_id,
mime_parser, mime_parser,
) )
.await .await?
{ {
chat_id = Some(new_chat_id); chat_id = Some(new_chat_id);
chat_id_blocked = new_chat_id_blocked; chat_id_blocked = new_chat_id_blocked;
@@ -548,7 +548,7 @@ async fn add_parts(
sender, sender,
mime_parser, mime_parser,
) )
.await .await?
{ {
chat_id = Some(new_chat_id); chat_id = Some(new_chat_id);
chat_id_blocked = new_chat_id_blocked; chat_id_blocked = new_chat_id_blocked;
@@ -1347,13 +1347,9 @@ async fn create_or_lookup_group(
}; };
// check, if we have a chat with this group ID // check, if we have a chat with this group ID
let mut chat_id = if let Ok((chat_id, _protected, _blocked)) = let mut chat_id = chat::get_chat_id_by_grpid(context, &grpid)
chat::get_chat_id_by_grpid(context, &grpid).await .await?
{ .map(|(chat_id, _protected, _blocked)| chat_id);
Some(chat_id)
} else {
None
};
// For chat messages, we don't have to guess (is_*probably*_private_reply()) but we know for sure that // For chat messages, we don't have to guess (is_*probably*_private_reply()) but we know for sure that
// they belong to the group because of the Chat-Group-Id or Message-Id header // they belong to the group because of the Chat-Group-Id or Message-Id header
@@ -1616,7 +1612,7 @@ async fn create_or_lookup_mailinglist(
allow_creation: bool, allow_creation: bool,
list_id_header: &str, list_id_header: &str,
mime_parser: &MimeMessage, mime_parser: &MimeMessage,
) -> Option<(ChatId, Blocked)> { ) -> Result<Option<(ChatId, Blocked)>> {
static LIST_ID: Lazy<Regex> = Lazy::new(|| Regex::new(r"^(.+)<(.+)>$").unwrap()); static LIST_ID: Lazy<Regex> = Lazy::new(|| Regex::new(r"^(.+)<(.+)>$").unwrap());
let (mut name, listid) = match LIST_ID.captures(list_id_header) { let (mut name, listid) = match LIST_ID.captures(list_id_header) {
Some(cap) => (cap[1].trim().to_string(), cap[2].trim().to_string()), Some(cap) => (cap[1].trim().to_string(), cap[2].trim().to_string()),
@@ -1630,8 +1626,8 @@ async fn create_or_lookup_mailinglist(
), ),
}; };
if let Ok((chat_id, _, blocked)) = chat::get_chat_id_by_grpid(context, &listid).await { if let Some((chat_id, _, blocked)) = chat::get_chat_id_by_grpid(context, &listid).await? {
return Some((chat_id, blocked)); return Ok(Some((chat_id, blocked)));
} }
// for mailchimp lists, the name in `ListId` is just a long number. // for mailchimp lists, the name in `ListId` is just a long number.
@@ -1678,7 +1674,7 @@ async fn create_or_lookup_mailinglist(
if allow_creation { if allow_creation {
// list does not exist but should be created // list does not exist but should be created
match create_multiuser_record( let chat_id = create_multiuser_record(
context, context,
Chattype::Mailinglist, Chattype::Mailinglist,
&listid, &listid,
@@ -1687,25 +1683,18 @@ async fn create_or_lookup_mailinglist(
ProtectionStatus::Unprotected, ProtectionStatus::Unprotected,
) )
.await .await
{ .map_err(|err| {
Ok(chat_id) => { err.context(format!(
chat::add_to_chat_contacts_table(context, chat_id, DC_CONTACT_ID_SELF).await; "Failed to create mailinglist '{}' for grpid={}",
Some((chat_id, Blocked::Request)) &name, &listid
} ))
Err(e) => { })?;
warn!(
context, chat::add_to_chat_contacts_table(context, chat_id, DC_CONTACT_ID_SELF).await;
"Failed to create mailinglist '{}' for grpid={}: {}", Ok(Some((chat_id, Blocked::Request)))
&name,
&listid,
e.to_string()
);
None
}
}
} else { } else {
info!(context, "creating list forbidden by caller"); info!(context, "creating list forbidden by caller");
None Ok(None)
} }
} }

View File

@@ -1636,7 +1636,7 @@ pub(crate) async fn prefetch_should_download(
// deleted from the database or has not arrived yet. // deleted from the database or has not arrived yet.
if let Some(rfc724_mid) = headers.get_header_value(HeaderDef::MessageId) { if let Some(rfc724_mid) = headers.get_header_value(HeaderDef::MessageId) {
if let Some(group_id) = dc_extract_grpid_from_rfc724_mid(&rfc724_mid) { if let Some(group_id) = dc_extract_grpid_from_rfc724_mid(&rfc724_mid) {
if let Ok((_chat_id, _, _)) = get_chat_id_by_grpid(context, group_id).await { if get_chat_id_by_grpid(context, group_id).await?.is_some() {
return Ok(true); return Ok(true);
} }
} }

View File

@@ -324,11 +324,9 @@ pub async fn set_config_from_qr(context: &Context, qr: &str) -> Result<(), Error
let chat_id = if lot.state == LotState::QrReviveVerifyContact { let chat_id = if lot.state == LotState::QrReviveVerifyContact {
None None
} else { } else {
Some( get_chat_id_by_grpid(context, &lot.text2.unwrap_or_default())
get_chat_id_by_grpid(context, &lot.text2.unwrap_or_default()) .await?
.await? .map(|(chat_id, _protected, _blocked)| chat_id)
.0,
)
}; };
token::save( token::save(
context, context,

View File

@@ -3,7 +3,7 @@
use std::convert::TryFrom; use std::convert::TryFrom;
use std::time::{Duration, Instant}; use std::time::{Duration, Instant};
use anyhow::{bail, Context as _, Error, Result}; use anyhow::{anyhow, bail, Context as _, Error, Result};
use async_std::channel::Receiver; use async_std::channel::Receiver;
use async_std::sync::Mutex; use async_std::sync::Mutex;
use percent_encoding::{utf8_percent_encode, AsciiSet, NON_ALPHANUMERIC}; use percent_encoding::{utf8_percent_encode, AsciiSet, NON_ALPHANUMERIC};
@@ -327,14 +327,14 @@ async fn securejoin(context: &Context, qr: &str) -> Result<ChatId, JoinError> {
let start = Instant::now(); let start = Instant::now();
let chatid = loop { let chatid = loop {
{ {
match chat::get_chat_id_by_grpid(context, &group_id).await { match chat::get_chat_id_by_grpid(context, &group_id).await? {
Ok((chatid, _is_protected, _blocked)) => break chatid, Some((chatid, _is_protected, _blocked)) => break chatid,
Err(err) => { None => {
if start.elapsed() > Duration::from_secs(7) { if start.elapsed() > Duration::from_secs(7) {
context.free_ongoing().await; context.free_ongoing().await;
return Err(err return Err(JoinError::Other(anyhow!(
.context("Ongoing sender dropped (this is a bug)") "Ongoing sender dropped (this is a bug)"
.into()); )));
} }
} }
} }
@@ -650,8 +650,8 @@ pub(crate) async fn handle_securejoin_handshake(
return Ok(HandshakeMessage::Ignore); return Ok(HandshakeMessage::Ignore);
} }
}; };
match chat::get_chat_id_by_grpid(context, field_grpid).await { match chat::get_chat_id_by_grpid(context, field_grpid).await? {
Ok((group_chat_id, _, _)) => { Some((group_chat_id, _, _)) => {
if let Err(err) = if let Err(err) =
chat::add_contact_to_chat_ex(context, group_chat_id, contact_id, true) chat::add_contact_to_chat_ex(context, group_chat_id, contact_id, true)
.await .await
@@ -659,12 +659,7 @@ pub(crate) async fn handle_securejoin_handshake(
error!(context, "failed to add contact: {}", err); error!(context, "failed to add contact: {}", err);
} }
} }
Err(err) => { None => bail!("Chat {} not found", &field_grpid),
error!(context, "Chat {} not found: {}", &field_grpid, err);
return Err(
err.context(format!("Chat for group {} not found", &field_grpid))
);
}
} }
} else { } else {
// Alice -> Bob // Alice -> Bob

View File

@@ -12,7 +12,7 @@ use anyhow::{Error, Result};
use async_std::sync::MutexGuard; use async_std::sync::MutexGuard;
use crate::chat::{self, ChatId}; use crate::chat::{self, ChatId};
use crate::constants::{Blocked, Viewtype}; use crate::constants::Viewtype;
use crate::contact::{Contact, Origin}; use crate::contact::{Contact, Origin};
use crate::context::Context; use crate::context::Context;
use crate::events::EventType; use crate::events::EventType;
@@ -336,9 +336,10 @@ impl BobState {
// the very handshake message we're handling now. But // the very handshake message we're handling now. But
// only after we have returned. It does not impact // only after we have returned. It does not impact
// the security invariants of secure-join however. // the security invariants of secure-join however.
let (_, is_verified_group, _) = chat::get_chat_id_by_grpid(context, grpid)
.await let is_verified_group = chat::get_chat_id_by_grpid(context, grpid)
.unwrap_or((ChatId::new(0), false, Blocked::Not)); .await?
.map_or(false, |(_chat_id, is_protected, _blocked)| is_protected);
// when joining a non-verified group // when joining a non-verified group
// the vg-member-added message may be unencrypted // the vg-member-added message may be unencrypted
// when not all group members have keys or prefer encryption. // when not all group members have keys or prefer encryption.