[geary/wip/714104-refine-account-dialog: 34/69] Rework the Engine's account validation process.

[Michael Gratton <mjog src gnome org>]

commit 49781e63eaa44613e376ae0919754ef83e73890c
Author: Michael James Gratton <mike vee net>
Date:   Mon Jul 23 14:52:48 2018 +1000

    Rework the Engine's account validation process.
    
    Split up account validation up into IMAP and SMTP validation so clients
    can manually manage it. Don't bother trying to validate email address and
    nick names, that can and should be done by clients.

 src/client/accounts/account-dialog.vala       |   6 +-
 src/client/application/geary-controller.vala  |  18 ++--
 src/engine/api/geary-account-information.vala |  21 ++--
 src/engine/api/geary-engine.vala              | 138 +++++++++++++-------------
 4 files changed, 86 insertions(+), 97 deletions(-)
---
diff --git a/src/client/accounts/account-dialog.vala b/src/client/accounts/account-dialog.vala
index 971269c8..0b31d884 100644
--- a/src/client/accounts/account-dialog.vala
+++ b/src/client/accounts/account-dialog.vala
@@ -173,9 +173,9 @@ public class AccountDialog : Gtk.Dialog {
         Geary.Engine.ValidationOption options) {
         Geary.Engine.ValidationResult validation_result = Geary.Engine.ValidationResult.OK;
         for (;;) {
-            validation_result = yield this.application.controller.validate_async(
-                account_information, options);
-            
+            //validation_result = yield this.application.controller.validate_async(
+            //    account_information, options);
+
             // If account was successfully added return to the account list.
             if (validation_result == Geary.Engine.ValidationResult.OK) {
                 account_list_pane.present();
diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala
index 27184295..72bf61d6 100644
--- a/src/client/application/geary-controller.vala
+++ b/src/client/application/geary-controller.vala
@@ -895,15 +895,15 @@ public class GearyController : Geary.BaseObject {
         validating_endpoints.add(account_information.smtp.endpoint);
 
         Geary.Engine.ValidationResult result = Geary.Engine.ValidationResult.OK;
-        try {
-            result = yield Geary.Engine.instance.validate_account_information_async(account_information,
-                options, cancellable);
-        } catch (Error err) {
-            debug("Error validating account: %s", err.message);
-            this.application.exit(-1); // Fatal error
-
-            return result;
-        }
+        // try {
+        //     result = yield Geary.Engine.instance.validate_account_information_async(account_information,
+        //         options, cancellable);
+        // } catch (Error err) {
+        //     debug("Error validating account: %s", err.message);
+        //     this.application.exit(-1); // Fatal error
+
+        //     return result;
+        // }
 
         validating_endpoints.remove(account_information.imap.endpoint);
         validating_endpoints.remove(account_information.smtp.endpoint);
diff --git a/src/engine/api/geary-account-information.vala b/src/engine/api/geary-account-information.vala
index c1b35d1b..3994b80f 100644
--- a/src/engine/api/geary-account-information.vala
+++ b/src/engine/api/geary-account-information.vala
@@ -584,13 +584,16 @@ public class Geary.AccountInformation : BaseObject {
         );
     }
 
-    internal void connect_endpoints() {
+    internal void connect_imap_endpoint() {
         if (this.imap.endpoint == null) {
             this.imap.endpoint = get_imap_endpoint();
             this.imap.endpoint.untrusted_host.connect(
                 on_imap_untrusted_host
             );
         }
+    }
+
+    internal void connect_smtp_endpoint() {
         if (this.smtp.endpoint == null) {
             this.smtp.endpoint = get_smtp_endpoint();
             this.smtp.endpoint.untrusted_host.connect(
@@ -639,7 +642,7 @@ public class Geary.AccountInformation : BaseObject {
                 imap_endpoint = new Endpoint(this.imap.host, this.imap.port,
                     imap_flags, Imap.ClientConnection.RECOMMENDED_TIMEOUT_SEC);
             break;
-            
+
             default:
                 assert_not_reached();
         }
@@ -647,12 +650,7 @@ public class Geary.AccountInformation : BaseObject {
         // look for existing one in the global pool; want to use that
         // because Endpoint is mutable and signalled in such a way
         // that it's better to share them
-        imap_endpoint = get_shared_endpoint(this.imap, imap_endpoint);
-
-        // bind shared Endpoint signal to this AccountInformation's signal
-        imap_endpoint.untrusted_host.connect(on_imap_untrusted_host);
-
-        return imap_endpoint;
+        return get_shared_endpoint(this.imap, imap_endpoint);
     }
 
     private Endpoint get_smtp_endpoint() {
@@ -688,12 +686,7 @@ public class Geary.AccountInformation : BaseObject {
         // look for existing one in the global pool; want to use that
         // because Endpoint is mutable and signalled in such a way
         // that it's better to share them
-        smtp_endpoint = get_shared_endpoint(this.smtp, smtp_endpoint);
-
-        // bind shared Endpoint signal to this AccountInformation's signal
-        smtp_endpoint.untrusted_host.connect(on_smtp_untrusted_host);
-
-        return smtp_endpoint;
+        return get_shared_endpoint(this.smtp, smtp_endpoint);
     }
 
     public static Geary.FolderPath? build_folder_path(Gee.List<string>? parts) {
diff --git a/src/engine/api/geary-engine.vala b/src/engine/api/geary-engine.vala
index 6ccd1786..cba9b3d6 100644
--- a/src/engine/api/geary-engine.vala
+++ b/src/engine/api/geary-engine.vala
@@ -201,103 +201,98 @@ public class Geary.Engine : BaseObject {
     }
 
     /**
-     * Returns whether the account information "validates."  If validate_connection is true,
-     * we check if we can connect to the endpoints and authenticate using the supplied credentials.
+     * Determines if an account's IMAP service can be connected to.
      */
-    public async ValidationResult validate_account_information_async(AccountInformation account,
-        ValidationOption options, Cancellable? cancellable = null) throws Error {
+    public async void validate_imap(AccountInformation account,
+                                    GLib.Cancellable? cancellable = null)
+        throws GLib.Error {
         check_opened();
-        
-        ValidationResult error_code = ValidationResult.OK;
-        
-        // Make sure the account nickname and email is not in use.
-        foreach (AccountInformation a in get_accounts().values) {
-            // Don't need to check a's alternate_emails since they
-            // can't be set at account creation time
-            bool has_email = account.has_email_address(a.primary_mailbox);
-
-            if (!has_email && Geary.String.stri_equal(account.nickname, a.nickname))
-                error_code |= ValidationResult.INVALID_NICKNAME;
-
-            // if creating a new Account, don't allow an existing email address
-            if (!options.is_all_set(ValidationOption.UPDATING_EXISTING) && has_email)
-                error_code |= ValidationResult.EMAIL_EXISTS;
+
+        if (account.imap.port == 0) {
+            account.imap.port = account.imap.use_ssl
+                ? Imap.ClientConnection.DEFAULT_PORT_SSL
+                : Imap.ClientConnection.DEFAULT_PORT;
         }
-        
-        // If we don't need to validate the connection, exit out here.
-        if (!options.is_all_set(ValidationOption.CHECK_CONNECTIONS))
-            return error_code;
 
         account.untrusted_host.connect(on_untrusted_host);
-        account.connect_endpoints();
+        account.connect_imap_endpoint();
 
-        // validate IMAP, which requires logging in and establishing an AUTHORIZED cx state
+        // validate IMAP, which requires logging in and establishing
+        // an AUTHORIZED cx state
         Geary.Imap.ClientSession? imap_session = new Imap.ClientSession(
             account.imap.endpoint
         );
+
+        // XXX initiate_session_async doesn't seem to actually throw
+        // an imap error on login failed. This is not worth fixing
+        // until wip/26-proton-mail-bridge lands though, so use
+        // signals as a workaround instead.
+        bool login_failed = false;
+        imap_session.login_failed.connect(() => login_failed = true);
+
         try {
             yield imap_session.connect_async(cancellable);
-        } catch (Error err) {
-            debug("Error connecting to IMAP server: %s", err.message);
-            error_code |= ValidationResult.IMAP_CONNECTION_FAILED;
-        }
-
-        if (!error_code.is_all_set(ValidationResult.IMAP_CONNECTION_FAILED)) {
+            yield imap_session.initiate_session_async(
+                account.imap.credentials, cancellable
+            );
+        } finally {
             try {
-                yield imap_session.initiate_session_async(account.imap.credentials, cancellable);
-                
-                // Connected and initiated, still need to be sure connection authorized
-                Imap.MailboxSpecifier current_mailbox;
-                if (imap_session.get_protocol_state(out current_mailbox) != 
Imap.ClientSession.ProtocolState.AUTHORIZED)
-                    error_code |= ValidationResult.IMAP_CREDENTIALS_INVALID;
-            } catch (Error err) {
-                debug("Error validating IMAP account info: %s", err.message);
-                if (err is ImapError.UNAUTHENTICATED)
-                    error_code |= ValidationResult.IMAP_CREDENTIALS_INVALID;
-                else
-                    error_code |= ValidationResult.IMAP_CONNECTION_FAILED;
+                yield imap_session.disconnect_async(cancellable);
+            } catch {
+                // Oh well
             }
+            account.disconnect_endpoints();
+            account.untrusted_host.disconnect(on_untrusted_host);
         }
 
-        try {
-            yield imap_session.disconnect_async(cancellable);
-        } catch (Error err) {
-            // ignored
-        } finally {
-            imap_session = null;
+        if (login_failed) {
+            // XXX This should be a LOGIN_FAILED error or something
+            throw new ImapError.UNAUTHENTICATED("Login failed");
+        }
+    }
+
+    /**
+     * Determines if an account's SMTP service can be connected to.
+     */
+    public async void validate_smtp(AccountInformation account,
+                                    GLib.Cancellable? cancellable = null)
+        throws GLib.Error {
+        check_opened();
+
+        if (account.smtp.port == 0) {
+            if (account.smtp.use_ssl) {
+                account.smtp.port = Smtp.ClientConnection.DEFAULT_PORT_SSL;
+            } else if (account.smtp.use_starttls) {
+                account.smtp.port = account.smtp.smtp_noauth
+                    ? Smtp.ClientConnection.DEFAULT_PORT
+                    : Smtp.ClientConnection.DEFAULT_PORT_STARTTLS;
+            } else {
+                account.smtp.port = Smtp.ClientConnection.DEFAULT_PORT;
+            }
         }
 
-        // SMTP is simpler, merely see if login works and done (throws
-        // an SmtpError if not)
+        account.untrusted_host.connect(on_untrusted_host);
+        account.connect_smtp_endpoint();
+
         Geary.Smtp.ClientSession? smtp_session = new Geary.Smtp.ClientSession(
             account.smtp.endpoint
         );
+
         try {
             yield smtp_session.login_async(
                 account.get_smtp_credentials(), cancellable
             );
-        } catch (Error err) {
-            debug("Error validating SMTP account info: %s", err.message);
-            if (err is SmtpError.AUTHENTICATION_FAILED)
-                error_code |= ValidationResult.SMTP_CREDENTIALS_INVALID;
-            else
-                error_code |= ValidationResult.SMTP_CONNECTION_FAILED;
-        }
-
-        try {
-            yield smtp_session.logout_async(true, cancellable);
-        } catch (Error err) {
-            // ignored
         } finally {
-            smtp_session = null;
+            try {
+                yield smtp_session.logout_async(true, cancellable);
+            } catch {
+                // Oh well
+            }
+            account.disconnect_endpoints();
+            account.untrusted_host.disconnect(on_untrusted_host);
         }
-
-        account.disconnect_endpoints();
-        account.untrusted_host.disconnect(on_untrusted_host);
-
-        return error_code;
     }
-    
+
     /**
      * Creates a Geary.Account from a Geary.AccountInformation (which is what
      * other methods in this interface deal in).
@@ -366,7 +361,8 @@ public class Geary.Engine : BaseObject {
 
         accounts.set(account.id, account);
 
-        account.connect_endpoints();
+        account.connect_imap_endpoint();
+        account.connect_smtp_endpoint();
         account.untrusted_host.connect(on_untrusted_host);
         account_available(account);
     }