Merge pull request #1539 from deltachat/sane-configure

refactor(configure): simplify logic and code

src/configure/mod.rs

@@ -4,7 +4,7 @@ mod auto_mozilla;
 mod auto_outlook;
 mod read_url;
 
-use anyhow::{bail, ensure, format_err, Result};
+use anyhow::{bail, ensure, Context as _, Result};
 use async_std::prelude::*;
 use percent_encoding::{utf8_percent_encode, NON_ALPHANUMERIC};
 
@@ -66,75 +66,11 @@ impl Context {
     }
 
     async fn inner_configure(&self) -> Result<()> {
-        let mut success = false;
-        let mut param_autoconfig: Option<LoginParam> = None;
-
         info!(self, "Configure ...");
 
-        // Variables that are shared between steps:
-        let mut param = LoginParam::from_database(self, "").await;
-        // need all vars here to be mutable because rust thinks the same step could be called multiple times
-        // and also initialize, because otherwise rust thinks it's used while unitilized, even if thats not the case as the loop goes only forward
-        let mut param_domain = "undefined.undefined".to_owned();
-        let mut param_addr_urlencoded: String =
-            "Internal Error: this value should never be used".to_owned();
-        let mut keep_flags = 0;
-
-        let mut step_counter: u8 = 0;
-        let (_s, r) = async_std::sync::channel(1);
-        let mut imap = Imap::new(r);
         let was_configured_before = self.is_configured().await;
-
+        let mut param = LoginParam::from_database(self, "").await;
-        while !self.shall_stop_ongoing().await {
+        let success = configure(self, &mut param).await;
-            step_counter += 1;
-
-            match exec_step(
-                self,
-                &mut imap,
-                &mut param,
-                &mut param_domain,
-                &mut param_autoconfig,
-                &mut param_addr_urlencoded,
-                &mut keep_flags,
-                &mut step_counter,
-            )
-            .await
-            {
-                Ok(step) => {
-                    success = true;
-                    match step {
-                        Step::Continue => {}
-                        Step::Done => break,
-                    }
-                }
-                Err(err) => {
-                    error!(self, "{}", err);
-                    success = false;
-                    break;
-                }
-            }
-        }
-
-        if imap.is_connected() {
-            imap.disconnect(self).await;
-        }
-
-        // remember the entered parameters on success
-        // and restore to last-entered on failure.
-        // this way, the parameters visible to the ui are always in-sync with the current configuration.
-        if success {
-            LoginParam::from_database(self, "")
-                .await
-                .save_to_database(self, "configured_raw_")
-                .await
-                .ok();
-        } else {
-            LoginParam::from_database(self, "configured_raw_")
-                .await
-                .save_to_database(self, "")
-                .await
-                .ok();
-        }
 
         if let Some(provider) = provider::get_provider_info(&param.addr) {
             if !was_configured_before {
@@ -158,42 +94,33 @@ impl Context {
             }
         }
 
-        if success {
+        match success {
+            Ok(_) => {
                 progress!(self, 1000);
                 Ok(())
-        } else {
+            }
+            Err(err) => {
+                error!(self, "Configure Failed: {}", err);
                 progress!(self, 0);
-            Err(format_err!("Configure failed"))
+                Err(err)
+            }
         }
     }
 }
 
-#[allow(clippy::too_many_arguments)]
+async fn configure(ctx: &Context, param: &mut LoginParam) -> Result<()> {
-async fn exec_step(
+    let mut param_autoconfig: Option<LoginParam> = None;
-    ctx: &Context,
+    let mut keep_flags = 0;
-    imap: &mut Imap,
-    param: &mut LoginParam,
-    param_domain: &mut String,
-    param_autoconfig: &mut Option<LoginParam>,
-    param_addr_urlencoded: &mut String,
-    keep_flags: &mut i32,
-    step_counter: &mut u8,
-) -> Result<Step> {
-    const STEP_12_USE_AUTOCONFIG: u8 = 12;
-    const STEP_13_AFTER_AUTOCONFIG: u8 = 13;
 
-    match *step_counter {
     // Read login parameters from the database
-        1 => {
     progress!(ctx, 1);
     ensure!(!param.addr.is_empty(), "Please enter an email address.");
-        }
+
     // Step 1: Load the parameters and check email-address and password
-        2 => {
+
     if 0 != param.server_flags & DC_LP_AUTH_OAUTH2 {
         // the used oauth2 addr may differ, check this.
-                // if dc_get_oauth2_addr() is not available in the oauth2 implementation,
+        // if dc_get_oauth2_addr() is not available in the oauth2 implementation, just use the given one.
-                // just use the given one.
         progress!(ctx, 10);
         if let Some(oauth2_addr) = dc_get_oauth2_addr(ctx, &param.addr, &param.mail_pw)
             .await
@@ -208,137 +135,57 @@ async fn exec_step(
         progress!(ctx, 20);
     }
     // no oauth? - just continue it's no error
-        }
+
-        3 => {
+    let parsed: EmailAddress = param.addr.parse().context("Bad email-address")?;
-            if let Ok(parsed) = param.addr.parse() {
+    let param_domain = parsed.domain;
-                let parsed: EmailAddress = parsed;
+    let param_addr_urlencoded = utf8_percent_encode(&param.addr, NON_ALPHANUMERIC).to_string();
-                *param_domain = parsed.domain;
+
-                *param_addr_urlencoded =
-                    utf8_percent_encode(&param.addr, NON_ALPHANUMERIC).to_string();
-            } else {
-                bail!("Bad email-address.");
-            }
-        }
     // Step 2: Autoconfig
-        4 => {
     progress!(ctx, 200);
 
+    // param.mail_user.is_empty() -- the user can enter a loginname which is used by autoconfig then
+    // param.send_pw.is_empty()   -- the password cannot be auto-configured and is no criterion for
+    //                               autoconfig or not
     if param.mail_server.is_empty()
         && param.mail_port == 0
-            /* && param.mail_user.is_empty() -- the user can enter a loginname which is used by autoconfig then */
         && param.send_server.is_empty()
         && param.send_port == 0
         && param.send_user.is_empty()
-            /* && param.send_pw.is_empty() -- the password cannot be auto-configured and is no criterion for autoconfig or not */
         && (param.server_flags & !DC_LP_AUTH_OAUTH2) == 0
     {
         // no advanced parameters entered by the user: query provider-database or do Autoconfig
-                *keep_flags = param.server_flags & DC_LP_AUTH_OAUTH2;
+        keep_flags = param.server_flags & DC_LP_AUTH_OAUTH2;
         if let Some(new_param) = get_offline_autoconfig(ctx, &param) {
             // got parameters from our provider-database, skip Autoconfig, preserve the OAuth2 setting
-                    *param_autoconfig = Some(new_param);
+            param_autoconfig = Some(new_param);
-                    *step_counter = STEP_12_USE_AUTOCONFIG - 1; // minus one as step_counter is increased on next loop
         }
-            } else {
+
-                // advanced parameters entered by the user: skip Autoconfig
-                *step_counter = STEP_13_AFTER_AUTOCONFIG - 1; // minus one as step_counter is increased on next loop
-            }
-        }
-        /* A.  Search configurations from the domain used in the email-address, prefer encrypted */
-        5 => {
         if param_autoconfig.is_none() {
-                let url = format!(
+            param_autoconfig =
-                    "https://autoconfig.{}/mail/config-v1.1.xml?emailaddress={}",
+                get_autoconfig(ctx, param, &param_domain, &param_addr_urlencoded).await;
-                    param_domain, param_addr_urlencoded
-                );
-                *param_autoconfig = moz_autoconfigure(ctx, &url, &param).await.ok();
         }
     }
-        6 => {
+
-            progress!(ctx, 300);
+    // C.  Do we have any autoconfig result?
-            if param_autoconfig.is_none() {
-                // the doc does not mention `emailaddress=`, however, Thunderbird adds it, see https://releases.mozilla.org/pub/thunderbird/ ,  which makes some sense
-                let url = format!(
-                    "https://{}/.well-known/autoconfig/mail/config-v1.1.xml?emailaddress={}",
-                    param_domain, param_addr_urlencoded
-                );
-                *param_autoconfig = moz_autoconfigure(ctx, &url, &param).await.ok();
-            }
-        }
-        /* Outlook section start ------------- */
-        /* Outlook uses always SSL but different domains (this comment describes the next two steps) */
-        7 => {
-            progress!(ctx, 310);
-            if param_autoconfig.is_none() {
-                let url = format!("https://{}/autodiscover/autodiscover.xml", param_domain);
-                *param_autoconfig = outlk_autodiscover(ctx, &url, &param).await.ok();
-            }
-        }
-        8 => {
-            progress!(ctx, 320);
-            if param_autoconfig.is_none() {
-                let url = format!(
-                    "https://{}{}/autodiscover/autodiscover.xml",
-                    "autodiscover.", param_domain
-                );
-                *param_autoconfig = outlk_autodiscover(ctx, &url, &param).await.ok();
-            }
-        }
-        /* ----------- Outlook section end */
-        9 => {
-            progress!(ctx, 330);
-            if param_autoconfig.is_none() {
-                let url = format!(
-                    "http://autoconfig.{}/mail/config-v1.1.xml?emailaddress={}",
-                    param_domain, param_addr_urlencoded
-                );
-                *param_autoconfig = moz_autoconfigure(ctx, &url, &param).await.ok();
-            }
-        }
-        10 => {
-            progress!(ctx, 340);
-            if param_autoconfig.is_none() {
-                // do not transfer the email-address unencrypted
-                let url = format!(
-                    "http://{}/.well-known/autoconfig/mail/config-v1.1.xml",
-                    param_domain
-                );
-                *param_autoconfig = moz_autoconfigure(ctx, &url, &param).await.ok();
-            }
-        }
-        /* B.  If we have no configuration yet, search configuration in Thunderbird's centeral database */
-        11 => {
-            progress!(ctx, 350);
-            if param_autoconfig.is_none() {
-                /* always SSL for Thunderbird's database */
-                let url = format!("https://autoconfig.thunderbird.net/v1.1/{}", param_domain);
-                *param_autoconfig = moz_autoconfigure(ctx, &url, &param).await.ok();
-            }
-        }
-        /* C.  Do we have any autoconfig result?
-           If you change the match-number here, also update STEP_12_COPY_AUTOCONFIG above
-        */
-        STEP_12_USE_AUTOCONFIG => {
     progress!(ctx, 500);
     if let Some(ref cfg) = param_autoconfig {
         info!(ctx, "Got autoconfig: {}", &cfg);
         if !cfg.mail_user.is_empty() {
             param.mail_user = cfg.mail_user.clone();
         }
-                param.mail_server = cfg.mail_server.clone(); /* all other values are always NULL when entering autoconfig */
+        // all other values are always NULL when entering autoconfig
+        param.mail_server = cfg.mail_server.clone();
         param.mail_port = cfg.mail_port;
         param.send_server = cfg.send_server.clone();
         param.send_port = cfg.send_port;
         param.send_user = cfg.send_user.clone();
         param.server_flags = cfg.server_flags;
-                /* although param_autoconfig's data are no longer needed from,
+        // although param_autoconfig's data are no longer needed from,
-                it is used to later to prevent trying variations of port/server/logins */
+        // it is used to later to prevent trying variations of port/server/logins
-            }
-            param.server_flags |= *keep_flags;
     }
+    param.server_flags |= keep_flags;
+
     // Step 3: Fill missing fields with defaults
-        // If you change the match-number here, also update STEP_13_AFTER_AUTOCONFIG above
-        STEP_13_AFTER_AUTOCONFIG => {
     if param.mail_server.is_empty() {
         param.mail_server = format!("imap.{}", param_domain,)
     }
@@ -395,77 +242,148 @@ async fn exec_step(
             DC_LP_SMTP_SOCKET_SSL as i32
         }
     }
-            /* do we have a complete configuration? */
+
-            if param.mail_server.is_empty()
+    // do we have a complete configuration?
-                || param.mail_port == 0
+    ensure!(
-                || param.mail_user.is_empty()
+        !param.mail_server.is_empty()
-                || param.mail_pw.is_empty()
+            && param.mail_port != 0
-                || param.send_server.is_empty()
+            && !param.mail_user.is_empty()
-                || param.send_port == 0
+            && !param.mail_pw.is_empty()
-                || param.send_user.is_empty()
+            && !param.send_server.is_empty()
-                || param.send_pw.is_empty()
+            && param.send_port != 0
-                || param.server_flags == 0
+            && !param.send_user.is_empty()
-            {
+            && !param.send_pw.is_empty()
-                bail!("Account settings incomplete.");
+            && param.server_flags != 0,
-            }
+        "Account settings incomplete."
-        }
+    );
-        14 => {
+
     progress!(ctx, 600);
-            /* try to connect to IMAP - if we did not got an autoconfig,
+    // try to connect to IMAP - if we did not got an autoconfig,
-            do some further tries with different settings and username variations */
+    // do some further tries with different settings and username variations
-            try_imap_connections(ctx, param, param_autoconfig.is_some(), imap).await?;
+    let (_s, r) = async_std::sync::channel(1);
-        }
+    let mut imap = Imap::new(r);
-        15 => {
+
+    try_imap_connections(ctx, param, param_autoconfig.is_some(), &mut imap).await?;
     progress!(ctx, 800);
+
     try_smtp_connections(ctx, param, param_autoconfig.is_some()).await?;
-        }
-        16 => {
     progress!(ctx, 900);
 
     let create_mvbox = ctx.get_config_bool(Config::MvboxWatch).await
         || ctx.get_config_bool(Config::MvboxMove).await;
 
-            if let Err(err) = imap.configure_folders(ctx, create_mvbox).await {
+    imap.configure_folders(ctx, create_mvbox)
-                bail!("configuring folders failed: {:?}", err);
+        .await
-            }
+        .context("configuring folders failed")?;
+
+    imap.select_with_uidvalidity(ctx, "INBOX")
+        .await
+        .context("could not read INBOX status")?;
+
+    drop(imap);
 
-            if let Err(err) = imap.select_with_uidvalidity(ctx, "INBOX").await {
-                bail!("could not read INBOX status: {:?}", err);
-            }
-        }
-        17 => {
     progress!(ctx, 910);
     // configuration success - write back the configured parameters with the
     // "configured_" prefix; also write the "configured"-flag */
     // the trailing underscore is correct
     param.save_to_database(ctx, "configured_").await?;
     ctx.sql.set_raw_config_bool(ctx, "configured", true).await?;
-        }
+
-        18 => {
     progress!(ctx, 920);
-            // we generate the keypair just now - we could also postpone this until the first message is sent, however,
+
-            // this may result in a unexpected and annoying delay when the user sends his very first message
-            // (~30 seconds on a Moto G4 play) and might looks as if message sending is always that slow.
     e2ee::ensure_secret_key_exists(ctx).await?;
     info!(ctx, "key generation completed");
+
     progress!(ctx, 940);
-            return Ok(Step::Done);
+
+    Ok(())
 }
-        _ => {
+
-            bail!("Internal error: step counter out of bound");
+#[derive(Debug, PartialEq, Eq)]
+enum AutoconfigProvider {
+    Mozilla,
+    Outlook,
+}
+
+#[derive(Debug, PartialEq, Eq)]
+struct AutoconfigSource {
+    provider: AutoconfigProvider,
+    url: String,
+}
+
+impl AutoconfigSource {
+    fn all(domain: &str, addr: &str) -> [Self; 5] {
+        [
+            AutoconfigSource {
+                provider: AutoconfigProvider::Mozilla,
+                url: format!(
+                    "https://autoconfig.{}/mail/config-v1.1.xml?emailaddress={}",
+                    domain, addr,
+                ),
+            },
+            // the doc does not mention `emailaddress=`, however, Thunderbird adds it, see https://releases.mozilla.org/pub/thunderbird/ ,  which makes some sense
+            AutoconfigSource {
+                provider: AutoconfigProvider::Mozilla,
+                url: format!(
+                    "https://{}/.well-known/autoconfig/mail/config-v1.1.xml?emailaddress={}",
+                    domain, addr
+                ),
+            },
+            AutoconfigSource {
+                provider: AutoconfigProvider::Outlook,
+                url: format!("https://{}/autodiscover/autodiscover.xml", domain),
+            },
+            // Outlook uses always SSL but different domains (this comment describes the next two steps)
+            AutoconfigSource {
+                provider: AutoconfigProvider::Outlook,
+                url: format!(
+                    "https://{}{}/autodiscover/autodiscover.xml",
+                    "autodiscover.", domain
+                ),
+            },
+            // always SSL for Thunderbird's database
+            AutoconfigSource {
+                provider: AutoconfigProvider::Mozilla,
+                url: format!("https://autoconfig.thunderbird.net/v1.1/{}", domain),
+            },
+        ]
+    }
+
+    async fn fetch(&self, ctx: &Context, param: &LoginParam) -> Result<LoginParam> {
+        let params = match self.provider {
+            AutoconfigProvider::Mozilla => moz_autoconfigure(ctx, &self.url, &param).await?,
+            AutoconfigProvider::Outlook => outlk_autodiscover(ctx, &self.url, &param).await?,
+        };
+
+        Ok(params)
     }
 }
 
-    Ok(Step::Continue)
+/// Retrieve available autoconfigurations.
+///
+/// A Search configurations from the domain used in the email-address, prefer encrypted
+/// B. If we have no configuration yet, search configuration in Thunderbird's centeral database
+async fn get_autoconfig(
+    ctx: &Context,
+    param: &LoginParam,
+    param_domain: &str,
+    param_addr_urlencoded: &str,
+) -> Option<LoginParam> {
+    let sources = AutoconfigSource::all(param_domain, param_addr_urlencoded);
+
+    let mut progress = 300;
+    for source in &sources {
+        let res = source.fetch(ctx, param).await;
+        progress!(ctx, progress);
+        progress += 10;
+        if let Ok(res) = res {
+            return Some(res);
+        }
     }
 
-#[derive(Debug)]
+    None
-enum Step {
-    Done,
-    Continue,
 }
 
-#[allow(clippy::unnecessary_unwrap)]
 fn get_offline_autoconfig(context: &Context, param: &LoginParam) -> Option<LoginParam> {
     info!(
         context,
@@ -481,10 +399,8 @@ fn get_offline_autoconfig(context: &Context, param: &LoginParam) -> Option<Login
                 // however, rewriting the code to "if let" would make things less obvious,
                 // esp. if we allow more combinations of servers (pop, jmap).
                 // therefore, #[allow(clippy::unnecessary_unwrap)] is added above.
-                if imap.is_some() && smtp.is_some() {
+                if let Some(imap) = imap {
-                    let imap = imap.unwrap();
+                    if let Some(smtp) = smtp {
-                    let smtp = smtp.unwrap();
-
                         let mut p = LoginParam::new();
                         p.addr = param.addr.clone();
 
@@ -508,11 +424,11 @@ fn get_offline_autoconfig(context: &Context, param: &LoginParam) -> Option<Login
 
                         info!(context, "offline autoconfig found: {}", p);
                         return Some(p);
-                } else {
+                    }
+                }
                 info!(context, "offline autoconfig found, but no servers defined");
                 return None;
             }
-            }
             provider::Status::BROKEN => {
                 info!(context, "offline autoconfig found, provider is broken");
                 return None;
@@ -592,20 +508,17 @@ async fn try_imap_one_param(context: &Context, param: &LoginParam, imap: &mut Im
         return Ok(());
     }
 
-    if context.shall_stop_ongoing().await {
-        bail!("Interrupted");
-    }
-
     bail!("Could not connect: {}", inf);
 }
 
 async fn try_smtp_connections(
     context: &Context,
-    mut param: &mut LoginParam,
+    param: &mut LoginParam,
     was_autoconfig: bool,
 ) -> Result<()> {
     let mut smtp = Smtp::new();
-    /* try to connect to SMTP - if we did not got an autoconfig, the first try was SSL-465 and we do a second try with STARTTLS-587 */
+    // try to connect to SMTP - if we did not got an autoconfig, the first try was SSL-465 and we do
+    // a second try with STARTTLS-587
     if try_smtp_one_param(context, &param, &mut smtp).await.is_ok() {
         return Ok(());
     }
@@ -621,6 +534,7 @@ async fn try_smtp_connections(
         return Ok(());
     }
     progress!(context, 860);
+
     param.server_flags &= !(DC_LP_SMTP_SOCKET_FLAGS as i32);
     param.server_flags |= DC_LP_SMTP_SOCKET_STARTTLS as i32;
     param.send_port = 25;