diff --git a/deltachat-rpc-client/tests/test_folders.py b/deltachat-rpc-client/tests/test_folders.py index 305cad563..bfa08326d 100644 --- a/deltachat-rpc-client/tests/test_folders.py +++ b/deltachat-rpc-client/tests/test_folders.py @@ -8,8 +8,10 @@ from imap_tools import AND, U from deltachat_rpc_client import Contact, EventType, Message -def test_move_works(acfactory): +def test_move_works(acfactory, direct_imap): ac1, ac2 = acfactory.get_online_accounts(2) + ac2_direct_imap = direct_imap(ac2) + ac2_direct_imap.create_folder("DeltaChat") ac2.set_config("mvbox_move", "1") ac2.bring_online() @@ -34,6 +36,8 @@ def test_move_avoids_loop(acfactory, direct_imap): We do not want to move this message from `INBOX.DeltaChat` to `DeltaChat` again. """ ac1, ac2 = acfactory.get_online_accounts(2) + ac2_direct_imap = direct_imap(ac2) + ac2_direct_imap.create_folder("DeltaChat") ac2.set_config("mvbox_move", "1") ac2.set_config("delete_server_after", "0") ac2.bring_online() @@ -97,6 +101,8 @@ def test_reactions_for_a_reordering_move(acfactory, direct_imap): addr, password = acfactory.get_credentials() ac2 = acfactory.get_unconfigured_account() ac2.add_or_update_transport({"addr": addr, "password": password}) + ac2_direct_imap = direct_imap(ac2) + ac2_direct_imap.create_folder("DeltaChat") ac2.set_config("mvbox_move", "1") assert ac2.is_configured() @@ -130,30 +136,6 @@ def test_reactions_for_a_reordering_move(acfactory, direct_imap): assert list(reactions.reactions_by_contact.values())[0] == [react_str] -def test_delete_deltachat_folder(acfactory, direct_imap): - """Test that DeltaChat folder is recreated if user deletes it manually.""" - ac1 = acfactory.new_configured_account() - ac1.set_config("mvbox_move", "1") - ac1.bring_online() - - ac1_direct_imap = direct_imap(ac1) - ac1_direct_imap.conn.folder.delete("DeltaChat") - assert "DeltaChat" not in ac1_direct_imap.list_folders() - - # Wait until new folder is created and UIDVALIDITY is updated. - while True: - event = ac1.wait_for_event() - if event.kind == EventType.INFO and "transport 1: UID validity for folder DeltaChat changed from " in event.msg: - break - - ac2 = acfactory.get_online_account() - ac2.create_chat(ac1).send_text("hello") - msg = ac1.wait_for_incoming_msg().get_snapshot() - assert msg.text == "hello" - - assert "DeltaChat" in ac1_direct_imap.list_folders() - - def test_dont_show_emails(acfactory, direct_imap, log): """Most mailboxes have a "Drafts" folder where constantly new emails appear but we don't actually want to show them. So: If it's outgoing AND there is no Received header, then ignore the email. @@ -294,10 +276,12 @@ def test_dont_show_emails(acfactory, direct_imap, log): assert len(msg.chat.get_messages()) == 2 -def test_move_works_on_self_sent(acfactory): +def test_move_works_on_self_sent(acfactory, direct_imap): ac1, ac2 = acfactory.get_online_accounts(2) - # Enable movebox and wait until it is created. + # Create and enable movebox. + ac1_direct_imap = direct_imap(ac1) + ac1_direct_imap.create_folder("DeltaChat") ac1.set_config("mvbox_move", "1") ac1.set_config("bcc_self", "1") ac1.bring_online() @@ -314,6 +298,8 @@ def test_move_works_on_self_sent(acfactory): def test_moved_markseen(acfactory, direct_imap): """Test that message already moved to DeltaChat folder is marked as seen.""" ac1, ac2 = acfactory.get_online_accounts(2) + ac2_direct_imap = direct_imap(ac2) + ac2_direct_imap.create_folder("DeltaChat") ac2.set_config("mvbox_move", "1") ac2.set_config("delete_server_after", "0") ac2.set_config("sync_msgs", "0") # Do not send a sync message when accepting a contact request. @@ -356,6 +342,8 @@ def test_markseen_message_and_mdn(acfactory, direct_imap, mvbox_move): for ac in ac1, ac2: ac.set_config("delete_server_after", "0") if mvbox_move: + ac_direct_imap = direct_imap(ac) + ac_direct_imap.create_folder("DeltaChat") ac.set_config("mvbox_move", "1") ac.bring_online() @@ -393,6 +381,8 @@ def test_markseen_message_and_mdn(acfactory, direct_imap, mvbox_move): 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() @@ -400,7 +390,6 @@ def test_mvbox_and_trash(acfactory, direct_imap, log): ac2 = acfactory.get_online_account() log.section("ac1: create trash") - ac1_direct_imap = direct_imap(ac1) ac1_direct_imap.create_folder("Trash") ac1.set_config("scan_all_folders_debounce_secs", "0") ac1.stop_io() diff --git a/deltachat-rpc-client/tests/test_something.py b/deltachat-rpc-client/tests/test_something.py index c1739c6e5..ddac6b39d 100644 --- a/deltachat-rpc-client/tests/test_something.py +++ b/deltachat-rpc-client/tests/test_something.py @@ -401,9 +401,6 @@ def test_dont_move_sync_msgs(acfactory, direct_imap): ac1_direct_imap.select_folder("Inbox") assert len(ac1_direct_imap.get_all_messages()) == inbox_msg_cnt + 2 - ac1_direct_imap.select_folder("DeltaChat") - assert len(ac1_direct_imap.get_all_messages()) == 0 - def test_reaction_seen_on_another_dev(acfactory) -> None: alice, bob = acfactory.get_online_accounts(2) diff --git a/python/tests/test_1_online.py b/python/tests/test_1_online.py index 142562549..e6205d446 100644 --- a/python/tests/test_1_online.py +++ b/python/tests/test_1_online.py @@ -220,21 +220,6 @@ def test_webxdc_huge_update(acfactory, data, lp): assert update["payload"] == payload -def test_enable_mvbox_move(acfactory, lp): - (ac1,) = acfactory.get_online_accounts(1) - - lp.sec("ac2: start without mvbox thread") - ac2 = acfactory.new_online_configuring_account(mvbox_move=False) - acfactory.bring_accounts_online() - - lp.sec("ac2: configuring mvbox") - ac2.set_config("mvbox_move", "1") - - lp.sec("ac1: send message and wait for ac2 to receive it") - acfactory.get_accepted_chat(ac1, ac2).send_text("message1") - assert ac2._evtracker.wait_next_incoming_message().text == "message1" - - def test_forward_messages(acfactory, lp): ac1, ac2 = acfactory.get_online_accounts(2) chat = ac1.create_chat(ac2) @@ -350,7 +335,7 @@ def test_long_group_name(acfactory, lp): def test_send_self_message(acfactory, lp): - ac1 = acfactory.new_online_configuring_account(mvbox_move=True, bcc_self=True) + ac1 = acfactory.new_online_configuring_account(bcc_self=True) acfactory.bring_accounts_online() lp.sec("ac1: create self chat") chat = ac1.get_self_contact().create_chat() @@ -509,7 +494,7 @@ def test_reply_privately(acfactory): def test_mdn_asymmetric(acfactory, lp): - ac1 = acfactory.new_online_configuring_account(mvbox_move=True) + ac1 = acfactory.new_online_configuring_account() ac2 = acfactory.new_online_configuring_account() acfactory.bring_accounts_online() @@ -538,20 +523,14 @@ def test_mdn_asymmetric(acfactory, lp): ac2.mark_seen_messages([msg]) lp.sec("ac1: waiting for incoming activity") - # MDN should be moved even though MDNs are already disabled - ac1._evtracker.get_matching("DC_EVENT_IMAP_MESSAGE_MOVED") - assert len(chat.get_messages()) == 1 + E2EE_INFO_MSGS # Wait for the message to be marked as seen on IMAP. - ac1._evtracker.get_info_contains("Marked messages [0-9]+ in folder DeltaChat as seen.") + ac1._evtracker.get_info_contains("Marked messages [0-9]+ in folder INBOX as seen.") # MDN is received even though MDNs are already disabled assert msg_out.is_out_mdn_received() - ac1.direct_imap.select_config_folder("mvbox") - assert len(list(ac1.direct_imap.conn.fetch(AND(seen=True)))) == 1 - def test_send_receive_encrypt(acfactory, lp): ac1, ac2 = acfactory.get_online_accounts(2) diff --git a/src/download.rs b/src/download.rs index a154d5fcd..8046540e2 100644 --- a/src/download.rs +++ b/src/download.rs @@ -189,10 +189,7 @@ impl Session { bail!("Attempt to fetch UID 0"); } - let create = false; - let folder_exists = self - .select_with_uidvalidity(context, folder, create) - .await?; + let folder_exists = self.select_with_uidvalidity(context, folder).await?; ensure!(folder_exists, "No folder {folder}"); // we are connected, and the folder is selected diff --git a/src/imap.rs b/src/imap.rs index f65e960d7..7bceba196 100644 --- a/src/imap.rs +++ b/src/imap.rs @@ -499,13 +499,7 @@ impl Imap { .get_raw_config_int(constants::DC_FOLDERS_CONFIGURED_KEY) .await?; if folders_configured.unwrap_or_default() < constants::DC_FOLDERS_CONFIGURED_VERSION { - let is_chatmail = match context.get_config_bool(Config::FixIsChatmail).await? { - false => session.is_chatmail(), - true => context.get_config_bool(Config::IsChatmail).await?, - }; - let create_mvbox = !is_chatmail || context.get_config_bool(Config::MvboxMove).await?; - self.configure_folders(context, &mut session, create_mvbox) - .await?; + self.configure_folders(context, &mut session).await?; } Ok(session) @@ -563,9 +557,8 @@ impl Imap { return Ok(false); } - let create = false; let folder_exists = session - .select_with_uidvalidity(context, folder, create) + .select_with_uidvalidity(context, folder) .await .with_context(|| format!("Failed to select folder {folder:?}"))?; if !folder_exists { @@ -873,10 +866,7 @@ impl Session { // Collect pairs of UID and Message-ID. let mut msgs = BTreeMap::new(); - let create = false; - let folder_exists = self - .select_with_uidvalidity(context, folder, create) - .await?; + let folder_exists = self.select_with_uidvalidity(context, folder).await?; let transport_id = self.transport_id(); if folder_exists { let mut list = self @@ -1082,10 +1072,7 @@ impl Session { // MOVE/DELETE operations. This does not result in multiple SELECT commands // being sent because `select_folder()` does nothing if the folder is already // selected. - let create = false; - let folder_exists = self - .select_with_uidvalidity(context, folder, create) - .await?; + let folder_exists = self.select_with_uidvalidity(context, folder).await?; ensure!(folder_exists, "No folder {folder}"); // Empty target folder name means messages should be deleted. @@ -1140,8 +1127,7 @@ impl Session { .await?; for (folder, rowid_set, uid_set) in UidGrouper::from(rows) { - let create = false; - let folder_exists = match self.select_with_uidvalidity(context, &folder, create).await { + let folder_exists = match self.select_with_uidvalidity(context, &folder).await { Err(err) => { warn!( context, @@ -1195,9 +1181,8 @@ impl Session { return Ok(()); } - let create = false; let folder_exists = self - .select_with_uidvalidity(context, folder, create) + .select_with_uidvalidity(context, folder) .await .context("Failed to select folder")?; if !folder_exists { @@ -1708,17 +1693,16 @@ impl Session { /// Attempts to configure mvbox. /// - /// Tries to find any folder examining `folders` in the order they go. If none is found, tries - /// to create any folder in the same order. This method does not use LIST command to ensure that + /// Tries to find any folder examining `folders` in the order they go. + /// This method does not use LIST command to ensure that /// configuration works even if mailbox lookup is forbidden via Access Control List (see /// ). /// - /// Returns first found or created folder name. + /// Returns first found folder name. async fn configure_mvbox<'a>( &mut self, context: &Context, folders: &[&'a str], - create_mvbox: bool, ) -> Result> { // Close currently selected folder if needed. // We are going to select folders using low-level EXAMINE operations below. @@ -1735,34 +1719,12 @@ impl Session { self.close().await?; // Before moving emails to the mvbox we need to remember its UIDVALIDITY, otherwise // emails moved before that wouldn't be fetched but considered "old" instead. - let create = false; - let folder_exists = self - .select_with_uidvalidity(context, folder, create) - .await?; + let folder_exists = self.select_with_uidvalidity(context, folder).await?; ensure!(folder_exists, "No MVBOX folder {:?}??", &folder); return Ok(Some(folder)); } } - if !create_mvbox { - return Ok(None); - } - // Some servers require namespace-style folder names like "INBOX.DeltaChat", so we try all - // the variants here. - for folder in folders { - match self - .select_with_uidvalidity(context, folder, create_mvbox) - .await - { - Ok(_) => { - info!(context, "MVBOX-folder {} created.", folder); - return Ok(Some(folder)); - } - Err(err) => { - warn!(context, "Cannot create MVBOX-folder {:?}: {}", folder, err); - } - } - } Ok(None) } } @@ -1772,7 +1734,6 @@ impl Imap { &mut self, context: &Context, session: &mut Session, - create_mvbox: bool, ) -> Result<()> { let mut folders = session .list(Some(""), Some("*")) @@ -1813,7 +1774,7 @@ impl Imap { let fallback_folder = format!("INBOX{delimiter}DeltaChat"); let mvbox_folder = session - .configure_mvbox(context, &["DeltaChat", &fallback_folder], create_mvbox) + .configure_mvbox(context, &["DeltaChat", &fallback_folder]) .await .context("failed to configure mvbox")?; diff --git a/src/imap/idle.rs b/src/imap/idle.rs index 44c014bde..0a5217a61 100644 --- a/src/imap/idle.rs +++ b/src/imap/idle.rs @@ -27,9 +27,7 @@ impl Session { idle_interrupt_receiver: Receiver<()>, folder: &str, ) -> Result { - let create = true; - self.select_with_uidvalidity(context, folder, create) - .await?; + self.select_with_uidvalidity(context, folder).await?; if self.drain_unsolicited_responses(context)? { self.new_mail = true; diff --git a/src/imap/select_folder.rs b/src/imap/select_folder.rs index 9d5ff7cda..1be6482e3 100644 --- a/src/imap/select_folder.rs +++ b/src/imap/select_folder.rs @@ -89,33 +89,6 @@ impl ImapSession { } } - /// Selects a folder. Tries to create it once and select again if the folder does not exist. - pub(super) async fn select_or_create_folder( - &mut self, - context: &Context, - folder: &str, - ) -> anyhow::Result { - match self.select_folder(context, folder).await { - Ok(newly_selected) => Ok(newly_selected), - Err(err) => match err { - Error::NoFolder(..) => { - info!(context, "Failed to select folder {folder:?} because it does not exist, trying to create it."); - let create_res = self.create(folder).await; - if let Err(ref err) = create_res { - info!(context, "Couldn't select folder, then create() failed: {err:#}."); - // Need to recheck, could have been created in parallel. - } - let select_res = self.select_folder(context, folder).await.with_context(|| format!("failed to select newely created folder {folder}")); - if select_res.is_err() { - create_res?; - } - select_res - } - _ => Err(err).with_context(|| format!("failed to select folder {folder} with error other than NO, not trying to create it")), - }, - } - } - /// Selects a folder optionally creating it and takes care of UIDVALIDITY changes. Returns false /// iff `folder` doesn't exist. /// @@ -129,23 +102,16 @@ impl ImapSession { &mut self, context: &Context, folder: &str, - create: bool, ) -> anyhow::Result { - let newly_selected = if create { - self.select_or_create_folder(context, folder) - .await - .with_context(|| format!("Failed to select or create folder {folder:?}"))? - } else { - match self.select_folder(context, folder).await { - Ok(newly_selected) => newly_selected, - Err(err) => match err { - Error::NoFolder(..) => return Ok(false), - _ => { - return Err(err) - .with_context(|| format!("Failed to select folder {folder:?}"))?; - } - }, - } + let newly_selected = match self.select_folder(context, folder).await { + Ok(newly_selected) => newly_selected, + Err(err) => match err { + Error::NoFolder(..) => return Ok(false), + _ => { + return Err(err) + .with_context(|| format!("Failed to select folder {folder:?}"))?; + } + }, }; let transport_id = self.transport_id();