Introduce a ContactId newtype

This makes the contact ID its own newtype instead of being a plain
u32.  The change purposefully does not yet try and reap any benefits
from this yet, instead aiming for a boring change that's easy to
review.  Only exception is the ToSql/FromSql as not doing that yet
would also have created churn in the database code and it is easier to
go straight for the right solution here.
This commit is contained in:
Floris Bruynooghe
2022-03-02 22:06:48 +01:00
parent f28fcec81d
commit 438940219e
20 changed files with 372 additions and 261 deletions

View File

@@ -20,7 +20,7 @@ use crate::constants::{
DC_CONTACT_ID_LAST_SPECIAL, DC_CONTACT_ID_SELF, DC_GCM_ADDDAYMARKER, DC_GCM_INFO_ONLY,
DC_RESEND_USER_AVATAR_DAYS,
};
use crate::contact::{addr_cmp, Contact, Origin, VerifiedStatus};
use crate::contact::{addr_cmp, Contact, ContactId, Origin, VerifiedStatus};
use crate::context::Context;
use crate::dc_receive_imf::ReceivedMsg;
use crate::dc_tools::{
@@ -153,7 +153,10 @@ impl ChatId {
/// Returns the [`ChatId`] for the 1:1 chat with `contact_id` if it exists.
///
/// If it does not exist, `None` is returned.
pub async fn lookup_by_contact(context: &Context, contact_id: u32) -> Result<Option<Self>> {
pub async fn lookup_by_contact(
context: &Context,
contact_id: ContactId,
) -> Result<Option<Self>> {
ChatIdBlocked::lookup_by_contact(context, contact_id)
.await
.map(|lookup| lookup.map(|chat| chat.id))
@@ -166,7 +169,7 @@ impl ChatId {
/// This is an internal API, if **a user action** needs to get a chat
/// [`ChatId::create_for_contact`] should be used as this also scales up the
/// [`Contact`]'s origin.
pub async fn get_for_contact(context: &Context, contact_id: u32) -> Result<Self> {
pub async fn get_for_contact(context: &Context, contact_id: ContactId) -> Result<Self> {
ChatIdBlocked::get_for_contact(context, contact_id, Blocked::Not)
.await
.map(|chat| chat.id)
@@ -176,7 +179,7 @@ impl ChatId {
///
/// This should be used when **a user action** creates a chat 1:1, it ensures the chat
/// exists, is unblocked and scales the [`Contact`]'s origin.
pub async fn create_for_contact(context: &Context, contact_id: u32) -> Result<Self> {
pub async fn create_for_contact(context: &Context, contact_id: ContactId) -> Result<Self> {
ChatId::create_for_contact_with_blocked(context, contact_id, Blocked::Not).await
}
@@ -185,7 +188,7 @@ impl ChatId {
/// `create_blocked` won't block already unblocked chats again.
pub(crate) async fn create_for_contact_with_blocked(
context: &Context,
contact_id: u32,
contact_id: ContactId,
create_blocked: Blocked,
) -> Result<Self> {
let chat_id = match ChatIdBlocked::lookup_by_contact(context, contact_id).await? {
@@ -420,7 +423,7 @@ impl ChatId {
context: &Context,
protect: ProtectionStatus,
promote: bool,
from_id: u32,
from_id: ContactId,
) -> Result<()> {
let msg_text = context.stock_protection_msg(protect, from_id).await;
let cmd = match protect {
@@ -1628,7 +1631,11 @@ pub(crate) async fn get_broadcast_icon(context: &Context) -> Result<String> {
Ok(icon)
}
async fn update_special_chat_name(context: &Context, contact_id: u32, name: String) -> Result<()> {
async fn update_special_chat_name(
context: &Context,
contact_id: ContactId,
name: String,
) -> Result<()> {
if let Some(chat_id) = ChatId::lookup_by_contact(context, contact_id).await? {
// the `!= name` condition avoids unneeded writes
context
@@ -1675,9 +1682,15 @@ impl ChatIdBlocked {
/// Searches the database for the 1:1 chat with this contact.
///
/// If no chat is found `None` is returned.
pub async fn lookup_by_contact(context: &Context, contact_id: u32) -> Result<Option<Self>> {
pub async fn lookup_by_contact(
context: &Context,
contact_id: ContactId,
) -> Result<Option<Self>> {
ensure!(context.sql.is_open().await, "Database not available");
ensure!(contact_id > 0, "Invalid contact id requested");
ensure!(
contact_id > ContactId::new(0),
"Invalid contact id requested"
);
context
.sql
@@ -1706,11 +1719,14 @@ impl ChatIdBlocked {
/// state.
pub async fn get_for_contact(
context: &Context,
contact_id: u32,
contact_id: ContactId,
create_blocked: Blocked,
) -> Result<Self> {
ensure!(context.sql.is_open().await, "Database not available");
ensure!(contact_id > 0, "Invalid contact id requested");
ensure!(
contact_id > ContactId::new(0),
"Invalid contact id requested"
);
if let Some(res) = Self::lookup_by_contact(context, contact_id).await? {
// Already exists, no need to create.
@@ -1914,7 +1930,7 @@ async fn prepare_msg_common(
pub async fn is_contact_in_chat(
context: &Context,
chat_id: ChatId,
contact_id: u32,
contact_id: ContactId,
) -> Result<bool> {
// this function works for group and for normal chats, however, it is more useful
// for group chats.
@@ -1925,7 +1941,7 @@ pub async fn is_contact_in_chat(
.sql
.exists(
"SELECT COUNT(*) FROM chats_contacts WHERE chat_id=? AND contact_id=?;",
paramsv![chat_id, contact_id as i32],
paramsv![chat_id, contact_id],
)
.await?;
Ok(exists)
@@ -2227,9 +2243,12 @@ pub async fn get_chat_msgs(
|row: &rusqlite::Row| {
// is_info logic taken from Message.is_info()
let params = row.get::<_, String>("param")?;
let (from_id, to_id) = (row.get::<_, u32>("from_id")?, row.get::<_, u32>("to_id")?);
let is_info_msg: bool = from_id == DC_CONTACT_ID_INFO as u32
|| to_id == DC_CONTACT_ID_INFO as u32
let (from_id, to_id) = (
row.get::<_, ContactId>("from_id")?,
row.get::<_, ContactId>("to_id")?,
);
let is_info_msg: bool = from_id == DC_CONTACT_ID_INFO
|| to_id == DC_CONTACT_ID_INFO
|| match Params::from_str(&params) {
Ok(p) => {
let cmd = p.get_cmd();
@@ -2530,7 +2549,7 @@ pub async fn get_next_media(
}
/// Returns a vector of contact IDs for given chat ID.
pub async fn get_chat_contacts(context: &Context, chat_id: ChatId) -> Result<Vec<u32>> {
pub async fn get_chat_contacts(context: &Context, chat_id: ChatId) -> Result<Vec<ContactId>> {
// Normal chats do not include SELF. Group chats do (as it may happen that one is deleted from a
// groupchat but the chats stays visible, moreover, this makes displaying lists easier)
@@ -2544,7 +2563,7 @@ pub async fn get_chat_contacts(context: &Context, chat_id: ChatId) -> Result<Vec
WHERE cc.chat_id=?
ORDER BY c.id=1, LOWER(c.name||c.addr), c.id;",
paramsv![chat_id],
|row| row.get::<_, u32>(0),
|row| row.get::<_, ContactId>(0),
|ids| ids.collect::<Result<Vec<_>, _>>().map_err(Into::into),
)
.await?;
@@ -2651,13 +2670,13 @@ pub async fn create_broadcast_list(context: &Context) -> Result<ChatId> {
pub(crate) async fn add_to_chat_contacts_table(
context: &Context,
chat_id: ChatId,
contact_id: u32,
contact_id: ContactId,
) -> Result<()> {
context
.sql
.execute(
"INSERT INTO chats_contacts (chat_id, contact_id) VALUES(?, ?)",
paramsv![chat_id, contact_id as i32],
paramsv![chat_id, contact_id],
)
.await?;
Ok(())
@@ -2667,13 +2686,13 @@ pub(crate) async fn add_to_chat_contacts_table(
pub(crate) async fn remove_from_chat_contacts_table(
context: &Context,
chat_id: ChatId,
contact_id: u32,
contact_id: ContactId,
) -> Result<()> {
context
.sql
.execute(
"DELETE FROM chats_contacts WHERE chat_id=? AND contact_id=?",
paramsv![chat_id, contact_id as i32],
paramsv![chat_id, contact_id],
)
.await?;
Ok(())
@@ -2683,7 +2702,7 @@ pub(crate) async fn remove_from_chat_contacts_table(
pub async fn add_contact_to_chat(
context: &Context,
chat_id: ChatId,
contact_id: u32,
contact_id: ContactId,
) -> Result<()> {
add_contact_to_chat_ex(context, chat_id, contact_id, false).await?;
Ok(())
@@ -2692,7 +2711,7 @@ pub async fn add_contact_to_chat(
pub(crate) async fn add_contact_to_chat_ex(
context: &Context,
chat_id: ChatId,
contact_id: u32,
contact_id: ContactId,
from_handshake: bool,
) -> Result<bool> {
ensure!(!chat_id.is_special(), "can not add member to special chats");
@@ -2769,9 +2788,8 @@ pub(crate) async fn add_contact_to_chat_ex(
if chat.typ == Chattype::Group && chat.is_promoted() {
msg.viewtype = Viewtype::Text;
msg.text = Some(
stock_str::msg_add_member(context, contact.get_addr(), DC_CONTACT_ID_SELF as u32).await,
);
msg.text =
Some(stock_str::msg_add_member(context, contact.get_addr(), DC_CONTACT_ID_SELF).await);
msg.param.set_cmd(SystemMessage::MemberAddedToGroup);
msg.param.set(Param::Arg, contact.get_addr());
msg.param.set_int(Param::Arg2, from_handshake.into());
@@ -2874,7 +2892,7 @@ pub async fn set_muted(context: &Context, chat_id: ChatId, duration: MuteDuratio
pub async fn remove_contact_from_chat(
context: &Context,
chat_id: ChatId,
contact_id: u32,
contact_id: ContactId,
) -> Result<()> {
ensure!(
!chat_id.is_special(),
@@ -2903,15 +2921,14 @@ pub async fn remove_contact_from_chat(
msg.viewtype = Viewtype::Text;
if contact.id == DC_CONTACT_ID_SELF {
set_group_explicitly_left(context, &chat.grpid).await?;
msg.text = Some(
stock_str::msg_group_left(context, DC_CONTACT_ID_SELF as u32).await,
);
msg.text =
Some(stock_str::msg_group_left(context, DC_CONTACT_ID_SELF).await);
} else {
msg.text = Some(
stock_str::msg_del_member(
context,
contact.get_addr(),
DC_CONTACT_ID_SELF as u32,
DC_CONTACT_ID_SELF,
)
.await,
);
@@ -3004,13 +3021,8 @@ pub async fn set_chat_name(context: &Context, chat_id: ChatId, new_name: &str) -
if chat.is_promoted() && !chat.is_mailing_list() && chat.typ != Chattype::Broadcast {
msg.viewtype = Viewtype::Text;
msg.text = Some(
stock_str::msg_grp_name(
context,
&chat.name,
&new_name,
DC_CONTACT_ID_SELF as u32,
)
.await,
stock_str::msg_grp_name(context, &chat.name, &new_name, DC_CONTACT_ID_SELF)
.await,
);
msg.param.set_cmd(SystemMessage::GroupNameChanged);
if !chat.name.is_empty() {
@@ -3063,7 +3075,7 @@ pub async fn set_chat_profile_image(
if new_image.as_ref().is_empty() {
chat.param.remove(Param::ProfileImage);
msg.param.remove(Param::Arg);
msg.text = Some(stock_str::msg_grp_img_deleted(context, DC_CONTACT_ID_SELF as u32).await);
msg.text = Some(stock_str::msg_grp_img_deleted(context, DC_CONTACT_ID_SELF).await);
} else {
let mut image_blob = match BlobObject::from_path(context, Path::new(new_image.as_ref())) {
Ok(blob) => Ok(blob),
@@ -3077,7 +3089,7 @@ pub async fn set_chat_profile_image(
image_blob.recode_to_avatar_size(context).await?;
chat.param.set(Param::ProfileImage, image_blob.as_name());
msg.param.set(Param::Arg, image_blob.as_name());
msg.text = Some(stock_str::msg_grp_img_changed(context, DC_CONTACT_ID_SELF as u32).await);
msg.text = Some(stock_str::msg_grp_img_changed(context, DC_CONTACT_ID_SELF).await);
}
chat.update_param(context).await?;
if chat.is_promoted() && !chat.is_mailing_list() {
@@ -3905,7 +3917,7 @@ mod tests {
async fn test_self_talk() -> Result<()> {
let t = TestContext::new_alice().await;
let chat = &t.get_self_chat().await;
assert_eq!(DC_CONTACT_ID_SELF, 1);
assert_eq!(DC_CONTACT_ID_SELF, ContactId::new(1));
assert!(!chat.id.is_special());
assert!(chat.is_self_talk());
assert!(chat.visibility == ChatVisibility::Normal);
@@ -4322,7 +4334,7 @@ mod tests {
let contact1 = Contact::create(&context.ctx, "bob", "bob@mail.de")
.await
.unwrap();
assert_ne!(contact1, 0);
assert_ne!(contact1, ContactId::new(0));
let chat_id = ChatId::create_for_contact(&context.ctx, contact1)
.await
@@ -4527,7 +4539,7 @@ mod tests {
// create contact, then unblocked chat
let contact_id = Contact::create(&ctx, "", "bob@foo.de").await.unwrap();
assert_ne!(contact_id, 0);
assert_ne!(contact_id, ContactId::new(0));
let found = ChatId::lookup_by_contact(&ctx, contact_id).await.unwrap();
assert!(found.is_none());
@@ -4553,10 +4565,14 @@ mod tests {
assert_eq!(chat2.blocked, Blocked::Yes);
// test nonexistent contact
let found = ChatId::lookup_by_contact(&ctx, 1234).await.unwrap();
let found = ChatId::lookup_by_contact(&ctx, ContactId::new(1234))
.await
.unwrap();
assert!(found.is_none());
let found = ChatIdBlocked::lookup_by_contact(&ctx, 1234).await.unwrap();
let found = ChatIdBlocked::lookup_by_contact(&ctx, ContactId::new(1234))
.await
.unwrap();
assert!(found.is_none());
}