[geary/wip/714104-refine-account-dialog] Update GOA loading and error handling to be a bit more robust



commit 816324e62c9c462585bf8acd2ebbc59bf4b4d2d6
Author: Michael Gratton <mike vee net>
Date:   Sun Dec 9 19:13:52 2018 +1100

    Update GOA loading and error handling to be a bit more robust

 src/client/accounts/accounts-manager.vala | 47 ++++++++---------
 src/client/application/goa-mediator.vala  | 88 ++++++++++++++++---------------
 2 files changed, 69 insertions(+), 66 deletions(-)
---
diff --git a/src/client/accounts/accounts-manager.vala b/src/client/accounts/accounts-manager.vala
index 211753b9..e9dc390d 100644
--- a/src/client/accounts/accounts-manager.vala
+++ b/src/client/accounts/accounts-manager.vala
@@ -85,8 +85,9 @@ public interface Accounts.ServiceConfig : GLib.Object {
 
 public errordomain Accounts.ConfigError {
     IO,
+    MANAGEMENT,
     SYNTAX,
-    UNSUPPORTED_VERSION,
+    VERSION,
     UNAVAILABLE,
     REMOVED;
 }
@@ -351,13 +352,10 @@ public class Accounts.Manager : GLib.Object {
                         Geary.AccountInformation info = yield load_account(
                             id, cancellable
                         );
-
-                        GoaMediator? mediator = info.mediator as GoaMediator;
-                        if (mediator == null || mediator.is_valid) {
-                            set_enabled(info, true);
-                        } else {
-                            set_available(info, false);
-                        }
+                        set_enabled(info, true);
+                    } catch (ConfigError.UNAVAILABLE err) {
+                        // All good, this was handled properly by
+                        // load_account.
                     } catch (ConfigError.REMOVED err) {
                         // All good, this was handled properly by
                         // load_account.
@@ -554,16 +552,17 @@ public class Accounts.Manager : GLib.Object {
             is_goa = true;
         }
 
+        Goa.Object? goa_handle = null;
         GoaMediator? goa_mediator = null;
         Geary.ServiceProvider? default_provider = null;
         Geary.CredentialsMediator mediator = this.libsecret;
 
         if (is_goa) {
             if (this.goa_service == null) {
-                throw new ConfigError.UNAVAILABLE("GOA service not available");
+                throw new ConfigError.MANAGEMENT("GOA service not available");
             }
 
-            Goa.Object? goa_handle = this.goa_service.lookup_by_id(goa_id);
+            goa_handle = this.goa_service.lookup_by_id(goa_id);
             if (goa_handle != null) {
                 mediator = goa_mediator = new GoaMediator(goa_handle);
                 default_provider = goa_mediator.get_service_provider();
@@ -596,7 +595,7 @@ public class Accounts.Manager : GLib.Object {
             break;
 
         default:
-            throw new ConfigError.UNSUPPORTED_VERSION(
+            throw new ConfigError.VERSION(
                 "Unsupported config version: %d", version
             );
         }
@@ -635,17 +634,21 @@ public class Accounts.Manager : GLib.Object {
             }
         } else {
             account.service_label = goa_mediator.get_service_label();
-
             try {
-                // This updates the service configs as well
                 yield goa_mediator.update(account, cancellable);
             } catch (GLib.Error err) {
-                // If we get an error here, there might have been a
-                // problem loading the GOA data, or the OAuth token
-                // has expired. XXX Need to distinguish between the
-                // two.
+                throw new ConfigError.MANAGEMENT(err.message);
+            }
+
+            if (goa_handle.get_mail() == null) {
+                // If we get here, the GOA account's mail service used
+                // to exist (we just loaded Geary's config for it) but
+                // no longer does. This indicates the mail service has
+                // been disabled, so set it as disabled.
                 set_available(account, false);
-                throw new ConfigError.UNAVAILABLE(err.message);
+                throw new ConfigError.UNAVAILABLE(
+                    "GOA Mail service not available"
+                );
             }
         }
 
@@ -836,11 +839,7 @@ public class Accounts.Manager : GLib.Object {
                     ));
             }
 
-            if (mediator.is_valid) {
-                set_enabled(info, true);
-            } else {
-                set_available(info, false);
-            }
+            set_enabled(info, true);
         } else {
             debug(
                 "Ignoring GOA %s account %s, mail service not enabled",
@@ -932,7 +931,7 @@ public class Accounts.Manager : GLib.Object {
                             ));
                     }
 
-                    set_available(state.account, mediator.is_valid);
+                    set_available(state.account, mediator.is_available);
                 }
             );
         } else {
diff --git a/src/client/application/goa-mediator.vala b/src/client/application/goa-mediator.vala
index c38e14b7..21e79d6e 100644
--- a/src/client/application/goa-mediator.vala
+++ b/src/client/application/goa-mediator.vala
@@ -1,42 +1,25 @@
 /*
  * Copyright 2017 Software Freedom Conservancy Inc.
+ * 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.
+ * (version 2.1 or later). See the COPYING file in this distribution.
  */
 
 /** GNOME Online Accounts token adapter. */
 public class GoaMediator : Geary.CredentialsMediator, Object {
 
 
-    public bool is_valid {
+    public bool is_available {
         get {
-            Goa.Account account = this.handle.get_account();
             // Goa.Account.mail_disabled doesn't seem to reflect if we
             // get can get a valid mail object or not, so just rely on
             // actually getting one instead.
-            Goa.Mail? mail = this.handle.get_mail();
-            return (
-                mail != null &&
-                !account.attention_needed &&
-                (this.oauth2 != null || this.password != null)
-            );
-        }
-    }
-
-    public Geary.Credentials.Method method {
-        get {
-            Geary.Credentials.Method method = Geary.Credentials.Method.PASSWORD;
-            if (this.oauth2 != null) {
-                method = Geary.Credentials.Method.OAUTH2;
-            }
-            return method;
+            return this.handle.get_mail() != null;
         }
     }
 
     private Goa.Object handle;
-    private Goa.OAuth2Based? oauth2 = null;
-    private Goa.PasswordBased? password = null;
 
 
     public GoaMediator(Goa.Object handle) {
@@ -64,43 +47,49 @@ public class GoaMediator : Geary.CredentialsMediator, Object {
     public async void update(Geary.AccountInformation geary_account,
                              GLib.Cancellable? cancellable)
         throws GLib.Error {
-        // XXX have to call the sync version of this since the async
-        // version seems to be broken
-        this.handle.get_account().call_ensure_credentials_sync(
-            null, cancellable
-        );
-        this.oauth2 = this.handle.get_oauth2_based();
-        this.password = this.handle.get_password_based();
+        debug("checking auth");
+        // Call this to get the exception thrown if no auth method is
+        // supported.
+        get_auth_method();
 
+        debug("updating imap");
         update_imap_config(geary_account.incoming);
+        debug("updating smtp");
         update_smtp_config(geary_account.outgoing);
+        debug("updating done");
     }
 
     public virtual async bool load_token(Geary.AccountInformation account,
                                          Geary.ServiceInformation service,
                                          Cancellable? cancellable)
         throws GLib.Error {
+        // XXX have to call the sync version of this since the async
+        // version seems to be broken. See
+        // https://gitlab.gnome.org/GNOME/vala/issues/709
+        this.handle.get_account().call_ensure_credentials_sync(
+            null, cancellable
+        );
+
         bool loaded = false;
         string? token = null;
 
-        if (this.method == Geary.Credentials.Method.OAUTH2) {
-            // XXX have to call the sync version of this since the async
-            // version seems to be broken
-            this.oauth2.call_get_access_token_sync(
+        switch (get_auth_method()) {
+        case OAUTH2:
+            this.handle.get_oauth2_based().call_get_access_token_sync(
                 out token, null, cancellable
             );
-        } else {
-            // XXX have to call the sync version of these since the
-            // async version seems to be broken
+            break;
+
+        case PASSWORD:
             switch (service.protocol) {
             case Geary.Protocol.IMAP:
-                this.password.call_get_password_sync(
+                this.handle.get_password_based().call_get_password_sync(
                     "imap-password", out token, cancellable
                 );
                 break;
 
             case Geary.Protocol.SMTP:
-                this.password.call_get_password_sync(
+                this.handle.get_password_based().call_get_password_sync(
                     "smtp-password", out token, cancellable
                 );
                 break;
@@ -108,6 +97,7 @@ public class GoaMediator : Geary.CredentialsMediator, Object {
             default:
                 return false;
             }
+            break;
         }
 
         if (token != null) {
@@ -130,10 +120,23 @@ public class GoaMediator : Geary.CredentialsMediator, Object {
         // something. Connect to the GOA service and wait until we
         // hear that needs attention is no longer true.
 
-        return this.is_valid;
+        return this.is_available;
+    }
+
+    private Geary.Credentials.Method get_auth_method() throws GLib.Error {
+        if (this.handle.get_oauth2_based() != null) {
+            return Geary.Credentials.Method.OAUTH2;
+        }
+        if (this.handle.get_password_based() != null) {
+            return Geary.Credentials.Method.PASSWORD;
+        }
+        throw new Geary.EngineError.UNSUPPORTED(
+            "GOA account supports neither password or OAuth2 auth"
+        );
     }
 
-    private void update_imap_config(Geary.ServiceInformation service) {
+    private void update_imap_config(Geary.ServiceInformation service)
+        throws GLib.Error {
         Goa.Mail? mail = this.handle.get_mail();
         if (mail != null) {
             parse_host_name(service, mail.imap_host);
@@ -147,7 +150,7 @@ public class GoaMediator : Geary.CredentialsMediator, Object {
             }
 
             service.credentials = new Geary.Credentials(
-                this.method, mail.imap_user_name
+                get_auth_method(), mail.imap_user_name
             );
 
             if (service.port == 0) {
@@ -156,7 +159,8 @@ public class GoaMediator : Geary.CredentialsMediator, Object {
         }
     }
 
-    private void update_smtp_config(Geary.ServiceInformation service) {
+    private void update_smtp_config(Geary.ServiceInformation service)
+        throws GLib.Error {
         Goa.Mail? mail = this.handle.get_mail();
         if (mail != null) {
             parse_host_name(service, mail.smtp_host);
@@ -177,7 +181,7 @@ public class GoaMediator : Geary.CredentialsMediator, Object {
 
             if (mail.smtp_use_auth) {
                 service.credentials = new Geary.Credentials(
-                    this.method, mail.smtp_user_name
+                    get_auth_method(), mail.smtp_user_name
                 );
             }
 


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