fix: multi relay connectivity (#8550)

this PR, created together with @adbenitez, improves the connectivity
state passed to UI, which has changed a lot since multi relay:

- change the algorithm for `get_connectivity()`: this is roughly the
"best" connectivity of all relays now, so if one is connected, we're
already fine. this was discussed widely one to one, and that part
already closes #8554

- the PR adds a test for that, previously, that was untested

- additionally, do not regard unpublished relays in
`get_connectivity()`: e.g. unpublished relays are no longer given to
peers - so if only that is connected, the overall state should not be
"connected". therefore, we just ignore unpublished relays there

- in `get_connectivity_html()`, we continue showing unpublished relays,
however, we tune them down visually and flag them as such

- for the docs, remove the "range for some future use" wording. it was
never used like that, and that future will probably not arrive :)

<img width="320"
src="https://github.com/user-attachments/assets/1805c525-72d1-4197-b865-6e2d6fbc8819"
/>

---------

Co-authored-by: Hocuri <hocuri@gmx.de>
This commit is contained in:
biørn
2026-08-09 22:31:16 +02:00
committed by GitHub
parent 137ac9a027
commit 805e555a99
7 changed files with 145 additions and 50 deletions

View File

@@ -600,13 +600,10 @@ char* dc_get_info (const dc_context_t* context);
/**
* Get the current connectivity, i.e. whether the device is connected to the IMAP server.
* One of:
* - DC_CONNECTIVITY_NOT_CONNECTED (1000-1999): Show e.g. the string "Not connected" or a red dot
* - DC_CONNECTIVITY_CONNECTING (2000-2999): Show e.g. the string "Connecting…" or a yellow dot
* - DC_CONNECTIVITY_WORKING (3000-3999): Show e.g. the string "Getting new messages" or a spinning wheel
* - DC_CONNECTIVITY_CONNECTED (>=4000): Show e.g. the string "Connected" or a green dot
*
* We don't use exact values but ranges here so that we can split up
* states into multiple states in the future.
* - DC_CONNECTIVITY_NOT_CONNECTED (1000): Show e.g. the string "Not connected" or a red dot
* - DC_CONNECTIVITY_CONNECTING (2000): Show e.g. the string "Connecting…" or a yellow dot
* - DC_CONNECTIVITY_WORKING (3000): Show e.g. the string "Getting new messages" or a spinning wheel
* - DC_CONNECTIVITY_CONNECTED (4000): Show e.g. the string "Connected" or a green dot
*
* Meant as a rough overview that can be shown
* e.g. in the title of the main screen.
@@ -7274,6 +7271,13 @@ void dc_event_unref(dc_event_t* event);
/// "Message pinned by %1$s."
#define DC_STR_MESSAGE_PINNED_BY_OTHER 244
/// "Phasing out"
///
/// Used in connectivity view to flag unpublished relays.
/// This should match the wording used for relay deletion confirmation,
/// saying "Before deletion, it will be gradually phased out so your contacts can switch over smoothly"
#define DC_STR_PHASING_OUT 245
/**
* @}
*/

View File

