[geary/wip/714104-refine-account-dialog: 58/69] Move endpoints from account config to implementation objects (1/3)



commit e760b074a2ece309171f7a48c22d1445fb7f3d2e
Author: Michael Gratton <mike vee net>
Date:   Sun Nov 18 21:31:35 2018 +1100

    Move endpoints from account config to implementation objects (1/3)
    
    This removes Endpoint from ServiceInformation and moves it to a new
    ClientService class, since ServiceInformation is for apps to provide
    configuration, and Endpoints are effectively implementation detail. The
    new ClientService class is in effect an analoge to ServiceInformation
    in the same way as Account is to AccountInformation, and so this is a
    better place to store an account's endpoints. Two instances have been
    added to Account as `incoming` and `outgoing` properties instead of imap
    and SMTP to be a bit more generic and with an eye to supporting other
    protocols in the future.
    
    This is possble to implement now non-generic providers are populating
    ServiceInformation classes rather than Endpoints directly, and reduces
    the complexity of the code needed to manage endpoints since all of the
    untrusted_host callback complexity in AccountInformation and the Engine
    can be removed, and will also allow simplifing credentials and SMTP
    codepaths somewhat.
    
    This work has been broken up in to thee parts to make the changes
    clearer.

 po/POTFILES.in                                     |   1 +
 src/client/application/geary-controller.vala       |  85 +++++++++-------
 src/client/application/secret-mediator.vala        |  12 +--
 src/client/components/main-window-info-bar.vala    |   7 +-
 src/client/dialogs/certificate-warning-dialog.vala |   4 +-
 src/engine/api/geary-account-information.vala      |  56 +----------
 src/engine/api/geary-account.vala                  |  45 +++++----
 src/engine/api/geary-client-service.vala           |  98 ++++++++++++++++++
 src/engine/api/geary-endpoint.vala                 |  12 +++
 src/engine/api/geary-engine.vala                   | 109 ++++++++++++---------
 src/engine/api/geary-service-information.vala      |   8 --
 src/engine/imap-db/outbox/smtp-outbox-folder.vala  |   2 +-
 .../imap-engine/imap-engine-generic-account.vala   |  53 +++++-----
 src/engine/meson.build                             |   1 +
 test/engine/api/geary-account-mock.vala            |  67 ++++++++++---
 15 files changed, 336 insertions(+), 224 deletions(-)
---
diff --git a/po/POTFILES.in b/po/POTFILES.in
index a71e05dd..543faa84 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -115,6 +115,7 @@ src/engine/api/geary-account.vala
 src/engine/api/geary-aggregated-folder-properties.vala
 src/engine/api/geary-attachment.vala
 src/engine/api/geary-base-object.vala
+src/engine/api/geary-client-service.vala
 src/engine/api/geary-composed-email.vala
 src/engine/api/geary-contact-flags.vala
 src/engine/api/geary-contact-importance.vala
diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala
index c2bf0c2e..2bc85320 100644
--- a/src/client/application/geary-controller.vala
+++ b/src/client/application/geary-controller.vala
@@ -69,6 +69,12 @@ public class GearyController : Geary.BaseObject {
         GearyController.untitled_file_name = _("Untitled");
     }
 
