Mirrors/chatmail-core
1
0
Fork 0
mirror of https://github.com/chatmail/core.git synced 2026-04-17 21:46:35 +03:00
Code Issues Packages Projects Releases Wiki Activity

feat: Remove Config::SentboxWatch (#7178)

Browse Source 
The motivation is to reduce code complexity, get rid of the extra IMAP connection and cases when
messages are added to chats by Inbox and Sentbox loops in parallel which leads to various message
sorting bugs, particularly to outgoing messages breaking sorting of incoming ones which are fetched
later, but may have a smaller "Date".
This commit is contained in:
iequidoo
2025-09-08 15:23:32 -03:00
committed by iequidoo
parent f428033d95
commit 18445c09c2
13 changed files with 34 additions and 90 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
python/src/deltachat/testplugin.py
View File

@@ -523,7 +523,6 @@ class ACFactory:
assert "addr" in configdict and "mail_pw" in configdict, configdict
configdict.setdefault("bcc_self", False)
configdict.setdefault("mvbox_move", False)
configdict.setdefault("sentbox_watch", False)
configdict.setdefault("sync_msgs", False)
configdict.setdefault("delete_server_after", 0)
ac.update_config(configdict)

14
python/tests/test_1_online.py
View File

@@ -269,19 +269,21 @@ def test_enable_mvbox_move(acfactory, lp):
assert ac2._evtracker.wait_next_incoming_message().text == "message1"
def test_mvbox_sentbox_threads(acfactory, lp):
def test_mvbox_thread_and_sentbox(acfactory, lp):
lp.sec("ac1: start with mvbox thread")
ac1 = acfactory.new_online_configuring_account(mvbox_move=True, sentbox_watch=False)
ac1 = acfactory.new_online_configuring_account(mvbox_move=True)
lp.sec("ac2: start without mvbox/sentbox threads")
ac2 = acfactory.new_online_configuring_account(mvbox_move=False, sentbox_watch=False)
lp.sec("ac2: start without a mvbox thread")
ac2 = acfactory.new_online_configuring_account(mvbox_move=False)
lp.sec("ac2 and ac1: waiting for configuration")
acfactory.bring_accounts_online()
lp.sec("ac1: create and configure sentbox")
lp.sec("ac1: create sentbox")
ac1.direct_imap.create_folder("Sent")
ac1.set_config("sentbox_watch", "1")
ac1.set_config("scan_all_folders_debounce_secs", "0")
ac1.stop_io()
ac1.start_io()
lp.sec("ac1: send message and wait for ac2 to receive it")
acfactory.get_accepted_chat(ac1, ac2).send_text("message1")

2
scripts/update-provider-database.sh
View File

@@ -6,7 +6,7 @@ set -euo pipefail
export TZ=UTC
# Provider database revision.
REV=1cce91c1f1065b47e4f307d6fe2f4cca68c74d2e
REV=d041136c19a48b493823b46d472f12b9ee94ae80
CORE_ROOT="$PWD"
TMP="$(mktemp -d)"

19
src/config.rs
View File

@@ -156,10 +156,6 @@ pub enum Config {
#[strum(props(default = "1"))]
MdnsEnabled,
/// True if "Sent" folder should be watched for changes.
#[strum(props(default = "0"))]
SentboxWatch,
/// True if chat messages should be moved to a separate folder. Auto-sent messages like sync
/// ones are moved there anyway.
#[strum(props(default = "1"))]
@@ -473,10 +469,7 @@ impl Config {
/// Whether the config option needs an IO scheduler restart to take effect.
pub(crate) fn needs_io_restart(&self) -> bool {
matches!(
self,
Config::MvboxMove | Config::OnlyFetchMvbox | Config::SentboxWatch
)
matches!(self, Config::MvboxMove | Config::OnlyFetchMvbox)
}
}
@@ -603,15 +596,6 @@ impl Context {
|| !self.get_config_bool(Config::IsChatmail).await?)
}
/// Returns true if sentbox ("Sent" folder) should be watched.
pub(crate) async fn should_watch_sentbox(&self) -> Result<bool> {
Ok(self.get_config_bool(Config::SentboxWatch).await?
&& self
.get_config(Config::ConfiguredSentboxFolder)
.await?
.is_some())
}
/// Returns true if sync messages should be sent.
pub(crate) async fn should_send_sync_msgs(&self) -> Result<bool> {
Ok(self.get_config_bool(Config::SyncMsgs).await?
@@ -700,7 +684,6 @@ impl Context {
| Config::ProxyEnabled
| Config::BccSelf
| Config::MdnsEnabled
| Config::SentboxWatch
| Config::MvboxMove
| Config::OnlyFetchMvbox
| Config::DeleteToTrash

1
src/configure.rs
View File

@@ -549,7 +549,6 @@ async fn configure(ctx: &Context, param: &EnteredLoginParam) -> Result<Option<&'
true => ctx.get_config_bool(Config::IsChatmail).await?,
};
if is_chatmail {
ctx.set_config(Config::SentboxWatch, None).await?;
ctx.set_config(Config::MvboxMove, Some("0")).await?;
ctx.set_config(Config::OnlyFetchMvbox, None).await?;
ctx.set_config(Config::ShowEmails, None).await?;

8
src/context.rs
View File

@@ -845,7 +845,6 @@ impl Context {
Err(err) => format!("<key failure: {err}>"),
};
let sentbox_watch = self.get_config_int(Config::SentboxWatch).await?;
let mvbox_move = self.get_config_int(Config::MvboxMove).await?;
let only_fetch_mvbox = self.get_config_int(Config::OnlyFetchMvbox).await?;
let folders_configured = self
@@ -949,7 +948,6 @@ impl Context {
.await?
.to_string(),
);
res.insert("sentbox_watch", sentbox_watch.to_string());
res.insert("mvbox_move", mvbox_move.to_string());
res.insert("only_fetch_mvbox", only_fetch_mvbox.to_string());
res.insert(
@@ -1259,12 +1257,6 @@ impl Context {
Ok(inbox.as_deref() == Some(folder_name))
}
/// Returns true if given folder name is the name of the "sent" folder.
pub async fn is_sentbox(&self, folder_name: &str) -> Result<bool> {
let sentbox = self.get_config(Config::ConfiguredSentboxFolder).await?;
Ok(sentbox.as_deref() == Some(folder_name))
}
/// Returns true if given folder name is the name of the "DeltaChat" folder.
pub async fn is_mvbox(&self, folder_name: &str) -> Result<bool> {
let mvbox = self.get_config(Config::ConfiguredMvboxFolder).await?;

6
src/imap.rs
View File

@@ -2044,7 +2044,7 @@ async fn spam_target_folder_cfg(
if needs_move_to_mvbox(context, headers).await?
// If OnlyFetchMvbox is set, we don't want to move the message to
// the inbox or sentbox where we wouldn't fetch it again:
// the inbox where we wouldn't fetch it again:
|| context.get_config_bool(Config::OnlyFetchMvbox).await?
{
Ok(Some(Config::ConfiguredMvboxFolder))
@@ -2568,10 +2568,6 @@ async fn should_ignore_folder(
if !context.get_config_bool(Config::OnlyFetchMvbox).await? {
return Ok(false);
}
if context.is_sentbox(folder).await? {
// Still respect the SentboxWatch setting.
return Ok(!context.get_config_bool(Config::SentboxWatch).await?);
}
Ok(!(context.is_mvbox(folder).await? || folder_meaning == FolderMeaning::Spam))
}

8
src/imap/imap_tests.rs
View File

@@ -183,10 +183,6 @@ const COMBINATIONS_ACCEPTED_CHAT: &[(&str, bool, bool, &str)] = &[
("INBOX", false, true, "INBOX"),
("INBOX", true, false, "INBOX"),
("INBOX", true, true, "DeltaChat"),
("Sent", false, false, "Sent"),
("Sent", false, true, "Sent"),
("Sent", true, false, "Sent"),
("Sent", true, true, "Sent"),
("Spam", false, false, "INBOX"), // Move classical emails in accepted chats from Spam to Inbox, not 100% sure on this, we could also just never move non-chat-msgs
("Spam", false, true, "INBOX"),
("Spam", true, false, "INBOX"), // Move classical emails in accepted chats from Spam to Inbox, not 100% sure on this, we could also just never move non-chat-msgs
@@ -199,10 +195,6 @@ const COMBINATIONS_REQUEST: &[(&str, bool, bool, &str)] = &[
("INBOX", false, true, "INBOX"),
("INBOX", true, false, "INBOX"),
("INBOX", true, true, "DeltaChat"),
("Sent", false, false, "Sent"),
("Sent", false, true, "Sent"),
("Sent", true, false, "Sent"),
("Sent", true, true, "Sent"),
("Spam", false, false, "Spam"),
("Spam", false, true, "INBOX"),
("Spam", true, false, "Spam"),

3
src/imap/scan_folders.rs
View File

@@ -108,9 +108,6 @@ impl Imap {
pub(crate) async fn get_watched_folder_configs(context: &Context) -> Result<Vec<Config>> {
let mut res = vec![Config::ConfiguredInboxFolder];
if context.get_config_bool(Config::SentboxWatch).await? {
res.push(Config::ConfiguredSentboxFolder);
}
if context.should_watch_mvbox().await? {
res.push(Config::ConfiguredMvboxFolder);
}

21
src/provider/data.rs
View File

@@ -510,10 +510,6 @@ static P_FIVE_CHAT: Provider = Provider {
key: Config::BccSelf,
value: "1",
},
ConfigDefault {
key: Config::SentboxWatch,
value: "0",
},
ConfigDefault {
key: Config::MvboxMove,
value: "0",
@@ -1084,10 +1080,6 @@ static P_NAUTA_CU: Provider = Provider {
key: Config::DeleteServerAfter,
value: "1",
},
ConfigDefault {
key: Config::SentboxWatch,
value: "0",
},
ConfigDefault {
key: Config::MvboxMove,
value: "0",
@@ -1629,10 +1621,6 @@ static P_TESTRUN: Provider = Provider {
key: Config::BccSelf,
value: "1",
},
ConfigDefault {
key: Config::SentboxWatch,
value: "0",
},
ConfigDefault {
key: Config::MvboxMove,
value: "0",
@@ -1898,11 +1886,11 @@ static P_WKPB_DE: Provider = Provider {
oauth2_authorizer: None,
};
// yahoo.md: yahoo.com, yahoo.de, yahoo.it, yahoo.fr, yahoo.es, yahoo.se, yahoo.co.uk, yahoo.co.nz, yahoo.com.au, yahoo.com.ar, yahoo.com.br, yahoo.com.mx, ymail.com, rocketmail.com, yahoodns.net
// yahoo.md: yahoo.com, yahoo.de, yahoo.it, yahoo.fr, yahoo.es, yahoo.se, yahoo.co.uk, yahoo.co.nz, yahoo.com.au, yahoo.com.ar, yahoo.com.br, yahoo.com.mx, myyahoo.com, ymail.com, rocketmail.com, yahoodns.net
static P_YAHOO: Provider = Provider {
id: "yahoo",
status: Status::Preparation,
before_login_hint: "To use your Yahoo email address you have to create an \"App-Password\" in the account security screen.",
before_login_hint: "To use your Yahoo email address you have to create an app password in the Yahoo account security screen.",
after_login_hint: "",
overview_page: "https://providers.delta.chat/yahoo",
server: &[
@@ -2041,7 +2029,7 @@ static P_ZOHO: Provider = Provider {
oauth2_authorizer: None,
};
pub(crate) static PROVIDER_DATA: [(&str, &Provider); 533] = [
pub(crate) static PROVIDER_DATA: [(&str, &Provider); 534] = [
("163.com", &P_163),
("aktivix.org", &P_AKTIVIX_ORG),
("aliyun.com", &P_ALIYUN),
@@ -2560,6 +2548,7 @@ pub(crate) static PROVIDER_DATA: [(&str, &Provider); 533] = [
("yahoo.com.ar", &P_YAHOO),
("yahoo.com.br", &P_YAHOO),
("yahoo.com.mx", &P_YAHOO),
("myyahoo.com", &P_YAHOO),
("ymail.com", &P_YAHOO),
("rocketmail.com", &P_YAHOO),
("yahoodns.net", &P_YAHOO),
@@ -2658,4 +2647,4 @@ pub(crate) static PROVIDER_IDS: LazyLock<HashMap<&'static str, &'static Provider
});
pub static _PROVIDER_UPDATED: LazyLock<chrono::NaiveDate> =
LazyLock::new(|| chrono::NaiveDate::from_ymd_opt(2025, 9, 4).unwrap());
LazyLock::new(|| chrono::NaiveDate::from_ymd_opt(2025, 9, 10).unwrap());

32
src/scheduler.rs
View File

@@ -254,7 +254,7 @@ impl SchedulerState {
}
}
/// Interrupt optional boxes (mvbox, sentbox) loops.
/// Interrupt optional boxes (mvbox currently) loops.
pub(crate) async fn interrupt_oboxes(&self) {
let inner = self.inner.read().await;
if let InnerSchedulerState::Started(ref scheduler) = *inner {
@@ -333,7 +333,7 @@ struct SchedBox {
#[derive(Debug)]
pub(crate) struct Scheduler {
inbox: SchedBox,
/// Optional boxes -- mvbox, sentbox.
/// Optional boxes -- mvbox.
oboxes: Vec<SchedBox>,
smtp: SmtpConnectionState,
smtp_handle: task::JoinHandle<()>,
@@ -875,22 +875,18 @@ impl Scheduler {
};
start_recvs.push(inbox_start_recv);
for (meaning, should_watch) in [
(FolderMeaning::Mvbox, ctx.should_watch_mvbox().await),
(FolderMeaning::Sent, ctx.should_watch_sentbox().await),
] {
if should_watch? {
let (conn_state, handlers) = ImapConnectionState::new(ctx).await?;
let (start_send, start_recv) = oneshot::channel();
let ctx = ctx.clone();
let handle = task::spawn(simple_imap_loop(ctx, start_send, handlers, meaning));
oboxes.push(SchedBox {
meaning,
conn_state,
handle,
});
start_recvs.push(start_recv);
}
if ctx.should_watch_mvbox().await? {
let (conn_state, handlers) = ImapConnectionState::new(ctx).await?;
let (start_send, start_recv) = oneshot::channel();
let ctx = ctx.clone();
let meaning = FolderMeaning::Mvbox;
let handle = task::spawn(simple_imap_loop(ctx, start_send, handlers, meaning));
oboxes.push(SchedBox {
meaning,
conn_state,
handle,
});
start_recvs.push(start_recv);
}
let smtp_handle = {

3
src/scheduler/connectivity.rs
View File

@@ -90,7 +90,7 @@ impl DetailedConnectivity {
DetailedConnectivity::Preparing => Some(Connectivity::Working),
// Just don't return a connectivity, probably the folder is configured not to be
// watched or there is e.g. no "Sent" folder, so we are not interested in it
// watched, so we are not interested in it.
DetailedConnectivity::NotConfigured => None,
DetailedConnectivity::Idle => Some(Connectivity::Connected),
@@ -378,7 +378,6 @@ impl Context {
// Add e.g.
// Incoming messages
// - "Inbox": Connected
// - "Sent": Connected
// =============================================================================================
let watched_folders = get_watched_folder_configs(self).await?;

6
src/tests/verified_chats.rs
View File

@@ -207,8 +207,8 @@ async fn test_degrade_verified_oneonone_chat() -> Result<()> {
Ok(())
}
/// Alice is offline for some time.
/// When she comes online, first her inbox is synced and then her sentbox.
/// Alice is offline for some time. mvbox_move is 0.
/// When she comes online, first her inbox is synced and then her mvbox.
/// This test tests that the messages are still in the right order.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_old_message_4() -> Result<()> {
@@ -246,7 +246,7 @@ async fn test_old_message_4() -> Result<()> {
}
/// Alice is offline for some time.
/// When they come online, first their sentbox is synced and then their inbox.
/// When they come online, first their mvbox is synced and then their inbox.
/// This test tests that the messages are still in the right order.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_old_message_5() -> Result<()> {