From 233c2cb37a035e7d1677a62903d44ccebe7dbd66 Mon Sep 17 00:00:00 2001 From: link2xt Date: Thu, 17 Sep 2026 15:13:42 +0000 Subject: [PATCH] WIP: remove maybe_update_sending_transport --- src/chat.rs | 6 +-- src/config.rs | 10 +++- src/configure.rs | 13 ++--- src/contact.rs | 20 +++----- src/contact/contact_tests.rs | 8 +-- src/securejoin/securejoin_tests.rs | 4 +- src/test_utils.rs | 12 +---- src/transport.rs | 40 ++------------- src/transport/transport_tests.rs | 79 ------------------------------ 9 files changed, 33 insertions(+), 159 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index 51a21e7b0..6f44e008f 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -2895,11 +2895,11 @@ async fn create_send_msg_jobs(context: &Context, msg: &mut Message) -> Result Result> { + if key == Config::ConfiguredAddr { + let addr: Option = self + .sql + .query_get_value("SELECT addr FROM transports", ()) + .await?; + return Ok(addr); + } + let value = self.get_config_opt(key).await?; if value.is_some() { return Ok(value); @@ -884,7 +892,7 @@ impl Context { .await } - /// Returns the address of the transport used for sending. + /// Returns address of some transport. /// Returns an error if no self addr is configured. pub async fn get_primary_self_addr(&self) -> Result { self.get_config(Config::ConfiguredAddr) diff --git a/src/configure.rs b/src/configure.rs index 71dd55152..9b1211e47 100644 --- a/src/configure.rs +++ b/src/configure.rs @@ -37,8 +37,8 @@ use crate::sync::Sync::Nosync; use crate::tools::time; use crate::transport::{ ConfiguredCertificateChecks, ConfiguredLoginParam, ConfiguredServerLoginParam, - ConnectionCandidate, delete_transport_row, maybe_update_sending_transport, - purge_transport_caches, send_sync_transports, transport_addrs, + ConnectionCandidate, delete_transport_row, purge_transport_caches, send_sync_transports, + transport_addrs, }; use crate::{EventType, stock_str}; @@ -215,7 +215,7 @@ impl Context { /// another one is chosen automatically. pub async fn delete_transport(&self, addr: &str) -> Result<()> { let now = time(); - let (removed_transport_id, reelected) = self + let removed_transport_id = self .sql .transaction(|transaction| { if transport_addrs(transaction)?.len() <= 1 { @@ -234,14 +234,9 @@ impl Context { let remove_timestamp = std::cmp::max(now, add_timestamp); let transport_id = delete_transport_row(transaction, addr, remove_timestamp)? .context("Transport disappeared")?; - let reelected = maybe_update_sending_transport(transaction)?; - Ok((transport_id, reelected)) + Ok(transport_id) }) .await?; - if let Some(new_addr) = reelected { - info!(self, "Using transport {new_addr:?} for sending now."); - self.sql.uncache_raw_config("configured_addr").await; - } send_sync_transports(self).await?; purge_transport_caches(self, removed_transport_id).await; // Restarting all IO also stops the removed transport's IMAP loop. diff --git a/src/contact.rs b/src/contact.rs index d5d5b9353..632abef65 100644 --- a/src/contact.rs +++ b/src/contact.rs @@ -694,9 +694,9 @@ impl Contact { .await? .unwrap_or_default(); contact.addr = context - .get_config(Config::ConfiguredAddr) - .await? - .unwrap_or_default(); + .get_primary_self_addr() + .await + .context("Cannot get address for self-contact")?; if let Some(self_fp) = self_fingerprint_opt(context).await? { contact.fingerprint = Some(self_fp.to_string()); } @@ -1384,10 +1384,6 @@ WHERE addr=? ); let contact = Contact::get_by_id(context, contact_id).await?; - let addr = context - .get_config(Config::ConfiguredAddr) - .await? - .unwrap_or_default(); let Some(fingerprint_other) = contact.fingerprint() else { return Ok(stock_str::encr_none(context)); @@ -1407,30 +1403,26 @@ WHERE addr=? .await? .dc_fingerprint() .human_readable(); - if addr < contact.addr { + if fingerprint_self < fingerprint_other { cat_fingerprint( &mut ret, &stock_str::self_msg(context), - &addr, &fingerprint_self, ); cat_fingerprint( &mut ret, contact.get_display_name(), - &contact.addr, &fingerprint_other, ); } else { cat_fingerprint( &mut ret, contact.get_display_name(), - &contact.addr, &fingerprint_other, ); cat_fingerprint( &mut ret, &stock_str::self_msg(context), - &addr, &fingerprint_self, ); } @@ -1931,8 +1923,8 @@ pub(crate) async fn update_last_seen( Ok(()) } -fn cat_fingerprint(ret: &mut String, name: &str, addr: &str, fingerprint: &str) { - *ret += &format!("\n\n{name} ({addr}):\n{fingerprint}"); +fn cat_fingerprint(ret: &mut String, name: &str, fingerprint: &str) { + *ret += &format!("\n\n{name}:\n{fingerprint}"); } fn split_address_book(book: &str) -> Vec<(&str, &str)> { diff --git a/src/contact/contact_tests.rs b/src/contact/contact_tests.rs index c0181cf7d..fb24c6301 100644 --- a/src/contact/contact_tests.rs +++ b/src/contact/contact_tests.rs @@ -887,11 +887,11 @@ async fn test_contact_get_encrinfo() -> Result<()> { "Messages are end-to-end encrypted. Fingerprints: -Me (alice@example.org): +Me: 2E6F A2CB 23B5 32D7 2863 4B58 64B0 8F61 A9ED 9443 -bob@example.net (bob@example.net): +bob@example.net: CCCB 5AA9 F6E1 141C 9431 65F1 DB18 B18C BCF7 0487 @@ -908,11 +908,11 @@ bob@example.net" "No encryption. Fingerprints: -Me (alice@example.org): +Me: 2E6F A2CB 23B5 32D7 2863 4B58 64B0 8F61 A9ED 9443 -bob@example.net (bob@example.net): +bob@example.net: CCCB 5AA9 F6E1 141C 9431 65F1 DB18 B18C BCF7 0487" ); diff --git a/src/securejoin/securejoin_tests.rs b/src/securejoin/securejoin_tests.rs index 5be9a1177..441ffbb8a 100644 --- a/src/securejoin/securejoin_tests.rs +++ b/src/securejoin/securejoin_tests.rs @@ -48,7 +48,7 @@ async fn test_setup_contact_ext(case: SetupContactCase) -> (TestContext, TestCon let mut tcm = TestContextManager::new(); let alice = tcm.alice().await; - let alice_addr = &alice + let alice_addr = alice .get_config(Config::ConfiguredAddr) .await .unwrap() @@ -116,7 +116,7 @@ async fn test_setup_contact_ext(case: SetupContactCase) -> (TestContext, TestCon let contact_alice_id = bob.add_or_lookup_contact_no_key(&alice).await.id; let sent = bob.pop_sent_msg().await; assert!(!sent.payload.contains("Bob Examplenet")); - assert_eq!(sent.recipient(), EmailAddress::new(alice_addr).unwrap()); + assert_eq!(sent.recipients, *alice_addr); let msg = alice.parse_msg(&sent).await; assert!(msg.signature.is_none()); assert_eq!( diff --git a/src/test_utils.rs b/src/test_utils.rs index b7c03ce69..e097333cf 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -193,21 +193,11 @@ impl TestContextManager { to.recv_msg(&sent).await } + // TODO: this just adds a transport pub async fn change_addr(&self, test_context: &TestContext, new_addr: &str) { - self.section(&format!( - "{} changes her self address and reconfigures", - test_context.name() - )); - test_context.add_transport(new_addr).await; - test_context.set_primary_self_addr(new_addr).await.unwrap(); // ensure_secret_key_exists() is called during configure key::ensure_secret_key_exists(test_context).await.unwrap(); - - assert_eq!( - test_context.get_primary_self_addr().await.unwrap(), - new_addr - ); } /// Executes SecureJoin protocol between `scanner` and `scanned`. diff --git a/src/transport.rs b/src/transport.rs index 8a15db498..d44a589fa 100644 --- a/src/transport.rs +++ b/src/transport.rs @@ -276,14 +276,14 @@ impl ConfiguredLoginParam { .await } - /// Loads legacy configured param. Only used for tests and the migration. + /// Loads legacy configured param. Only used for tests and migration 131. pub(crate) async fn load_legacy(context: &Context) -> Result> { if !context.get_config_bool(Config::Configured).await? { return Ok(None); } let addr = context - .get_config(Config::ConfiguredAddr) + .get_config_opt(Config::ConfiguredAddr) .await? .unwrap_or_default() .trim() @@ -625,7 +625,7 @@ pub(crate) async fn sync_transports( ) .collect(); - let (deleted_ids, reelected) = context + let deleted_ids = context .sql .transaction(|transaction| { let mut deleted_ids = Vec::new(); @@ -638,8 +638,7 @@ pub(crate) async fn sync_transports( } modified |= !deleted_ids.is_empty(); - let reelected = maybe_update_sending_transport(transaction)?; - Ok((deleted_ids, reelected)) + Ok(deleted_ids) }) .await?; @@ -647,12 +646,6 @@ pub(crate) async fn sync_transports( purge_transport_caches(context, *transport_id).await; } - if let Some(new_addr) = reelected { - info!(context, "Re-elected sending transport {new_addr:?}."); - context.sql.uncache_raw_config("configured_addr").await; - modified = true; - } - if modified { context.self_public_key.lock().await.take(); context @@ -711,31 +704,6 @@ pub(crate) async fn purge_transport_caches(context: &Context, transport_id: u32) context.metadata.write().await.remove(&transport_id); } -/// Elects another transport for sending if the current one vanished. -/// Any remaining transport works and selection is anyway moving -/// to the authority of the SMTP loop, see -pub(crate) fn maybe_update_sending_transport( - transaction: &mut rusqlite::Transaction, -) -> Result> { - let configured_addr: String = transaction.query_row( - "SELECT value FROM config WHERE keyname='configured_addr'", - (), - |row| row.get(0), - )?; - let addrs = transport_addrs(transaction)?; - if addrs.contains(&configured_addr) { - return Ok(None); - } - let Some(new_addr) = addrs.into_iter().next() else { - return Ok(None); - }; - transaction.execute( - "UPDATE config SET value=? WHERE keyname='configured_addr'", - (&new_addr,), - )?; - Ok(Some(new_addr)) -} - /// Adds transport entry to the `transports` table with empty configuration. pub async fn add_pseudo_transport(context: &Context, addr: &str) -> Result<()> { context.sql diff --git a/src/transport/transport_tests.rs b/src/transport/transport_tests.rs index a981c40b6..a7fb4ae6f 100644 --- a/src/transport/transport_tests.rs +++ b/src/transport/transport_tests.rs @@ -226,50 +226,6 @@ async fn test_delete_transport() -> Result<()> { Ok(()) } -/// Tests that selecting sending transport by setting "configured_addr" does not -/// send the sync message, is not synchronized between devices even if sync message is forced, -/// and does not bump sending transport `add_timestamp`. -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn test_no_configured_addr_synchronization() -> Result<()> { - let mut tcm = TestContextManager::new(); - let alice = &tcm.alice().await; - let alice2 = &tcm.alice().await; - for a in [alice, alice2] { - a.set_config_bool(Config::SyncMsgs, true).await?; - a.set_config_bool(Config::BccSelf, true).await?; - } - - let addr = "alice@otherprovider.com"; - add_dummy_transport(alice, addr).await?; - send_sync_transports(alice).await?; - sync_and_check_recipients(alice, alice2, &format!("{addr} alice@example.org")).await; - - // Selects `addr` on `alice` as the sending transport - // and syncs the transport update to `alice2`, - // whose own sending transport must stay unchanged. - let old_timestamp = add_timestamp(alice2, addr).await; - let alice2_primary = alice2.get_config(Config::ConfiguredAddr).await?; - alice.set_config(Config::ConfiguredAddr, Some(addr)).await?; - assert_eq!(add_timestamp(alice, addr).await, old_timestamp); - - send_sync_transports(alice).await?; - alice.send_sync_msg().await?.unwrap(); - let sync_msg = alice.pop_sent_msg().await; - assert_eq!(sync_msg.recipients, format!("alice@example.org {addr}")); - // The sync message comes from the new sending address, - // which must not make `alice2` adopt it as its own sending address. - assert!(sync_msg.payload.contains(&format!("From: <{addr}>"))); - alice2.recv_msg_trash(&sync_msg).await; - - // add_timestamp must not change. - assert_eq!(add_timestamp(alice2, addr).await, old_timestamp); - assert_eq!( - alice2.get_config(Config::ConfiguredAddr).await?, - alice2_primary - ); - Ok(()) -} - /// Tests that `sync_transports()` requests an IO restart /// if and only if it modified anything. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -288,14 +244,6 @@ async fn test_sync_transports_requests_io_restart() -> Result<()> { Ok(()) } -async fn add_timestamp(t: &TestContext, addr: &str) -> i64 { - t.sql - .query_get_value("SELECT add_timestamp FROM transports WHERE addr=?", (addr,)) - .await - .unwrap() - .unwrap() -} - /// Tests that removing the last transport keeps it. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_removing_last_transport() -> Result<()> { @@ -326,33 +274,6 @@ async fn test_removing_last_transport() -> Result<()> { Ok(()) } -/// Tests which transport is elected for sending. -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn test_maybe_update_sending_transport() -> Result<()> { - let t = &TestContext::new_alice().await; - - add_dummy_transport(t, "alice@one.com").await?; - assert_eq!( - t.sql.transaction(maybe_update_sending_transport).await?, - None - ); - - t.sql - .execute( - "DELETE FROM transports WHERE addr=?", - ("alice@example.org",), - ) - .await?; - assert_eq!( - t.sql - .transaction(maybe_update_sending_transport) - .await? - .as_deref(), - Some("alice@one.com") - ); - Ok(()) -} - /// Tests that a transport an older core unpublished is removed. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_sync_unpublished_transport_removes_it() -> Result<()> {