Skip to content

Commit

Permalink
feat: Limit the size of aggregated WebXDC update to 100 KiB (#4825)
Browse files Browse the repository at this point in the history
Before, update sending might be delayed due to rate limits and later merged into large
messages. This is undesirable for apps that want to send large files over WebXDC updates because the
message with aggregated update may be too large for actual sending and hit the provider limit or
require multiple attempts on a flaky SMTP connection.

So, don't aggregate updates if the size of an aggregated update will exceed the limit of 100
KiB. This is a soft limit, so it may be exceeded if a single update is larger and it limits only the
update JSON size, so the message with all envelopes still may be larger. Also don't send any updates
together with the WebXDC instance when resending it to not complicate the code, the only downside is
sending one message more.
  • Loading branch information
iequidoo committed May 21, 2024
1 parent e7d9ff1 commit 7136e48
Show file tree
Hide file tree
Showing 3 changed files with 275 additions and 74 deletions.
56 changes: 49 additions & 7 deletions src/chat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ use crate::tools::{
create_smeared_timestamps, get_abs_path, gm2local_offset, improve_single_line_input,
smeared_time, time, IsNoneOrEmpty, SystemTime,
};
use crate::webxdc::WEBXDC_SUFFIX;
use crate::webxdc::{StatusUpdateSerial, WEBXDC_SUFFIX};

/// An chat item, such as a message or a marker.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
Expand Down Expand Up @@ -2870,7 +2870,7 @@ async fn prepare_send_msg(
);
message::update_msg_state(context, msg.id, MessageState::OutPending).await?;
}
create_send_msg_jobs(context, msg).await
create_send_msg_jobs(context, msg, false).await
}

/// Constructs jobs for sending a message and inserts them into the `smtp` table.
Expand All @@ -2879,9 +2879,15 @@ async fn prepare_send_msg(
/// group with only self and no BCC-to-self configured.
///
/// The caller has to interrupt SMTP loop or otherwise process new rows.
pub(crate) async fn create_send_msg_jobs(context: &Context, msg: &mut Message) -> Result<Vec<i64>> {
pub(crate) async fn create_send_msg_jobs(
context: &Context,
msg: &mut Message,
resend: bool,
) -> Result<Vec<i64>> {
let needs_encryption = msg.param.get_bool(Param::GuaranteeE2ee).unwrap_or_default();
let mimefactory = MimeFactory::from_msg(context, msg).await?;
let mimefactory = MimeFactory::from_msg(context, msg)
.await?
.set_resend(resend);
let attach_selfavatar = mimefactory.attach_selfavatar;
let mut recipients = mimefactory.recipients();

Expand Down Expand Up @@ -4180,7 +4186,10 @@ pub async fn forward_msgs(context: &Context, msg_ids: &[MsgId], chat_id: ChatId)
.prepare_msg_raw(context, &mut msg, None, curr_timestamp)
.await?;
curr_timestamp += 1;
if !create_send_msg_jobs(context, &mut msg).await?.is_empty() {
if !create_send_msg_jobs(context, &mut msg, false)
.await?
.is_empty()
{
context.scheduler.interrupt_smtp().await;
}
}
Expand Down Expand Up @@ -4240,9 +4249,42 @@ pub async fn resend_msgs(context: &Context, msg_ids: &[MsgId]) -> Result<()> {
msg.timestamp_sort = create_smeared_timestamp(context);
// note(treefit): only matters if it is the last message in chat (but probably to expensive to check, debounce also solves it)
chatlist_events::emit_chatlist_item_changed(context, msg.chat_id);
if !create_send_msg_jobs(context, &mut msg).await?.is_empty() {
context.scheduler.interrupt_smtp().await;
let resend = true;
if create_send_msg_jobs(context, &mut msg, resend)
.await?
.is_empty()
{
continue;
}
if msg.viewtype == Viewtype::Webxdc {
let conn_fn = |conn: &mut rusqlite::Connection| {
let range = conn.query_row(
"SELECT IFNULL(min(id), 1), IFNULL(max(id), 0) \
FROM msgs_status_updates WHERE msg_id=?",
(msg.id,),
|row| {
let min_id: StatusUpdateSerial = row.get(0)?;
let max_id: StatusUpdateSerial = row.get(1)?;
Ok((min_id, max_id))
},
)?;
if range.0 > range.1 {
return Ok(());
};
// `first_serial` must be decreased to make parallel running
// `Context::flush_status_updates()` send the updates again.
conn.execute(
"INSERT INTO smtp_status_updates (msg_id, first_serial, last_serial, descr) \
VALUES(?, ?, ?, '') \
ON CONFLICT(msg_id) \
DO UPDATE SET first_serial=min(first_serial - 1, excluded.first_serial)",
(msg.id, range.0, range.1),
)?;
Ok(())
};
context.sql.call_write(conn_fn).await?;
}
context.scheduler.interrupt_smtp().await;
}
Ok(())
}
Expand Down
21 changes: 19 additions & 2 deletions src/mimefactory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use crate::tools::IsNoneOrEmpty;
use crate::tools::{
create_outgoing_rfc724_mid, create_smeared_timestamp, remove_subject_prefix, time,
};
use crate::webxdc::StatusUpdateSerial;
use crate::{location, peer_channels};

// attachments of 25 mb brutto should work on the majority of providers
Expand Down Expand Up @@ -77,6 +78,7 @@ pub struct MimeFactory<'a> {

req_mdn: bool,
last_added_location_id: Option<u32>,
resend: bool,

/// If the created mime-structure contains sync-items,
/// the IDs of these items are listed here.
Expand Down Expand Up @@ -232,6 +234,7 @@ impl<'a> MimeFactory<'a> {
references,
req_mdn,
last_added_location_id: None,
resend: false,
sync_ids_to_delete: None,
attach_selfavatar,
};
Expand Down Expand Up @@ -273,13 +276,21 @@ impl<'a> MimeFactory<'a> {
references: String::default(),
req_mdn: false,
last_added_location_id: None,
resend: false,
sync_ids_to_delete: None,
attach_selfavatar: false,
};

Ok(res)
}

/// Returns `MimeFactory` for resending the message if `resend`. E.g. for a resent WebXDC
/// instance the status updates must not be sent in the same message to not exceeed the size
/// limit.
pub(crate) fn set_resend(self, resend: bool) -> Self {
Self { resend, ..self }
}

async fn peerstates_for_recipients(
&self,
context: &Context,
Expand Down Expand Up @@ -1332,8 +1343,14 @@ impl<'a> MimeFactory<'a> {
headers
.protected
.push(create_iroh_header(context, topic, self.msg.id).await?);
if let Some(json) = context
.render_webxdc_status_update_object(self.msg.id, None)
if self.resend {
} else if let (Some(json), _) = context
.render_webxdc_status_update_object(
self.msg.id,
StatusUpdateSerial::MIN,
StatusUpdateSerial::MAX,
None,
)
.await?
{
parts.push(context.build_status_update_part(&json));
Expand Down

0 comments on commit 7136e48

Please sign in to comment.