@@ -2091,13 +2091,10 @@ impl CommandApi {
/// Get the current connectivity, i.e. whether the device is connected to the IMAP server.
/// One of:
/// - DC_CONNECTIVITY_NOT_CONNECTED (1000-1999): Show e.g. the string "Not connected" or a red dot
/// - DC_CONNECTIVITY_CONNECTING (2000-2999): Show e.g. the string "Connecting…" or a yellow dot
/// - DC_CONNECTIVITY_WORKING (3000-3999): Show e.g. the string "Getting new messages" or a spinning wheel
/// - DC_CONNECTIVITY_CONNECTED (>=4000): Show e.g. the string "Connected" or a green dot
///
/// We don't use exact values but ranges here so that we can split up
/// states into multiple states in the future.
/// - DC_CONNECTIVITY_NOT_CONNECTED (1000): Show e.g. the string "Not connected" or a red dot
/// - DC_CONNECTIVITY_CONNECTING (2000): Show e.g. the string "Connecting…" or a yellow dot
/// - DC_CONNECTIVITY_WORKING (3000): Show e.g. the string "Getting new messages" or a spinning wheel
/// - DC_CONNECTIVITY_CONNECTED (4000): Show e.g. the string "Connected" or a green dot
///
/// Meant as a rough overview that can be shown
/// e.g. in the title of the main screen.

View File

@@ -320,9 +320,9 @@ pub struct InnerContext {
/// Mutex is also held while generating the key to avoid generating the key twice.
pub(crate) self_public_key: Mutex<Option<SignedPublicKey>>,
/// `Connectivity` values for mailboxes, unordered. Used to compute the aggregate connectivity,
/// `Connectivity` values for published relays, unordered. Used to compute the aggregate connectivity,
/// see [`Context::get_connectivity()`].
pub(crate) connectivities: parking_lot::Mutex<Vec<ConnectivityStore>>,
pub(crate) published_connectivities: parking_lot::Mutex<Vec<ConnectivityStore>>,
}
/// The state of ongoing process.
@@ -498,7 +498,7 @@ impl Context {
iroh: Arc::new(RwLock::new(None)),
self_fingerprint: OnceLock::new(),
self_public_key: Mutex::new(None),
connectivities: parking_lot::Mutex::new(Vec::new()),
published_connectivities: parking_lot::Mutex::new(Vec::new()),
};
let ctx = Context {
@@ -830,7 +830,7 @@ impl Context {
let all_transports: Vec<String> = ConfiguredLoginParam::load_all(self)
.await?
.into_iter()
.map(|(transport_id, param)| format!("{transport_id}: {param}"))
.map(|(transport_id, param, _)| format!("{transport_id}: {param}"))
.collect();
let all_transports = if all_transports.is_empty() {
"Not configured".to_string()

View File

@@ -321,6 +321,9 @@ struct SchedBox {
/// IMAP loop task handle.
handle: task::JoinHandle<()>,
/// Relay published status.
is_published: bool,
}
/// Job and connection scheduler.
@@ -637,7 +640,9 @@ impl Scheduler {
let mut inboxes = Vec::new();
let mut start_recvs = Vec::new();
for (transport_id, configured_login_param) in ConfiguredLoginParam::load_all(ctx).await? {
for (transport_id, configured_login_param, is_published) in
ConfiguredLoginParam::load_all(ctx).await?
{
let (conn_state, inbox_handlers) =
ImapConnectionState::new(ctx, transport_id, configured_login_param.clone()).await?;
let (inbox_start_send, inbox_start_recv) = oneshot::channel();
@@ -654,6 +659,7 @@ impl Scheduler {
folder,
conn_state,
handle,
is_published,
};
inboxes.push(inbox);
start_recvs.push(inbox_start_recv);

View File

@@ -225,13 +225,27 @@ impl fmt::Debug for ConnectivityStore {
}
}
/// Combines per-relay connectivities into a single, overall connectivity as shown in the UI.
///
/// - If any relay is `Working`, this is the state we want the UIs to show.
/// - Otherwise, show the max, `Connected` takes precedence over `Connecting` and over `NotConnected`.
fn combine_connectivities(connectivities: &[Connectivity]) -> Connectivity {
if connectivities.contains(&Connectivity::Working) {
return Connectivity::Working;
}
*connectivities
.iter()
.max()
.unwrap_or(&Connectivity::NotConnected)
}
impl Context {
/// Get the current connectivity, i.e. whether the device is connected to the IMAP server.
/// One of:
/// - DC_CONNECTIVITY_NOT_CONNECTED (1000-1999): Show e.g. the string "Not connected" or a red dot
/// - DC_CONNECTIVITY_CONNECTING (2000-2999): Show e.g. the string "Connecting…" or a yellow dot
/// - DC_CONNECTIVITY_WORKING (3000-3999): Show e.g. the string "Updating…" or a spinning wheel
/// - DC_CONNECTIVITY_CONNECTED (>=4000): Show e.g. the string "Connected" or a green dot
/// - `Connectivity::NotConnected` (1000): Show e.g. the string "Not connected" or a red dot
/// - `Connectivity::Connecting` (2000): Show e.g. the string "Connecting…" or a yellow dot
/// - `Connectivity::Working` (3000): Show e.g. the string "Updating…" or a spinning wheel
/// - `Connectivity::Connected` (4000): Show e.g. the string "Connected" or a green dot
///
/// We don't use exact values but ranges here so that we can split up
/// states into multiple states in the future.
@@ -241,27 +255,21 @@ impl Context {
///
/// If the connectivity changes, a DC_EVENT_CONNECTIVITY_CHANGED will be emitted.
pub fn get_connectivity(&self) -> Connectivity {
let stores = self.connectivities.lock().clone();
let mut connectivities = Vec::new();
for s in stores {
let connectivity = s.get_basic();
connectivities.push(connectivity);
}
connectivities
.into_iter()
.min()
.unwrap_or(Connectivity::NotConnected)
let stores: Vec<ConnectivityStore> = self.published_connectivities.lock().clone();
let connectivities: Vec<Connectivity> = stores.into_iter().map(|s| s.get_basic()).collect();
combine_connectivities(&connectivities)
}
pub(crate) fn update_connectivities(&self, sched: &InnerSchedulerState) {
let stores: Vec<_> = match sched {
InnerSchedulerState::Started(sched) => sched
.boxes()
.filter(|b| b.is_published)
.map(|b| b.conn_state.state.connectivity.clone())
.collect(),
_ => Vec::new(),
};
*self.connectivities.lock() = stores;
*self.published_connectivities.lock() = stores;
}
/// Get an overview of the current connectivity, and possibly more statistics.
@@ -323,6 +331,9 @@ impl Context {
.transport {
margin-bottom: 1em;
}
.unpublished {
opacity: 0.5;
}
.quota-list {
padding-left: 0;
}
@@ -386,19 +397,28 @@ impl Context {
let transports = self
.sql
.query_map_vec("SELECT id, addr FROM transports", (), |row| {
let transport_id: u32 = row.get(0)?;
let addr: String = row.get(1)?;
Ok((transport_id, addr))
})
.query_map_vec(
"SELECT id, addr, is_published FROM transports ORDER BY is_published DESC, id",
(),
|row| {
let transport_id: u32 = row.get(0)?;
let addr: String = row.get(1)?;
let is_published: bool = row.get(2)?;
Ok((transport_id, addr, is_published))
},
)
.await?;
let quota = self.quota.read().await;
for (transport_id, transport_addr) in transports {
for (transport_id, transport_addr, is_published) in transports {
let domain = &deltachat_contact_tools::EmailAddress::new(&transport_addr)
.map_or(transport_addr.clone(), |email| email.domain);
let domain_escaped = escaper::encode_minimal(domain);
ret += "<li class=\"transport\">";
ret += if is_published {
"<li class=\"transport\">"
} else {
"<li class=\"transport unpublished\">"
};
let folders = folders_states
.iter()
.filter(|(folder_addr, ..)| *folder_addr == transport_addr);
@@ -408,10 +428,18 @@ impl Context {
ret += " <b>";
ret += &*domain_escaped;
ret += ":</b> ";
ret += &*escaper::encode_minimal(&detailed.to_string_imap(self));
if is_published {
ret += &*escaper::encode_minimal(&detailed.to_string_imap(self));
} else {
ret += &*escaper::encode_minimal(&stock_str::phasing_out(self));
}
ret += "<br />";
}
if !is_published {
ret += "</li>"; // quota is of no big interest for unpublished relays
continue;
};
let Some(quota) = quota.get(&transport_id) else {
ret += "</li>";
continue;
@@ -565,3 +593,50 @@ impl Context {
}
}
}
#[cfg(test)]
mod tests {
use super::*;
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_combine_connectivities() {
assert_eq!(combine_connectivities(&[]), Connectivity::NotConnected);
assert_eq!(
combine_connectivities(&[Connectivity::NotConnected]),
Connectivity::NotConnected
);
assert_eq!(
combine_connectivities(&[Connectivity::Connecting]),
Connectivity::Connecting
);
assert_eq!(
combine_connectivities(&[Connectivity::Working]),
Connectivity::Working
);
assert_eq!(
combine_connectivities(&[Connectivity::Connected]),
Connectivity::Connected
);
assert_eq!(
combine_connectivities(&[
Connectivity::Working,
Connectivity::Connected,
Connectivity::NotConnected,
Connectivity::Connecting
]),
Connectivity::Working
);
assert_eq!(
combine_connectivities(&[
Connectivity::Connected,
Connectivity::NotConnected,
Connectivity::Connecting
]),
Connectivity::Connected
);
assert_eq!(
combine_connectivities(&[Connectivity::NotConnected, Connectivity::Connecting]),
Connectivity::Connecting
);
}
}

View File

@@ -427,6 +427,9 @@ https://delta.chat/donate"))]
#[strum(props(fallback = "Message pinned by %1$s."))]
MsgMessagePinnedBy = 244,
#[strum(props(fallback = "Phasing out"))]
PhasingOut = 245,
}
impl StockMessage {
@@ -1145,6 +1148,11 @@ pub(crate) fn last_msg_sent_successfully(context: &Context) -> String {
translated(context, StockMessage::LastMsgSentSuccessfully)
}
/// Stock string: `Phasing out`.
pub(crate) fn phasing_out(context: &Context) -> String {
translated(context, StockMessage::PhasingOut)
}
/// Stock string: `Error: %1$s…`.
/// `%1$s` will be replaced by a possibly more detailed, typically english, error description.
pub(crate) fn error(context: &Context, error: &str) -> String {

View File

@@ -289,16 +289,21 @@ impl ConfiguredLoginParam {
/// Loads configured login parameters for all transports.
///
/// Returns a vector of all transport IDs
/// paired with the configured parameters for the transports.
pub(crate) async fn load_all(context: &Context) -> Result<Vec<(u32, Self)>> {
/// paired with the configured parameters for the transports and the published state.
pub(crate) async fn load_all(context: &Context) -> Result<Vec<(u32, Self, bool)>> {
context
.sql
.query_map_vec("SELECT id, configured_param FROM transports", (), |row| {
let id: u32 = row.get(0)?;
let json: String = row.get(1)?;
let param = Self::from_json(&json)?;
Ok((id, param))
})
.query_map_vec(
"SELECT id, configured_param, is_published FROM transports",
(),
|row| {
let id: u32 = row.get(0)?;
let json: String = row.get(1)?;
let param = Self::from_json(&json)?;
let is_published: bool = row.get(2)?;
Ok((id, param, is_published))
},
)
.await
}