[geary/wip/714104-refine-account-dialog] Fix some issues with reporting errors when loading accounts

[Michael Gratton <mjog src gnome org>]

commit 63abedbb6820e9273dbaec50913ce34b07ace8ce
Author: Michael Gratton <mike vee net>
Date:   Sun Dec 9 16:46:00 2018 +1100

    Fix some issues with reporting errors when loading accounts
    
    Ensure controller is actually listening for problem reports from
    accounts manager, provide a means for parsing enums from ConfigFile
    that differenciates between the key not being found and the value being
    bad, and handle ConfigError.REMOVED property.

 src/client/accounts/accounts-manager.vala    | 93 +++++++++++++++++-----------
 src/client/application/geary-controller.vala |  5 +-
 src/engine/util/util-config-file.vala        | 31 ++++++++++
 3 files changed, 91 insertions(+), 38 deletions(-)
---
diff --git a/src/client/accounts/accounts-manager.vala b/src/client/accounts/accounts-manager.vala
index b0baa56a..211753b9 100644
--- a/src/client/accounts/accounts-manager.vala
+++ b/src/client/accounts/accounts-manager.vala
@@ -346,9 +346,10 @@ public class Accounts.Manager : GLib.Object {
             for (uint i = 0; i < len && !cancellable.is_cancelled(); i++) {
                 GLib.FileInfo file = info_list.nth_data(i);
                 if (file.get_file_type() == FileType.DIRECTORY) {
+                    string id = file.get_name();
                     try {
                         Geary.AccountInformation info = yield load_account(
-                            file.get_name(), cancellable
+                            id, cancellable
                         );
 
                         GoaMediator? mediator = info.mediator as GoaMediator;
@@ -357,7 +358,11 @@ public class Accounts.Manager : GLib.Object {
                         } else {
                             set_available(info, false);
                         }
+                    } catch (ConfigError.REMOVED err) {
+                        // All good, this was handled properly by
+                        // load_account.
                     } catch (GLib.Error err) {
+                        debug("Error loading account %s", id);
                         report_problem(
                             new Geary.ProblemReport(
                                 Geary.ProblemType.GENERIC_ERROR,
@@ -1011,15 +1016,20 @@ public class Accounts.AccountConfigV1 : AccountConfig, GLib.Object {
             throw new ConfigError.SYNTAX("%s: No sender addresses found", id);
         }
 
-        Geary.ServiceProvider? provider = null;
-        try {
-            provider = default_provider ??
-                Geary.ServiceProvider.for_value(
-                    account_config.get_required_string(ACCOUNT_PROVIDER)
-                );
-        } catch (Geary.EngineError err) {
-            throw new ConfigError.SYNTAX("%s: No/bad service provider", id);
-        }
+        Geary.ServiceProvider provider = (
+            default_provider != null
+            ? default_provider
+            : account_config.parse_required_value<Geary.ServiceProvider>(
+                ACCOUNT_PROVIDER,
+                (value) => {
+                    try {
+                        return Geary.ServiceProvider.for_value(value);
+                    } catch (Geary.EngineError err) {
+                        throw new GLib.KeyFileError.INVALID_VALUE(err.message);
+                    }
+                }
+            )
+        );
 
         Geary.AccountInformation account = new Geary.AccountInformation(
             id, provider, mediator, senders.remove_at(0)
@@ -1154,15 +1164,20 @@ public class Accounts.AccountConfigLegacy : AccountConfig, GLib.Object {
         string primary_email = config.get_required_string(PRIMARY_EMAIL_KEY);
         string real_name = config.get_string(REAL_NAME_KEY, default_name);
 
-        Geary.ServiceProvider? provider = null;
-        try {
-            provider = default_provider ??
-                Geary.ServiceProvider.for_value(
-                    config.get_required_string(SERVICE_PROVIDER_KEY)
-                );
-        } catch (Geary.EngineError err) {
-            throw new ConfigError.SYNTAX("%s: No/bad service provider", id);
-        }
+        Geary.ServiceProvider provider = (
+            default_provider != null
+            ? default_provider
+            : config.parse_required_value<Geary.ServiceProvider>(
+                SERVICE_PROVIDER_KEY,
+                (value) => {
+                    try {
+                        return Geary.ServiceProvider.for_value(value);
+                    } catch (Geary.EngineError err) {
+                        throw new GLib.KeyFileError.INVALID_VALUE(err.message);
+                    }
+                }
+            )
+        );
 
         Geary.AccountInformation info = new Geary.AccountInformation(
             id, provider, mediator,
@@ -1311,25 +1326,29 @@ public class Accounts.ServiceConfigV1 : ServiceConfig, GLib.Object {
             service.host = service_config.get_required_string(HOST);
             service.port = (uint16) service_config.get_int(PORT, service.port);
 
-            try {
-                service.transport_security =
-                Geary.TlsNegotiationMethod.for_value(
-                    service_config.get_string(SECURITY, null)
-                );
-            } catch (GLib.Error err) {
-                // Oh well
-                debug("%s: No/invalid transport security config for %s",
-                      account.id, service.protocol.to_value());
-            }
+            service.transport_security = service_config.parse_required_value
+            <Geary.TlsNegotiationMethod>(
+                SECURITY,
+                (value) => {
+                    try {
+                        return Geary.TlsNegotiationMethod.for_value(value);
+                    } catch (GLib.Error err) {
+                        throw new GLib.KeyFileError.INVALID_VALUE(err.message);
+                    }
+                }
+            );
 
-            try {
-                service.credentials_requirement =
-                    Geary.Credentials.Requirement.for_value(
-                        service_config.get_required_string(CREDENTIALS)
-                    );
-            } catch (Geary.EngineError err) {
-                debug("%s: No/invalid SMTP auth config", account.id);
-            }
+            service.credentials_requirement = service_config.parse_required_value
+            <Geary.Credentials.Requirement>(
+                CREDENTIALS,
+                (value) => {
+                    try {
+                        return Geary.Credentials.Requirement.for_value(value);
+                    } catch (GLib.Error err) {
+                        throw new GLib.KeyFileError.INVALID_VALUE(err.message);
+                    }
+                }
+            );
 
             if (service.port == 0) {
                 service.port = service.get_default_port();
diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala
index 7deae462..38b937e0 100644
--- a/src/client/application/geary-controller.vala
+++ b/src/client/application/geary-controller.vala
@@ -319,6 +319,9 @@ public class GearyController : Geary.BaseObject {
         this.account_manager.account_removed.connect(
             on_account_removed
         );
+        this.account_manager.report_problem.connect(
+            on_report_problem
+        );
 
         try {
             yield this.account_manager.connect_libsecret(cancellable);
@@ -944,7 +947,7 @@ public class GearyController : Geary.BaseObject {
         }
     }
 
-    private void on_report_problem(Geary.Account account, Geary.ProblemReport problem) {
+    private void on_report_problem(Geary.ProblemReport problem) {
         report_problem(problem);
     }
 
diff --git a/src/engine/util/util-config-file.vala b/src/engine/util/util-config-file.vala
index 245ab098..61ced4c4 100644
--- a/src/engine/util/util-config-file.vala
+++ b/src/engine/util/util-config-file.vala
@@ -14,6 +14,10 @@
 public class Geary.ConfigFile {
 
 
+    /** A string parser that can be used to extract custom values. */
+    public delegate T Parser<T>(string value) throws GLib.KeyFileError;
+
+
     /**
      * A set of configuration keys grouped under a "[Name]" heading.
      */
@@ -188,6 +192,33 @@ public class Geary.ConfigFile {
             this.backing.set_integer(this.name, key, (int) value);
         }
 
+        public T? parse_value<T>(string key, Parser<T> parser, T? def = null) {
+            T value = def;
+            string? str = get_string(key);
+            if (str != null) {
+                try {
+                    value = parser(str);
+                } catch (GLib.KeyFileError err) {
+                    debug(
+                        "%s:%s value is invalid: %s", this.name, key, err.message
+                    );
+                }
+            }
+            return value;
+        }
+
+        public T parse_required_value<T>(string key, Parser<T> parser)
+            throws GLib.KeyFileError {
+            string? str = get_required_string(key);
+            try {
+                return parser(str);
+            } catch (GLib.KeyFileError err) {
+                throw new GLib.KeyFileError.INVALID_VALUE(
+                    "%s:%s value is invalid: %s", this.name, key, err.message
+                );
+            }
+        }
+
         /** Removes a key from this group. */
         public void remove_key(string name) throws GLib.KeyFileError {
             this.backing.remove_key(this.name, name);