[geary/wip/714104-refine-account-dialog] Fix credentials not being updated in the engine when changed in editor

[Michael Gratton <mjog src gnome org>]

commit ebe4e190ac5465a7c283de82e8ec950d6d4cfd56
Author: Michael Gratton <mike vee net>
Date:   Thu Dec 27 10:24:37 2018 +1030

    Fix credentials not being updated in the engine when changed in editor
    
    Add a method to Accounts.Manager to update credentials for local
    accounts. Update Geary.ClientService to enable both an endpoint and
    service config to be updated when changed, and simplify how that gets
    done. Update Accounts.EditorServerPane to ensure these get called.

 .../accounts/accounts-editor-servers-pane.vala     | 50 +++++++++++++++++-----
 src/client/accounts/accounts-manager.vala          | 21 +++++++++
 src/engine/api/geary-account.vala                  | 14 ------
 src/engine/api/geary-client-service.vala           | 12 +++---
 src/engine/api/geary-engine.vala                   | 34 +++++----------
 .../imap-engine/imap-engine-generic-account.vala   | 13 ------
 test/engine/api/geary-account-mock.vala            |  9 ----
 7 files changed, 78 insertions(+), 75 deletions(-)
---
diff --git a/src/client/accounts/accounts-editor-servers-pane.vala 
b/src/client/accounts/accounts-editor-servers-pane.vala
index 12c24362..2502196a 100644
--- a/src/client/accounts/accounts-editor-servers-pane.vala
+++ b/src/client/accounts/accounts-editor-servers-pane.vala
@@ -200,16 +200,12 @@ internal class Accounts.EditorServersPane :
             is_valid = yield validate(cancellable);
 
             if (is_valid) {
-                try {
-                    has_changed = this.engine.update_account_service(
-                        this.account, incoming_mutable
-                    );
-                    has_changed = this.engine.update_account_service(
-                        this.account, outgoing_mutable
-                    );
-                } catch (Geary.EngineError err) {
-                    warning("Could not update account services: %s", err.message);
-                }
+                has_changed |= yield update_service(
+                    this.account.incoming, this.incoming_mutable, cancellable
+                );
+                has_changed |= yield update_service(
+                    this.account.outgoing, this.outgoing_mutable, cancellable
+                );
             }
         }
 
@@ -299,6 +295,40 @@ internal class Accounts.EditorServersPane :
         return is_valid;
     }
 
+    private async bool update_service(Geary.ServiceInformation existing,
+                                      Geary.ServiceInformation copy,
+                                      GLib.Cancellable cancellable) {
+        bool has_changed = !existing.equal_to(copy);
+        if (has_changed) {
+            try {
+                yield this.editor.accounts.update_local_credentials(
+                    this.account, existing, copy, cancellable
+                );
+            } catch (GLib.Error err) {
+                warning(
+                    "Could not update %s %s credentials: %s",
+                    this.account.id,
+                    existing.protocol.to_value(),
+                    err.message
+                );
+            }
+
+            try {
+                yield this.engine.update_account_service(
+                    this.account, copy, cancellable
+                );
+            } catch (GLib.Error err) {
+                warning(
+                    "Could not update %s %s service: %s",
+                    this.account.id,
+                    existing.protocol.to_value(),
+                    err.message
+                );
+            }
+        }
+        return has_changed;
+    }
+
     private void add_notification(InAppNotification notification) {
         this.osd_overlay.add_overlay(notification);
         notification.show();
diff --git a/src/client/accounts/accounts-manager.vala b/src/client/accounts/accounts-manager.vala
index e5af1bf1..ffdd1d58 100644
--- a/src/client/accounts/accounts-manager.vala
+++ b/src/client/accounts/accounts-manager.vala
@@ -359,6 +359,27 @@ public class Accounts.Manager : GLib.Object {
         }
     }
 
+    /** Updates a local account service's credentials. */
+    public async void update_local_credentials(Geary.AccountInformation account,
+                                               Geary.ServiceInformation old_service,
+                                               Geary.ServiceInformation new_service,
+                                               GLib.Cancellable? cancellable)
+        throws GLib.Error {
+        SecretMediator? mediator = account.mediator as SecretMediator;
+        if (mediator != null) {
+            if (new_service.credentials != null) {
+                yield mediator.update_token(account, new_service, cancellable);
+            }
+
+            if (old_service.credentials != null &&
+                (new_service.credentials == null ||
+                 (new_service.credentials != null &&
+                  old_service.credentials.user != old_service.credentials.user))) {
+                yield mediator.clear_token(account, old_service, cancellable);
+            }
+        }
+    }
+
     /**
      * Determines if an account is a GOA account or not.
      */
diff --git a/src/engine/api/geary-account.vala b/src/engine/api/geary-account.vala
index fe53657f..d381208a 100644
--- a/src/engine/api/geary-account.vala
+++ b/src/engine/api/geary-account.vala
@@ -389,20 +389,6 @@ public abstract class Geary.Account : BaseObject {
         return this.id;
     }
 
