WIP: remove maybe_update_sending_transport (and remaining usage of ConfiguredAddr)

src/chat.rs

@@ -2895,11 +2895,11 @@ async fn create_send_msg_jobs(context: &Context, msg: &mut Message) -> Result<Ve
     let (queued_msg, side_effects) = queued_msg_pair;
 
     if needs_encryption && !is_encrypted {
-        let addr = context.get_config(Config::ConfiguredAddr).await?;
+        // FIXME: remove this message? We don't know yet which provider we are going to send the message through.
+        let addr = context.get_primary_self_addr().await?;
         let text = stock_str::unencrypted_email(
             context,
-            addr.unwrap_or_default()
-                .split('@')
+            addr.split('@')
                 .nth(1)
                 .unwrap_or_default(),
         )

src/config.rs

@@ -526,6 +526,14 @@ impl Context {
 
     /// Get a config key value if set, or a default value. Returns `None` if no value exists.
     pub async fn get_config(&self, key: Config) -> Result<Option<String>> {
+        if key == Config::ConfiguredAddr {
+            let addr: Option<String> = 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<String> {
         self.get_config(Config::ConfiguredAddr)

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.

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)> {

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"
     );

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!(

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`.

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<Option<Self>> {
         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,21 +638,14 @@ 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?;
 
     for transport_id in &deleted_ids {
         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 <https://github.com/chatmail/core/pull/8619>
-pub(crate) fn maybe_update_sending_transport(
-    transaction: &mut rusqlite::Transaction,
-) -> Result<Option<String>> {
-    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

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<()> {