mirror of
https://github.com/chatmail/core.git
synced 2026-05-08 17:36:29 +03:00
fix: Mark a message as delivered only after it has been fully sent out (#8062)
Fix https://github.com/chatmail/core/issues/8042 The problem was that after receiving the bcc_self'ed pre-message in `receive_imf`, the logic there only looked for a pending `smtp`-table-entry that matches the rfc724_mid, and if there was none then it thought "Great, apparently the message is fully sent out, we can mark it as delivered!". But with pre-messages, the same message can have two `smtp` entries (one for the pre-message and one for the post-message), and the message should only be marked as delivered once both of them are sent out. Now, I changed the logic to look for all entries with the same msg_id. This is actually the same SQL query used in smtp.rs, so, I extracted it into a new function; feel free to suggest a better name for it. I tested on Android that it now works fine. I'll add a test in a follow-up PR. There are a lot of other problems with sending large files, though: - The pre-message is sent before the post-message, so that for the receiver it looks as if the message arrived, but stays in "downloading..." forever - There is quite a time delay between clicking on "Send" and the outgoing message appearing in the chat - The message shortly gets a letter icon right after it is sent - I'm wondering if there is a way to give feedback to the user immediately if the message is too big - It's unclear when exactly we want to send read receipts I'll open a follow-up issue for these.
This commit is contained in:
@@ -45,6 +45,7 @@ use crate::securejoin::{
|
|||||||
self, get_secure_join_step, handle_securejoin_handshake, observe_securejoin_on_other_device,
|
self, get_secure_join_step, handle_securejoin_handshake, observe_securejoin_on_other_device,
|
||||||
};
|
};
|
||||||
use crate::simplify;
|
use crate::simplify;
|
||||||
|
use crate::smtp::msg_has_pending_smtp_job;
|
||||||
use crate::stats::STATISTICS_BOT_EMAIL;
|
use crate::stats::STATISTICS_BOT_EMAIL;
|
||||||
use crate::stock_str;
|
use crate::stock_str;
|
||||||
use crate::sync::Sync::*;
|
use crate::sync::Sync::*;
|
||||||
@@ -582,14 +583,7 @@ pub(crate) async fn receive_imf_inner(
|
|||||||
(rfc724_mid_orig, &self_addr),
|
(rfc724_mid_orig, &self_addr),
|
||||||
)
|
)
|
||||||
.await?;
|
.await?;
|
||||||
if !context
|
if !msg_has_pending_smtp_job(context, msg_id).await? {
|
||||||
.sql
|
|
||||||
.exists(
|
|
||||||
"SELECT COUNT(*) FROM smtp WHERE rfc724_mid=?",
|
|
||||||
(rfc724_mid_orig,),
|
|
||||||
)
|
|
||||||
.await?
|
|
||||||
{
|
|
||||||
msg_id.set_delivered(context).await?;
|
msg_id.set_delivered(context).await?;
|
||||||
}
|
}
|
||||||
return Ok(None);
|
return Ok(None);
|
||||||
|
|||||||
16
src/smtp.rs
16
src/smtp.rs
@@ -465,11 +465,7 @@ pub(crate) async fn send_msg_to_smtp(
|
|||||||
match status {
|
match status {
|
||||||
SendResult::Retry => Err(format_err!("Retry")),
|
SendResult::Retry => Err(format_err!("Retry")),
|
||||||
SendResult::Success => {
|
SendResult::Success => {
|
||||||
if !context
|
if !msg_has_pending_smtp_job(context, msg_id).await? {
|
||||||
.sql
|
|
||||||
.exists("SELECT COUNT(*) FROM smtp WHERE msg_id=?", (msg_id,))
|
|
||||||
.await?
|
|
||||||
{
|
|
||||||
msg_id.set_delivered(context).await?;
|
msg_id.set_delivered(context).await?;
|
||||||
}
|
}
|
||||||
Ok(())
|
Ok(())
|
||||||
@@ -478,6 +474,16 @@ pub(crate) async fn send_msg_to_smtp(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub(crate) async fn msg_has_pending_smtp_job(
|
||||||
|
context: &Context,
|
||||||
|
msg_id: MsgId,
|
||||||
|
) -> Result<bool, Error> {
|
||||||
|
context
|
||||||
|
.sql
|
||||||
|
.exists("SELECT COUNT(*) FROM smtp WHERE msg_id=?", (msg_id,))
|
||||||
|
.await
|
||||||
|
}
|
||||||
|
|
||||||
/// Attempts to send queued MDNs.
|
/// Attempts to send queued MDNs.
|
||||||
async fn send_mdns(context: &Context, connection: &mut Smtp) -> Result<()> {
|
async fn send_mdns(context: &Context, connection: &mut Smtp) -> Result<()> {
|
||||||
loop {
|
loop {
|
||||||
|
|||||||
@@ -40,6 +40,7 @@ use crate::message::{Message, MessageState, MsgId, update_msg_state};
|
|||||||
use crate::mimeparser::{MimeMessage, SystemMessage};
|
use crate::mimeparser::{MimeMessage, SystemMessage};
|
||||||
use crate::receive_imf::receive_imf;
|
use crate::receive_imf::receive_imf;
|
||||||
use crate::securejoin::{get_securejoin_qr, join_securejoin};
|
use crate::securejoin::{get_securejoin_qr, join_securejoin};
|
||||||
|
use crate::smtp::msg_has_pending_smtp_job;
|
||||||
use crate::stock_str::StockStrings;
|
use crate::stock_str::StockStrings;
|
||||||
use crate::tools::time;
|
use crate::tools::time;
|
||||||
|
|
||||||
@@ -658,10 +659,7 @@ impl TestContext {
|
|||||||
.execute("DELETE FROM smtp WHERE id=?;", (rowid,))
|
.execute("DELETE FROM smtp WHERE id=?;", (rowid,))
|
||||||
.await
|
.await
|
||||||
.expect("failed to remove job");
|
.expect("failed to remove job");
|
||||||
if !self
|
if !msg_has_pending_smtp_job(self, msg_id)
|
||||||
.ctx
|
|
||||||
.sql
|
|
||||||
.exists("SELECT COUNT(*) FROM smtp WHERE msg_id=?", (msg_id,))
|
|
||||||
.await
|
.await
|
||||||
.expect("Failed to check for more jobs")
|
.expect("Failed to check for more jobs")
|
||||||
{
|
{
|
||||||
|
|||||||
Reference in New Issue
Block a user