[wip]

python/tests/test_account.py

@@ -642,7 +642,7 @@ class TestOnlineAccount:
         assert os.path.exists(msg_in.filename)
         assert os.stat(msg_in.filename).st_size == os.stat(path).st_size
 
-    def test_import_export_online_all(self, acfactory, tmpdir, lp):
+    def test_import_export_online_all_twice(self, acfactory, tmpdir, lp):
         ac1 = acfactory.get_online_configuring_account()
         wait_configuration_progress(ac1, 1000)
 
@@ -675,7 +675,6 @@ class TestOnlineAccount:
         assert len(messages) == 1
         assert messages[0].text == "msg1"
 
-        pytest.xfail("cannot export twice yet, probably due to interrupt_idle failing")
         # wait until a second passed since last backup
         # because get_latest_backupfile() shall return the latest backup
         # from a UI it's unlikely anyone manages to export two

src/context.rs

@@ -398,24 +398,21 @@ impl Context {
             .unwrap_or_default()
     }
 
-    pub fn is_inbox(&self, folder_name: impl AsRef<str>) -> bool {
-        folder_name.as_ref() == "INBOX"
+    pub fn is_inbox(&self, folder_name: &str) -> bool {
+        folder_name == "INBOX"
     }
 
-    pub fn is_sentbox(&self, folder_name: impl AsRef<str>) -> bool {
-        let sentbox_name = self.sql.get_raw_config(self, "configured_sentbox_folder");
-        if let Some(name) = sentbox_name {
-            name == folder_name.as_ref()
+    pub fn is_sentbox(&self, folder_name: &str) -> bool {
+        if let Some(name) = self.sql.get_raw_config(self, "configured_sentbox_folder") {
+            name == folder_name
         } else {
             false
         }
     }
 
-    pub fn is_mvbox(&self, folder_name: impl AsRef<str>) -> bool {
-        let mvbox_name = self.sql.get_raw_config(self, "configured_mvbox_folder");
-
-        if let Some(name) = mvbox_name {
-            name == folder_name.as_ref()
+    pub fn is_mvbox(&self, folder_name: &str) -> bool {
+        if let Some(name) = self.sql.get_raw_config(self, "configured_mvbox_folder") {
+            name == folder_name
         } else {
             false
         }

src/imap.rs

@@ -380,10 +380,8 @@ impl Imap {
         self.should_reconnect.load(Ordering::Relaxed)
     }
 
-    fn setup_handle_if_needed(&self, context: &Context) -> bool {
-        if self.config.read().unwrap().imap_server.is_empty() {
-            return false;
-        }
+    fn setup_handle_if_needed(&self, context: &Context) -> Result<(), Error> {
+        ensure!(!self.config.read().unwrap().imap_server.is_empty(), "no imap server configured");
 
         if self.should_reconnect() {
             self.unsetup_handle(context);
@@ -391,7 +389,7 @@ impl Imap {
 
         if self.is_connected() && self.stream.read().unwrap().is_some() {
             self.should_reconnect.store(false, Ordering::Relaxed);
-            return true;
+            return Ok(());
         }
 
         let server_flags = self.config.read().unwrap().server_flags as i32;
@@ -437,7 +435,7 @@ impl Imap {
                         };
                         client.authenticate("XOAUTH2", &auth)
                     } else {
-                        return false;
+                        bail!("could not authenticate with XOAUTH2");
                     }
                 } else {
                     client.login(imap_user, imap_pw)
@@ -455,7 +453,7 @@ impl Imap {
 
                 emit_event!(context, Event::ErrorNetwork(message));
 
-                return false;
+                bail!("login error: {}", err);
             }
         };
 
@@ -465,7 +463,7 @@ impl Imap {
             Ok((session, stream)) => {
                 *self.session.lock().unwrap() = Some(session);
                 *self.stream.write().unwrap() = Some(stream);
-                true
+                Ok(())
             }
             Err((err, _)) => {
                 let imap_user = self.config.read().unwrap().imap_user.to_owned();
@@ -477,7 +475,7 @@ impl Imap {
                 );
                 self.unsetup_handle(context);
 
-                false
+                bail!("{}", err)
             }
         }
     }
@@ -549,7 +547,7 @@ impl Imap {
             config.server_flags = server_flags;
         }
 
-        if !self.setup_handle_if_needed(context) {
+        if self.setup_handle_if_needed(context).is_err() {
             self.free_connect_params();
             return false;
         }
@@ -622,7 +620,7 @@ impl Imap {
             // get any more. if IDLE is called directly after, there is only a small chance that
             // messages are missed and delayed until the next IDLE call
             loop {
-                if self.fetch_from_single_folder(context, watch_folder) == 0 {
+                if self.fetch_from_single_folder(context, watch_folder).is_err() {
                     break;
                 }
             }
@@ -632,69 +630,20 @@ impl Imap {
         }
     }
 
-    fn select_folder<S: AsRef<str>>(&self, context: &Context, folder: Option<S>) -> usize {
-        if self.session.lock().unwrap().is_none() {
-            let mut cfg = self.config.write().unwrap();
-            cfg.selected_folder = None;
-            cfg.selected_folder_needs_expunge = false;
-            return 0;
-        }
+    fn expunge_folder(&self, context: &Context) -> bool {
+        if let Some(ref folder) = self.config.read().unwrap().selected_folder {
+            info!(context, "Expunge messages in \"{}\".", folder);
 
-        // if there is a new folder and the new folder is equal to the selected one, there's nothing to do.
-        // if there is _no_ new folder, we continue as we might want to expunge below.
-        if let Some(ref folder) = folder {
-            if let Some(ref selected_folder) = self.config.read().unwrap().selected_folder {
-                if folder.as_ref() == selected_folder {
-                    return 1;
-                }
-            }
-        }
-
-        // deselect existing folder, if needed (it's also done implicitly by SELECT, however, without EXPUNGE then)
-        let needs_expunge = { self.config.read().unwrap().selected_folder_needs_expunge };
-        if needs_expunge {
-            if let Some(ref folder) = self.config.read().unwrap().selected_folder {
-                info!(context, "Expunge messages in \"{}\".", folder);
-
-                // A CLOSE-SELECT is considerably faster than an EXPUNGE-SELECT, see
-                // https://tools.ietf.org/html/rfc3501#section-6.4.2
-                if let Some(ref mut session) = &mut *self.session.lock().unwrap() {
-                    match session.close() {
-                        Ok(_) => {
-                            info!(context, "close/expunge succeeded");
-                        }
-                        Err(err) => {
-                            warn!(context, "failed to close session: {:?}", err);
-                            return 0;
-                        }
-                    }
-                } else {
-                    return 0;
-                }
-            }
-            self.config.write().unwrap().selected_folder_needs_expunge = false;
-        }
-
-        // select new folder
-        if let Some(ref folder) = folder {
+            // A CLOSE-SELECT is considerably faster than an EXPUNGE-SELECT, see
+            // https://tools.ietf.org/html/rfc3501#section-6.4.2
             if let Some(ref mut session) = &mut *self.session.lock().unwrap() {
-                match session.select(folder) {
-                    Ok(mailbox) => {
-                        let mut config = self.config.write().unwrap();
-                        config.selected_folder = Some(folder.as_ref().to_string());
-                        config.selected_mailbox = Some(mailbox);
+                match session.close() {
+                    Ok(_) => {
+                        info!(context, "close/expunge succeeded");
                     }
                     Err(err) => {
-                        info!(
-                            context,
-                            "Cannot select folder: {}; {:?}.",
-                            folder.as_ref(),
-                            err
-                        );
-
-                        self.config.write().unwrap().selected_folder = None;
-                        self.should_reconnect.store(true, Ordering::Relaxed);
-                        return 0;
+                        warn!(context, "failed to close session: {:?}", err);
+                        return false;
                     }
                 }
             } else {
@@ -702,11 +651,55 @@ impl Imap {
             }
         }
 
-        1
+        true
     }
 
-    fn get_config_last_seen_uid<S: AsRef<str>>(&self, context: &Context, folder: S) -> (u32, u32) {
-        let key = format!("imap.mailbox.{}", folder.as_ref());
+    fn select_folder(&self, context: &Context, folder: &str) -> Result<(), Error> {
+        if self.session.lock().unwrap().is_none() {
+            let mut cfg = self.config.write().unwrap();
+            cfg.selected_folder = None;
+            cfg.selected_folder_needs_expunge = false;
+            bail!("select_folder: session is closed");
+        }
+
+        // if there is a new folder and the new folder is equal to the selected one, there's nothing to do.
+        // if there is _no_ new folder, we continue as we might want to expunge below.
+        if let Some(ref selected_folder) = self.config.read().unwrap().selected_folder {
+            if folder == selected_folder {
+                return Ok(());
+            }
+        }
+
+        // deselect existing folder, if needed (it's also done implicitly by SELECT, however, without EXPUNGE then)
+        let needs_expunge = { self.config.read().unwrap().selected_folder_needs_expunge };
+        if needs_expunge {
+            self.expunge_folder(context);
+            self.config.write().unwrap().selected_folder_needs_expunge = false;
+        }
+
+        // select new folder
+        if let Some(ref mut session) = &mut *self.session.lock().unwrap() {
+            match session.select(&folder) {
+                Ok(mailbox) => {
+                    let mut config = self.config.write().unwrap();
+                    config.selected_folder = Some(folder.to_string());
+                    config.selected_mailbox = Some(mailbox);
+                }
+                Err(err) => {
+                    self.config.write().unwrap().selected_folder = None;
+                    self.should_reconnect.store(true, Ordering::Relaxed);
+                    bail!("Cannot select folder: {}; {:?}.", folder, err);
+                }
+            }
+        } else {
+            unreachable!();
+        }
+
+        Ok(())
+    }
+
+    fn get_config_last_seen_uid(&self, context: &Context, folder: &str) -> (u32, u32) {
+        let key = format!("imap.mailbox.{}", folder);
         if let Some(entry) = context.sql.get_raw_config(context, &key) {
             // the entry has the format `imap.mailbox.<folder>=<uidvalidity>:<lastseenuid>`
             let mut parts = entry.split(':');
@@ -727,49 +720,25 @@ impl Imap {
         }
     }
 
-    fn fetch_from_single_folder<S: AsRef<str>>(&self, context: &Context, folder: S) -> usize {
-        if !self.is_connected() {
-            info!(
-                context,
-                "Cannot fetch from \"{}\" - not connected.",
-                folder.as_ref()
-            );
-
-            return 0;
-        }
-
-        if self.select_folder(context, Some(&folder)) == 0 {
-            info!(
-                context,
-                "Cannot select folder \"{}\" for fetching.",
-                folder.as_ref()
-            );
-
-            return 0;
-        }
 
+    fn 
+        
         // compare last seen UIDVALIDITY against the current one
-        let (mut uid_validity, mut last_seen_uid) = self.get_config_last_seen_uid(context, &folder);
+        let (last_uid_validity, last_seen_uid) = self.get_config_last_seen_uid(context, &folder);
+        let mailbox = self.config.read().unwrap().selected_mailbox.as_ref().expect("just selected");
 
-        let config = self.config.read().unwrap();
-        let mailbox = config.selected_mailbox.as_ref().expect("just selected");
+        ensure!(mailbox.uid_validity.is_some(),
+            "Cannot get UIDVALIDITY for folder \"{}\".", folder,);
 
-        if mailbox.uid_validity.is_none() {
-            error!(
-                context,
-                "Cannot get UIDVALIDITY for folder \"{}\".",
-                folder.as_ref(),
-            );
+        let mailbox = mailbox.unwrap();
+        let current_uid_validity = mailbox.uid_validity.unwrap_or_default();
 
-            return 0;
-        }
-
-        if mailbox.uid_validity.unwrap_or_default() != uid_validity {
-            // first time this folder is selected or UIDVALIDITY has changed, init lastseenuid and save it to config
+        if current_uid_validity != last_uid_validity {
+            // first time this folder is selected or UIDVALIDITY has changed, 
+            // init lastseenuid and save it to config
 
             if mailbox.exists == 0 {
-                info!(context, "Folder \"{}\" is empty.", folder.as_ref());
-
+                info!(context, "Folder \"{}\" is empty.", folder);
                 // set lastseenuid=0 for empty folders.
                 // id we do not do this here, we'll miss the first message
                 // as we will get in here again and fetch from lastseenuid+1 then
@@ -777,10 +746,10 @@ impl Imap {
                 self.set_config_last_seen_uid(
                     context,
                     &folder,
-                    mailbox.uid_validity.unwrap_or_default(),
+                    current_uid_validity,
                     0,
                 );
-                return 0;
+                return Ok(false);
             }
 
             let list = if let Some(ref mut session) = &mut *self.session.lock().unwrap() {
@@ -788,56 +757,57 @@ impl Imap {
                 let set = format!("{}", mailbox.exists);
                 match session.fetch(set, PREFETCH_FLAGS) {
                     Ok(list) => list,
-                    Err(_err) => {
+                    Err(err) => {
                         self.should_reconnect.store(true, Ordering::Relaxed);
-                        info!(
-                            context,
-                            "No result returned for folder \"{}\".",
-                            folder.as_ref()
-                        );
-
-                        return 0;
+                        bail!("IMAP FETCH {} {} failed on folder '{}': {}'", 
+                              set, PREFETCH_FLAGS, folder, err);
                     }
                 }
             } else {
-                return 0;
+                unreachable!();
             };
 
-            last_seen_uid = list[0].uid.unwrap_or_else(|| 0);
+            last_seen_uid = list[0].uid.unwrap_or_default();
 
             // if the UIDVALIDITY has _changed_, decrease lastseenuid by one to avoid gaps (well add 1 below
-            if uid_validity > 0 && last_seen_uid > 1 {
+            if last_uid_validity > 0 && last_seen_uid > 1 {
                 last_seen_uid -= 1;
             }
 
-            uid_validity = mailbox.uid_validity.unwrap_or_default();
             self.set_config_last_seen_uid(context, &folder, uid_validity, last_seen_uid);
             info!(
                 context,
                 "lastseenuid initialized to {} for {}@{}",
                 last_seen_uid,
-                folder.as_ref(),
+                folder,
                 uid_validity,
             );
+            Ok(true)
         }
 
+    fn fetch_from_single_folder(&self, context: &Context, folder: &str) -> Result<(), Error> {
+        self.select_folder(context, &folder)?;
+
+        if self.folder_is_uidvalid_and_empty(&self, context: &Context, folder: &str)? {
+            return  
+
         let mut read_cnt = 0;
         let mut read_errors = 0;
         let mut new_last_seen_uid = 0;
 
+        let set = format!("{}:*", last_seen_uid + 1);
         let list = if let Some(ref mut session) = &mut *self.session.lock().unwrap() {
             // fetch messages with larger UID than the last one seen
             // (`UID FETCH lastseenuid+1:*)`, see RFC 4549
-            let set = format!("{}:*", last_seen_uid + 1);
             match session.uid_fetch(set, PREFETCH_FLAGS) {
                 Ok(list) => list,
                 Err(err) => {
                     warn!(context, "failed to fetch uids: {}", err);
-                    return 0;
+                    bail!("failed to fetch uids: {}", err)
                 }
             }
         } else {
-            return 0;
+            unreachable!();
         };
 
         // go through all mails in folder (this is typically _fast_ as we already have the whole list)
@@ -848,14 +818,14 @@ impl Imap {
 
                 let message_id = prefetch_get_message_id(msg).unwrap_or_default();
 
-                if !precheck_imf(context, &message_id, folder.as_ref(), cur_uid) {
+                if !precheck_imf(context, &message_id, &folder, cur_uid) {
                     // check passed, go fetch the rest
                     if self.fetch_single_msg(context, &folder, cur_uid) == 0 {
                         info!(
                             context,
                             "Read error for message {} from \"{}\", trying over later.",
                             message_id,
-                            folder.as_ref()
+                            folder
                         );
 
                         read_errors += 1;
@@ -866,7 +836,7 @@ impl Imap {
                         context,
                         "Skipping message {} from \"{}\" by precheck.",
                         message_id,
-                        folder.as_ref(),
+                        folder
                     );
                 }
                 if cur_uid > new_last_seen_uid {
@@ -886,7 +856,7 @@ impl Imap {
                 context,
                 "{} mails read from \"{}\" with {} errors.",
                 read_cnt,
-                folder.as_ref(),
+                folder,
                 read_errors
             );
         } else {
@@ -894,30 +864,30 @@ impl Imap {
                 context,
                 "{} mails read from \"{}\".",
                 read_cnt,
-                folder.as_ref()
+                folder
             );
         }
 
-        read_cnt
+        Ok(read_cnt)
     }
 
-    fn set_config_last_seen_uid<S: AsRef<str>>(
+    fn set_config_last_seen_uid(
         &self,
         context: &Context,
-        folder: S,
+        folder: &str,
         uidvalidity: u32,
         lastseenuid: u32,
     ) {
-        let key = format!("imap.mailbox.{}", folder.as_ref());
+        let key = format!("imap.mailbox.{}", folder);
         let val = format!("{}:{}", uidvalidity, lastseenuid);
 
         context.sql.set_raw_config(context, &key, Some(&val)).ok();
     }
 
-    fn fetch_single_msg<S: AsRef<str>>(
+    fn fetch_single_msg(
         &self,
         context: &Context,
-        folder: S,
+        folder: &str,
         server_uid: u32,
     ) -> usize {
         // the function returns:
@@ -938,7 +908,7 @@ impl Imap {
                         context,
                         "Error on fetching message #{} from folder \"{}\"; retry={}; error={}.",
                         server_uid,
-                        folder.as_ref(),
+                        folder,
                         self.should_reconnect(),
                         err
                     );
@@ -954,7 +924,7 @@ impl Imap {
                 context,
                 "Message #{} does not exist in folder \"{}\".",
                 server_uid,
-                folder.as_ref()
+                folder,
             );
         } else {
             let msg = &msgs[0];
@@ -974,7 +944,7 @@ impl Imap {
             if !is_deleted && msg.body().is_some() {
                 let body = msg.body().unwrap_or_default();
                 unsafe {
-                    dc_receive_imf(context, &body, folder.as_ref(), server_uid, flags as u32);
+                    dc_receive_imf(context, &body, &folder, server_uid, flags as u32);
                 }
             }
         }
@@ -982,18 +952,16 @@ impl Imap {
         1
     }
 
-    pub fn idle(&self, context: &Context) {
+    pub fn idle(&self, context: &Context) -> Result<(), Error> {
         if !self.config.read().unwrap().can_idle {
-            return self.fake_idle(context);
+            self.fake_idle(context);
         }
 
-        self.setup_handle_if_needed(context);
+        self.setup_handle_if_needed(context)?;
 
         let watch_folder = self.config.read().unwrap().watch_folder.clone();
-        if self.select_folder(context, watch_folder.as_ref()) == 0 {
-            warn!(context, "IMAP-IDLE not setup.",);
-
-            return self.fake_idle(context);
+        if let Some(wfolder) = watch_folder {
+            self.select_folder(context, &wfolder)?;
         }
 
         let session = self.session.clone();
@@ -1087,6 +1055,7 @@ impl Imap {
         }
 
         *watch = false;
+        Ok(())
     }
 
     fn fake_idle(&self, context: &Context) {
@@ -1148,7 +1117,7 @@ impl Imap {
 
             let watch_folder = self.config.read().unwrap().watch_folder.clone();
             if let Some(watch_folder) = watch_folder {
-                if 0 != self.fetch_from_single_folder(context, watch_folder) {
+                if self.fetch_from_single_folder(context, &watch_folder).is_ok() {
                     do_fake_idle = false;
                 }
             }
@@ -1285,14 +1254,12 @@ impl Imap {
                 return Some(ImapResult::RetryLater);
             }
         }
-        if self.select_folder(context, Some(&folder)) == 0 {
-            warn!(
-                context,
-                "Cannot select folder {} for preparing IMAP operation", folder
-            );
-            Some(ImapResult::RetryLater)
-        } else {
-            None
+        match self.select_folder(context, &folder) {
+            Ok(()) => None,
+            Err(err) => { 
+                warn!(context, "Cannot select folder {}: {}", folder, err);
+                Some(ImapResult::RetryLater)
+            }
         }
     }
 
@@ -1498,28 +1465,17 @@ impl Imap {
         }
     }
 
-    pub fn empty_folder(&self, context: &Context, folder: &str) {
+    pub fn empty_folder(&self, context: &Context, folder: &str) -> Result<(), Error> {
         info!(context, "emptying folder {}", folder);
 
-        if folder.is_empty() || self.select_folder(context, Some(&folder)) == 0 {
-            warn!(context, "Cannot select folder '{}' for emptying", folder);
-            return;
-        }
+        self.select_folder(context, &folder)?;
 
-        if !self.add_flag_finalized_with_set(context, SELECT_ALL, "\\Deleted") {
-            warn!(context, "Cannot empty folder {}", folder);
-        } else {
-            // we now trigger expunge to actually delete messages
-            self.config.write().unwrap().selected_folder_needs_expunge = true;
-            if self.select_folder::<String>(context, None) == 0 {
-                warn!(
-                    context,
-                    "could not perform expunge on empty-marked folder {}", folder
-                );
-            } else {
-                emit_event!(context, Event::ImapFolderEmptied(folder.to_string()));
-            }
-        }
+        ensure!(self.add_flag_finalized_with_set(context, SELECT_ALL, "\\Deleted"),
+                "Could not set \\Deleted flags to empty {}", folder);
+
+        ensure!(self.expunge_folder(context), "Could not expunge");
+        emit_event!(context, Event::ImapFolderEmptied(folder.to_string()));
+        Ok(())
     }
 }
 

src/job.rs

@@ -291,19 +291,20 @@ impl Job {
     }
 
     #[allow(non_snake_case)]
-    fn do_DC_JOB_EMPTY_SERVER(&mut self, context: &Context) {
+    fn do_DC_JOB_EMPTY_SERVER(&mut self, context: &Context) -> Result<(), Error> {
         let inbox = context.inbox.read().unwrap();
         if self.foreign_id & DC_EMPTY_MVBOX > 0 {
             if let Some(mvbox_folder) = context
                 .sql
                 .get_raw_config(context, "configured_mvbox_folder")
             {
-                inbox.empty_folder(context, &mvbox_folder);
+                inbox.empty_folder(context, &mvbox_folder)?;
             }
         }
         if self.foreign_id & DC_EMPTY_INBOX > 0 {
-            inbox.empty_folder(context, "INBOX");
+            inbox.empty_folder(context, "INBOX")?;
         }
+        Ok(())
     }
 
     #[allow(non_snake_case)]
@@ -793,7 +794,14 @@ fn job_perform(context: &Context, thread: Thread, probe_network: bool) {
                     warn!(context, "Unknown job id found");
                 }
                 Action::SendMsgToSmtp => job.do_DC_JOB_SEND(context),
-                Action::EmptyServer => job.do_DC_JOB_EMPTY_SERVER(context),
+                Action::EmptyServer => {
+                    match job.do_DC_JOB_EMPTY_SERVER(context) {
+                        Ok(()) => {},
+                        Err(err) => {
+                            warn!(context, "emptying server folder(s) failed: {}", err);
+                        }
+                    }
+                }
                 Action::DeleteMsgOnImap => job.do_DC_JOB_DELETE_MSG_ON_IMAP(context),
                 Action::MarkseenMsgOnImap => job.do_DC_JOB_MARKSEEN_MSG_ON_IMAP(context),
                 Action::MarkseenMdnOnImap => job.do_DC_JOB_MARKSEEN_MDN_ON_IMAP(context),