Put it behind config option, fill relay_candidates with defaults, exclude candidates that are already in use

src/automatic_transport_management.rs

@@ -15,6 +15,9 @@ const AUTOMATIC_ADDITION_DEBOUNCE_SECONDS: i64 = 60 * 60; // one hour
 const BACKOFF_PERIOD_FOR_NOT_WORKING_TRANSPORT: i64 = 60 * 60 * 24 * 7; // one week
 
 // TODO think about how this interacts with stop_io()/start_io()
+
+// TODO decide if this should be done asynchronously in a task;
+// generally we should be able to try adding relays while io is running.
 pub(crate) async fn maybe_add_additional_transports(context: &Context) -> Result<()> {
     let now = time();
 
@@ -37,14 +40,24 @@ pub(crate) async fn maybe_add_additional_transports(context: &Context) -> Result
         )
         .await?;
 
-    while context.count_transports().await? < NUM_TRANSPORTS_TARGET {
-        let cutoff_timestamp = now.saturating_sub(BACKOFF_PERIOD_FOR_NOT_WORKING_TRANSPORT);
+    // Using `for` instead of `while` to prevent infinite loop
+    for _ in 0..NUM_TRANSPORTS_TARGET {
+        if context.count_transports().await? >= NUM_TRANSPORTS_TARGET {
+            return Ok(());
+        }
 
-        // TODO find out why login_param_from_account_qr() uses OsRng rather than rng()
+        // First, query all candidates that were not tried since `BACKOFF_PERIOD_FOR_NOT_WORKING_TRANSPORT` seconds.
+        // Domains that are already used are excluded.
+        let cutoff_timestamp = now.saturating_sub(BACKOFF_PERIOD_FOR_NOT_WORKING_TRANSPORT);
         let candidates: Vec<String> = context
             .sql
             .query_map_vec(
-                "SELECT domain FROM relay_candidates WHERE last_tried<?",
+                "SELECT domain FROM relay_candidates WHERE last_tried<?
+                AND NOT EXISTS (
+                    SELECT 1
+                    FROM transports
+                    WHERE substr(addr, instr(addr, '@') + 1) = domain
+                )",
                 (cutoff_timestamp,),
                 |row| Ok(row.get::<_, String>(0)?),
             )
@@ -58,14 +71,17 @@ pub(crate) async fn maybe_add_additional_transports(context: &Context) -> Result
             return Ok(());
         };
 
-        let mut param = login_param_from_domain(domain);
-        let res = context.add_transport_inner(&mut param).await;
+        let param = login_param_from_domain(domain);
+        let res = crate::configure::configure(context, &param).await;
         if res.is_err() {
             context
                 .sql
                 .execute("UPDATE relay_candidates SET last_tried=?", (now,))
                 .await?;
         }
+
+        // TODO: Decide whether we want to immediately try again with another relay,
+        // if this one failed. If yes, remove the next line.
         res?;
     }
 

src/config.rs

@@ -357,6 +357,9 @@ pub enum Config {
     /// [`crate::automatic_transport_management::maybe_add_additional_transports`]
     LastAutomaticTransportManagement,
 
+    /// Whether to automatically add/remove transports
+    AutomaticTransportManagement,
+
     /// Whether to avoid using IMAP IDLE even if the server supports it.
     ///
     /// This is a developer option for testing "fake idle".

src/configure.rs

@@ -49,21 +49,6 @@ use crate::{chat, provider};
 /// See <https://github.com/chatmail/core/issues/7608>.
 pub(crate) const MAX_RELAYS: usize = 5;
 
-/// Hard-coded candidates for default relays.
-/// In the future, we want to use it during onboarding;
-/// note that before onboarding automatically on any of these,
-/// we need to ask the admins whether their relay is able to handle this.
-/// For now, this is just the first 6 relays from chatmail.at/relays.
-#[allow(unused)]
-const DEFAULT_RELAY_CANDIDATES: &[&str] = &[
-    "mehl.cloud",
-    "mailchat.pl",
-    "chatmail.woodpeckersnest.space",
-    "chatmail.culturanerd.it",
-    "tarpit.fun",
-    "d.gaufr.es",
-];
-
 macro_rules! progress {
     ($context:tt, $progress:expr, $comment:expr) => {
         assert!(
@@ -604,7 +589,11 @@ async fn get_configured_param(
     Ok(configured_login_param)
 }
 
-async fn configure(ctx: &Context, param: &EnteredLoginParam) -> Result<Option<&'static Provider>> {
+// TODO maybe this function name should be changed
+pub(crate) async fn configure(
+    ctx: &Context,
+    param: &EnteredLoginParam,
+) -> Result<Option<&'static Provider>> {
     progress!(ctx, 1);
 
     let configured_param = get_configured_param(ctx, param).await?;

src/sql/migrations.rs

@@ -2525,11 +2525,32 @@ UPDATE msgs SET state=24 WHERE state=18; -- Change OutPreparing to OutFailed.
     // TODO should we call it relay_candidates or transport_candidates?
     inc_and_check(&mut migration_version, 159)?;
     if dbversion < migration_version {
-        sql.execute_migration(
-            "CREATE TABLE relay_candidates(
-                domain TEXT PRIMARY KEY NOT NULL,
-                last_tried TEXT NOT NULL DEFAULT 0
-            ) STRICT",
+        // TODO put a better list here
+        const DEFAULT_RELAY_CANDIDATES: &[&str] = &[
+            "mehl.cloud",
+            "mailchat.pl",
+            "chatmail.woodpeckersnest.space",
+            "chatmail.culturanerd.it",
+            "tarpit.fun",
+            "d.gaufr.es",
+        ];
+
+        sql.execute_migration_transaction(
+            |transaction| {
+                transaction.execute(
+                    "CREATE TABLE relay_candidates(
+                        domain TEXT PRIMARY KEY NOT NULL,
+                        last_tried TEXT NOT NULL DEFAULT 0
+                    ) STRICT",
+                    (),
+                )?;
+                let mut statement =
+                    transaction.prepare("INSERT INTO relay_candidates(domain) VALUES (?)")?;
+                for domain in DEFAULT_RELAY_CANDIDATES {
+                    statement.execute((domain,));
+                }
+                Ok(())
+            },
             migration_version,
         )
         .await?;