fix: remove Config::DeleteToTrash and Config::ConfiguredTrashFolder

`delete_to_trash` is an option that was added for Gmail
as Gmail archives the messages by default
when they are deleted over IMAP:
<https://github.com/chatmail/core/issues/3957>
(implemented in <https://github.com/chatmail/core/pull/3972>).

Closes <https://github.com/chatmail/core/issues/6444>.

deltachat-rpc-client/tests/test_folders.py

@@ -378,32 +378,6 @@ def test_markseen_message_and_mdn(acfactory, direct_imap, mvbox_move):
     assert len(list(ac2_direct_imap.conn.fetch(AND(seen=True), mark_seen=False))) == 1
 
 
-def test_mvbox_and_trash(acfactory, direct_imap, log):
-    log.section("ac1: start with mvbox")
-    ac1 = acfactory.get_online_account()
-    ac1_direct_imap = direct_imap(ac1)
-    ac1_direct_imap.create_folder("DeltaChat")
-    ac1.set_config("mvbox_move", "1")
-    ac1.bring_online()
-
-    log.section("ac2: start without a mvbox")
-    ac2 = acfactory.get_online_account()
-
-    log.section("ac1: create trash")
-    ac1_direct_imap.create_folder("Trash")
-    ac1.set_config("scan_all_folders_debounce_secs", "0")
-    ac1.stop_io()
-    ac1.start_io()
-
-    log.section("ac1: send message and wait for ac2 to receive it")
-    acfactory.get_accepted_chat(ac1, ac2).send_text("message1")
-    assert ac2.wait_for_incoming_msg().get_snapshot().text == "message1"
-
-    assert ac1.get_config("configured_mvbox_folder") == "DeltaChat"
-    while ac1.get_config("configured_trash_folder") != "Trash":
-        ac1.wait_for_event(EventType.CONNECTIVITY_CHANGED)
-
-
 @pytest.mark.parametrize(
     ("folder", "move", "expected_destination"),
     [
@@ -479,20 +453,8 @@ def test_trash_multiple_messages(acfactory, direct_imap, log):
     ac1, ac2 = acfactory.get_online_accounts(2)
     ac2.stop_io()
 
-    # Create the Trash folder on IMAP server and configure deletion to it. There was a bug that if
-    # Trash wasn't configured initially, it can't be configured later, let's check this.
-    log.section("Creating trash folder")
-    ac2_direct_imap = direct_imap(ac2)
-    ac2_direct_imap.create_folder("Trash")
     ac2.set_config("delete_server_after", "0")
     ac2.set_config("sync_msgs", "0")
-    ac2.set_config("delete_to_trash", "1")
-
-    log.section("Check that Trash can be configured initially as well")
-    ac3 = ac2.clone()
-    ac3.bring_online()
-    assert ac3.get_config("configured_trash_folder")
-    ac3.stop_io()
 
     ac2.start_io()
     chat12 = acfactory.get_accepted_chat(ac1, ac2)
@@ -509,17 +471,15 @@ def test_trash_multiple_messages(acfactory, direct_imap, log):
         assert msg.text in texts
         if text != "second":
             to_delete.append(msg)
-    # ac2 has received some messages, this is impossible w/o the trash folder configured, let's
-    # check the configuration.
-    assert ac2.get_config("configured_trash_folder") == "Trash"
 
     log.section("ac2: deleting all messages except second")
     assert len(to_delete) == len(texts) - 1
     ac2.delete_messages(to_delete)
 
     log.section("ac2: test that only one message is left")
+    ac2_direct_imap = direct_imap(ac2)
     while 1:
-        ac2.wait_for_event(EventType.IMAP_MESSAGE_MOVED)
+        ac2.wait_for_event(EventType.IMAP_MESSAGE_DELETED)
         ac2_direct_imap.select_config_folder("inbox")
         nr_msgs = len(ac2_direct_imap.get_all_messages())
         assert nr_msgs > 0

src/chat.rs

@@ -618,7 +618,6 @@ impl ChatId {
         );
 
         let chat = Chat::load_from_db(context, self).await?;
-        let delete_msgs_target = context.get_delete_msgs_target().await?;
         let sync_id = match sync {
             Nosync => None,
             Sync => chat.get_sync_id(context).await?,
@@ -628,12 +627,12 @@ impl ChatId {
             .sql
             .transaction(|transaction| {
                 transaction.execute(
-                    "UPDATE imap SET target=? WHERE rfc724_mid IN (SELECT rfc724_mid FROM msgs WHERE chat_id=? AND rfc724_mid!='')",
+                    "UPDATE imap SET target='' WHERE rfc724_mid IN (SELECT rfc724_mid FROM msgs WHERE chat_id=? AND rfc724_mid!='')",
-                    (&delete_msgs_target, self,),
+                    (self,),
                 )?;
                 transaction.execute(
-                    "UPDATE imap SET target=? WHERE rfc724_mid IN (SELECT pre_rfc724_mid FROM msgs WHERE chat_id=? AND pre_rfc724_mid!='')",
+                    "UPDATE imap SET target='' WHERE rfc724_mid IN (SELECT pre_rfc724_mid FROM msgs WHERE chat_id=? AND pre_rfc724_mid!='')",
-                    (&delete_msgs_target, self,),
+                    (self,),
                 )?;
                 transaction.execute(
                     "DELETE FROM msgs_mdns WHERE msg_id IN (SELECT id FROM msgs WHERE chat_id=?)",

src/config.rs

@@ -194,10 +194,6 @@ pub enum Config {
     #[strum(props(default = "0"))]
     DeleteDeviceAfter,
 
-    /// Move messages to the Trash folder instead of marking them "\Deleted". Overrides
-    /// `ProviderOptions::delete_to_trash`.
-    DeleteToTrash,
-
     /// The primary email address.
     ConfiguredAddr,
 
@@ -275,9 +271,6 @@ pub enum Config {
     /// Configured folder for chat messages.
     ConfiguredMvboxFolder,
 
-    /// Configured "Trash" folder.
-    ConfiguredTrashFolder,
-
     /// Unix timestamp of the last successful configuration.
     ConfiguredTimestamp,
 
@@ -696,7 +689,6 @@ impl Context {
             | Config::MdnsEnabled
             | Config::MvboxMove
             | Config::OnlyFetchMvbox
-            | Config::DeleteToTrash
             | Config::Configured
             | Config::Bot
             | Config::NotifyAboutWrongPw

src/context.rs

@@ -8,7 +8,7 @@ use std::sync::atomic::AtomicBool;
 use std::sync::{Arc, OnceLock, Weak};
 use std::time::Duration;
 
-use anyhow::{Context as _, Result, bail, ensure};
+use anyhow::{Result, bail, ensure};
 use async_channel::{self as channel, Receiver, Sender};
 use ratelimit::Ratelimit;
 use tokio::sync::{Mutex, Notify, RwLock};
@@ -880,10 +880,6 @@ impl Context {
             .get_config(Config::ConfiguredMvboxFolder)
             .await?
             .unwrap_or_else(|| "<unset>".to_string());
-        let configured_trash_folder = self
-            .get_config(Config::ConfiguredTrashFolder)
-            .await?
-            .unwrap_or_else(|| "<unset>".to_string());
 
         let mut res = get_info();
 
@@ -968,7 +964,6 @@ impl Context {
         );
         res.insert("configured_inbox_folder", configured_inbox_folder);
         res.insert("configured_mvbox_folder", configured_mvbox_folder);
-        res.insert("configured_trash_folder", configured_trash_folder);
         res.insert("mdns_enabled", mdns_enabled.to_string());
         res.insert("bcc_self", bcc_self.to_string());
         res.insert("sync_msgs", sync_msgs.to_string());
@@ -992,12 +987,6 @@ impl Context {
                 .await?
                 .to_string(),
         );
-        res.insert(
-            "delete_to_trash",
-            self.get_config(Config::DeleteToTrash)
-                .await?
-                .unwrap_or_else(|| "<unset>".to_string()),
-        );
         res.insert(
             "last_housekeeping",
             self.get_config_int(Config::LastHousekeeping)
@@ -1286,33 +1275,6 @@ ORDER BY m.timestamp DESC,m.id DESC",
         Ok(mvbox.as_deref() == Some(folder_name))
     }
 
-    /// Returns true if given folder name is the name of the trash folder.
-    pub async fn is_trash(&self, folder_name: &str) -> Result<bool> {
-        let trash = self.get_config(Config::ConfiguredTrashFolder).await?;
-        Ok(trash.as_deref() == Some(folder_name))
-    }
-
-    pub(crate) async fn should_delete_to_trash(&self) -> Result<bool> {
-        if let Some(v) = self.get_config_bool_opt(Config::DeleteToTrash).await? {
-            return Ok(v);
-        }
-        if let Some(provider) = self.get_configured_provider().await? {
-            return Ok(provider.opt.delete_to_trash);
-        }
-        Ok(false)
-    }
-
-    /// Returns `target` for deleted messages as per `imap` table. Empty string means "delete w/o
-    /// moving to trash".
-    pub(crate) async fn get_delete_msgs_target(&self) -> Result<String> {
-        if !self.should_delete_to_trash().await? {
-            return Ok("".into());
-        }
-        self.get_config(Config::ConfiguredTrashFolder)
-            .await?
-            .context("No configured trash folder")
-    }
-
     pub(crate) fn derive_blobdir(dbfile: &Path) -> PathBuf {
         let mut blob_fname = OsString::new();
         blob_fname.push(dbfile.file_name().unwrap_or_default());

src/ephemeral.rs

@@ -665,25 +665,19 @@ pub(crate) async fn delete_expired_imap_messages(context: &Context) -> Result<()
                 now - max(delete_server_after, MIN_DELETE_SERVER_AFTER),
             ),
         };
-    let target = context.get_delete_msgs_target().await?;
 
     context
         .sql
         .execute(
             "UPDATE imap
-             SET target=?
+             SET target=''
              WHERE rfc724_mid IN (
                SELECT rfc724_mid FROM msgs
                WHERE ((download_state = 0 AND timestamp < ?) OR
                       (download_state != 0 AND timestamp < ?) OR
                       (ephemeral_timestamp != 0 AND ephemeral_timestamp <= ?))
              )",
-            (
+            (threshold_timestamp, threshold_timestamp_extended, now),
-                &target,
-                threshold_timestamp,
-                threshold_timestamp_extended,
-                now,
-            ),
         )
         .await?;
 

src/imap.rs

@@ -183,7 +183,7 @@ impl FolderMeaning {
             FolderMeaning::Spam => None,
             FolderMeaning::Inbox => Some(Config::ConfiguredInboxFolder),
             FolderMeaning::Mvbox => Some(Config::ConfiguredMvboxFolder),
-            FolderMeaning::Trash => Some(Config::ConfiguredTrashFolder),
+            FolderMeaning::Trash => None,
             FolderMeaning::Virtual => None,
         }
     }
@@ -611,7 +611,6 @@ impl Imap {
         let mut download_later: Vec<String> = Vec::new();
         let mut uid_message_ids = BTreeMap::new();
         let mut largest_uid_skipped = None;
-        let delete_target = context.get_delete_msgs_target().await?;
 
         let download_limit: Option<u32> = context
             .get_config_parsed(Config::DownloadLimit)
@@ -661,7 +660,7 @@ impl Imap {
 
             let _target;
             let target = if delete {
-                &delete_target
+                ""
             } else {
                 _target = target_folder(context, folder, folder_meaning, &headers).await?;
                 &_target
@@ -988,17 +987,6 @@ impl Session {
                     return Ok(());
                 }
                 Err(err) => {
-                    if context.should_delete_to_trash().await? {
-                        error!(
-                            context,
-                            "Cannot move messages {} to {}, no fallback to COPY/DELETE because \
-                            delete_to_trash is set. Error: {:#}",
-                            set,
-                            target,
-                            err,
-                        );
-                        return Err(err.into());
-                    }
                     warn!(
                         context,
                         "Cannot move messages, fallback to COPY/DELETE {} to {}: {}",
@@ -1012,19 +1000,11 @@ impl Session {
 
         // Server does not support MOVE or MOVE failed.
         // Copy messages to the destination folder if needed and mark records for deletion.
-        let copy = !context.is_trash(target).await?;
+        info!(
-        if copy {
+            context,
-            info!(
+            "Server does not support MOVE, fallback to COPY/DELETE {} to {}", set, target
-                context,
+        );
-                "Server does not support MOVE, fallback to COPY/DELETE {} to {}", set, target
+        self.uid_copy(&set, &target).await?;
-            );
-            self.uid_copy(&set, &target).await?;
-        } else {
-            error!(
-                context,
-                "Server does not support MOVE, fallback to DELETE {} to {}", set, target,
-            );
-        }
         context
             .sql
             .transaction(|transaction| {
@@ -1036,11 +1016,9 @@ impl Session {
             })
             .await
             .context("Cannot plan deletion of messages")?;
-        if copy {
+        context.emit_event(EventType::ImapMessageMoved(format!(
-            context.emit_event(EventType::ImapMessageMoved(format!(
+            "IMAP messages {set} copied to {target}"
-                "IMAP messages {set} copied to {target}"
+        )));
-            )));
-        }
         Ok(())
     }
 

src/imap/scan_folders.rs

@@ -84,17 +84,6 @@ impl Imap {
             }
         }
 
-        // Set config for the Trash folder. Or reset if the folder was deleted.
-        let conf = Config::ConfiguredTrashFolder;
-        let val = folder_configs.get(&conf).map(|s| s.as_str());
-        let interrupt = val.is_some() && context.get_config(conf).await?.is_none();
-        context.set_config_internal(conf, val).await?;
-        if interrupt {
-            // `Imap::fetch_move_delete()`, particularly message deletion, is possible now for other
-            // folders (NB: we are in the Inbox loop).
-            context.scheduler.interrupt_oboxes().await;
-        }
-
         info!(context, "Found folders: {folder_names:?}.");
         Ok(true)
     }

src/message.rs

@@ -1762,12 +1762,11 @@ pub async fn delete_msgs_ex(
         modified_chat_ids.insert(msg.chat_id);
         deleted_rfc724_mid.push(msg.rfc724_mid.clone());
 
-        let target = context.get_delete_msgs_target().await?;
         let update_db = |trans: &mut rusqlite::Transaction| {
-            let mut stmt = trans.prepare("UPDATE imap SET target=? WHERE rfc724_mid=?")?;
+            let mut stmt = trans.prepare("UPDATE imap SET target='' WHERE rfc724_mid=?")?;
-            stmt.execute((&target, &msg.rfc724_mid))?;
+            stmt.execute((&msg.rfc724_mid,))?;
             if !msg.pre_rfc724_mid.is_empty() {
-                stmt.execute((&target, &msg.pre_rfc724_mid))?;
+                stmt.execute((&msg.pre_rfc724_mid,))?;
             }
             trans.execute("DELETE FROM smtp WHERE msg_id=?", (msg_id,))?;
             trans.execute(

src/provider.rs

@@ -153,9 +153,6 @@ pub struct ProviderOptions {
 
     /// Maximum number of recipients the provider allows to send a single email to.
     pub max_smtp_rcpt_to: Option<u16>,
-
-    /// Move messages to the Trash folder instead of marking them "\Deleted".
-    pub delete_to_trash: bool,
 }
 
 impl ProviderOptions {
@@ -163,7 +160,6 @@ impl ProviderOptions {
         Self {
             strict_tls: true,
             max_smtp_rcpt_to: None,
-            delete_to_trash: false,
         }
     }
 }

src/receive_imf.rs

@@ -952,7 +952,7 @@ UPDATE config SET value=? WHERE keyname='configured_addr' AND value!=?1
 
     if !received_msg.msg_ids.is_empty() {
         let target = if received_msg.needs_delete_job || delete_server_after == Some(0) {
-            Some(context.get_delete_msgs_target().await?)
+            Some("".to_string())
         } else {
             None
         };

src/scheduler.rs

@@ -256,14 +256,6 @@ impl SchedulerState {
         }
     }
 
-    /// 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 {
-            scheduler.interrupt_oboxes();
-        }
-    }
-
     pub(crate) async fn interrupt_smtp(&self) {
         let inner = self.inner.read().await;
         if let InnerSchedulerState::Started(ref scheduler) = *inner {
@@ -519,33 +511,24 @@ async fn fetch_idle(
             .context("store_seen_flags_on_imap")?;
     }
 
-    if !ctx.should_delete_to_trash().await?
+    // Fetch the watched folder.
-        || ctx
+    connection
-            .get_config(Config::ConfiguredTrashFolder)
+        .fetch_move_delete(ctx, &mut session, &watch_folder, folder_meaning)
-            .await?
+        .await
-            .is_some()
+        .context("fetch_move_delete")?;
-    {
-        // Fetch the watched folder.
-        connection
-            .fetch_move_delete(ctx, &mut session, &watch_folder, folder_meaning)
-            .await
-            .context("fetch_move_delete")?;
 
-        // Mark expired messages for deletion. Marked messages will be deleted from the server
+    // Mark expired messages for deletion. Marked messages will be deleted from the server
-        // on the next iteration of `fetch_move_delete`. `delete_expired_imap_messages` is not
+    // on the next iteration of `fetch_move_delete`. `delete_expired_imap_messages` is not
-        // called right before `fetch_move_delete` because it is not well optimized and would
+    // called right before `fetch_move_delete` because it is not well optimized and would
-        // otherwise slow down message fetching.
+    // otherwise slow down message fetching.
-        delete_expired_imap_messages(ctx)
+    delete_expired_imap_messages(ctx)
-            .await
+        .await
-            .context("delete_expired_imap_messages")?;
+        .context("delete_expired_imap_messages")?;
 
-        download_known_post_messages_without_pre_message(ctx, &mut session).await?;
+    download_known_post_messages_without_pre_message(ctx, &mut session).await?;
-        download_msgs(ctx, &mut session)
+    download_msgs(ctx, &mut session)
-            .await
+        .await
-            .context("download_msgs")?;
+        .context("download_msgs")?;
-    } else if folder_config == Config::ConfiguredInboxFolder {
-        session.last_full_folder_scan.lock().await.take();
-    }
 
     // Scan additional folders only after finishing fetching the watched folder.
     //
@@ -902,12 +885,6 @@ impl Scheduler {
         }
     }
 
-    fn interrupt_oboxes(&self) {
-        for b in &self.oboxes {
-            b.conn_state.interrupt();
-        }
-    }
-
     fn interrupt_smtp(&self) {
         self.smtp.interrupt();
     }