WIP: remove maybe_update_sending_transport

This commit is contained in:
link2xt
2026-09-17 15:13:42 +00:00
parent fae6a63023
commit 233c2cb37a
9 changed files with 33 additions and 159 deletions

View File

@@ -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(),
)

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

@@ -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,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 <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

View File

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