[geary/mjog/account-command-stacks: 1/27] Improve Geary.Account API for listing folders

[Michael Gratton <mjog src gnome org>]

commit fb3165f7c11def259cae8b38a3df5acc89f6432c
Author: Michael Gratton <mike vee net>
Date:   Thu Oct 3 07:52:43 2019 +1000

    Improve Geary.Account API for listing folders
    
    Make both Account.list_folders and Account.get_special_folder not throw
    any errors - either the folders exist, or they don't. Update call sites.

 src/client/application/application-controller.vala | 50 +++++-----------------
 src/engine/api/geary-account.vala                  | 36 +++++++---------
 src/engine/app/app-conversation-monitor.vala       | 10 ++---
 .../imap-engine-account-synchronizer.vala          |  6 +--
 .../imap-engine/imap-engine-generic-account.vala   | 36 +++++++---------
 test/engine/api/geary-account-mock.vala            | 25 +++++++----
 6 files changed, 61 insertions(+), 102 deletions(-)
---
diff --git a/src/client/application/application-controller.vala 
b/src/client/application/application-controller.vala
index 22f22758..31e7302b 100644
--- a/src/client/application/application-controller.vala
+++ b/src/client/application/application-controller.vala
@@ -1013,12 +1013,7 @@ public class Application.Controller : Geary.BaseObject {
         bool is_descendent = false;
 
         Geary.Account account = target.account;
-        Geary.Folder? inbox = null;
-        try {
-            inbox = account.get_special_folder(Geary.SpecialFolderType.INBOX);
-        } catch (Error err) {
-            debug("Failed to get inbox for account %s", account.information.id);
-        }
+        Geary.Folder? inbox = account.get_special_folder(Geary.SpecialFolderType.INBOX);
 
         if (inbox != null) {
             is_descendent = inbox.path.is_descendant(target.path);
@@ -1435,17 +1430,12 @@ public class Application.Controller : Geary.BaseObject {
         Gee.Collection<Geary.FolderPath>? blacklist = null;
         if (this.current_folder != null &&
             this.current_folder.special_folder_type != Geary.SpecialFolderType.OUTBOX) {
-            Geary.Folder? outbox = null;
-            try {
-                outbox = this.current_account.get_special_folder(
-                    Geary.SpecialFolderType.OUTBOX
-                );
+            Geary.Folder? outbox = this.current_account.get_special_folder(
+                Geary.SpecialFolderType.OUTBOX
+            );
 
-                blacklist = new Gee.ArrayList<Geary.FolderPath>();
-                blacklist.add(outbox.path);
-            } catch (GLib.Error err) {
-                // Oh well
-            }
+            blacklist = new Gee.ArrayList<Geary.FolderPath>();
+            blacklist.add(outbox.path);
         }
 
         foreach(Geary.App.Conversation conversation in conversations) {
@@ -1642,11 +1632,7 @@ public class Application.Controller : Geary.BaseObject {
             }
         } else {
             // Move out of spam folder, back to inbox.
-            try {
-                destination_folder = current_account.get_special_folder(Geary.SpecialFolderType.INBOX);
-            } catch (Error e) {
-                debug("Error getting inbox folder: %s", e.message);
-            }
+            destination_folder = current_account.get_special_folder(Geary.SpecialFolderType.INBOX);
         }
 
         if (destination_folder != null)
@@ -2184,16 +2170,7 @@ public class Application.Controller : Geary.BaseObject {
         if (current_account == null)
             return;
 
-        Geary.Folder? folder = null;
-        try {
-            folder = current_account.get_special_folder(special_folder_type);
-        } catch (Error err) {
-            debug("%s: Unable to get special folder %s: %s", current_account.to_string(),
-                special_folder_type.to_string(), err.message);
-
-            // fall through
-        }
-
+        Geary.Folder? folder = current_account.get_special_folder(special_folder_type);
         if (folder == null)
             return;
 
@@ -2618,14 +2595,9 @@ public class Application.Controller : Geary.BaseObject {
     private void do_search(string search_text) {
         Geary.SearchFolder? search_folder = null;
         if (this.current_account != null) {
-            try {
-                search_folder =
-                    this.current_account.get_special_folder(
-                        Geary.SpecialFolderType.SEARCH
-                    ) as Geary.SearchFolder;
-            } catch (Error e) {
-                debug("Could not get search folder: %s", e.message);
-            }
+            search_folder = this.current_account.get_special_folder(
+                Geary.SpecialFolderType.SEARCH
+            ) as Geary.SearchFolder;
         }
 
         if (Geary.String.is_empty_or_whitespace(search_text)) {
diff --git a/src/engine/api/geary-account.vala b/src/engine/api/geary-account.vala
index af97bfb1..9d4040ae 100644
--- a/src/engine/api/geary-account.vala
+++ b/src/engine/api/geary-account.vala
@@ -369,34 +369,28 @@ public abstract class Geary.Account : BaseObject, Loggable {
         throws EngineError.NOT_FOUND;
 
     /**
-     * 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
-     * parent path cannot be found, EngineError.NOT_FOUND is thrown.  If no
-     * folders exist in the root, EngineError.NOT_FOUND may be thrown as well.
-     * However, the caller should be prepared to deal with an empty list being
-     * returned instead.
-     *
-     * The same Geary.Folder objects (instances) will be returned if the same path is submitted
-     * multiple times.  This means that multiple callers may be holding references to the same
-     * Folders.  This is important when thinking of opening and closing folders and signal
-     * notifications.
-     */
-    public abstract Gee.Collection<Geary.Folder> list_matching_folders(Geary.FolderPath? parent)
-        throws Error;
+     * Lists all currently-available folders.
+     *
+     * @see list_matching_folders
+     */
+    public abstract Gee.Collection<Folder> list_folders();
 
     /**
-     * Lists all currently-available folders.  See caveats under
-     * list_matching_folders().
+     * Lists all currently-available folders found a under parent.
+     *
+     * If the parent path cannot be found, EngineError.NOT_FOUND is
+     * thrown. However, the caller should be prepared to deal with an
+     * empty list being returned instead.
      */
-    public abstract Gee.Collection<Geary.Folder> list_folders() throws Error;
+    public abstract Gee.Collection<Folder> list_matching_folders(FolderPath? parent)
+        throws EngineError.NOT_FOUND;
 
     /**
-     * Returns the folder representing the given special folder type.  If no such folder exists,
-     * null is returned.
+     * Returns a folder for the given special folder type, it is exists.
      */
-    public virtual Geary.Folder? get_special_folder(Geary.SpecialFolderType special) throws Error {
+    public virtual Geary.Folder? get_special_folder(Geary.SpecialFolderType type){
         return traverse<Folder>(list_folders())
-            .first_matching(f => f.special_folder_type == special);
+            .first_matching(f => f.special_folder_type == type);
     }
 
     /**
diff --git a/src/engine/app/app-conversation-monitor.vala b/src/engine/app/app-conversation-monitor.vala
index fc9f4f7b..c3f522a1 100644
--- a/src/engine/app/app-conversation-monitor.vala
+++ b/src/engine/app/app-conversation-monitor.vala
@@ -391,13 +391,9 @@ public class Geary.App.ConversationMonitor : BaseObject {
 
         Gee.ArrayList<Geary.FolderPath?> blacklist = new Gee.ArrayList<Geary.FolderPath?>();
         foreach (Geary.SpecialFolderType type in blacklisted_folder_types) {
-            try {
-                Geary.Folder? blacklist_folder = this.base_folder.account.get_special_folder(type);
-                if (blacklist_folder != null)
-                    blacklist.add(blacklist_folder.path);
-            } catch (Error e) {
-                debug("Error finding special folder %s on account %s: %s",
-                    type.to_string(), this.base_folder.account.to_string(), e.message);
+            Geary.Folder? blacklist_folder = this.base_folder.account.get_special_folder(type);
+            if (blacklist_folder != null) {
+                blacklist.add(blacklist_folder.path);
             }
         }
 
diff --git a/src/engine/imap-engine/imap-engine-account-synchronizer.vala 
b/src/engine/imap-engine/imap-engine-account-synchronizer.vala
index f3397a09..0bb7432b 100644
--- a/src/engine/imap-engine/imap-engine-account-synchronizer.vala
+++ b/src/engine/imap-engine/imap-engine-account-synchronizer.vala
@@ -64,11 +64,7 @@ private class Geary.ImapEngine.AccountSynchronizer : Geary.BaseObject {
         // just opened) because just because this value has changed
         // doesn't mean the contents in the folders have changed
         if (this.account.is_open()) {
-            try {
-                send_all(this.account.list_folders(), true);
-            } catch (Error err) {
-                debug("Failed to list account folders for sync: %s", err.message);
-            }
+            send_all(this.account.list_folders(), true);
         }
     }
 
diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala 
b/src/engine/imap-engine/imap-engine-generic-account.vala
index c5ada125..56be71ad 100644
--- a/src/engine/imap-engine/imap-engine-generic-account.vala
+++ b/src/engine/imap-engine/imap-engine-generic-account.vala
@@ -450,29 +450,28 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
     }
 
     /** {@inheritDoc} */
-    public override Gee.Collection<Geary.Folder> list_matching_folders(Geary.FolderPath? parent)
-        throws Error {
-        check_open();
+    public override Gee.Collection<Folder> list_folders() {
+        Gee.HashSet<Folder> all_folders = new Gee.HashSet<Folder>();
+        all_folders.add_all(this.folder_map.values);
+        all_folders.add_all(this.local_only.values);
+
+        return all_folders;
+    }
 
-        return Geary.traverse<FolderPath>(folder_map.keys)
+    /** {@inheritDoc} */
+    public override Gee.Collection<Folder> list_matching_folders(FolderPath? parent)
+        throws EngineError.NOT_FOUND {
+        return traverse<FolderPath>(folder_map.keys)
             .filter(p => {
                 FolderPath? path_parent = p.parent;
                 return ((parent == null && path_parent == null) ||
-                    (parent != null && path_parent != null && path_parent.equal_to(parent)));
+                    (parent != null && path_parent != null &&
+                     path_parent.equal_to(parent)));
             })
             .map<Geary.Folder>(p => folder_map.get(p))
             .to_array_list();
     }
 
-    public override Gee.Collection<Geary.Folder> list_folders() throws Error {
-        check_open();
-        Gee.HashSet<Geary.Folder> all_folders = new Gee.HashSet<Geary.Folder>();
-        all_folders.add_all(folder_map.values);
-        all_folders.add_all(local_only.values);
-
-        return all_folders;
-    }
-
     public override async Geary.Folder get_required_special_folder_async(Geary.SpecialFolderType special,
         Cancellable? cancellable) throws Error {
         if (!(special in get_supported_special_folders())) {
@@ -652,13 +651,8 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
                 minimal.set_special_folder_type(special);
                 changed.add(minimal);
 
-                MinimalFolder? existing = null;
-                try {
-                    existing = get_special_folder(special) as MinimalFolder;
-                } catch (Error err) {
-                    debug("Error getting special folder: %s", err.message);
-                }
-
+                MinimalFolder? existing =
+                    get_special_folder(special) as MinimalFolder;
                 if (existing != null && existing != minimal) {
                     existing.set_special_folder_type(SpecialFolderType.NONE);
                     changed.add(existing);
diff --git a/test/engine/api/geary-account-mock.vala b/test/engine/api/geary-account-mock.vala
index 53bd38d0..45c90774 100644
--- a/test/engine/api/geary-account-mock.vala
+++ b/test/engine/api/geary-account-mock.vala
@@ -151,17 +151,24 @@ public class Geary.MockAccount : Account, MockObject {
         }
     }
 
-    public override Gee.Collection<Folder> list_folders() throws Error {
-        return object_call<Gee.Collection<Folder>>(
-            "list_folders", {}, Gee.List.empty<Folder>()
-        );
+    public override Gee.Collection<Folder> list_folders() {
+        try {
+            return object_call<Gee.Collection<Folder>>(
+                "list_folders", {}, Gee.List.empty<Folder>()
+            );
+        } catch (GLib.Error err) {
+            return Gee.List.empty<Folder>();
+        }
     }
 
-    public override Folder? get_special_folder(SpecialFolderType special)
-        throws Error {
-        return object_call<Folder?>(
-            "get_special_folder", {box_arg(special)}, null
-        );
+    public override Folder? get_special_folder(SpecialFolderType special) {
+        try {
+            return object_call<Folder?>(
+                "get_special_folder", {box_arg(special)}, null
+            );
+        } catch (GLib.Error err) {
+            return null;
+        }
     }
 
     public override async Folder get_required_special_folder_async(SpecialFolderType special,