mirror of
https://github.com/chatmail/core.git
synced 2026-04-29 11:26:29 +03:00
fix: Change BccSelf default to 0 for chatmail (#6340)
Change `BccSelf` default to 0 for chatmail configurations and enable it upon a backup export. As for `DeleteServerAfter` who was set to 0 upon a backup export before, make its default dependent on `BccSelf` for chatmail. We don't need `BccSelf` for chatmail by default because we assume single-device use. Also `BccSelf` is needed for "classic" email accounts even if `DeleteServerAfter` is set to "immediately" to detect that a message was sent if SMTP server is slow to respond and connection is lost before receiving the status line which isn't a problem for chatmail servers.
This commit is contained in:
@@ -2943,7 +2943,8 @@ pub(crate) async fn create_send_msg_jobs(context: &Context, msg: &mut Message) -
|
||||
// because BCC-self messages are also used to detect
|
||||
// that message was sent if SMTP server is slow to respond
|
||||
// and connection is frequently lost
|
||||
// before receiving status line.
|
||||
// before receiving status line. NB: This is not a problem for chatmail servers, so `BccSelf`
|
||||
// disabled by default is fine.
|
||||
//
|
||||
// `from` must be the last addr, see `receive_imf_inner()` why.
|
||||
if context.get_config_bool(Config::BccSelf).await?
|
||||
|
||||
@@ -143,7 +143,7 @@ pub enum Config {
|
||||
/// Send BCC copy to self.
|
||||
///
|
||||
/// Should be enabled for multidevice setups.
|
||||
#[strum(props(default = "1"))]
|
||||
/// Default is 0 for chatmail accounts before a backup export, 1 otherwise.
|
||||
BccSelf,
|
||||
|
||||
/// True if encryption is preferred according to Autocrypt standard.
|
||||
@@ -202,7 +202,7 @@ pub enum Config {
|
||||
/// Value 1 is treated as "delete at once": messages are deleted
|
||||
/// immediately, without moving to DeltaChat folder.
|
||||
///
|
||||
/// Default is 1 for chatmail accounts before a backup export, 0 otherwise.
|
||||
/// Default is 1 for chatmail accounts without `BccSelf`, 0 otherwise.
|
||||
DeleteServerAfter,
|
||||
|
||||
/// Timer in seconds after which the message is deleted from the
|
||||
@@ -519,11 +519,19 @@ impl Context {
|
||||
|
||||
// Default values
|
||||
let val = match key {
|
||||
Config::ConfiguredInboxFolder => Some("INBOX"),
|
||||
Config::DeleteServerAfter => match Box::pin(self.is_chatmail()).await? {
|
||||
false => Some("0"),
|
||||
true => Some("1"),
|
||||
Config::BccSelf => match Box::pin(self.is_chatmail()).await? {
|
||||
false => Some("1"),
|
||||
true => Some("0"),
|
||||
},
|
||||
Config::ConfiguredInboxFolder => Some("INBOX"),
|
||||
Config::DeleteServerAfter => {
|
||||
match !Box::pin(self.get_config_bool(Config::BccSelf)).await?
|
||||
&& Box::pin(self.is_chatmail()).await?
|
||||
{
|
||||
true => Some("1"),
|
||||
false => Some("0"),
|
||||
}
|
||||
}
|
||||
_ => key.get_str("default"),
|
||||
};
|
||||
Ok(val.map(|s| s.to_string()))
|
||||
@@ -1105,6 +1113,28 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn test_delete_server_after_default() -> Result<()> {
|
||||
let t = &TestContext::new_alice().await;
|
||||
|
||||
// Check that the settings are displayed correctly.
|
||||
assert_eq!(t.get_config(Config::BccSelf).await?, Some("1".to_string()));
|
||||
assert_eq!(
|
||||
t.get_config(Config::DeleteServerAfter).await?,
|
||||
Some("0".to_string())
|
||||
);
|
||||
|
||||
// Leaving emails on the server even w/o `BccSelf` is a good default at least because other
|
||||
// MUAs do so even if the server doesn't save sent messages to some sentbox (like Gmail
|
||||
// does).
|
||||
t.set_config_bool(Config::BccSelf, false).await?;
|
||||
assert_eq!(
|
||||
t.get_config(Config::DeleteServerAfter).await?,
|
||||
Some("0".to_string())
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn test_sync() -> Result<()> {
|
||||
let alice0 = TestContext::new_alice().await;
|
||||
|
||||
35
src/imex.rs
35
src/imex.rs
@@ -416,7 +416,7 @@ async fn import_backup_stream_inner<R: tokio::io::AsyncRead + Unpin>(
|
||||
.context("cannot import unpacked database");
|
||||
}
|
||||
if res.is_ok() {
|
||||
res = adjust_delete_server_after(context).await;
|
||||
res = adjust_bcc_self(context).await;
|
||||
}
|
||||
fs::remove_file(unpacked_database)
|
||||
.await
|
||||
@@ -796,7 +796,7 @@ async fn export_database(
|
||||
.to_str()
|
||||
.with_context(|| format!("path {} is not valid unicode", dest.display()))?;
|
||||
|
||||
adjust_delete_server_after(context).await?;
|
||||
adjust_bcc_self(context).await?;
|
||||
context
|
||||
.sql
|
||||
.set_raw_config_int("backup_time", timestamp)
|
||||
@@ -826,15 +826,14 @@ async fn export_database(
|
||||
.await
|
||||
}
|
||||
|
||||
/// Sets `Config::DeleteServerAfter` to "never" if needed so that new messages are present on the
|
||||
/// server after a backup restoration or available for all devices in multi-device case.
|
||||
/// NB: Calling this after a backup import isn't reliable as we can crash in between, but this is a
|
||||
/// problem only for old backups, new backups already have `DeleteServerAfter` set if necessary.
|
||||
async fn adjust_delete_server_after(context: &Context) -> Result<()> {
|
||||
if context.is_chatmail().await? && !context.config_exists(Config::DeleteServerAfter).await? {
|
||||
context
|
||||
.set_config(Config::DeleteServerAfter, Some("0"))
|
||||
.await?;
|
||||
/// Sets `Config::BccSelf` (and `DeleteServerAfter` to "never" in effect) if needed so that new
|
||||
/// messages are present on the server after a backup restoration or available for all devices in
|
||||
/// multi-device case. NB: Calling this after a backup import isn't reliable as we can crash in
|
||||
/// between, but this is a problem only for old backups, new backups already have `BccSelf` set if
|
||||
/// necessary.
|
||||
async fn adjust_bcc_self(context: &Context) -> Result<()> {
|
||||
if context.is_chatmail().await? && !context.config_exists(Config::BccSelf).await? {
|
||||
context.set_config(Config::BccSelf, Some("1")).await?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
@@ -1030,12 +1029,20 @@ mod tests {
|
||||
|
||||
let context1 = &TestContext::new_alice().await;
|
||||
|
||||
// Check that the setting is displayed correctly.
|
||||
// Check that the settings are displayed correctly.
|
||||
assert_eq!(
|
||||
context1.get_config(Config::BccSelf).await?,
|
||||
Some("1".to_string())
|
||||
);
|
||||
assert_eq!(
|
||||
context1.get_config(Config::DeleteServerAfter).await?,
|
||||
Some("0".to_string())
|
||||
);
|
||||
context1.set_config_bool(Config::IsChatmail, true).await?;
|
||||
assert_eq!(
|
||||
context1.get_config(Config::BccSelf).await?,
|
||||
Some("0".to_string())
|
||||
);
|
||||
assert_eq!(
|
||||
context1.get_config(Config::DeleteServerAfter).await?,
|
||||
Some("1".to_string())
|
||||
@@ -1058,6 +1065,10 @@ mod tests {
|
||||
assert!(context2.is_configured().await?);
|
||||
assert!(context2.is_chatmail().await?);
|
||||
for ctx in [context1, context2] {
|
||||
assert_eq!(
|
||||
ctx.get_config(Config::BccSelf).await?,
|
||||
Some("1".to_string())
|
||||
);
|
||||
assert_eq!(
|
||||
ctx.get_config(Config::DeleteServerAfter).await?,
|
||||
Some("0".to_string())
|
||||
|
||||
@@ -1121,6 +1121,23 @@ CREATE INDEX msgs_status_updates_index2 ON msgs_status_updates (uid);
|
||||
.await?;
|
||||
}
|
||||
|
||||
inc_and_check(&mut migration_version, 127)?;
|
||||
if dbversion < migration_version {
|
||||
// Existing chatmail configurations having `delete_server_after` disabled should get
|
||||
// `bcc_self` enabled, they may be multidevice configurations because before,
|
||||
// `delete_server_after` was set to 0 upon a backup export for them, but together with this
|
||||
// migration `bcc_self` is enabled instead (whose default is changed to 0 for chatmail). We
|
||||
// don't check `is_chatmail` for simplicity.
|
||||
sql.execute_migration(
|
||||
"INSERT OR IGNORE INTO config (keyname, value)
|
||||
SELECT 'bcc_self', '1'
|
||||
FROM config WHERE keyname='delete_server_after' AND value='0'
|
||||
",
|
||||
migration_version,
|
||||
)
|
||||
.await?;
|
||||
}
|
||||
|
||||
let new_version = sql
|
||||
.get_raw_config_int(VERSION_CFG)
|
||||
.await?
|
||||
|
||||
Reference in New Issue
Block a user