+    private static void get_gcr_params(Geary.Endpoint endpoint, out Gcr.Certificate cert,
+        out string peer) {
+        cert = new Gcr.SimpleCertificate(endpoint.untrusted_certificate.certificate.data);
+        peer = "%s:%u".printf(endpoint.remote_address.hostname, endpoint.remote_address.port);
+    }
+
 
     internal class AccountContext : Geary.BaseObject {
 
@@ -666,15 +672,19 @@ public class GearyController : Geary.BaseObject {
         locked_prompt_untrusted_host_async(Geary.AccountInformation account,
                                            Geary.ServiceInformation service,
                                            Geary.TlsNegotiationMethod method,
-                                           TlsConnection cx) {
+                                           GLib.TlsConnection cx) {
+        Geary.Endpoint endpoint = get_endpoint(account, service);
+
         // possible while waiting on mutex that this endpoint became trusted or untrusted
-        if (service.endpoint.trust_untrusted_host != Geary.Trillian.UNKNOWN)
+        if (endpoint == null ||
+            endpoint.trust_untrusted_host != Geary.Trillian.UNKNOWN) {
             return;
+        }
 
         // get GCR parameters
         Gcr.Certificate cert;
         string peer;
-        get_gcr_params(service.endpoint, out cert, out peer);
+        get_gcr_params(endpoint, out cert, out peer);
 
         // Geary allows for user to auto-revoke all questionable server certificates without
         // digging around in a keyring/pk manager
@@ -696,7 +706,7 @@ public class GearyController : Geary.BaseObject {
         try {
             if (gcr_trust_is_certificate_pinned(cert, GCR_PURPOSE_SERVER_AUTH, peer, null)) {
                 debug("Certificate for %s is pinned, accepting connection...", peer);
-                service.endpoint.trust_untrusted_host = Geary.Trillian.TRUE;
+                endpoint.trust_untrusted_host = Geary.Trillian.TRUE;
                 return;
             }
         } catch (Error err) {
@@ -711,10 +721,10 @@ public class GearyController : Geary.BaseObject {
                                            Geary.AccountInformation account,
                                            Geary.ServiceInformation service,
                                            bool is_validation) {
+        Geary.Endpoint endpoint = get_endpoint(account, service);
         CertificateWarningDialog dialog = new CertificateWarningDialog(
-            parent, account, service, is_validation
+            parent, account, service, endpoint, is_validation
         );
-        Geary.Endpoint endpoint = service.endpoint;
         switch (dialog.run()) {
             case CertificateWarningDialog.Result.TRUST:
                 endpoint.trust_untrusted_host = Geary.Trillian.TRUE;
@@ -763,7 +773,7 @@ public class GearyController : Geary.BaseObject {
         else
             parent = main_window;
 
-        Geary.Endpoint endpoint = service.endpoint;
+        Geary.Endpoint endpoint = get_endpoint(account, service);
 
         // If Endpoint had unresolved TLS issues, prompt user about them
         if (endpoint.tls_validation_warnings != 0 && endpoint.trust_untrusted_host != Geary.Trillian.TRUE) {
@@ -792,7 +802,7 @@ public class GearyController : Geary.BaseObject {
     //
     // Returns possibly modified validation results
     public async Geary.Engine.ValidationResult validation_check_for_tls_warnings_async(
-        Geary.AccountInformation account, Geary.Engine.ValidationResult validation_result,
+        Geary.AccountInformation config, Geary.Engine.ValidationResult validation_result,
         out bool retry_required) {
         retry_required = false;
 
@@ -804,8 +814,8 @@ public class GearyController : Geary.BaseObject {
         // check each service for problems, prompting user each time for verification
         bool imap_prompted, imap_retry_required;
         validation_result = validation_check_endpoint_for_tls_warnings(
-            account,
-            account.imap,
+            config,
+            config.imap,
             validation_result,
             out imap_prompted,
             out imap_retry_required
@@ -813,17 +823,19 @@ public class GearyController : Geary.BaseObject {
 
         bool smtp_prompted, smtp_retry_required;
         validation_result = validation_check_endpoint_for_tls_warnings(
-            account,
-            account.smtp,
+            config,
+            config.smtp,
             validation_result,
             out smtp_prompted,
             out smtp_retry_required
         );
 
-        // if prompted for user acceptance of bad certificates and they agreed to both, try again
+        // if prompted for user acceptance of bad certificates and
+        // they agreed to both, try again
+        Geary.Account account = this.accounts.get(config).account;
         if (imap_prompted && smtp_prompted
-            && account.imap.endpoint.is_trusted_or_never_connected
-            && account.smtp.endpoint.is_trusted_or_never_connected) {
+            && account.incoming.endpoint.is_trusted_or_never_connected
+            && account.outgoing.endpoint.is_trusted_or_never_connected) {
             retry_required = true;
         } else if (validation_result == Geary.Engine.ValidationResult.OK) {
             retry_required = true;
@@ -831,7 +843,7 @@ public class GearyController : Geary.BaseObject {
             // if prompt requires retry or otherwise detected it, retry
             retry_required = imap_retry_required && smtp_retry_required;
         }
-        
+
         return validation_result;
     }
 
@@ -868,21 +880,15 @@ public class GearyController : Geary.BaseObject {
             info_bar.report as Geary.ServiceProblemReport;
         Error retry_err = null;
         if (service_report != null) {
-            Geary.Account? account = null;
-            try {
-                account = this.application.engine.get_account_instance(
-                    service_report.account
-                );
-            } catch (Error err) {
-                debug("Error getting account for error retry: %s", err.message);
-            }
-
-            if (account != null && account.is_open()) {
+            AccountContext? context = this.accounts.get(service_report.account);
+            if (context != null && context.account.is_open()) {
                 switch (service_report.service.protocol) {
                 case Geary.Protocol.IMAP:
-                    account.start_incoming_client.begin((obj, ret) => {
+                    context.account.incoming.start.begin(
+                        context.cancellable,
+                        (obj, ret) => {
                             try {
-                                account.start_incoming_client.end(ret);
+                                context.account.incoming.start.end(ret);
                             } catch (Error err) {
                                 retry_err = err;
                             }
@@ -890,9 +896,11 @@ public class GearyController : Geary.BaseObject {
                     break;
 
                 case Geary.Protocol.SMTP:
-                    account.start_outgoing_client.begin((obj, ret) => {
+                    context.account.outgoing.start.begin(
+                        context.cancellable,
+                        (obj, ret) => {
                             try {
-                                account.start_outgoing_client.end(ret);
+                                context.account.outgoing.start.end(ret);
                             } catch (Error err) {
                                 retry_err = err;
                             }
@@ -2913,10 +2921,19 @@ public class GearyController : Geary.BaseObject {
         return selected_conversations.read_only_view;
     }
 
-    private static void get_gcr_params(Geary.Endpoint endpoint, out Gcr.Certificate cert,
-        out string peer) {
-        cert = new Gcr.SimpleCertificate(endpoint.untrusted_certificate.certificate.data);
-        peer = "%s:%u".printf(endpoint.remote_address.hostname, endpoint.remote_address.port);
+    private inline Geary.Endpoint? get_endpoint(Geary.AccountInformation config,
+                                               Geary.ServiceInformation service) {
+        Geary.Account account = this.accounts.get(config).account;
+        Geary.Endpoint? endpoint = null;
+        switch (service.protocol) {
+        case Geary.Protocol.IMAP:
+            endpoint = account.incoming.endpoint;
+            break;
+        case Geary.Protocol.SMTP:
+            endpoint = account.outgoing.endpoint;
+            break;
+        }
+        return endpoint;
     }
 
     private inline Geary.App.EmailStore get_store_for_folder(Geary.Folder target) {
diff --git a/src/client/application/secret-mediator.vala b/src/client/application/secret-mediator.vala
index aad73993..0e41be55 100644
--- a/src/client/application/secret-mediator.vala
+++ b/src/client/application/secret-mediator.vala
@@ -66,10 +66,10 @@ public class SecretMediator : Geary.CredentialsMediator, Object {
                 loaded = true;
             } else {
                 debug(
-                    "Unable to fetch password in liqbsecret keyring for %s: %s %s",
+                    "Unable to fetch libsecret password for %s: %s %s",
+                    account.id,
                     service.protocol.to_string(),
-                    service.credentials.user,
-                    service.endpoint.remote_address.get_hostname()
+                    service.credentials.user
                 );
             }
         } else {
@@ -123,10 +123,10 @@ public class SecretMediator : Geary.CredentialsMediator, Object {
                 yield do_store(service, service.credentials.token, cancellable);
             } catch (Error e) {
                 debug(
-                    "Unable to store password in libsecret keyring for %s: %s %s",
+                    "Unable to store libsecret password for %s: %s %s",
+                    account.id,
                     service.protocol.to_string(),
-                    service.credentials.user,
-                    service.endpoint.remote_address.get_hostname()
+                    service.credentials.user
                 );
             }
         } else {
diff --git a/src/client/components/main-window-info-bar.vala b/src/client/components/main-window-info-bar.vala
index f2b20d0b..17902e36 100644
--- a/src/client/components/main-window-info-bar.vala
+++ b/src/client/components/main-window-info-bar.vala
@@ -44,9 +44,8 @@ public class MainWindowInfoBar : Gtk.InfoBar {
 
         if (report is Geary.ServiceProblemReport) {
             Geary.ServiceProblemReport service_report = (Geary.ServiceProblemReport) report;
-            Geary.Endpoint endpoint = service_report.service.endpoint;
             string account = service_report.account.display_name;
-            string server = endpoint.remote_address.hostname;
+            string server = service_report.service.host;
 
             if (report.problem_type == Geary.ProblemType.CONNECTION_ERROR &&
                 service_report.service.protocol == Geary.Protocol.IMAP) {
@@ -216,8 +215,8 @@ public class MainWindowInfoBar : Gtk.InfoBar {
                 service_report.service.protocol.to_string()
             );
             details.append_printf(
-                "Endpoint: %s\n",
-                service_report.service.endpoint.to_string()
+                "Service host: %s\n",
+                service_report.service.host
             );
         }
         if (this.report.error == null) {
diff --git a/src/client/dialogs/certificate-warning-dialog.vala 
b/src/client/dialogs/certificate-warning-dialog.vala
index bcdc4bb9..f8e49f31 100644
--- a/src/client/dialogs/certificate-warning-dialog.vala
+++ b/src/client/dialogs/certificate-warning-dialog.vala
@@ -18,6 +18,7 @@ public class CertificateWarningDialog {
     public CertificateWarningDialog(Gtk.Window? parent,
                                     Geary.AccountInformation account,
                                     Geary.ServiceInformation service,
+                                    Geary.Endpoint endpoint,
                                     bool is_validation) {
         Gtk.Builder builder = GioUtil.create_builder("certificate_warning_dialog.glade");
 
@@ -34,12 +35,11 @@ public class CertificateWarningDialog {
 
         title_label.label = _("Untrusted Connection: %s").printf(account.display_name);
 
-        Geary.Endpoint endpoint = service.endpoint;
         top_label.label = _("The identity of the %s mail server at %s:%u could not be verified.").printf(
             service.protocol.to_value(), endpoint.remote_address.hostname, endpoint.remote_address.port);
 
         warnings_label.label = generate_warning_list(
-            service.endpoint.tls_validation_warnings
+            endpoint.tls_validation_warnings
         );
         warnings_label.use_markup = true;
 
diff --git a/src/engine/api/geary-account-information.vala b/src/engine/api/geary-account-information.vala
index 886c071f..aed9397f 100644
--- a/src/engine/api/geary-account-information.vala
+++ b/src/engine/api/geary-account-information.vala
@@ -171,13 +171,10 @@ public class Geary.AccountInformation : BaseObject {
 
 
     /**
-     * Indicates the supplied {@link Endpoint} has reported TLS certificate warnings during
-     * connection.
+     * Emitted when a service has reported TLS certificate warnings.
      *
-     * Since this {@link Endpoint} persists for the lifetime of the {@link AccountInformation},
-     * marking it as trusted once will survive the application session.  It is up to the caller to
-     * pin the certificate appropriately if the user does not want to receive these warnings in
-     * the future.
+     * It is up to the caller to pin the certificate appropriately if
+     * the user does not want to receive these warnings in the future.
      */
     public signal void untrusted_host(ServiceInformation service,
                                       TlsNegotiationMethod method,
@@ -224,10 +221,6 @@ public class Geary.AccountInformation : BaseObject {
         this.is_copy = true;
     }
 
-    ~AccountInformation() {
-        disconnect_service_endpoints();
-    }
-
 
     /** Copies all properties from an instance into this one. */
     public void copy_from(AccountInformation from) {
@@ -561,39 +554,6 @@ public class Geary.AccountInformation : BaseObject {
         );
     }
 
-    internal void connect_imap_service(Endpoint service) {
-        if (this.imap.endpoint == null) {
-            this.imap.endpoint = service;
-            this.imap.endpoint.untrusted_host.connect(
-                on_imap_untrusted_host
-            );
-        }
-    }
-
-    internal void connect_smtp_service(Endpoint service) {
-        if (this.smtp.endpoint == null) {
-            this.smtp.endpoint = service;
-            this.smtp.endpoint.untrusted_host.connect(
-                on_smtp_untrusted_host
-            );
-        }
-    }
-
-    internal void disconnect_service_endpoints() {
-        if (this.imap.endpoint != null) {
-            this.imap.endpoint.untrusted_host.disconnect(
-                on_imap_untrusted_host
-            );
-            this.imap.endpoint = null;
-        }
-        if (this.smtp.endpoint != null) {
-            this.smtp.endpoint.untrusted_host.disconnect(
-                on_smtp_untrusted_host
-            );
-            this.smtp.endpoint = null;
-        }
-    }
-
     public static Geary.FolderPath? build_folder_path(Gee.List<string>? parts) {
         if (parts == null || parts.size == 0)
             return null;
@@ -604,14 +564,4 @@ public class Geary.AccountInformation : BaseObject {
         return path;
     }
 
-    private void on_imap_untrusted_host(TlsNegotiationMethod method,
-                                        GLib.TlsConnection cx) {
-        untrusted_host(this.imap, method, cx);
-    }
-
-    private void on_smtp_untrusted_host(TlsNegotiationMethod method,
-                                        GLib.TlsConnection cx) {
-        untrusted_host(this.smtp, method, cx);
-    }
-
 }
diff --git a/src/engine/api/geary-account.vala b/src/engine/api/geary-account.vala
index 6e11ae6e..06af0eb6 100644
--- a/src/engine/api/geary-account.vala
+++ b/src/engine/api/geary-account.vala
@@ -27,7 +27,10 @@ public abstract class Geary.Account : BaseObject {
     internal const uint AUTH_ATTEMPTS_MAX = 3;
 
 
-    public Geary.AccountInformation information { get; protected set; }
+    /**
+     * The account's current configuration.
+     */
+    public AccountInformation information { get; protected set; }
 
     /**
      * Determines if this account appears to be online.
@@ -41,6 +44,16 @@ public abstract class Geary.Account : BaseObject {
      */
     public abstract bool is_online { get; protected set; }
 
+    /**
+     * The service manager for the incoming email service.
+     */
+    public abstract ClientService incoming { get; }
+
+    /**
+     * The service manager for the outgoing email service.
+     */
+    public abstract ClientService outgoing { get; }
+
     public Geary.ProgressMonitor search_upgrade_monitor { get; protected set; }
     public Geary.ProgressMonitor db_upgrade_monitor { get; protected set; }
     public Geary.ProgressMonitor db_vacuum_monitor { get; protected set; }
@@ -221,26 +234,6 @@ public abstract class Geary.Account : BaseObject {
      */
     public abstract async void rebuild_async(Cancellable? cancellable = null) throws Error;
 
-    /**
-     * Starts delivery of messages to the outgoing server.
-     *
-     * Outgoing delivery will be started by default when the account
-     * is opened. This method is mostly useful when re-starting it
-     * after an error has occurred.
-     */
-    public abstract async void start_outgoing_client()
-        throws Error;
-
-    /**
-     * Starts receiving messages from the incoming server.
-     *
-     * The incoming client will be started by default when the account
-     * is opened. This method is mostly useful when re-starting it
-     * after an error has occurred.
-     */
-    public abstract async void start_incoming_client()
-        throws Error;
-
     /**
      * Lists all the currently-available folders found under the parent path
      * unless it's null, in which case it lists all the root folders.  If the
@@ -392,6 +385,16 @@ public abstract class Geary.Account : BaseObject {
         return name;
     }
 
+    /**
+     * Sets network endpoints for incoming and outgoing client services.
+     *
+     * This is called by {@link Engine} after creating the account and
+     * if the network configuration changes. Implementations should
+     * pass these to their incoming and outgoing client services and
+     * restart them as required.
+     */
+    internal abstract void set_endpoints(Endpoint incoming, Endpoint outgoing);
+
     /** Fires a {@link opened} signal. */
     protected virtual void notify_opened() {
         opened();
diff --git a/src/engine/api/geary-client-service.vala b/src/engine/api/geary-client-service.vala
new file mode 100644
index 00000000..0582188a
--- /dev/null
+++ b/src/engine/api/geary-client-service.vala
@@ -0,0 +1,98 @@
+/*
+ * Copyright 2018 Michael Gratton <mike vee net>
+ *
+ * This software is licensed under the GNU Lesser General Public License
+ * (version 2.1 or later). See the COPYING file in this distribution.
+ */
+
+/**
+ * Manages client connections to a specific network service.
+ *
+ * Derived classes are used by accounts to manage client connections
+ * to a specific network service, such as IMAP or SMTP. This class
+ * does connect to the service itself, rather manages the
+ * configuration and life-cycle of client sessions that do connect to
+ * the service.
+ */
+public abstract class Geary.ClientService : BaseObject {
+
+
+    /**
+     * The configuration for the account the service belongs to.
+     */
+    public AccountInformation account { get; private set; }
+
+    /**
+     * The configuration for the service.
+     */
+    public ServiceInformation service { get; private set; }
+
+    /**
+     * The network endpoint the service will connect to.
+     */
+    public Endpoint? endpoint { get; private set; default = null; }
+
+    /** Determines if this manager has been started. */
+    public bool is_running { get; protected set; default = false; }
+
+
+    protected ClientService(AccountInformation account,
+                            ServiceInformation service) {
+        this.account = account;
+        this.service = service;
+    }
+
+    /**
+     * Updates the service's network endpoint and restarts if needed.
+     *
+     * The service will be restarted if it is already running, and if
+     * so will be stopped before the old endpoint is replaced by the
+     * new one, then started again.
+     */
+    public async void set_endpoint_restart(Endpoint endpoint,
+                                           GLib.Cancellable? cancellable = null)
+        throws GLib.Error {
+        if ((this.endpoint == null && endpoint != null) ||
+            (this.endpoint != null && this.endpoint.equal_to(endpoint))) {
+            if (this.endpoint != null) {
+                this.endpoint.untrusted_host.disconnect(on_untrusted_host);
+            }
+
+            bool do_restart = this.is_running;
+            if (do_restart) {
+                yield stop(cancellable);
+            }
+
+            this.endpoint = endpoint;
+            this.endpoint.untrusted_host.connect(on_untrusted_host);
+
+            if (do_restart) {
+                yield start(cancellable);
+            }
+        }
+    }
+
+    /**
+     * Starts the service manager running.
+     *
+     * This may cause the manager to establish connections to the
+     * network service.
+     */
+    public abstract async void start(GLib.Cancellable? cancellable = null)
+        throws GLib.Error;
+
+    /**
+     * Stops the service manager running.
+     *
+     * Any existing connections to the network service will be closed.
+     */
+    public abstract async void stop(GLib.Cancellable? cancellable = null)
+        throws GLib.Error;
+
+
+    private void on_untrusted_host(TlsNegotiationMethod method,
+                                   GLib.TlsConnection cx) {
+        this.account.untrusted_host(this.service, method, cx);
+    }
+
+}
diff --git a/src/engine/api/geary-endpoint.vala b/src/engine/api/geary-endpoint.vala
index 6ea42eea..fe23146d 100644
--- a/src/engine/api/geary-endpoint.vala
+++ b/src/engine/api/geary-endpoint.vala
@@ -233,7 +233,19 @@ public class Geary.Endpoint : BaseObject {
         }
     }
 
+    public bool equal_to(Geary.Endpoint other) {
+        if (this == other) {
+            return true;
+        }
+
+        return (
+            this.remote_address.hostname == other.remote_address.hostname &&
+            this.remote_address.port == other.remote_address.port
+        );
+    }
+
     public string to_string() {
         return "%s/default:%u".printf(remote_address.hostname, remote_address.port);
     }
+
 }
diff --git a/src/engine/api/geary-engine.vala b/src/engine/api/geary-engine.vala
index 977589c5..9e20dd58 100644
--- a/src/engine/api/geary-engine.vala
+++ b/src/engine/api/geary-engine.vala
@@ -109,11 +109,13 @@ public class Geary.Engine : BaseObject {
     public signal void account_unavailable(AccountInformation account);
 
     /**
-     * Fired when an {@link Endpoint} associated with the {@link AccountInformation} reports
-     * TLS certificate warnings during connection.
+     * Emitted when a service has reported TLS certificate warnings.
      *
-     * This may be fired during normal operation or while validating the AccountInformation, in
-     * which case there is no {@link Account} associated with it.
+     * This may be fired during normal operation or while validating
+     * the account information, in which case there is no {@link
+     * Account} associated with it.
+     *
+     * @see AccountInformation.untrusted_host
      */
     public signal void untrusted_host(AccountInformation account,
                                       ServiceInformation service,
@@ -240,42 +242,52 @@ public class Geary.Engine : BaseObject {
                 : Imap.IMAP_PORT;
         }
 
-        account.untrusted_host.connect(on_untrusted_host);
-        account.connect_imap_service(
-            get_shared_endpoint(account.service_provider, account.imap)
+        Endpoint endpoint = get_shared_endpoint(
+            account.service_provider, service
         );
-
-        // validate IMAP, which requires logging in and establishing
-        // an AUTHORIZED cx state
-        Geary.Imap.ClientSession? imap_session = new Imap.ClientSession(
-            service.endpoint
+        ulong untrusted_id = endpoint.untrusted_host.connect(
+            (security, cx) => account.untrusted_host(service, security, cx)
         );
 
-        // 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);
-
-        GLib.Error? login_err = null;
+        Geary.Imap.ClientSession client = new Imap.ClientSession(endpoint);
+        GLib.Error? connect_err = null;
         try {
-            yield imap_session.connect_async(cancellable);
-            yield imap_session.initiate_session_async(
-                service.credentials, cancellable
-            );
+            yield client.connect_async(cancellable);
         } catch (GLib.Error err) {
-            login_err = err;
+            connect_err = err;
         }
 
-        try {
-            yield imap_session.disconnect_async(cancellable);
-        } catch {
-            // Oh well
+        bool login_failed = false;
+        GLib.Error? login_err = null;
+        if (connect_err == null) {
+            // 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.
+            client.login_failed.connect(() => login_failed = true);
+
+            try {
+                yield client.initiate_session_async(
+                    service.credentials, cancellable
+                );
+            } catch (GLib.Error err) {
+                login_err = err;
+            }
+
+            try {
+                yield client.disconnect_async(cancellable);
+            } catch {
+                // Oh well
+            }
         }
 
-        account.disconnect_service_endpoints();
-        account.untrusted_host.disconnect(on_untrusted_host);
+        // This always needs to be disconnected, even when there's an
+        // error
+        endpoint.disconnect(untrusted_id);
+
+        if (connect_err != null) {
+            throw connect_err;
+        }
 
         if (login_failed) {
             // XXX This should be a LOGIN_FAILED error or something
@@ -306,18 +318,17 @@ public class Geary.Engine : BaseObject {
             }
         }
 
-        account.untrusted_host.connect(on_untrusted_host);
-        account.connect_smtp_service(
-            get_shared_endpoint(account.service_provider, account.smtp)
+        Endpoint endpoint = get_shared_endpoint(
+            account.service_provider, service
         );
-
-        Geary.Smtp.ClientSession? smtp_session = new Geary.Smtp.ClientSession(
-            service.endpoint
+        ulong untrusted_id = endpoint.untrusted_host.connect(
+            (security, cx) => account.untrusted_host(service, security, cx)
         );
 
+        Geary.Smtp.ClientSession client = new Geary.Smtp.ClientSession(endpoint);
         GLib.Error? login_err = null;
         try {
-            yield smtp_session.login_async(
+            yield client.login_async(
                 account.get_smtp_credentials(), cancellable
             );
         } catch (GLib.Error err) {
@@ -325,13 +336,14 @@ public class Geary.Engine : BaseObject {
         }
 
         try {
-            yield smtp_session.logout_async(true, cancellable);
+            yield client.logout_async(true, cancellable);
         } catch {
             // Oh well
         }
 
-        account.disconnect_service_endpoints();
-        account.untrusted_host.disconnect(on_untrusted_host);
+        // This always needs to be disconnected, even when there's an
+        // error
+        endpoint.disconnect(untrusted_id);
 
         if (login_err != null) {
             throw login_err;
@@ -388,6 +400,13 @@ public class Geary.Engine : BaseObject {
                 assert_not_reached();
         }
 
+        Endpoint imap = get_shared_endpoint(
+            account_information.service_provider, account_information.imap
+        );
+        Endpoint smtp = get_shared_endpoint(
+            account_information.service_provider, account_information.smtp);
+        account.set_endpoints(imap, smtp);
+
         account_instances.set(account_information.id, account);
         return account;
     }
@@ -405,13 +424,6 @@ public class Geary.Engine : BaseObject {
         }
 
         accounts.set(account.id, account);
-
-        account.connect_imap_service(
-            get_shared_endpoint(account.service_provider, account.imap)
-        );
-        account.connect_smtp_service(
-            get_shared_endpoint(account.service_provider, account.smtp)
-        );
         account.untrusted_host.connect(on_untrusted_host);
         account_available(account);
     }
@@ -433,7 +445,6 @@ public class Geary.Engine : BaseObject {
 
         if (this.accounts.has_key(account.id)) {
             account.untrusted_host.disconnect(on_untrusted_host);
-            account.disconnect_service_endpoints();
 
             // Removal *MUST* be done in the following order:
             // 1. Send the account-unavailable signal.
diff --git a/src/engine/api/geary-service-information.vala b/src/engine/api/geary-service-information.vala
index f14d7651..4f3eb8bc 100644
--- a/src/engine/api/geary-service-information.vala
+++ b/src/engine/api/geary-service-information.vala
@@ -226,14 +226,6 @@ public abstract class Geary.ServiceInformation : GLib.Object {
      */
     public bool smtp_use_imap_credentials { get; set; default = true; }
 
-    /**
-     * The network endpoint for this service.
-     *
-     * This will be null until the service's account has been added to
-     * the engine, and after it has been removed from the engine.
-     */
-    public Endpoint? endpoint { get; internal set; }
-
 
     protected ServiceInformation(Protocol proto, CredentialsMediator mediator) {
         this.protocol = proto;
diff --git a/src/engine/imap-db/outbox/smtp-outbox-folder.vala 
b/src/engine/imap-db/outbox/smtp-outbox-folder.vala
index 6ee36f98..1215b287 100644
--- a/src/engine/imap-db/outbox/smtp-outbox-folder.vala
+++ b/src/engine/imap-db/outbox/smtp-outbox-folder.vala
@@ -73,7 +73,7 @@ private class Geary.SmtpOutboxFolder :
     }
 
     private Endpoint smtp_endpoint {
-        get { return this._account.information.smtp.endpoint; }
+        get { return null; }
     }
 
     private weak Account _account;
diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala 
b/src/engine/imap-engine/imap-engine-generic-account.vala
index e2ff420c..c3ad84e7 100644
--- a/src/engine/imap-engine/imap-engine-generic-account.vala
+++ b/src/engine/imap-engine/imap-engine-generic-account.vala
@@ -27,11 +27,24 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
 
     public override bool is_online { get; protected set; default = false; }
 
+    /** Returns the IMAP client service. */
+    public override ClientService incoming { get { return this.imap; } }
+
+    /** Returns the SMTP client service. */
+    public override ClientService outgoing { get { return this.smtp; } }
+
+    /** Service for incoming IMAP connections. */
+    public ClientService imap  { get; private set; }
+
+    /** Service for outgoing SMTP connections. */
+    public ClientService smtp { get; private set; }
+
+    /** Local database for the account. */
+    public ImapDB.Account local { get; private set; }
+
     /** This account's IMAP session pool. */
     public Imap.ClientSessionManager session_pool { get; private set; }
 
-    internal ImapDB.Account local { get; private set; }
-
     private bool open = false;
     private Cancellable? open_cancellable = null;
     private Nonblocking.Semaphore? remote_ready_lock = null;
@@ -60,7 +73,7 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
 
         this.session_pool = new Imap.ClientSessionManager(
             this.information.id,
-            this.information.imap.endpoint,
+            null, //this.information.imap.endpoint,
             this.information.imap.credentials
         );
         this.session_pool.min_pool_size = IMAP_MIN_POOL_SIZE;
@@ -251,33 +264,6 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
         message("%s: Rebuild complete", to_string());
     }
 
-    /**
-     * Starts the outbox postman running.
-     */
-    public override async void start_outgoing_client()
-        throws Error {
-        check_open();
-        this.local.outbox.start_postman_async.begin();
-    }
-
-    /**
-     * Closes then reopens the IMAP account if it is not ready.
-     */
-    public override async void start_incoming_client()
-        throws Error {
-        check_open();
-        if (!this.session_pool.is_ready) {
-            try {
-                yield this.session_pool.close_async(this.open_cancellable);
-            } catch (Error err) {
-                debug("Ignoring error closing IMAP session pool for restart: %s",
-                      err.message);
-            }
-
-            yield this.session_pool.open_async(this.open_cancellable);
-        }
-    }
-
     /**
      * Queues an operation for execution by this account.
      *
@@ -498,7 +484,7 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
         // that's supposed to be globally unique...
         Geary.RFC822.Message rfc822 = new Geary.RFC822.Message.from_composed_email(
             composed, GMime.utils_generate_message_id(
-                information.smtp.endpoint.remote_address.hostname
+                null //information.smtp.endpoint.remote_address.hostname
             ));
 
         // don't use create_email_async() as that requires the folder be open to use
@@ -563,6 +549,11 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
         return yield local.get_containing_folders_async(ids, cancellable);
     }
 
+    internal override void set_endpoints(Endpoint incoming, Endpoint outgoing) {
+        this.imap.set_endpoint_restart.begin(incoming, this.open_cancellable);
+        this.smtp.set_endpoint_restart.begin(outgoing, this.open_cancellable);
+    }
+
     /**
      * Constructs a set of folders and adds them to the account.
      *
diff --git a/src/engine/meson.build b/src/engine/meson.build
index a0762bcd..f327c375 100644
--- a/src/engine/meson.build
+++ b/src/engine/meson.build
@@ -7,6 +7,7 @@ geary_engine_vala_sources = files(
   'api/geary-aggregated-folder-properties.vala',
   'api/geary-attachment.vala',
   'api/geary-base-object.vala',
+  'api/geary-client-service.vala',
   'api/geary-composed-email.vala',
   'api/geary-contact.vala',
   'api/geary-contact-flags.vala',
diff --git a/test/engine/api/geary-account-mock.vala b/test/engine/api/geary-account-mock.vala
index 78831ab7..850987c3 100644
--- a/test/engine/api/geary-account-mock.vala
+++ b/test/engine/api/geary-account-mock.vala
@@ -31,8 +31,38 @@ public class Geary.MockAccount : Account, MockObject {
     }
 
 
+    public class MockClientService : ClientService {
+
+        public MockClientService(AccountInformation account,
+                                 ServiceInformation service) {
+            base(account, service);
+        }
+
+        public override async void start(GLib.Cancellable? cancellable = null)
+            throws GLib.Error {
+            throw new EngineError.UNSUPPORTED("Mock method");
+        }
+
+        public override async void stop(GLib.Cancellable? cancellable = null)
+            throws GLib.Error {
+            throw new EngineError.UNSUPPORTED("Mock method");
+        }
+
+    }
+
+
     public override bool is_online { get; protected set; default = false; }
 
+    public override ClientService incoming {
+        get { return this.incoming; }
+    }
+    private ClientService _incoming;
+
+    public override ClientService outgoing {
+        get { return this._outgoing; }
+    }
+    private ClientService _outgoing;
+
     protected Gee.Queue<ExpectedCall> expected {
         get; set; default = new Gee.LinkedList<ExpectedCall>();
     }
@@ -40,6 +70,12 @@ public class Geary.MockAccount : Account, MockObject {
 
     public MockAccount(string name, AccountInformation information) {
         base(name, information);
+        this._incoming = new MockClientService(
+            this.information, this.information.imap
+        );
+        this._outgoing = new MockClientService(
+            this.information, this.information.smtp
+        );
     }
 
     public override async void open_async(Cancellable? cancellable = null) throws Error {
@@ -62,21 +98,14 @@ public class Geary.MockAccount : Account, MockObject {
         void_call("rebuild_async", { cancellable });
     }
 
-    public override async void start_outgoing_client()
-        throws Error {
-        void_call("start_outgoing_client", {});
-    }
-
-    public override async void start_incoming_client()
-        throws Error {
-        void_call("start_incoming_client", {});
-    }
-
-    public override Gee.Collection<Folder> list_matching_folders(FolderPath? parent)
-        throws Error {
-        return object_call<Gee.Collection<Folder>>(
-            "get_containing_folders_async", {parent}, Gee.List.empty<Folder>()
-        );
+    public override Gee.Collection<Folder> list_matching_folders(FolderPath? parent) {
+        try {
+            return object_call<Gee.Collection<Folder>>(
+                "get_containing_folders_async", {parent}, Gee.List.empty<Folder>()
+            );
+        } catch (GLib.Error err) {
+            return Gee.Collection.empty<Folder>();
+        }
     }
 
     public override Gee.Collection<Folder> list_folders() throws Error {
@@ -205,4 +234,12 @@ public class Geary.MockAccount : Account, MockObject {
         );
     }
 
+    internal override void set_endpoints(Endpoint incoming, Endpoint outgoing) {
+        try {
+            void_call("set_endpoints", {incoming, outgoing});
+        } catch (GLib.Error err) {
+            // oh well
+        }
+    }
+
 }



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]