Remove delete_server_after code. Rust tests are passing

This commit is contained in:
Hocuri
2026-05-05 22:40:41 +02:00
parent 741d1beed8
commit 0b4d8f5fee
18 changed files with 132 additions and 219 deletions

View File

@@ -413,11 +413,6 @@ char* dc_get_blobdir (const dc_context_t* context);
* Messages in the "saved messages" chat (see dc_chat_is_self_talk()) are skipped.
* Messages are deleted whether they were seen or not, the UI should clearly point that out.
* See also dc_estimate_deletion_cnt().
* - `delete_server_after` = 0=do not delete messages from server automatically (default),
* 1=delete messages directly after receiving from server, mvbox is skipped.
* >1=seconds, after which messages are deleted automatically from the server, mvbox is used as defined.
* "Saved messages" are deleted from the server as well as emails, the UI should clearly point that out.
* See also dc_estimate_deletion_cnt().
* - `media_quality` = DC_MEDIA_QUALITY_BALANCED (0) =
* good outgoing images/videos/voice quality at reasonable sizes (default)
* DC_MEDIA_QUALITY_WORSE (1)
@@ -1461,13 +1456,13 @@ dc_chatlist_t* dc_get_similar_chatlist (dc_context_t* context, uint32_t ch
/**
* Estimate the number of messages that will be deleted
* by the dc_set_config()-options `delete_device_after` or `delete_server_after`.
* by the dc_set_config()-option `delete_device_after`.
* This is typically used to show the estimated impact to the user
* before actually enabling deletion of old messages.
*
* @memberof dc_context_t
* @param context The context object as returned from dc_context_new().
* @param from_server 1=Estimate deletion count for server, 0=Estimate deletion count for device
* @param from_server Deprecated, pass 0 here
* @param seconds Count messages older than the given number of seconds.
* @return Number of messages that are older than the given number of seconds.
* Messages in the "saved messages" folder are not counted as they will not be deleted automatically.

View File

@@ -735,10 +735,19 @@ impl CommandApi {
Ok(msg_ids)
}
/// Estimate the number of messages that will be deleted
/// by the set_config()-options `delete_device_after` or `delete_server_after`.
/// Estimates the number of messages that will be deleted
/// by the `set_config()`-option `delete_device_after`.
///
/// This is typically used to show the estimated impact to the user
/// before actually enabling deletion of old messages.
///
/// Messages in the "saved messages" folder are not counted as they will not be deleted automatically.
///
/// Parameters:
/// - `from_server`: Deprecated, pass `false` here
/// - `seconds`: Count messages older than the given number of seconds.
///
/// Returns the number of messages that are older than the given number of seconds.
async fn estimate_auto_deletion_count(
&self,
account_id: u32,

View File

@@ -17,7 +17,7 @@ def test_moved_markseen(acfactory, direct_imap, log):
log.section("ac2: creating DeltaChat folder")
ac2_direct_imap = direct_imap(ac2)
ac2_direct_imap.create_folder("DeltaChat")
ac2.set_config("delete_server_after", "0")
# ac2.set_config("delete_server_after", "0") # TODO check if this causes a test failure
ac2.set_config("sync_msgs", "0") # Do not send a sync message when accepting a contact request.
ac2.add_or_update_transport({"addr": addr, "password": password, "imapFolder": "DeltaChat"})
@@ -58,7 +58,9 @@ def test_markseen_message_and_mdn(acfactory, direct_imap):
ac1, ac2 = acfactory.get_online_accounts(2)
for ac in ac1, ac2:
ac.set_config("delete_server_after", "0")
# TODO check if this causes a test failure
# ac.set_config("delete_server_after", "0")
pass
# Do not send BCC to self, we only want to test MDN on ac1.
ac1.set_config("bcc_self", "0")
@@ -91,7 +93,7 @@ def test_trash_multiple_messages(acfactory, direct_imap, log):
ac1, ac2 = acfactory.get_online_accounts(2)
ac2.stop_io()
ac2.set_config("delete_server_after", "0")
# TODO check if this causes a test failure: ac2.set_config("delete_server_after", "0")
ac2.set_config("sync_msgs", "0")
ac2.start_io()

View File

@@ -4,41 +4,6 @@ from deltachat_rpc_client import EventType
from deltachat_rpc_client.const import MessageState
def test_bcc_self_delete_server_after_defaults(acfactory):
"""Test default values for bcc_self and delete_server_after."""
ac = acfactory.get_online_account()
# Initially after getting online
# the setting bcc_self is set to 0 because there is only one device
# and delete_server_after is "1", meaning immediate deletion.
assert ac.get_config("bcc_self") == "0"
assert ac.get_config("delete_server_after") == "1"
# Setup a second device.
ac_clone = ac.clone()
ac_clone.bring_online()
# Second device setup
# enables bcc_self and changes default delete_server_after.
assert ac.get_config("bcc_self") == "1"
assert ac.get_config("delete_server_after") == "0"
assert ac_clone.get_config("bcc_self") == "1"
assert ac_clone.get_config("delete_server_after") == "0"
# Manually disabling bcc_self
# also restores the default for delete_server_after.
ac.set_config("bcc_self", "0")
assert ac.get_config("bcc_self") == "0"
assert ac.get_config("delete_server_after") == "1"
# Cloning the account again enables bcc_self
# even though it was manually disabled.
ac_clone = ac.clone()
assert ac.get_config("bcc_self") == "1"
assert ac.get_config("delete_server_after") == "0"
def test_one_account_send_bcc_setting(acfactory, log, direct_imap):
ac1, ac2 = acfactory.get_online_accounts(2)
ac1_clone = ac1.clone()

View File

@@ -1235,7 +1235,7 @@ def test_immediate_autodelete(acfactory, direct_imap, log):
ac1, ac2 = acfactory.get_online_accounts(2)
# "1" means delete immediately, while "0" means do not delete
ac2.set_config("delete_server_after", "1")
# TODO check if this causes a test failure: ac2.set_config("delete_server_after", "1")
log.section("ac1: create chat with ac2")
chat1 = ac1.create_chat(ac2)

View File

@@ -521,7 +521,6 @@ class ACFactory:
assert "addr" in configdict and "mail_pw" in configdict, configdict
configdict.setdefault("bcc_self", False)
configdict.setdefault("sync_msgs", False)
configdict.setdefault("delete_server_after", 0)
ac.update_config(configdict)
self._acsetup._account2config[ac] = configdict
self._preconfigure_key(ac)

View File

@@ -314,7 +314,7 @@ def test_verified_group_vs_delete_server_after(acfactory, tmp_path, lp):
ac2_offl = acfactory.new_online_configuring_account(cloned_from=ac2)
for ac in [ac2, ac2_offl]:
ac.set_config("bcc_self", "1")
ac2.set_config("delete_server_after", "1")
# TODO check if this causes a test failure: ac2.set_config("delete_server_after", "1")
ac2.set_config("gossip_period", "0") # Re-gossip in every message
acfactory.bring_accounts_online()
dir = tmp_path / "exportdir"
@@ -337,7 +337,7 @@ def test_verified_group_vs_delete_server_after(acfactory, tmp_path, lp):
ac2._evtracker.get_matching("DC_EVENT_IMAP_MESSAGE_DELETED")
lp.sec("ac1: sending 'hi' to the group")
ac2.set_config("delete_server_after", "0")
# TODO check if this causes a test failure: ac2.set_config("delete_server_after", "0")
chat1.send_text("hi")
lp.sec("ac2_offl: going online, checking the 'hi' message")

View File

@@ -194,17 +194,6 @@ pub enum Config {
#[strum(props(default = "0"))] // also change MediaQuality.default() on changes
MediaQuality,
/// Timer in seconds after which the message is deleted from the
/// server.
///
/// 0 means messages are never deleted by Delta Chat.
///
/// Value 1 is treated as "delete at once": messages are deleted
/// immediately, without moving to DeltaChat folder.
///
/// Default is 1 for chatmail accounts without `BccSelf`, 0 otherwise.
DeleteServerAfter,
/// Timer in seconds after which the message is deleted from the
/// device.
///
@@ -554,14 +543,6 @@ impl Context {
// Default values
let val = match key {
Config::ConfiguredInboxFolder => Some("INBOX".to_string()),
Config::DeleteServerAfter => {
match !Box::pin(self.get_config_bool(Config::BccSelf)).await?
&& Box::pin(self.is_chatmail()).await?
{
true => Some("1".to_string()),
false => Some("0".to_string()),
}
}
Config::Addr => self.get_config_opt(Config::ConfiguredAddr).await?,
_ => key.get_str("default").map(|s| s.to_string()),
};
@@ -642,19 +623,15 @@ impl Context {
self.get_config_bool(Config::MdnsEnabled).await
}
/// Gets configured "delete_server_after" value.
/// Gets configured "delete_server_after"
///
/// `None` means never delete the message, `Some(0)` means delete
/// at once, `Some(x)` means delete after `x` seconds.
/// at once, `Some(x)` is never returned
// TODO rename and refactor
pub async fn get_config_delete_server_after(&self) -> Result<Option<i64>> {
let val = match self
.get_config_parsed::<i64>(Config::DeleteServerAfter)
.await?
.unwrap_or(0)
{
0 => None,
1 => Some(0),
x => Some(x),
let val = match !self.get_config_bool(Config::BccSelf).await? && self.is_chatmail().await? {
true => Some(0),
false => None,
};
Ok(val)
}

View File

@@ -142,28 +142,6 @@ async fn test_mdns_default_behaviour() -> Result<()> {
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(())
}
const SAVED_MESSAGES_DEDUPLICATED_FILE: &str = "969142cb84015bc135767bc2370934a.png";
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]

View File

@@ -973,12 +973,7 @@ impl Context {
.await?
.to_string(),
);
res.insert(
"delete_server_after",
self.get_config_int(Config::DeleteServerAfter)
.await?
.to_string(),
);
res.insert(
"last_housekeeping",
self.get_config_int(Config::LastHousekeeping)

View File

@@ -15,12 +15,6 @@ use crate::{EventType, chatlist_events};
pub(crate) mod post_msg_metadata;
pub(crate) use post_msg_metadata::PostMsgMetadata;
/// If a message is downloaded only partially
/// and `delete_server_after` is set to small timeouts (eg. "at once"),
/// the user might have no chance to actually download that message.
/// `MIN_DELETE_SERVER_AFTER` increases the timeout in this case.
pub(crate) const MIN_DELETE_SERVER_AFTER: i64 = 48 * 60 * 60;
/// From this point onward outgoing messages are considered large
/// and get a Pre-Message, which announces the Post-Message.
/// This is only about sending so we can modify it any time.

View File

@@ -23,16 +23,15 @@
//! ## Device settings
//!
//! In addition to per-chat ephemeral message setting, each device has
//! two global user-configured settings that complement per-chat
//! settings: `delete_device_after` and `delete_server_after`. These
//! settings are not synchronized among devices and apply to all
//! a global user-configured setting that complements per-chat
//! settings, `delete_device_after`.
//! This setting is not synchronized among devices and applies to all
//! messages known to the device, including messages sent or received
//! before configuring the setting.
//! It deletes messages only from the device, not from the server.
//!
//! `delete_device_after` configures the maximum time device is
//! storing the messages locally. `delete_server_after` configures the
//! time after which device will delete the messages it knows about
//! from the server.
//! storing the messages locally.
//!
//! ## How messages are deleted
//!
@@ -60,7 +59,7 @@
//!
//! Server deletion happens by updating the `imap` table based on
//! the database entries which are expired either according to their
//! ephemeral message timers or global `delete_server_after` setting.
//! ephemeral message timers.
use std::cmp::max;
use std::collections::BTreeSet;
@@ -78,7 +77,6 @@ use crate::chat::{ChatId, ChatIdBlocked, send_msg};
use crate::constants::{DC_CHAT_ID_LAST_SPECIAL, DC_CHAT_ID_TRASH};
use crate::contact::ContactId;
use crate::context::Context;
use crate::download::MIN_DELETE_SERVER_AFTER;
use crate::events::EventType;
use crate::log::{LogExt, warn};
use crate::message::{Message, MessageState, MsgId, Viewtype};
@@ -654,6 +652,8 @@ pub(crate) async fn ephemeral_loop(context: &Context, interrupt_receiver: Receiv
#[expect(clippy::arithmetic_side_effects)]
pub(crate) async fn delete_expired_imap_messages(context: &Context) -> Result<()> {
let now = time();
// TODO if is_chatmail, but not bcc_self, then delete after downloading
// apart from this, we may be able to remove the delete_server_after part
let (threshold_timestamp, threshold_timestamp_extended) =
match context.get_config_delete_server_after().await? {
@@ -664,7 +664,7 @@ pub(crate) async fn delete_expired_imap_messages(context: &Context) -> Result<()
0 => i64::MAX,
_ => now - delete_server_after,
},
now - max(delete_server_after, MIN_DELETE_SERVER_AFTER),
now - max(delete_server_after, 48 * 60 * 60),
),
};

View File

@@ -455,7 +455,6 @@ async fn test_delete_expired_imap_messages() -> Result<()> {
let uidvalidity = 12345;
for (id, timestamp, ephemeral_timestamp) in &[
(900, now - 2 * HOUR, 0),
(1000, now - 23 * HOUR - MIN_DELETE_SERVER_AFTER, 0),
(1010, now - 23 * HOUR, 0),
(1020, now - 21 * HOUR, 0),
(1030, now - 19 * HOUR, 0),
@@ -512,28 +511,29 @@ async fn test_delete_expired_imap_messages() -> Result<()> {
0
);
t.set_config(Config::DeleteServerAfter, Some(&*(25 * HOUR).to_string()))
.await?;
delete_expired_imap_messages(&t).await?;
test_marked_for_deletion(&t, 1000).await?;
// t.set_config(Config::DeleteServerAfter, Some(&*(25 * HOUR).to_string()))
// .await?;
// delete_expired_imap_messages(&t).await?;
// test_marked_for_deletion(&t, 1000).await?;
MsgId::new(1000)
.update_download_state(&t, DownloadState::Available)
.await?;
t.sql
.execute("UPDATE imap SET target=folder WHERE rfc724_mid='1000'", ())
.await?;
delete_expired_imap_messages(&t).await?;
test_marked_for_deletion(&t, 1000).await?; // Delete downloadable anyway.
remove_uid(&t, 1000).await?;
// MsgId::new(1000)
// .update_download_state(&t, DownloadState::Available)
// .await?;
// t.sql
// .execute("UPDATE imap SET target=folder WHERE rfc724_mid='1000'", ())
// .await?;
// delete_expired_imap_messages(&t).await?;
// test_marked_for_deletion(&t, 1000).await?; // Delete downloadable anyway.
// remove_uid(&t, 1000).await?;
t.set_config(Config::DeleteServerAfter, Some(&*(22 * HOUR).to_string()))
.await?;
delete_expired_imap_messages(&t).await?;
test_marked_for_deletion(&t, 1010).await?;
t.sql
.execute("UPDATE imap SET target=folder WHERE rfc724_mid='1010'", ())
.await?;
// t.set_config(Config::DeleteServerAfter, Some(&*(22 * HOUR).to_string()))
// .await?;
// delete_expired_imap_messages(&t).await?;
// test_marked_for_deletion(&t, 1010).await?;
// t.sql
// .execute("UPDATE imap SET target=folder WHERE rfc724_mid='1010'", ())
// .await?;
// TODO check if removing this code makes the test fail
MsgId::new(1010)
.update_download_state(&t, DownloadState::Available)
@@ -547,9 +547,10 @@ async fn test_delete_expired_imap_messages() -> Result<()> {
0
);
t.set_config(Config::DeleteServerAfter, Some("1")).await?;
delete_expired_imap_messages(&t).await?;
test_marked_for_deletion(&t, 3000).await?;
// TODO instead, test that setting bcc_self to 0 removes messages
// t.set_config(Config::DeleteServerAfter, Some("1")).await?;
// delete_expired_imap_messages(&t).await?;
// test_marked_for_deletion(&t, 3000).await?;
Ok(())
}

View File

@@ -979,21 +979,19 @@ mod tests {
context1.set_config(Config::BccSelf, None).await?;
// Check that the settings are displayed correctly.
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())
);
context1.set_config_bool(Config::IsChatmail, true).await?;
assert_eq!(context1.get_config(Config::IsMuted).await?, None);
context1.set_config_bool(Config::IsMuted, true).await?;
assert_eq!(
context1.get_config(Config::DeleteServerAfter).await?,
context1.get_config(Config::IsMuted).await?,
Some("1".to_string())
);
assert_eq!(context1.get_config_delete_server_after().await?, Some(0));
imex(context1, ImexMode::ExportBackup, backup_dir.path(), None).await?;
let _event = context1
.evtracker
@@ -1010,15 +1008,15 @@ mod tests {
assert!(context2.is_configured().await?);
assert!(context2.is_chatmail().await?);
for ctx in [context1, context2] {
// BccSelf should be enabled automatically when exporting a backup
assert_eq!(
ctx.get_config(Config::BccSelf).await?,
Some("1".to_string())
);
assert_eq!(
ctx.get_config(Config::DeleteServerAfter).await?,
Some("0".to_string())
ctx.get_config(Config::IsMuted).await?,
Some("1".to_string())
);
assert_eq!(ctx.get_config_delete_server_after().await?, None);
}
Ok(())
}

View File

@@ -2105,63 +2105,52 @@ pub async fn get_request_msg_cnt(context: &Context) -> usize {
}
/// Estimates the number of messages that will be deleted
/// by the options `delete_device_after` or `delete_server_after`.
/// by the `set_config()`-option `delete_device_after`.
///
/// This is typically used to show the estimated impact to the user
/// before actually enabling deletion of old messages.
///
/// If `from_server` is true,
/// estimate deletion count for server,
/// otherwise estimate deletion count for device.
/// Messages in the "saved messages" folder are not counted as they will not be deleted automatically.
///
/// Count messages older than the given number of `seconds`.
/// Parameters:
/// - `from_server`: Deprecated, pass `false` here
/// - `seconds`: Count messages older than the given number of seconds.
///
/// Returns the number of messages that are older than the given number of seconds.
/// Messages in the "saved messages" folder are not counted as they will not be deleted automatically.
#[expect(clippy::arithmetic_side_effects)]
pub async fn estimate_deletion_cnt(
context: &Context,
from_server: bool,
seconds: i64,
) -> Result<usize> {
ensure!(
!from_server,
"The `delete_server_after` config option was removed."
);
let self_chat_id = ChatIdBlocked::lookup_by_contact(context, ContactId::SELF)
.await?
.map(|c| c.id)
.unwrap_or_default();
let threshold_timestamp = time() - seconds;
let cnt = if from_server {
context
.sql
.count(
"SELECT COUNT(*)
FROM msgs m
WHERE m.id > ?
AND timestamp < ?
AND chat_id != ?
AND EXISTS (SELECT * FROM imap WHERE rfc724_mid=m.rfc724_mid);",
(DC_MSG_ID_LAST_SPECIAL, threshold_timestamp, self_chat_id),
)
.await?
} else {
context
.sql
.count(
"SELECT COUNT(*)
let cnt = context
.sql
.count(
"SELECT COUNT(*)
FROM msgs m
WHERE m.id > ?
AND timestamp < ?
AND chat_id != ?
AND chat_id != ? AND hidden = 0;",
(
DC_MSG_ID_LAST_SPECIAL,
threshold_timestamp,
self_chat_id,
DC_CHAT_ID_TRASH,
),
)
.await?
};
(
DC_MSG_ID_LAST_SPECIAL,
threshold_timestamp,
self_chat_id,
DC_CHAT_ID_TRASH,
),
)
.await?;
Ok(cnt)
}

View File

@@ -890,16 +890,10 @@ static P_NAUTA_CU: Provider = Provider {
strict_tls: false,
..ProviderOptions::new()
},
config_defaults: Some(&[
ConfigDefault {
key: Config::DeleteServerAfter,
value: "1",
},
ConfigDefault {
key: Config::MediaQuality,
value: "1",
},
]),
config_defaults: Some(&[ConfigDefault {
key: Config::MediaQuality,
value: "1",
}]),
oauth2_authorizer: None,
};

View File

@@ -1978,15 +1978,16 @@ async fn test_no_smtp_job_for_self_chat() -> Result<()> {
assert!(bob.pop_sent_msg_opt(Duration::ZERO).await.is_none());
bob.set_config_bool(Config::BccSelf, true).await?;
bob.set_config(Config::DeleteServerAfter, Some("1")).await?;
let mut msg = Message::new_text("Happy birthday to me".to_string());
chat::send_msg(bob, chat_id, &mut msg).await?;
assert!(bob.pop_sent_msg_opt(Duration::ZERO).await.is_none());
// TODO instead, test setting bcc_self
// bob.set_config(Config::DeleteServerAfter, Some("1")).await?;
// let mut msg = Message::new_text("Happy birthday to me".to_string());
// chat::send_msg(bob, chat_id, &mut msg).await?;
// assert!(bob.pop_sent_msg_opt(Duration::ZERO).await.is_none());
bob.set_config(Config::DeleteServerAfter, None).await?;
let mut msg = Message::new_text("Happy birthday to me".to_string());
chat::send_msg(bob, chat_id, &mut msg).await?;
assert!(bob.pop_sent_msg_opt(Duration::ZERO).await.is_some());
// bob.set_config(Config::DeleteServerAfter, None).await?;
// let mut msg = Message::new_text("Happy birthday to me".to_string());
// chat::send_msg(bob, chat_id, &mut msg).await?;
// assert!(bob.pop_sent_msg_opt(Duration::ZERO).await.is_some());
Ok(())
}

View File

@@ -707,18 +707,34 @@ pub(crate) async fn add_self_recipients(
// and connection is frequently lost
// before receiving status line. NB: This is not a problem for chatmail servers, so `BccSelf`
// disabled by default is fine.
if context.get_config_delete_server_after().await? != Some(0) || !recipients.is_empty() {
// Avoid sending unencrypted messages to all transports, chatmail relays won't accept
// them. Normally the user should have a non-chatmail primary transport to send unencrypted
// messages.
if encrypted {
for addr in context.get_published_secondary_self_addrs().await? {
recipients.push(addr);
}
// Seems like the correct replacement is `if true`.
// Before my change, we're adding the self-recipient iff:
// - Messages are NOT deleted at once
// - OR there are recipients
//
// i.e. we skip adding the self-recipient iff:
// - Messages are deleted at once
// - AND there are no recipients
// probably because in this case, it's not necesary to send anything.
//
// Messages are deleted at once iff BccSelf is off in a chatmail profile now.
// But BccSelf is always on when this function is called.
// So, we can just remove the condition.
// TODO remove commented-out code
// if context.get_config_delete_server_after().await? != Some(0) || !recipients.is_empty() {
// Avoid sending unencrypted messages to all transports, chatmail relays won't accept
// them. Normally the user should have a non-chatmail primary transport to send unencrypted
// messages.
if encrypted {
for addr in context.get_published_secondary_self_addrs().await? {
recipients.push(addr);
}
// `from` must be the last addr, see `receive_imf_inner()` why.
let from = context.get_primary_self_addr().await?;
recipients.push(from);
}
// `from` must be the last addr, see `receive_imf_inner()` why.
let from = context.get_primary_self_addr().await?;
recipients.push(from);
// }
Ok(())
}