-    /**
-     * Sets network endpoints for the incoming or outgoing service.
-     *
-     * 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.
-     *
-     * The service parameter is one of {@link incoming} or {@link
-     * outgoing}.
-     */
-    internal abstract void set_endpoint(ClientService service,
-                                        Endpoint endpoint);
-
     /** 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
index 5033fee5..c0617205 100644
--- a/src/engine/api/geary-client-service.vala
+++ b/src/engine/api/geary-client-service.vala
@@ -45,14 +45,15 @@ public abstract class Geary.ClientService : BaseObject {
     }
 
     /**
-     * Updates the service's network endpoint and restarts if needed.
+     * Updates the configuration for the service.
      *
      * 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.
+     * so will be stopped before the old configuration and endpoint is
+     * replaced by the new one, then started again.
      */
-    public async void set_endpoint_restart(Endpoint endpoint,
-                                           GLib.Cancellable? cancellable = null)
+    public async void update_configuration(ServiceInformation configuration,
+                                           Endpoint remote,
+                                           GLib.Cancellable? cancellable)
         throws GLib.Error {
         if (this.remote != null) {
             this.remote.untrusted_host.disconnect(on_untrusted_host);
@@ -63,6 +64,7 @@ public abstract class Geary.ClientService : BaseObject {
             yield stop(cancellable);
         }
 
+        this.configuration = configuration;
         this.remote = remote;
         this.remote.untrusted_host.connect(on_untrusted_host);
 
diff --git a/src/engine/api/geary-engine.vala b/src/engine/api/geary-engine.vala
index b5b9d72b..66bedf3a 100644
--- a/src/engine/api/geary-engine.vala
+++ b/src/engine/api/geary-engine.vala
@@ -448,13 +448,11 @@ public class Geary.Engine : BaseObject {
      * Account.incoming} or {@link Account.outgoing} client service
      * will also be updated so that the new configuration will start
      * taking effect immediately.
-     *
-     * Returns true if the account's service was updated, or false if
-     * the configuration was the same.
      */
-    public bool update_account_service(AccountInformation account,
-                                       ServiceInformation updated)
-        throws EngineError {
+    public async void update_account_service(AccountInformation account,
+                                             ServiceInformation updated,
+                                             GLib.Cancellable? cancellable)
+        throws GLib.Error {
         Account? impl = this.account_instances.get(account.id);
         if (impl == null) {
             throw new EngineError.BAD_PARAMETERS(
@@ -463,33 +461,21 @@ public class Geary.Engine : BaseObject {
         }
 
         ClientService? service = null;
-        bool was_updated = false;
         switch (updated.protocol) {
         case Protocol.IMAP:
-            if (!account.incoming.equal_to(updated)) {
-                was_updated = true;
-                account.incoming = updated;
-            }
+            account.incoming = updated;
             service = impl.incoming;
             break;
+
         case Protocol.SMTP:
-            if (!account.outgoing.equal_to(updated)) {
-                was_updated = true;
-                account.outgoing = updated;
-            }
+            account.outgoing = updated;
             service = impl.outgoing;
             break;
         }
 
-        if (was_updated) {
-            Endpoint endpoint = get_shared_endpoint(
-                account.service_provider, updated
-            );
-            impl.set_endpoint(service, endpoint);
-            account.changed();
-        }
-
-        return was_updated;
+        Endpoint remote = get_shared_endpoint(account.service_provider, updated);
+        yield service.update_configuration(updated, remote, cancellable);
+        account.changed();
     }
 
     private Geary.Endpoint get_shared_endpoint(ServiceProvider provider,
diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala 
b/src/engine/imap-engine/imap-engine-generic-account.vala
index 785374fe..2bde62d3 100644
--- a/src/engine/imap-engine/imap-engine-generic-account.vala
+++ b/src/engine/imap-engine/imap-engine-generic-account.vala
@@ -566,19 +566,6 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
         return (map.size == 0) ? null : map;
     }
 
-    internal override void set_endpoint(ClientService service,
-                                        Endpoint endpoint) {
-        if (service == this.incoming) {
-            this.imap.set_endpoint_restart.begin(
-                endpoint, this.open_cancellable
-            );
-        } else if (service == this.outgoing) {
-            this.smtp.set_endpoint_restart.begin(
-                endpoint, this.open_cancellable
-            );
-        }
-    }
-
     /**
      * Constructs a set of folders and adds them to the account.
      *
diff --git a/test/engine/api/geary-account-mock.vala b/test/engine/api/geary-account-mock.vala
index 4696d2eb..8c978f5f 100644
--- a/test/engine/api/geary-account-mock.vala
+++ b/test/engine/api/geary-account-mock.vala
@@ -239,13 +239,4 @@ public class Geary.MockAccount : Account, MockObject {
         );
     }
 
-    internal override void set_endpoint(ClientService service,
-                                        Endpoint endpoint) {
-        try {
-            void_call("set_endpoint", {service, endpoint});
-        } catch (GLib.Error err) {
-            // oh well
-        }
-    }
-
 }