From aca0c77887c1741490bd521de0323b4fdbbeea3b Mon Sep 17 00:00:00 2001 From: Simon Laux Date: Mon, 11 Dec 2023 23:21:39 +0100 Subject: [PATCH 01/21] refactor: move convert folder meaning logic in own method also unify the error handling for the cases where it can go wrong. --- src/scheduler.rs | 60 ++++++++++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 25 deletions(-) diff --git a/src/scheduler.rs b/src/scheduler.rs index 7ad7b3c960..8725247195 100644 --- a/src/scheduler.rs +++ b/src/scheduler.rs @@ -477,46 +477,56 @@ async fn inbox_loop( .await; } -/// Implement a single iteration of IMAP loop. -/// -/// This function performs all IMAP operations on a single folder, selecting it if necessary and -/// handling all the errors. In case of an error, it is logged, but not propagated upwards. If -/// critical operation fails such as fetching new messages fails, connection is reset via -/// `trigger_reconnect`, so a fresh one can be opened. -async fn fetch_idle(ctx: &Context, connection: &mut Imap, folder_meaning: FolderMeaning) { +/// Convert folder meaning +/// used internally by [fetch_idle] and [Context::background_fetch] +pub async fn convert_folder_meaning( + ctx: &Context, + folder_meaning: FolderMeaning, +) -> Result<(Config, String)> { let folder_config = match folder_meaning.to_config() { Some(c) => c, None => { - error!(ctx, "Bad folder meaning: {}", folder_meaning); - connection - .fake_idle(ctx, None, FolderMeaning::Unknown) - .await; - return; + bail!("Bad folder meaning: {}", folder_meaning); } }; + let folder = match ctx.get_config(folder_config).await { Ok(folder) => folder, Err(err) => { - warn!( - ctx, - "Can not watch {} folder, failed to retrieve config: {:#}", folder_config, err + bail!( + "Can not watch {} folder, failed to retrieve config: {:#}", + folder_config, + err ); - connection - .fake_idle(ctx, None, FolderMeaning::Unknown) - .await; - return; } }; let watch_folder = if let Some(watch_folder) = folder { watch_folder } else { - connection.connectivity.set_not_configured(ctx).await; - info!(ctx, "Can not watch {} folder, not set", folder_config); - connection - .fake_idle(ctx, None, FolderMeaning::Unknown) - .await; - return; + bail!("Can not watch {} folder, not set", folder_config); + }; + + Ok((folder_config, watch_folder)) +} + +/// Implement a single iteration of IMAP loop. +/// +/// This function performs all IMAP operations on a single folder, selecting it if necessary and +/// handling all the errors. In case of an error, it is logged, but not propagated upwards. If +/// critical operation fails such as fetching new messages fails, connection is reset via +/// `trigger_reconnect`, so a fresh one can be opened. +async fn fetch_idle(ctx: &Context, connection: &mut Imap, folder_meaning: FolderMeaning) { + let (folder_config, watch_folder) = match convert_folder_meaning(ctx, folder_meaning).await { + Ok(meaning) => meaning, + Err(error) => { + error!(ctx, "Error converting IMAP Folder name: {:?}", error); + connection.connectivity.set_not_configured(ctx).await; + connection + .fake_idle(ctx, None, FolderMeaning::Unknown) + .await; + return; + } }; // connect and fake idle if unable to connect From 5bfe3152b7e36eb26375b7e83caa40bdcb82dbb8 Mon Sep 17 00:00:00 2001 From: Simon Laux Date: Tue, 12 Dec 2023 00:03:47 +0100 Subject: [PATCH 02/21] feat: add background fetch method --- src/accounts.rs | 28 ++++++++++++++++++++++++++++ src/context.rs | 33 +++++++++++++++++++++++++++++++-- 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/accounts.rs b/src/accounts.rs index e0d39c9525..0cad4ffee5 100644 --- a/src/accounts.rs +++ b/src/accounts.rs @@ -5,6 +5,7 @@ use std::future::Future; use std::path::{Path, PathBuf}; use anyhow::{ensure, Context as _, Result}; +use futures::future::join_all; use serde::{Deserialize, Serialize}; use tokio::fs; use tokio::io::AsyncWriteExt; @@ -291,6 +292,33 @@ impl Accounts { } } + /// Performs a background fetch for all accounts in parallel. + /// + /// If you need a timeout, then use [Accounts::background_fetch_with_timeout] instead. + pub async fn background_fetch(&self) { + async fn background_fetch_and_log_error(account: Context) { + if let Err(error) = account.background_fetch().await { + warn!(account, "{error:#}"); + } + } + + join_all( + self.accounts + .values() + .cloned() + .map(background_fetch_and_log_error), + ) + .await; + } + + /// Performs a background fetch for all accounts in parallel with a timeout. + /// + /// If you want no timeout, then use [Accounts::background_fetch] instead. + pub async fn background_fetch_with_timeout(&self, timeout: Duration) -> Result<()> { + tokio::time::timeout(timeout, self.background_fetch()).await?; + Ok(()) + } + /// Emits a single event. pub fn emit_event(&self, event: EventType) { self.events.emit(Event { id: 0, typ: event }) diff --git a/src/context.rs b/src/context.rs index cfe7a91182..60f901d8d7 100644 --- a/src/context.rs +++ b/src/context.rs @@ -19,12 +19,12 @@ use crate::constants::DC_VERSION_STR; use crate::contact::Contact; use crate::debug_logging::DebugLogging; use crate::events::{Event, EventEmitter, EventType, Events}; -use crate::imap::ServerMetadata; +use crate::imap::{FolderMeaning, Imap, ServerMetadata}; use crate::key::{load_self_public_key, DcKey as _}; use crate::login_param::LoginParam; use crate::message::{self, MessageState, MsgId}; use crate::quota::QuotaInfo; -use crate::scheduler::SchedulerState; +use crate::scheduler::{convert_folder_meaning, SchedulerState}; use crate::sql::Sql; use crate::stock_str::StockStrings; use crate::timesmearing::SmearedTimestamp; @@ -441,6 +441,35 @@ impl Context { self.scheduler.maybe_network().await; } + /// Do a background fetch + /// pauses the scheduler and does one imap fetch, then unpauses and returns + pub async fn background_fetch(&self) -> Result<()> { + if !(self.is_configured().await?) { + return Ok(()); + } + + let _pause_guard = self.scheduler.pause(self.clone()).await?; + + // connection + let mut connection = Imap::new_configured(self, channel::bounded(1).1).await?; + connection.prepare(self).await?; + + // fetch imap folders + for folder_meaning in [FolderMeaning::Inbox, FolderMeaning::Mvbox] { + let (_, watch_folder) = convert_folder_meaning(self, folder_meaning).await?; + connection + .fetch_move_delete(self, &watch_folder, folder_meaning) + .await?; + } + + // update quota (to send warning if full) + if let Err(err) = self.update_recent_quota(&mut connection).await { + warn!(self, "Failed to update quota: {:#}.", err); + } + + Ok(()) + } + pub(crate) async fn schedule_resync(&self) -> Result<()> { self.resync_request.store(true, Ordering::Relaxed); self.scheduler.interrupt_inbox().await; From 935755cd54d3fcd1fc1ee0ef64dec1ef5f04fa26 Mon Sep 17 00:00:00 2001 From: Simon Laux Date: Tue, 12 Dec 2023 01:26:10 +0100 Subject: [PATCH 03/21] api: jsonrpc: add `background_fetch_for_all_accounts` --- deltachat-jsonrpc/src/api.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/deltachat-jsonrpc/src/api.rs b/deltachat-jsonrpc/src/api.rs index fb8a2166be..749bf0064f 100644 --- a/deltachat-jsonrpc/src/api.rs +++ b/deltachat-jsonrpc/src/api.rs @@ -231,6 +231,16 @@ impl CommandApi { Ok(()) } + /// Performs a background fetch for all accounts in parallel with a timeout. + async fn background_fetch_for_all_accounts(&self, timeout_in_seconds: f64) -> Result<()> { + self.accounts + .write() + .await + .background_fetch_with_timeout(std::time::Duration::from_secs_f64(timeout_in_seconds)) + .await?; + Ok(()) + } + // --------------------------------------------- // Methods that work on individual accounts // --------------------------------------------- From e113ef26938b3cc7a7019ab9749db8f71975d1ad Mon Sep 17 00:00:00 2001 From: Simon Laux Date: Tue, 12 Dec 2023 01:26:36 +0100 Subject: [PATCH 04/21] api: cffi: add `dc_accounts_background_fetch_with_timeout` --- deltachat-ffi/deltachat.h | 14 ++++++++++++++ deltachat-ffi/src/lib.rs | 28 ++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index 97fe035c3a..8ff305b952 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -3151,6 +3151,20 @@ void dc_accounts_maybe_network (dc_accounts_t* accounts); void dc_accounts_maybe_network_lost (dc_accounts_t* accounts); +/** + * Perform a Background fetch for all accounts in parallel with a timeout. + * Pauses the scheduler, fetches messages from imap and then resumes the scheduler. + * + * dc_accounts_background_fetch_with_timeout() was created for the iOS Background fetch. + * + * @memberof dc_accounts_t + * @param timeout The timeout in seconds + * @return Return 1 on success and 0 on failure (like timeout) + * But note that this only indicates that the fetch of all accounts was done before the timeout. + * To know wether it worked you need to look for the events. + */ +int dc_accounts_background_fetch_with_timeout (dc_accounts_t* accounts, uint64_t timeout); + /** * Create the event emitter that is used to receive events. * diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index 5d68a940d4..3793ab5468 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -4898,6 +4898,34 @@ pub unsafe extern "C" fn dc_accounts_maybe_network_lost(accounts: *mut dc_accoun block_on(async move { accounts.write().await.maybe_network_lost().await }); } +#[no_mangle] +pub unsafe extern "C" fn dc_accounts_background_fetch_with_timeout( + accounts: *mut dc_accounts_t, + timeout_in_seconds: u64, +) -> libc::c_int { + if accounts.is_null() || timeout_in_seconds <= 2 { + eprintln!("ignoring careless call to dc_accounts_background_fetch_with_timeout()"); + return 0; + } + + let accounts = &*accounts; + block_on(async move { + let accounts = accounts.write().await; + match accounts + .background_fetch_with_timeout(Duration::from_secs(timeout_in_seconds)) + .await + { + Ok(()) => 1, + Err(err) => { + accounts.emit_event(EventType::Error(format!( + "Failed to do background fetch: {err:#}" + ))); + 0 + } + } + }) +} + #[no_mangle] pub unsafe extern "C" fn dc_accounts_get_event_emitter( accounts: *mut dc_accounts_t, From 002028e85fc8a8d5bbf60cc601a0a7a9870869c2 Mon Sep 17 00:00:00 2001 From: Simon Laux Date: Tue, 12 Dec 2023 01:52:18 +0100 Subject: [PATCH 05/21] Test server has no sentbox folder --- python/tests/test_1_online.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/tests/test_1_online.py b/python/tests/test_1_online.py index ac0fde6856..d79b2b8a17 100644 --- a/python/tests/test_1_online.py +++ b/python/tests/test_1_online.py @@ -381,7 +381,7 @@ def test_webxdc_download_on_demand(acfactory, data, lp): assert msgs_changed_event.data1 == msg2.chat.id assert msgs_changed_event.data2 == 0 - +@pytest.mark.xfail(reason="Test server has no sentbox folder") def test_mvbox_sentbox_threads(acfactory, lp): lp.sec("ac1: start with mvbox thread") ac1 = acfactory.new_online_configuring_account(mvbox_move=True, sentbox_watch=True) From 86fd139837bd46790e9891115b317008939198fe Mon Sep 17 00:00:00 2001 From: Simon Laux Date: Tue, 12 Dec 2023 02:15:57 +0100 Subject: [PATCH 06/21] fix iOS build issue --- src/accounts.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/accounts.rs b/src/accounts.rs index 0cad4ffee5..de4daab57d 100644 --- a/src/accounts.rs +++ b/src/accounts.rs @@ -314,7 +314,7 @@ impl Accounts { /// Performs a background fetch for all accounts in parallel with a timeout. /// /// If you want no timeout, then use [Accounts::background_fetch] instead. - pub async fn background_fetch_with_timeout(&self, timeout: Duration) -> Result<()> { + pub async fn background_fetch_with_timeout(&self, timeout: std::time::Duration) -> Result<()> { tokio::time::timeout(timeout, self.background_fetch()).await?; Ok(()) } From ffe53c45bcf6355b693ee2b640873f9765d3150b Mon Sep 17 00:00:00 2001 From: Simon Laux Date: Tue, 12 Dec 2023 02:53:43 +0100 Subject: [PATCH 07/21] log time that the function took --- src/context.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/context.rs b/src/context.rs index 60f901d8d7..634d53fdb8 100644 --- a/src/context.rs +++ b/src/context.rs @@ -448,6 +448,10 @@ impl Context { return Ok(()); } + let address = self.get_primary_self_addr().await?; + let time_start = std::time::SystemTime::now(); + info!(self, "background_fetch started fetching {address}"); + let _pause_guard = self.scheduler.pause(self.clone()).await?; // connection @@ -467,6 +471,12 @@ impl Context { warn!(self, "Failed to update quota: {:#}.", err); } + info!( + self, + "background_fetch done for {address} took {:?}", + time_start.elapsed().unwrap_or_default() + ); + Ok(()) } From 79119de98337fbc483c6cf7378665b6166be630c Mon Sep 17 00:00:00 2001 From: Simon Laux Date: Tue, 12 Dec 2023 03:08:41 +0100 Subject: [PATCH 08/21] don't hold write lock in cffi (this blocked events) --- deltachat-ffi/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index 3793ab5468..3cb1a7fb17 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -4910,7 +4910,7 @@ pub unsafe extern "C" fn dc_accounts_background_fetch_with_timeout( let accounts = &*accounts; block_on(async move { - let accounts = accounts.write().await; + let accounts = accounts.read().await; match accounts .background_fetch_with_timeout(Duration::from_secs(timeout_in_seconds)) .await From 5c7e2769ff0ba07cec46ae6a5bd113af55145541 Mon Sep 17 00:00:00 2001 From: Simon Laux Date: Tue, 12 Dec 2023 17:59:14 +0100 Subject: [PATCH 09/21] Apply suggestions from code review Co-authored-by: link2xt --- deltachat-ffi/deltachat.h | 4 ++-- src/context.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index 8ff305b952..bcce887764 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -3152,7 +3152,7 @@ void dc_accounts_maybe_network_lost (dc_accounts_t* accounts); /** - * Perform a Background fetch for all accounts in parallel with a timeout. + * Perform a background fetch for all accounts in parallel with a timeout. * Pauses the scheduler, fetches messages from imap and then resumes the scheduler. * * dc_accounts_background_fetch_with_timeout() was created for the iOS Background fetch. @@ -3161,7 +3161,7 @@ void dc_accounts_maybe_network_lost (dc_accounts_t* accounts); * @param timeout The timeout in seconds * @return Return 1 on success and 0 on failure (like timeout) * But note that this only indicates that the fetch of all accounts was done before the timeout. - * To know wether it worked you need to look for the events. + * To know whether it worked you need to look for the events. */ int dc_accounts_background_fetch_with_timeout (dc_accounts_t* accounts, uint64_t timeout); diff --git a/src/context.rs b/src/context.rs index 634d53fdb8..9ed75e231f 100644 --- a/src/context.rs +++ b/src/context.rs @@ -441,7 +441,7 @@ impl Context { self.scheduler.maybe_network().await; } - /// Do a background fetch + /// Does a background fetch /// pauses the scheduler and does one imap fetch, then unpauses and returns pub async fn background_fetch(&self) -> Result<()> { if !(self.is_configured().await?) { @@ -468,7 +468,7 @@ impl Context { // update quota (to send warning if full) if let Err(err) = self.update_recent_quota(&mut connection).await { - warn!(self, "Failed to update quota: {:#}.", err); + warn!(self, "Failed to update quota: {err:#}."); } info!( From baef362af95d677cc1a7df7f880ee564a847fd37 Mon Sep 17 00:00:00 2001 From: Simon Laux Date: Tue, 12 Dec 2023 18:00:26 +0100 Subject: [PATCH 10/21] rename cffi function --- deltachat-ffi/deltachat.h | 4 ++-- deltachat-ffi/src/lib.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index bcce887764..6f94fc5fd7 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -3155,7 +3155,7 @@ void dc_accounts_maybe_network_lost (dc_accounts_t* accounts); * Perform a background fetch for all accounts in parallel with a timeout. * Pauses the scheduler, fetches messages from imap and then resumes the scheduler. * - * dc_accounts_background_fetch_with_timeout() was created for the iOS Background fetch. + * dc_accounts_background_fetch() was created for the iOS Background fetch. * * @memberof dc_accounts_t * @param timeout The timeout in seconds @@ -3163,7 +3163,7 @@ void dc_accounts_maybe_network_lost (dc_accounts_t* accounts); * But note that this only indicates that the fetch of all accounts was done before the timeout. * To know whether it worked you need to look for the events. */ -int dc_accounts_background_fetch_with_timeout (dc_accounts_t* accounts, uint64_t timeout); +int dc_accounts_background_fetch (dc_accounts_t* accounts, uint64_t timeout); /** * Create the event emitter that is used to receive events. diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index 3cb1a7fb17..2ffe634ae0 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -4899,12 +4899,12 @@ pub unsafe extern "C" fn dc_accounts_maybe_network_lost(accounts: *mut dc_accoun } #[no_mangle] -pub unsafe extern "C" fn dc_accounts_background_fetch_with_timeout( +pub unsafe extern "C" fn dc_accounts_background_fetch( accounts: *mut dc_accounts_t, timeout_in_seconds: u64, ) -> libc::c_int { if accounts.is_null() || timeout_in_seconds <= 2 { - eprintln!("ignoring careless call to dc_accounts_background_fetch_with_timeout()"); + eprintln!("ignoring careless call to dc_accounts_background_fetch()"); return 0; } From a6101b7cd9c9dba353711106956ae997a1567063 Mon Sep 17 00:00:00 2001 From: Simon Laux Date: Thu, 4 Jan 2024 06:53:46 +0100 Subject: [PATCH 11/21] add rate limit for quota check in background fetch (12h for now) --- src/constants.rs | 3 +++ src/context.rs | 18 ++++++++++++++---- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/constants.rs b/src/constants.rs index ac3c281ed0..61bc4909cb 100644 --- a/src/constants.rs +++ b/src/constants.rs @@ -214,6 +214,9 @@ pub(crate) const DC_FOLDERS_CONFIGURED_VERSION: i32 = 4; // `max_smtp_rcpt_to` in the provider db. pub(crate) const DEFAULT_MAX_SMTP_RCPT_TO: usize = 50; +/// How far the last quota check needs to be in the past to be checked by the background function (in seconds). +pub(crate) const DC_BACKGROUND_FETCH_QUOTA_CHECK_RATELIMIT: i64 = 12 * 60 * 60; // 12 hours + #[cfg(test)] mod tests { use num_traits::FromPrimitive; diff --git a/src/context.rs b/src/context.rs index 9ed75e231f..38bf11e0d8 100644 --- a/src/context.rs +++ b/src/context.rs @@ -15,7 +15,7 @@ use tokio::sync::{Mutex, Notify, RwLock}; use crate::chat::{get_chat_cnt, ChatId}; use crate::config::Config; -use crate::constants::DC_VERSION_STR; +use crate::constants::{DC_BACKGROUND_FETCH_QUOTA_CHECK_RATELIMIT, DC_VERSION_STR}; use crate::contact::Contact; use crate::debug_logging::DebugLogging; use crate::events::{Event, EventEmitter, EventType, Events}; @@ -466,9 +466,19 @@ impl Context { .await?; } - // update quota (to send warning if full) - if let Err(err) = self.update_recent_quota(&mut connection).await { - warn!(self, "Failed to update quota: {err:#}."); + // update quota (to send warning if full) - but only check it once in a while + let quota_needs_update = { + let quota = self.quota.read().await; + quota + .as_ref() + .filter(|quota| quota.modified + DC_BACKGROUND_FETCH_QUOTA_CHECK_RATELIMIT > time()) + .is_none() + }; + + if quota_needs_update { + if let Err(err) = self.update_recent_quota(&mut connection).await { + warn!(self, "Failed to update quota: {err:#}."); + } } info!( From 1cbe8abcf17a6e539b4b84724344b86c03cb10aa Mon Sep 17 00:00:00 2001 From: Simon Laux Date: Thu, 4 Jan 2024 07:27:08 +0100 Subject: [PATCH 12/21] BackgroundFetchCompletedForAllAccounts event --- deltachat-ffi/deltachat.h | 10 ++++++++++ deltachat-ffi/src/lib.rs | 6 +++++- deltachat-jsonrpc/src/api/types/events.rs | 10 ++++++++++ node/constants.js | 1 + node/events.js | 3 ++- node/lib/constants.ts | 2 ++ src/accounts.rs | 7 +++++-- src/events/payload.rs | 7 +++++++ 8 files changed, 42 insertions(+), 4 deletions(-) diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index 6f94fc5fd7..48d9a60de1 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -6269,6 +6269,16 @@ void dc_event_unref(dc_event_t* event); #define DC_EVENT_WEBXDC_INSTANCE_DELETED 2121 +/** + * Tells that the Background fetch was completed (or timed out). + * + * This event acts as a marker, when you reach this event you can be sure + * that all events emitted during the background fetch were processed. + * + * This event is only emitted by the account manager + */ + +#define DC_EVENT_BACKGROUND_FETCH_COMPLETED_FOR_ALL_ACCOUNTS 2200 /** * @} diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index 2ffe634ae0..c52404cc42 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -559,6 +559,7 @@ pub unsafe extern "C" fn dc_event_get_id(event: *mut dc_event_t) -> libc::c_int EventType::ConfigSynced { .. } => 2111, EventType::WebxdcStatusUpdate { .. } => 2120, EventType::WebxdcInstanceDeleted { .. } => 2121, + EventType::BackgroundFetchCompletedForAllAccounts => 2200, } } @@ -586,7 +587,8 @@ pub unsafe extern "C" fn dc_event_get_data1_int(event: *mut dc_event_t) -> libc: | EventType::SelfavatarChanged | EventType::ConfigSynced { .. } | EventType::IncomingMsgBunch { .. } - | EventType::ErrorSelfNotInGroup(_) => 0, + | EventType::ErrorSelfNotInGroup(_) + | EventType::BackgroundFetchCompletedForAllAccounts => 0, EventType::MsgsChanged { chat_id, .. } | EventType::ReactionsChanged { chat_id, .. } | EventType::IncomingMsg { chat_id, .. } @@ -646,6 +648,7 @@ pub unsafe extern "C" fn dc_event_get_data2_int(event: *mut dc_event_t) -> libc: | EventType::WebxdcInstanceDeleted { .. } | EventType::IncomingMsgBunch { .. } | EventType::SelfavatarChanged + | EventType::BackgroundFetchCompletedForAllAccounts | EventType::ConfigSynced { .. } => 0, EventType::ChatModified(_) => 0, EventType::MsgsChanged { msg_id, .. } @@ -708,6 +711,7 @@ pub unsafe extern "C" fn dc_event_get_data2_str(event: *mut dc_event_t) -> *mut | EventType::SelfavatarChanged | EventType::WebxdcStatusUpdate { .. } | EventType::WebxdcInstanceDeleted { .. } + | EventType::BackgroundFetchCompletedForAllAccounts | EventType::ChatEphemeralTimerModified { .. } => ptr::null_mut(), EventType::ConfigureProgress { comment, .. } => { if let Some(comment) = comment { diff --git a/deltachat-jsonrpc/src/api/types/events.rs b/deltachat-jsonrpc/src/api/types/events.rs index c1feee63c7..a92ef2e038 100644 --- a/deltachat-jsonrpc/src/api/types/events.rs +++ b/deltachat-jsonrpc/src/api/types/events.rs @@ -245,6 +245,13 @@ pub enum EventType { /// Inform that a message containing a webxdc instance has been deleted #[serde(rename_all = "camelCase")] WebxdcInstanceDeleted { msg_id: u32 }, + + /// Tells that the Background fetch was completed (or timed out). + /// This event acts as a marker, when you reach this event you can be sure + /// that all events emitted during the background fetch were processed. + /// + /// This event is only emitted by the account manager + BackgroundFetchCompletedForAllAccounts, } impl From for EventType { @@ -353,6 +360,9 @@ impl From for EventType { CoreEventType::WebxdcInstanceDeleted { msg_id } => WebxdcInstanceDeleted { msg_id: msg_id.to_u32(), }, + CoreEventType::BackgroundFetchCompletedForAllAccounts => { + BackgroundFetchCompletedForAllAccounts + } } } } diff --git a/node/constants.js b/node/constants.js index 7c50ca55db..d980baf2d8 100644 --- a/node/constants.js +++ b/node/constants.js @@ -29,6 +29,7 @@ module.exports = { DC_DOWNLOAD_FAILURE: 20, DC_DOWNLOAD_IN_PROGRESS: 1000, DC_DOWNLOAD_UNDECIPHERABLE: 30, + DC_EVENT_BACKGROUND_FETCH_COMPLETED_FOR_ALL_ACCOUNTS: 2200, DC_EVENT_CHAT_EPHEMERAL_TIMER_MODIFIED: 2021, DC_EVENT_CHAT_MODIFIED: 2020, DC_EVENT_CONFIGURE_PROGRESS: 2041, diff --git a/node/events.js b/node/events.js index a618d4599d..b418bf6abf 100644 --- a/node/events.js +++ b/node/events.js @@ -36,5 +36,6 @@ module.exports = { 2110: 'DC_EVENT_SELFAVATAR_CHANGED', 2111: 'DC_EVENT_CONFIG_SYNCED', 2120: 'DC_EVENT_WEBXDC_STATUS_UPDATE', - 2121: 'DC_EVENT_WEBXDC_INSTANCE_DELETED' + 2121: 'DC_EVENT_WEBXDC_INSTANCE_DELETED', + 2200: 'DC_EVENT_BACKGROUND_FETCH_COMPLETED_FOR_ALL_ACCOUNTS' } diff --git a/node/lib/constants.ts b/node/lib/constants.ts index 9eba1b4b60..5c4c3b42da 100644 --- a/node/lib/constants.ts +++ b/node/lib/constants.ts @@ -29,6 +29,7 @@ export enum C { DC_DOWNLOAD_FAILURE = 20, DC_DOWNLOAD_IN_PROGRESS = 1000, DC_DOWNLOAD_UNDECIPHERABLE = 30, + DC_EVENT_BACKGROUND_FETCH_COMPLETED_FOR_ALL_ACCOUNTS = 2200, DC_EVENT_CHAT_EPHEMERAL_TIMER_MODIFIED = 2021, DC_EVENT_CHAT_MODIFIED = 2020, DC_EVENT_CONFIGURE_PROGRESS = 2041, @@ -326,4 +327,5 @@ export const EventId2EventName: { [key: number]: string } = { 2111: 'DC_EVENT_CONFIG_SYNCED', 2120: 'DC_EVENT_WEBXDC_STATUS_UPDATE', 2121: 'DC_EVENT_WEBXDC_INSTANCE_DELETED', + 2200: 'DC_EVENT_BACKGROUND_FETCH_COMPLETED_FOR_ALL_ACCOUNTS', } diff --git a/src/accounts.rs b/src/accounts.rs index de4daab57d..c3977bbeb4 100644 --- a/src/accounts.rs +++ b/src/accounts.rs @@ -309,14 +309,17 @@ impl Accounts { .map(background_fetch_and_log_error), ) .await; + + self.emit_event(EventType::BackgroundFetchCompletedForAllAccounts); } /// Performs a background fetch for all accounts in parallel with a timeout. /// /// If you want no timeout, then use [Accounts::background_fetch] instead. pub async fn background_fetch_with_timeout(&self, timeout: std::time::Duration) -> Result<()> { - tokio::time::timeout(timeout, self.background_fetch()).await?; - Ok(()) + let result = tokio::time::timeout(timeout, self.background_fetch()).await; + self.emit_event(EventType::BackgroundFetchCompletedForAllAccounts); + result.map_err(|err| err.into()) } /// Emits a single event. diff --git a/src/events/payload.rs b/src/events/payload.rs index d2cf3f4768..43c1673629 100644 --- a/src/events/payload.rs +++ b/src/events/payload.rs @@ -287,4 +287,11 @@ pub enum EventType { /// ID of the deleted message. msg_id: MsgId, }, + + /// Tells that the Background fetch was completed (or timed out). + /// This event acts as a marker, when you reach this event you can be sure + /// that all events emitted during the background fetch were processed. + /// + /// This event is only emitted by the account manager + BackgroundFetchCompletedForAllAccounts, } From cc043caaefa2cafb1e74f314da90e80b3c5d2a67 Mon Sep 17 00:00:00 2001 From: Simon Laux Date: Fri, 12 Jan 2024 21:20:43 +0100 Subject: [PATCH 13/21] rename event and mention event in method documentation --- deltachat-ffi/deltachat.h | 6 +++++- deltachat-jsonrpc/src/api.rs | 4 ++++ node/lib/constants.ts | 4 ++-- src/accounts.rs | 8 ++++++++ 4 files changed, 19 insertions(+), 3 deletions(-) diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index 48d9a60de1..76d7dfb002 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -3157,6 +3157,10 @@ void dc_accounts_maybe_network_lost (dc_accounts_t* accounts); * * dc_accounts_background_fetch() was created for the iOS Background fetch. * + * The `DC_EVENT_ACCOUNTS_BACKGROUND_FETCH_DONE` event is emitted at the end, + * process all events until you get this one and you can safely return to the background + * without forgeting to create notifications caused by timing race conditions. + * * @memberof dc_accounts_t * @param timeout The timeout in seconds * @return Return 1 on success and 0 on failure (like timeout) @@ -6278,7 +6282,7 @@ void dc_event_unref(dc_event_t* event); * This event is only emitted by the account manager */ -#define DC_EVENT_BACKGROUND_FETCH_COMPLETED_FOR_ALL_ACCOUNTS 2200 +#define DC_EVENT_ACCOUNTS_BACKGROUND_FETCH_DONE 2200 /** * @} diff --git a/deltachat-jsonrpc/src/api.rs b/deltachat-jsonrpc/src/api.rs index 749bf0064f..48a1e42a99 100644 --- a/deltachat-jsonrpc/src/api.rs +++ b/deltachat-jsonrpc/src/api.rs @@ -232,6 +232,10 @@ impl CommandApi { } /// Performs a background fetch for all accounts in parallel with a timeout. + /// + /// The `BackgroundFetchCompletedForAllAccounts` event is emitted at the end, + /// process all events until you get this one and you can safely return to the background + /// without forgeting to create notifications caused by timing race conditions. async fn background_fetch_for_all_accounts(&self, timeout_in_seconds: f64) -> Result<()> { self.accounts .write() diff --git a/node/lib/constants.ts b/node/lib/constants.ts index 5c4c3b42da..2fe4f9af50 100644 --- a/node/lib/constants.ts +++ b/node/lib/constants.ts @@ -29,7 +29,7 @@ export enum C { DC_DOWNLOAD_FAILURE = 20, DC_DOWNLOAD_IN_PROGRESS = 1000, DC_DOWNLOAD_UNDECIPHERABLE = 30, - DC_EVENT_BACKGROUND_FETCH_COMPLETED_FOR_ALL_ACCOUNTS = 2200, + DC_EVENT_ACCOUNTS_BACKGROUND_FETCH_DONE = 2200, DC_EVENT_CHAT_EPHEMERAL_TIMER_MODIFIED = 2021, DC_EVENT_CHAT_MODIFIED = 2020, DC_EVENT_CONFIGURE_PROGRESS = 2041, @@ -327,5 +327,5 @@ export const EventId2EventName: { [key: number]: string } = { 2111: 'DC_EVENT_CONFIG_SYNCED', 2120: 'DC_EVENT_WEBXDC_STATUS_UPDATE', 2121: 'DC_EVENT_WEBXDC_INSTANCE_DELETED', - 2200: 'DC_EVENT_BACKGROUND_FETCH_COMPLETED_FOR_ALL_ACCOUNTS', + 2200: 'DC_EVENT_ACCOUNTS_BACKGROUND_FETCH_DONE', } diff --git a/src/accounts.rs b/src/accounts.rs index c3977bbeb4..9ea09d5f34 100644 --- a/src/accounts.rs +++ b/src/accounts.rs @@ -295,6 +295,10 @@ impl Accounts { /// Performs a background fetch for all accounts in parallel. /// /// If you need a timeout, then use [Accounts::background_fetch_with_timeout] instead. + /// + /// The `BackgroundFetchCompletedForAllAccounts` event is emitted at the end, + /// process all events until you get this one and you can safely return to the background + /// without forgeting to create notifications caused by timing race conditions. pub async fn background_fetch(&self) { async fn background_fetch_and_log_error(account: Context) { if let Err(error) = account.background_fetch().await { @@ -316,6 +320,10 @@ impl Accounts { /// Performs a background fetch for all accounts in parallel with a timeout. /// /// If you want no timeout, then use [Accounts::background_fetch] instead. + /// + /// The `BackgroundFetchCompletedForAllAccounts` event is emitted at the end, + /// process all events until you get this one and you can safely return to the background + /// without forgeting to create notifications caused by timing race conditions. pub async fn background_fetch_with_timeout(&self, timeout: std::time::Duration) -> Result<()> { let result = tokio::time::timeout(timeout, self.background_fetch()).await; self.emit_event(EventType::BackgroundFetchCompletedForAllAccounts); From 02090a8a29aa9d5fe76c34e46589a43a287c3458 Mon Sep 17 00:00:00 2001 From: Simon Laux Date: Fri, 12 Jan 2024 21:22:56 +0100 Subject: [PATCH 14/21] cargo fmt --- deltachat-jsonrpc/src/api.rs | 2 +- src/accounts.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/deltachat-jsonrpc/src/api.rs b/deltachat-jsonrpc/src/api.rs index 48a1e42a99..4d009f6363 100644 --- a/deltachat-jsonrpc/src/api.rs +++ b/deltachat-jsonrpc/src/api.rs @@ -232,7 +232,7 @@ impl CommandApi { } /// Performs a background fetch for all accounts in parallel with a timeout. - /// + /// /// The `BackgroundFetchCompletedForAllAccounts` event is emitted at the end, /// process all events until you get this one and you can safely return to the background /// without forgeting to create notifications caused by timing race conditions. diff --git a/src/accounts.rs b/src/accounts.rs index 9ea09d5f34..11c1db6412 100644 --- a/src/accounts.rs +++ b/src/accounts.rs @@ -295,7 +295,7 @@ impl Accounts { /// Performs a background fetch for all accounts in parallel. /// /// If you need a timeout, then use [Accounts::background_fetch_with_timeout] instead. - /// + /// /// The `BackgroundFetchCompletedForAllAccounts` event is emitted at the end, /// process all events until you get this one and you can safely return to the background /// without forgeting to create notifications caused by timing race conditions. @@ -320,7 +320,7 @@ impl Accounts { /// Performs a background fetch for all accounts in parallel with a timeout. /// /// If you want no timeout, then use [Accounts::background_fetch] instead. - /// + /// /// The `BackgroundFetchCompletedForAllAccounts` event is emitted at the end, /// process all events until you get this one and you can safely return to the background /// without forgeting to create notifications caused by timing race conditions. From 72fda7508c91a4dcafe5aedd2f57a30039c6f5e8 Mon Sep 17 00:00:00 2001 From: Simon Laux Date: Sat, 13 Jan 2024 11:40:02 +0100 Subject: [PATCH 15/21] rename event also in core --- deltachat-ffi/src/lib.rs | 8 ++++---- deltachat-jsonrpc/src/api.rs | 2 +- deltachat-jsonrpc/src/api/types/events.rs | 6 ++---- src/accounts.rs | 8 ++++---- src/events/payload.rs | 2 +- 5 files changed, 12 insertions(+), 14 deletions(-) diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index c52404cc42..5ecb317098 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -559,7 +559,7 @@ pub unsafe extern "C" fn dc_event_get_id(event: *mut dc_event_t) -> libc::c_int EventType::ConfigSynced { .. } => 2111, EventType::WebxdcStatusUpdate { .. } => 2120, EventType::WebxdcInstanceDeleted { .. } => 2121, - EventType::BackgroundFetchCompletedForAllAccounts => 2200, + EventType::AccountsBackgroundFetchDone => 2200, } } @@ -588,7 +588,7 @@ pub unsafe extern "C" fn dc_event_get_data1_int(event: *mut dc_event_t) -> libc: | EventType::ConfigSynced { .. } | EventType::IncomingMsgBunch { .. } | EventType::ErrorSelfNotInGroup(_) - | EventType::BackgroundFetchCompletedForAllAccounts => 0, + | EventType::AccountsBackgroundFetchDone => 0, EventType::MsgsChanged { chat_id, .. } | EventType::ReactionsChanged { chat_id, .. } | EventType::IncomingMsg { chat_id, .. } @@ -648,7 +648,7 @@ pub unsafe extern "C" fn dc_event_get_data2_int(event: *mut dc_event_t) -> libc: | EventType::WebxdcInstanceDeleted { .. } | EventType::IncomingMsgBunch { .. } | EventType::SelfavatarChanged - | EventType::BackgroundFetchCompletedForAllAccounts + | EventType::AccountsBackgroundFetchDone | EventType::ConfigSynced { .. } => 0, EventType::ChatModified(_) => 0, EventType::MsgsChanged { msg_id, .. } @@ -711,7 +711,7 @@ pub unsafe extern "C" fn dc_event_get_data2_str(event: *mut dc_event_t) -> *mut | EventType::SelfavatarChanged | EventType::WebxdcStatusUpdate { .. } | EventType::WebxdcInstanceDeleted { .. } - | EventType::BackgroundFetchCompletedForAllAccounts + | EventType::AccountsBackgroundFetchDone | EventType::ChatEphemeralTimerModified { .. } => ptr::null_mut(), EventType::ConfigureProgress { comment, .. } => { if let Some(comment) = comment { diff --git a/deltachat-jsonrpc/src/api.rs b/deltachat-jsonrpc/src/api.rs index 4d009f6363..f8e46c50f0 100644 --- a/deltachat-jsonrpc/src/api.rs +++ b/deltachat-jsonrpc/src/api.rs @@ -233,7 +233,7 @@ impl CommandApi { /// Performs a background fetch for all accounts in parallel with a timeout. /// - /// The `BackgroundFetchCompletedForAllAccounts` event is emitted at the end, + /// The `AccountsBackgroundFetchDone` event is emitted at the end, /// process all events until you get this one and you can safely return to the background /// without forgeting to create notifications caused by timing race conditions. async fn background_fetch_for_all_accounts(&self, timeout_in_seconds: f64) -> Result<()> { diff --git a/deltachat-jsonrpc/src/api/types/events.rs b/deltachat-jsonrpc/src/api/types/events.rs index a92ef2e038..e6f441f840 100644 --- a/deltachat-jsonrpc/src/api/types/events.rs +++ b/deltachat-jsonrpc/src/api/types/events.rs @@ -251,7 +251,7 @@ pub enum EventType { /// that all events emitted during the background fetch were processed. /// /// This event is only emitted by the account manager - BackgroundFetchCompletedForAllAccounts, + AccountsBackgroundFetchDone, } impl From for EventType { @@ -360,9 +360,7 @@ impl From for EventType { CoreEventType::WebxdcInstanceDeleted { msg_id } => WebxdcInstanceDeleted { msg_id: msg_id.to_u32(), }, - CoreEventType::BackgroundFetchCompletedForAllAccounts => { - BackgroundFetchCompletedForAllAccounts - } + CoreEventType::AccountsBackgroundFetchDone => AccountsBackgroundFetchDone, } } } diff --git a/src/accounts.rs b/src/accounts.rs index 11c1db6412..41ca2f4360 100644 --- a/src/accounts.rs +++ b/src/accounts.rs @@ -296,7 +296,7 @@ impl Accounts { /// /// If you need a timeout, then use [Accounts::background_fetch_with_timeout] instead. /// - /// The `BackgroundFetchCompletedForAllAccounts` event is emitted at the end, + /// The `AccountsBackgroundFetchDone` event is emitted at the end, /// process all events until you get this one and you can safely return to the background /// without forgeting to create notifications caused by timing race conditions. pub async fn background_fetch(&self) { @@ -314,19 +314,19 @@ impl Accounts { ) .await; - self.emit_event(EventType::BackgroundFetchCompletedForAllAccounts); + self.emit_event(EventType::AccountsBackgroundFetchDone); } /// Performs a background fetch for all accounts in parallel with a timeout. /// /// If you want no timeout, then use [Accounts::background_fetch] instead. /// - /// The `BackgroundFetchCompletedForAllAccounts` event is emitted at the end, + /// The `AccountsBackgroundFetchDone` event is emitted at the end, /// process all events until you get this one and you can safely return to the background /// without forgeting to create notifications caused by timing race conditions. pub async fn background_fetch_with_timeout(&self, timeout: std::time::Duration) -> Result<()> { let result = tokio::time::timeout(timeout, self.background_fetch()).await; - self.emit_event(EventType::BackgroundFetchCompletedForAllAccounts); + self.emit_event(EventType::AccountsBackgroundFetchDone); result.map_err(|err| err.into()) } diff --git a/src/events/payload.rs b/src/events/payload.rs index 43c1673629..771088ab11 100644 --- a/src/events/payload.rs +++ b/src/events/payload.rs @@ -293,5 +293,5 @@ pub enum EventType { /// that all events emitted during the background fetch were processed. /// /// This event is only emitted by the account manager - BackgroundFetchCompletedForAllAccounts, + AccountsBackgroundFetchDone, } From 74e0631a54309b162daa0aa64ee0a120f2d52062 Mon Sep 17 00:00:00 2001 From: link2xt Date: Sun, 14 Jan 2024 02:43:24 +0000 Subject: [PATCH 16/21] fix: do not log error if watched folder is not configured This may happen if Sent folder does not exist but configuration option to watch it is enabled. --- python/tests/test_1_online.py | 2 +- src/scheduler.rs | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/python/tests/test_1_online.py b/python/tests/test_1_online.py index d79b2b8a17..ac0fde6856 100644 --- a/python/tests/test_1_online.py +++ b/python/tests/test_1_online.py @@ -381,7 +381,7 @@ def test_webxdc_download_on_demand(acfactory, data, lp): assert msgs_changed_event.data1 == msg2.chat.id assert msgs_changed_event.data2 == 0 -@pytest.mark.xfail(reason="Test server has no sentbox folder") + def test_mvbox_sentbox_threads(acfactory, lp): lp.sec("ac1: start with mvbox thread") ac1 = acfactory.new_online_configuring_account(mvbox_move=True, sentbox_watch=True) diff --git a/src/scheduler.rs b/src/scheduler.rs index 8725247195..9a8b651ea1 100644 --- a/src/scheduler.rs +++ b/src/scheduler.rs @@ -520,7 +520,10 @@ async fn fetch_idle(ctx: &Context, connection: &mut Imap, folder_meaning: Folder let (folder_config, watch_folder) = match convert_folder_meaning(ctx, folder_meaning).await { Ok(meaning) => meaning, Err(error) => { - error!(ctx, "Error converting IMAP Folder name: {:?}", error); + // Warning instead of error because the folder may not be configured. + // For example, this happens if the server does not have Sent folder + // but watching Sent folder is enabled. + warn!(ctx, "Error converting IMAP Folder name: {:?}", error); connection.connectivity.set_not_configured(ctx).await; connection .fake_idle(ctx, None, FolderMeaning::Unknown) From 02f6b9d453b60122f572203cdbfacb034e471827 Mon Sep 17 00:00:00 2001 From: Simon Laux Date: Sun, 14 Jan 2024 21:19:21 +0100 Subject: [PATCH 17/21] Apply suggestions from code review Co-authored-by: bjoern --- deltachat-jsonrpc/src/api.rs | 2 +- node/constants.js | 2 +- node/events.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/deltachat-jsonrpc/src/api.rs b/deltachat-jsonrpc/src/api.rs index f8e46c50f0..c034002656 100644 --- a/deltachat-jsonrpc/src/api.rs +++ b/deltachat-jsonrpc/src/api.rs @@ -236,7 +236,7 @@ impl CommandApi { /// The `AccountsBackgroundFetchDone` event is emitted at the end, /// process all events until you get this one and you can safely return to the background /// without forgeting to create notifications caused by timing race conditions. - async fn background_fetch_for_all_accounts(&self, timeout_in_seconds: f64) -> Result<()> { + async fn accounts_background_fetch(&self, timeout_in_seconds: f64) -> Result<()> { self.accounts .write() .await diff --git a/node/constants.js b/node/constants.js index d980baf2d8..59031ee1d8 100644 --- a/node/constants.js +++ b/node/constants.js @@ -29,7 +29,7 @@ module.exports = { DC_DOWNLOAD_FAILURE: 20, DC_DOWNLOAD_IN_PROGRESS: 1000, DC_DOWNLOAD_UNDECIPHERABLE: 30, - DC_EVENT_BACKGROUND_FETCH_COMPLETED_FOR_ALL_ACCOUNTS: 2200, + DC_EVENT_ACCOUNTS_BACKGROUND_FETCH_DONE: 2200, DC_EVENT_CHAT_EPHEMERAL_TIMER_MODIFIED: 2021, DC_EVENT_CHAT_MODIFIED: 2020, DC_EVENT_CONFIGURE_PROGRESS: 2041, diff --git a/node/events.js b/node/events.js index b418bf6abf..a5c9281b98 100644 --- a/node/events.js +++ b/node/events.js @@ -37,5 +37,5 @@ module.exports = { 2111: 'DC_EVENT_CONFIG_SYNCED', 2120: 'DC_EVENT_WEBXDC_STATUS_UPDATE', 2121: 'DC_EVENT_WEBXDC_INSTANCE_DELETED', - 2200: 'DC_EVENT_BACKGROUND_FETCH_COMPLETED_FOR_ALL_ACCOUNTS' + 2200: 'DC_EVENT_ACCOUNTS_BACKGROUND_FETCH_DONE' } From ec92b027b2a56b41d2ecb1aa28e04df48445f9e1 Mon Sep 17 00:00:00 2001 From: link2xt Date: Mon, 29 Jan 2024 23:29:57 +0000 Subject: [PATCH 18/21] Hide `background_fetch_without_timeout` from public API --- deltachat-ffi/src/lib.rs | 2 +- deltachat-jsonrpc/src/api.rs | 2 +- src/accounts.rs | 17 +++++------------ 3 files changed, 7 insertions(+), 14 deletions(-) diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index 5ecb317098..32c0a3ba45 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -4916,7 +4916,7 @@ pub unsafe extern "C" fn dc_accounts_background_fetch( block_on(async move { let accounts = accounts.read().await; match accounts - .background_fetch_with_timeout(Duration::from_secs(timeout_in_seconds)) + .background_fetch(Duration::from_secs(timeout_in_seconds)) .await { Ok(()) => 1, diff --git a/deltachat-jsonrpc/src/api.rs b/deltachat-jsonrpc/src/api.rs index c034002656..5b98bf12a5 100644 --- a/deltachat-jsonrpc/src/api.rs +++ b/deltachat-jsonrpc/src/api.rs @@ -240,7 +240,7 @@ impl CommandApi { self.accounts .write() .await - .background_fetch_with_timeout(std::time::Duration::from_secs_f64(timeout_in_seconds)) + .background_fetch(std::time::Duration::from_secs_f64(timeout_in_seconds)) .await?; Ok(()) } diff --git a/src/accounts.rs b/src/accounts.rs index 41ca2f4360..3454c637ae 100644 --- a/src/accounts.rs +++ b/src/accounts.rs @@ -294,12 +294,9 @@ impl Accounts { /// Performs a background fetch for all accounts in parallel. /// - /// If you need a timeout, then use [Accounts::background_fetch_with_timeout] instead. - /// - /// The `AccountsBackgroundFetchDone` event is emitted at the end, - /// process all events until you get this one and you can safely return to the background - /// without forgeting to create notifications caused by timing race conditions. - pub async fn background_fetch(&self) { + /// This is an auxiliary function and not part of public API. + /// Use [Accounts::background_fetch] instead. + async fn background_fetch_without_timeout(&self) { async fn background_fetch_and_log_error(account: Context) { if let Err(error) = account.background_fetch().await { warn!(account, "{error:#}"); @@ -313,19 +310,15 @@ impl Accounts { .map(background_fetch_and_log_error), ) .await; - - self.emit_event(EventType::AccountsBackgroundFetchDone); } /// Performs a background fetch for all accounts in parallel with a timeout. /// - /// If you want no timeout, then use [Accounts::background_fetch] instead. - /// /// The `AccountsBackgroundFetchDone` event is emitted at the end, /// process all events until you get this one and you can safely return to the background /// without forgeting to create notifications caused by timing race conditions. - pub async fn background_fetch_with_timeout(&self, timeout: std::time::Duration) -> Result<()> { - let result = tokio::time::timeout(timeout, self.background_fetch()).await; + pub async fn background_fetch(&self, timeout: std::time::Duration) -> Result<()> { + let result = tokio::time::timeout(timeout, self.background_fetch_without_timeout()).await; self.emit_event(EventType::AccountsBackgroundFetchDone); result.map_err(|err| err.into()) } From a40d426f8a8288b4ef4f8da14cf14c394c1ba3c6 Mon Sep 17 00:00:00 2001 From: link2xt Date: Tue, 30 Jan 2024 00:17:34 +0000 Subject: [PATCH 19/21] s/forgeting/forgetting/ --- deltachat-ffi/deltachat.h | 2 +- deltachat-jsonrpc/src/api.rs | 2 +- src/accounts.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index 76d7dfb002..c636521d78 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -3159,7 +3159,7 @@ void dc_accounts_maybe_network_lost (dc_accounts_t* accounts); * * The `DC_EVENT_ACCOUNTS_BACKGROUND_FETCH_DONE` event is emitted at the end, * process all events until you get this one and you can safely return to the background - * without forgeting to create notifications caused by timing race conditions. + * without forgetting to create notifications caused by timing race conditions. * * @memberof dc_accounts_t * @param timeout The timeout in seconds diff --git a/deltachat-jsonrpc/src/api.rs b/deltachat-jsonrpc/src/api.rs index 5b98bf12a5..c84e95d4b4 100644 --- a/deltachat-jsonrpc/src/api.rs +++ b/deltachat-jsonrpc/src/api.rs @@ -235,7 +235,7 @@ impl CommandApi { /// /// The `AccountsBackgroundFetchDone` event is emitted at the end, /// process all events until you get this one and you can safely return to the background - /// without forgeting to create notifications caused by timing race conditions. + /// without forgetting to create notifications caused by timing race conditions. async fn accounts_background_fetch(&self, timeout_in_seconds: f64) -> Result<()> { self.accounts .write() diff --git a/src/accounts.rs b/src/accounts.rs index 3454c637ae..8c4a73a641 100644 --- a/src/accounts.rs +++ b/src/accounts.rs @@ -316,7 +316,7 @@ impl Accounts { /// /// The `AccountsBackgroundFetchDone` event is emitted at the end, /// process all events until you get this one and you can safely return to the background - /// without forgeting to create notifications caused by timing race conditions. + /// without forgetting to create notifications caused by timing race conditions. pub async fn background_fetch(&self, timeout: std::time::Duration) -> Result<()> { let result = tokio::time::timeout(timeout, self.background_fetch_without_timeout()).await; self.emit_event(EventType::AccountsBackgroundFetchDone); From fdba0b2e7e37f9d7c30c8cd621fa47ff90b23084 Mon Sep 17 00:00:00 2001 From: link2xt Date: Tue, 30 Jan 2024 00:34:55 +0000 Subject: [PATCH 20/21] Emit DC_EVENT_ACCOUNTS_BACKGROUND_FETCH_DONE even on timeout Otherwise if there is a timeout, UI will wait for DC_EVENT_ACCOUNTS_BACKGROUND_FETCH_DONE forever. --- deltachat-ffi/deltachat.h | 9 ++++----- deltachat-jsonrpc/src/api.rs | 5 +++-- src/accounts.rs | 12 ++++++++++-- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index c636521d78..f710106621 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -3157,15 +3157,14 @@ void dc_accounts_maybe_network_lost (dc_accounts_t* accounts); * * dc_accounts_background_fetch() was created for the iOS Background fetch. * - * The `DC_EVENT_ACCOUNTS_BACKGROUND_FETCH_DONE` event is emitted at the end, - * process all events until you get this one and you can safely return to the background + * The `DC_EVENT_ACCOUNTS_BACKGROUND_FETCH_DONE` event is emitted at the end + * even in case of timeout, unless the function fails and returns 0. + * Process all events until you get this one and you can safely return to the background * without forgetting to create notifications caused by timing race conditions. * * @memberof dc_accounts_t * @param timeout The timeout in seconds - * @return Return 1 on success and 0 on failure (like timeout) - * But note that this only indicates that the fetch of all accounts was done before the timeout. - * To know whether it worked you need to look for the events. + * @return Return 1 if DC_EVENT_ACCOUNTS_BACKGROUND_FETCH_DONE was emitted and 0 otherwise. */ int dc_accounts_background_fetch (dc_accounts_t* accounts, uint64_t timeout); diff --git a/deltachat-jsonrpc/src/api.rs b/deltachat-jsonrpc/src/api.rs index c84e95d4b4..85cb1c7b69 100644 --- a/deltachat-jsonrpc/src/api.rs +++ b/deltachat-jsonrpc/src/api.rs @@ -233,8 +233,9 @@ impl CommandApi { /// Performs a background fetch for all accounts in parallel with a timeout. /// - /// The `AccountsBackgroundFetchDone` event is emitted at the end, - /// process all events until you get this one and you can safely return to the background + /// The `AccountsBackgroundFetchDone` event is emitted at the end + /// if the method returns sucessfully, even in case of timeout. + /// Process all events until you get this one and you can safely return to the background /// without forgetting to create notifications caused by timing race conditions. async fn accounts_background_fetch(&self, timeout_in_seconds: f64) -> Result<()> { self.accounts diff --git a/src/accounts.rs b/src/accounts.rs index 8c4a73a641..99053719cc 100644 --- a/src/accounts.rs +++ b/src/accounts.rs @@ -317,10 +317,18 @@ impl Accounts { /// The `AccountsBackgroundFetchDone` event is emitted at the end, /// process all events until you get this one and you can safely return to the background /// without forgetting to create notifications caused by timing race conditions. + /// + /// On error no `AccountsBackgroundFetchDone` event is emitted. pub async fn background_fetch(&self, timeout: std::time::Duration) -> Result<()> { - let result = tokio::time::timeout(timeout, self.background_fetch_without_timeout()).await; + if let Err(_err) = + tokio::time::timeout(timeout, self.background_fetch_without_timeout()).await + { + self.emit_event(EventType::Warning( + "Background fetch timed out.".to_string(), + )); + } self.emit_event(EventType::AccountsBackgroundFetchDone); - result.map_err(|err| err.into()) + Ok(()) } /// Emits a single event. From fa5c6ad1fc2ab2434b50da4010d9242ebaf35201 Mon Sep 17 00:00:00 2001 From: link2xt Date: Tue, 30 Jan 2024 15:40:36 +0000 Subject: [PATCH 21/21] Make Accounts::background_fetch() not return Result --- deltachat-ffi/src/lib.rs | 16 ++++------------ deltachat-jsonrpc/src/api.rs | 5 ++--- src/accounts.rs | 5 +---- 3 files changed, 7 insertions(+), 19 deletions(-) diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index 32c0a3ba45..6f28a89b96 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -4915,19 +4915,11 @@ pub unsafe extern "C" fn dc_accounts_background_fetch( let accounts = &*accounts; block_on(async move { let accounts = accounts.read().await; - match accounts + accounts .background_fetch(Duration::from_secs(timeout_in_seconds)) - .await - { - Ok(()) => 1, - Err(err) => { - accounts.emit_event(EventType::Error(format!( - "Failed to do background fetch: {err:#}" - ))); - 0 - } - } - }) + .await; + }); + 1 } #[no_mangle] diff --git a/deltachat-jsonrpc/src/api.rs b/deltachat-jsonrpc/src/api.rs index 85cb1c7b69..e2bf6c62f9 100644 --- a/deltachat-jsonrpc/src/api.rs +++ b/deltachat-jsonrpc/src/api.rs @@ -233,8 +233,7 @@ impl CommandApi { /// Performs a background fetch for all accounts in parallel with a timeout. /// - /// The `AccountsBackgroundFetchDone` event is emitted at the end - /// if the method returns sucessfully, even in case of timeout. + /// The `AccountsBackgroundFetchDone` event is emitted at the end even in case of timeout. /// Process all events until you get this one and you can safely return to the background /// without forgetting to create notifications caused by timing race conditions. async fn accounts_background_fetch(&self, timeout_in_seconds: f64) -> Result<()> { @@ -242,7 +241,7 @@ impl CommandApi { .write() .await .background_fetch(std::time::Duration::from_secs_f64(timeout_in_seconds)) - .await?; + .await; Ok(()) } diff --git a/src/accounts.rs b/src/accounts.rs index 99053719cc..626d279eff 100644 --- a/src/accounts.rs +++ b/src/accounts.rs @@ -317,9 +317,7 @@ impl Accounts { /// The `AccountsBackgroundFetchDone` event is emitted at the end, /// process all events until you get this one and you can safely return to the background /// without forgetting to create notifications caused by timing race conditions. - /// - /// On error no `AccountsBackgroundFetchDone` event is emitted. - pub async fn background_fetch(&self, timeout: std::time::Duration) -> Result<()> { + pub async fn background_fetch(&self, timeout: std::time::Duration) { if let Err(_err) = tokio::time::timeout(timeout, self.background_fetch_without_timeout()).await { @@ -328,7 +326,6 @@ impl Accounts { )); } self.emit_event(EventType::AccountsBackgroundFetchDone); - Ok(()) } /// Emits a single event.