From 09f6fc094a58af09d8081e28315b75d0f226fc2b Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Thu, 3 Oct 2019 07:52:43 +1000 Subject: [PATCH] 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. --- .../application/application-controller.vala | 50 ++++--------------- src/engine/api/geary-account.vala | 32 +++++------- src/engine/app/app-conversation-monitor.vala | 10 ++-- .../imap-engine-account-synchronizer.vala | 6 +-- .../imap-engine-generic-account.vala | 36 ++++++------- test/engine/api/geary-account-mock.vala | 25 ++++++---- 6 files changed, 59 insertions(+), 100 deletions(-) diff --git a/src/client/application/application-controller.vala b/src/client/application/application-controller.vala index 204b5599..afeb4b4a 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? 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(); - blacklist.add(outbox.path); - } catch (GLib.Error err) { - // Oh well - } + blacklist = new Gee.ArrayList(); + 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) @@ -2189,16 +2175,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; @@ -2623,14 +2600,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. + * Lists all currently-available folders. * - * 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. + * @see list_matching_folders */ - public abstract Gee.Collection list_matching_folders(Geary.FolderPath? parent) - throws Error; + public abstract Gee.Collection 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 list_folders() throws Error; + public abstract Gee.Collection 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(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 blacklist = new Gee.ArrayList(); 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 bddfe1f8..ff58e4d7 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 list_matching_folders(Geary.FolderPath? parent) - throws Error { - check_open(); + public override Gee.Collection list_folders() { + Gee.HashSet all_folders = new Gee.HashSet(); + all_folders.add_all(this.folder_map.values); + all_folders.add_all(this.local_only.values); - return Geary.traverse(folder_map.keys) + return all_folders; + } + + /** {@inheritDoc} */ + public override Gee.Collection list_matching_folders(FolderPath? parent) + throws EngineError.NOT_FOUND { + return traverse(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(p => folder_map.get(p)) .to_array_list(); } - public override Gee.Collection list_folders() throws Error { - check_open(); - Gee.HashSet all_folders = new Gee.HashSet(); - 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 list_folders() throws Error { - return object_call>( - "list_folders", {}, Gee.List.empty() - ); + public override Gee.Collection list_folders() { + try { + return object_call>( + "list_folders", {}, Gee.List.empty() + ); + } catch (GLib.Error err) { + return Gee.List.empty(); + } } - public override Folder? get_special_folder(SpecialFolderType special) - throws Error { - return object_call( - "get_special_folder", {box_arg(special)}, null - ); + public override Folder? get_special_folder(SpecialFolderType special) { + try { + return object_call( + "get_special_folder", {box_arg(special)}, null + ); + } catch (GLib.Error err) { + return null; + } } public override async Folder get_required_special_folder_async(SpecialFolderType special,