refactor(add_transport_from_qr): do not set deprecated config values

src/configure.rs

@@ -36,6 +36,7 @@ use crate::login_param::{
 use crate::message::Message;
 use crate::oauth2::get_oauth2_addr;
 use crate::provider::{Protocol, Provider, Socket, UsernamePattern};
+use crate::qr::{login_param_from_account_qr, login_param_from_login_qr};
 use crate::smtp::Smtp;
 use crate::sync::Sync::*;
 use crate::tools::time;
@@ -117,7 +118,7 @@ impl Context {
         Ok(())
     }
 
-    async fn add_transport_inner(&self, param: &mut EnteredLoginParam) -> Result<()> {
+    pub(crate) async fn add_transport_inner(&self, param: &mut EnteredLoginParam) -> Result<()> {
         ensure!(
             !self.scheduler.is_running().await,
             "cannot configure, already running"
@@ -162,20 +163,15 @@ impl Context {
     pub async fn add_transport_from_qr(&self, qr: &str) -> Result<()> {
         self.stop_io().await;
 
-        // This code first sets the deprecated Config::Addr, Config::MailPw, etc.
-        // and then calls configure(), which loads them again.
-        // At some point, we will remove configure()
-        // and then simplify the code
-        // to directly create an EnteredLoginParam.
         let result = async move {
-            match crate::qr::check_qr(self, qr).await? {
+            let mut param = match crate::qr::check_qr(self, qr).await? {
-                crate::qr::Qr::Account { .. } => crate::qr::set_account_from_qr(self, qr).await?,
+                crate::qr::Qr::Account { .. } => login_param_from_account_qr(self, qr).await?,
                 crate::qr::Qr::Login { address, options } => {
-                    crate::qr::configure_from_login_qr(self, &address, options).await?
+                    login_param_from_login_qr(&address, options)?
                 }
                 _ => bail!("QR code does not contain account"),
-            }
+            };
-            self.configure().await?;
+            self.add_transport_inner(&mut param).await?;
             Ok(())
         }
         .await;

src/qr.rs

@@ -6,16 +6,17 @@ use std::sync::LazyLock;
 
 use anyhow::{Context as _, Result, anyhow, bail, ensure};
 pub use dclogin_scheme::LoginOptions;
+pub(crate) use dclogin_scheme::login_param_from_login_qr;
 use deltachat_contact_tools::{ContactAddress, addr_normalize, may_be_valid_addr};
 use percent_encoding::{NON_ALPHANUMERIC, percent_decode_str, percent_encode};
 use serde::Deserialize;
 
-pub(crate) use self::dclogin_scheme::configure_from_login_qr;
 use crate::config::Config;
 use crate::contact::{Contact, ContactId, Origin};
 use crate::context::Context;
 use crate::events::EventType;
 use crate::key::Fingerprint;
+use crate::login_param::{EnteredCertificateChecks, EnteredLoginParam, EnteredServerLoginParam};
 use crate::net::http::post_empty;
 use crate::net::proxy::{DEFAULT_SOCKS_PORT, ProxyConfig};
 use crate::token;
@@ -651,10 +652,13 @@ struct CreateAccountErrorResponse {
     reason: String,
 }
 
-/// take a qr of the type DC_QR_ACCOUNT, parse it's parameters,
+/// Takes a QR with `DCACCOUNT:` scheme, parses its parameters,
-/// download additional information from the contained url and set the parameters.
+/// downloads additional information from the contained URL
-/// on success, a configure::configure() should be able to log in to the account
+/// and returns the login parameters.
-pub(crate) async fn set_account_from_qr(context: &Context, qr: &str) -> Result<()> {
+pub(crate) async fn login_param_from_account_qr(
+    context: &Context,
+    qr: &str,
+) -> Result<EnteredLoginParam> {
     let url_str = qr
         .get(DCACCOUNT_SCHEME.len()..)
         .context("Invalid DCACCOUNT scheme")?;
@@ -669,14 +673,19 @@ pub(crate) async fn set_account_from_qr(context: &Context, qr: &str) -> Result<(
             .with_context(|| {
                 format!("Cannot create account, response is malformed:\n{response_text:?}")
             })?;
-        context
-            .set_config_internal(Config::Addr, Some(&email))
-            .await?;
-        context
-            .set_config_internal(Config::MailPw, Some(&password))
-            .await?;
 
-        Ok(())
+        let param = EnteredLoginParam {
+            addr: email,
+            imap: EnteredServerLoginParam {
+                password,
+                ..Default::default()
+            },
+            smtp: Default::default(),
+            certificate_checks: EnteredCertificateChecks::Strict,
+            oauth2: false,
+        };
+
+        Ok(param)
     } else {
         match serde_json::from_str::<CreateAccountErrorResponse>(&response_text) {
             Ok(error) => Err(anyhow!(error.reason)),
@@ -693,7 +702,10 @@ pub(crate) async fn set_account_from_qr(context: &Context, qr: &str) -> Result<(
 /// Sets configuration values from a QR code.
 pub async fn set_config_from_qr(context: &Context, qr: &str) -> Result<()> {
     match check_qr(context, qr).await? {
-        Qr::Account { .. } => set_account_from_qr(context, qr).await?,
+        Qr::Account { .. } => {
+            let mut param = login_param_from_account_qr(context, qr).await?;
+            context.add_transport_inner(&mut param).await?
+        }
         Qr::Proxy { url, .. } => {
             let old_proxy_url_value = context
                 .get_config(Config::ProxyUrl)
@@ -781,7 +793,8 @@ pub async fn set_config_from_qr(context: &Context, qr: &str) -> Result<()> {
             context.scheduler.interrupt_inbox().await;
         }
         Qr::Login { address, options } => {
-            configure_from_login_qr(context, &address, options).await?
+            let mut param = login_param_from_login_qr(&address, options)?;
+            context.add_transport_inner(&mut param).await?
         }
         _ => bail!("QR code does not contain config"),
     }

src/qr/dclogin_scheme.rs

@@ -3,12 +3,9 @@ use std::collections::HashMap;
 use anyhow::{Context as _, Result, bail};
 
 use deltachat_contact_tools::may_be_valid_addr;
-use num_traits::cast::ToPrimitive;
 
 use super::{DCLOGIN_SCHEME, Qr};
-use crate::config::Config;
+use crate::login_param::{EnteredCertificateChecks, EnteredLoginParam, EnteredServerLoginParam};
-use crate::context::Context;
-use crate::login_param::EnteredCertificateChecks;
 use crate::provider::Socket;
 
 /// Options for `dclogin:` scheme.
@@ -157,15 +154,10 @@ fn parse_certificate_checks(
     })
 }
 
-pub(crate) async fn configure_from_login_qr(
+pub(crate) fn login_param_from_login_qr(
-    context: &Context,
+    addr: &str,
-    address: &str,
     options: LoginOptions,
-) -> Result<()> {
+) -> Result<EnteredLoginParam> {
-    context
-        .set_config_internal(Config::Addr, Some(address))
-        .await?;
-
     match options {
         LoginOptions::V1 {
             mail_pw,
@@ -181,77 +173,26 @@ pub(crate) async fn configure_from_login_qr(
             smtp_security,
             certificate_checks,
         } => {
-            context
+            let param = EnteredLoginParam {
-                .set_config_internal(Config::MailPw, Some(&mail_pw))
+                addr: addr.to_string(),
-                .await?;
+                imap: EnteredServerLoginParam {
-            if let Some(value) = imap_host {
+                    server: imap_host.unwrap_or_default(),
-                context
+                    port: imap_port.unwrap_or_default(),
-                    .set_config_internal(Config::MailServer, Some(&value))
+                    security: imap_security.unwrap_or_default(),
-                    .await?;
+                    user: imap_username.unwrap_or_default(),
-            }
+                    password: imap_password.unwrap_or(mail_pw),
-            if let Some(value) = imap_port {
+                },
-                context
+                smtp: EnteredServerLoginParam {
-                    .set_config_internal(Config::MailPort, Some(&value.to_string()))
+                    server: smtp_host.unwrap_or_default(),
-                    .await?;
+                    port: smtp_port.unwrap_or_default(),
-            }
+                    security: smtp_security.unwrap_or_default(),
-            if let Some(value) = imap_username {
+                    user: smtp_username.unwrap_or_default(),
-                context
+                    password: smtp_password.unwrap_or_default(),
-                    .set_config_internal(Config::MailUser, Some(&value))
+                },
-                    .await?;
+                certificate_checks: certificate_checks.unwrap_or_default(),
-            }
+                oauth2: false,
-            if let Some(value) = imap_password {
+            };
-                context
+            Ok(param)
-                    .set_config_internal(Config::MailPw, Some(&value))
-                    .await?;
-            }
-            if let Some(value) = imap_security {
-                let code = value
-                    .to_u8()
-                    .context("could not convert imap security value to number")?;
-                context
-                    .set_config_internal(Config::MailSecurity, Some(&code.to_string()))
-                    .await?;
-            }
-            if let Some(value) = smtp_host {
-                context
-                    .set_config_internal(Config::SendServer, Some(&value))
-                    .await?;
-            }
-            if let Some(value) = smtp_port {
-                context
-                    .set_config_internal(Config::SendPort, Some(&value.to_string()))
-                    .await?;
-            }
-            if let Some(value) = smtp_username {
-                context
-                    .set_config_internal(Config::SendUser, Some(&value))
-                    .await?;
-            }
-            if let Some(value) = smtp_password {
-                context
-                    .set_config_internal(Config::SendPw, Some(&value))
-                    .await?;
-            }
-            if let Some(value) = smtp_security {
-                let code = value
-                    .to_u8()
-                    .context("could not convert smtp security value to number")?;
-                context
-                    .set_config_internal(Config::SendSecurity, Some(&code.to_string()))
-                    .await?;
-            }
-            if let Some(value) = certificate_checks {
-                let code = value
-                    .to_u32()
-                    .context("could not convert certificate checks value to number")?;
-                context
-                    .set_config_internal(Config::ImapCertificateChecks, Some(&code.to_string()))
-                    .await?;
-                context
-                    .set_config_internal(Config::SmtpCertificateChecks, Some(&code.to_string()))
-                    .await?;
-            }
-            Ok(())
         }
         _ => bail!(
             "DeltaChat does not understand this QR Code yet, please update the app and try again."

src/qr/qr_tests.rs

@@ -1,6 +1,8 @@
 use super::*;
 use crate::chat::create_group;
 use crate::config::Config;
+use crate::login_param::EnteredCertificateChecks;
+use crate::provider::Socket;
 use crate::securejoin::get_securejoin_qr;
 use crate::test_utils::{TestContext, TestContextManager, sync};
 
@@ -581,101 +583,58 @@ async fn test_withdraw_multidevice() -> Result<()> {
 }
 
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
-async fn test_decode_and_apply_dclogin() -> Result<()> {
+async fn test_decode_dclogin() -> Result<()> {
-    let ctx = TestContext::new().await;
+    let ctx = &TestContext::new().await;
 
-    let result = check_qr(&ctx.ctx, "dclogin:usename+extension@host?p=1234&v=1").await?;
+    let Qr::Login { address, options } =
-    if let Qr::Login { address, options } = result {
+        check_qr(ctx, "dclogin:username+extension@host?p=1234&v=1").await?
-        assert_eq!(address, "usename+extension@host".to_owned());
+    else {
+        unreachable!();
+    };
 
-        if let LoginOptions::V1 { mail_pw, .. } = options {
+    assert_eq!(address, "username+extension@host".to_owned());
-            assert_eq!(mail_pw, "1234".to_owned());
+
-        } else {
+    if let LoginOptions::V1 { ref mail_pw, .. } = options {
-            bail!("wrong type")
+        assert_eq!(mail_pw, "1234");
-        }
     } else {
         bail!("wrong type")
     }
 
-    assert!(ctx.ctx.get_config(Config::Addr).await?.is_none());
+    let param = login_param_from_login_qr(&address, options)?;
-    assert!(ctx.ctx.get_config(Config::MailPw).await?.is_none());
+    assert_eq!(param.addr, "username+extension@host");
-
+    assert_eq!(param.imap.password, "1234");
-    set_config_from_qr(&ctx.ctx, "dclogin:username+extension@host?p=1234&v=1").await?;
-    assert_eq!(
-        ctx.ctx.get_config(Config::Addr).await?,
-        Some("username+extension@host".to_owned())
-    );
-    assert_eq!(
-        ctx.ctx.get_config(Config::MailPw).await?,
-        Some("1234".to_owned())
-    );
 
     Ok(())
 }
 
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
-async fn test_decode_and_apply_dclogin_advanced_options() -> Result<()> {
+async fn test_decode_dclogin_advanced_options() -> Result<()> {
-    let ctx = TestContext::new().await;
+    let ctx = &TestContext::new().await;
-    set_config_from_qr(&ctx.ctx, "dclogin:username+extension@host?p=1234&spw=4321&sh=send.host&sp=7273&su=SendUser&ih=host.tld&ip=4343&iu=user&ipw=password&is=ssl&ic=1&sc=3&ss=plain&v=1").await?;
+
-    assert_eq!(
+    let Qr::Login { address, options } = check_qr(ctx, "dclogin:username+extension@host?p=1234&spw=4321&sh=send.host&sp=7273&su=SendUser&ih=host.tld&ip=4343&iu=user&ipw=password&is=ssl&ic=1&sc=3&ss=plain&v=1").await? else {
-        ctx.ctx.get_config(Config::Addr).await?,
+        unreachable!();
-        Some("username+extension@host".to_owned())
+    };
-    );
+
+    assert_eq!(address, "username+extension@host");
+
+    let param = login_param_from_login_qr(&address, options)?;
+    assert_eq!(param.imap.server, "host.tld");
+    assert_eq!(param.imap.port, 4343);
+    assert_eq!(param.imap.user, "user");
 
     // `p=1234` is ignored, because `ipw=password` is set
+    assert_eq!(param.imap.password, "password");
+    assert_eq!(param.imap.security, Socket::Ssl);
 
-    assert_eq!(
+    assert_eq!(param.smtp.password, "4321");
-        ctx.ctx.get_config(Config::MailServer).await?,
+    assert_eq!(param.smtp.server, "send.host");
-        Some("host.tld".to_owned())
+    assert_eq!(param.smtp.port, 7273);
-    );
+    assert_eq!(param.smtp.user, "SendUser");
-    assert_eq!(
+    assert_eq!(param.smtp.security, Socket::Plain);
-        ctx.ctx.get_config(Config::MailPort).await?,
-        Some("4343".to_owned())
-    );
-    assert_eq!(
-        ctx.ctx.get_config(Config::MailUser).await?,
-        Some("user".to_owned())
-    );
-    assert_eq!(
-        ctx.ctx.get_config(Config::MailPw).await?,
-        Some("password".to_owned())
-    );
-    assert_eq!(
-        ctx.ctx.get_config(Config::MailSecurity).await?,
-        Some("1".to_owned()) // ssl
-    );
-    assert_eq!(
-        ctx.ctx.get_config(Config::ImapCertificateChecks).await?,
-        Some("1".to_owned())
-    );
-
-    assert_eq!(
-        ctx.ctx.get_config(Config::SendPw).await?,
-        Some("4321".to_owned())
-    );
-    assert_eq!(
-        ctx.ctx.get_config(Config::SendServer).await?,
-        Some("send.host".to_owned())
-    );
-    assert_eq!(
-        ctx.ctx.get_config(Config::SendPort).await?,
-        Some("7273".to_owned())
-    );
-    assert_eq!(
-        ctx.ctx.get_config(Config::SendUser).await?,
-        Some("SendUser".to_owned())
-    );
 
     // `sc` option is actually ignored and `ic` is used instead
     // because `smtp_certificate_checks` is deprecated.
-    assert_eq!(
+    assert_eq!(param.certificate_checks, EnteredCertificateChecks::Strict);
-        ctx.ctx.get_config(Config::SmtpCertificateChecks).await?,
-        Some("1".to_owned())
-    );
-    assert_eq!(
-        ctx.ctx.get_config(Config::SendSecurity).await?,
-        Some("3".to_owned()) // plain
-    );
 
     Ok(())
 }