[geary/wip/224-subfolders-missing: 1/2] Tidy up how accounts pass sorted sets of folders around

[Michael Gratton <mjog src gnome org>]

commit 3aebb98c9c54393f9b2089fd604f139d1a882675
Author: Michael Gratton <mike vee net>
Date:   Tue Feb 19 16:39:11 2019 +1100

    Tidy up how accounts pass sorted sets of folders around
    
    If the collection of folders is sorted, require it be stored in a sorted
    collection.

 src/client/application/geary-controller.vala       | 19 ++++--
 src/engine/api/geary-account.vala                  | 77 ++++++++++++----------
 .../imap-engine/imap-engine-generic-account.vala   | 40 ++++++-----
 .../imap-engine/imap-engine-revokable-move.vala    | 11 ++--
 4 files changed, 85 insertions(+), 62 deletions(-)
---
diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala
index f94b8fb4..e76c5c20 100644
--- a/src/client/application/geary-controller.vala
+++ b/src/client/application/geary-controller.vala
@@ -1371,9 +1371,10 @@ public class GearyController : Geary.BaseObject {
         }
     }
 
-    private void on_folders_available_unavailable(Geary.Account account,
-                                                  Gee.List<Geary.Folder>? available,
-                                                  Gee.List<Geary.Folder>? unavailable) {
+    private void on_folders_available_unavailable(
+        Geary.Account account,
+        Gee.BidirSortedSet<Geary.Folder>? available,
+        Gee.BidirSortedSet<Geary.Folder>? unavailable) {
         AccountContext context = this.accounts.get(account.information);
 
         if (available != null && available.size > 0) {
@@ -1438,8 +1439,13 @@ public class GearyController : Geary.BaseObject {
         }
 
         if (unavailable != null) {
-            for (int i = (unavailable.size - 1); i >= 0; i--) {
-                Geary.Folder folder = unavailable[i];
+            Gee.BidirIterator<Geary.Folder> unavailable_iterator =
+                unavailable.bidir_iterator();
+            unavailable_iterator.last();
+            while (unavailable_iterator.valid) {
+                Geary.Folder folder = unavailable_iterator.get();
+                unavailable_iterator.previous();
+
                 main_window.folder_list.remove_folder(folder);
                 if (folder.account == current_account) {
                     if (main_window.main_toolbar.copy_folder_menu.has_folder(folder))
@@ -2817,7 +2823,8 @@ public class GearyController : Geary.BaseObject {
         return context != null ? context.store : null;
     }
 
-    private bool should_add_folder(Gee.List<Geary.Folder>? all, Geary.Folder folder) {
+    private bool should_add_folder(Gee.Collection<Geary.Folder>? all,
+                                   Geary.Folder folder) {
         // if folder is openable, add it
         if (folder.properties.is_openable != Geary.Trillian.FALSE)
             return true;
diff --git a/src/engine/api/geary-account.vala b/src/engine/api/geary-account.vala
index 8a9aaeb7..d4e77012 100644
--- a/src/engine/api/geary-account.vala
+++ b/src/engine/api/geary-account.vala
@@ -68,8 +68,31 @@ public abstract class Geary.Account : BaseObject {
 
     }
 
+    /**
+     * A utility method to sort a Gee.Collection of {@link Folder}s by
+     * their {@link FolderPath}s to ensure they comport with {@link
+     * folders_available_unavailable}, {@link folders_created}, {@link
+     * folders_deleted} signals' contracts.
+     */
+    public static Gee.BidirSortedSet<Folder>
+        sort_by_path(Gee.Collection<Folder> folders) {
+        Gee.TreeSet<Folder> sorted =
+            new Gee.TreeSet<Folder>(Account.folder_path_comparator);
+        sorted.add_all(folders);
+        return sorted;
+    }
 
-   /**
+    /**
+     * Comparator used to sort folders.
+     *
+     * @see sort_by_path
+     */
+    public static int folder_path_comparator(Geary.Folder a, Geary.Folder b) {
+        return a.path.compare_to(b.path);
+    }
+
+
+    /**
      * The account's current configuration.
      */
     public AccountInformation information { get; protected set; }
@@ -127,7 +150,7 @@ public abstract class Geary.Account : BaseObject {
     public signal void report_problem(Geary.ProblemReport problem);
 
     public signal void contacts_loaded();
-    
+
     /**
      * Fired when folders become available or unavailable in the account.
      *
@@ -135,14 +158,16 @@ public abstract class Geary.Account : BaseObject {
      * they're created later; they become unavailable when the account is
      * closed or they're deleted later.
      *
-     * Folders are ordered for the convenience of the caller from the top of the hierarchy to
-     * lower in the hierarchy.  In other words, parents are listed before children, assuming the
-     * lists are traversed in natural order.
+     * Folders are ordered for the convenience of the caller from the
+     * top of the hierarchy to lower in the hierarchy.  In other
+     * words, parents are listed before children, assuming the
+     * collections are traversed in natural order.
      *
      * @see sort_by_path
      */
-    public signal void folders_available_unavailable(Gee.List<Geary.Folder>? available,
-        Gee.List<Geary.Folder>? unavailable);
+    public signal void
+        folders_available_unavailable(Gee.BidirSortedSet<Folder>? available,
+                                      Gee.BidirSortedSet<Folder>? unavailable);
 
     /**
      * Fired when new folders have been created.
@@ -154,10 +179,10 @@ public abstract class Geary.Account : BaseObject {
      *
      * Folders are ordered for the convenience of the caller from the
      * top of the hierarchy to lower in the hierarchy.  In other
-     * words, parents are listed before children, assuming the lists
-     * are traversed in natural order.
+     * words, parents are listed before children, assuming the
+     * collection is traversed in natural order.
      */
-    public signal void folders_created(Gee.List<Geary.Folder> created);
+    public signal void folders_created(Gee.BidirSortedSet<Geary.Folder> created);
 
     /**
      * Fired when existing folders are deleted.
@@ -169,10 +194,10 @@ public abstract class Geary.Account : BaseObject {
      *
      * Folders are ordered for the convenience of the caller from the
      * top of the hierarchy to lower in the hierarchy.  In other
-     * words, parents are listed before children, assuming the lists
-     * are traversed in natural order.
+     * words, parents are listed before children, assuming the
+     * collection is traversed in natural order.
      */
-    public signal void folders_deleted(Gee.List<Geary.Folder> deleted);
+    public signal void folders_deleted(Gee.BidirSortedSet<Geary.Folder> deleted);
 
     /**
      * Fired when a Folder's contents is detected having changed.
@@ -239,23 +264,6 @@ public abstract class Geary.Account : BaseObject {
         );
     }
 
-    /**
-     * A utility method to sort a Gee.Collection of {@link Folder}s by
-     * their {@link FolderPath}s to ensure they comport with {@link
-     * folders_available_unavailable}, {@link folders_created}, {@link
-     * folders_deleted} signals' contracts.
-     */
-    protected Gee.List<Geary.Folder> sort_by_path(Gee.Collection<Geary.Folder> folders) {
-        Gee.TreeSet<Geary.Folder> sorted = new Gee.TreeSet<Geary.Folder>(folder_path_comparator);
-        sorted.add_all(folders);
-
-        return Collection.to_array_list<Geary.Folder>(sorted);
-    }
-
-    private int folder_path_comparator(Geary.Folder a, Geary.Folder b) {
-        return a.path.compare_to(b.path);
-    }
-    
     /**
      * Opens the {@link Account} and makes it and its {@link Folder}s available for use.
      *
@@ -458,18 +466,19 @@ public abstract class Geary.Account : BaseObject {
     }
 
     /** Fires a {@link folders_available_unavailable} signal. */
-    protected virtual void notify_folders_available_unavailable(Gee.List<Geary.Folder>? available,
-                                                                Gee.List<Geary.Folder>? unavailable) {
+    protected virtual void
+        notify_folders_available_unavailable(Gee.BidirSortedSet<Folder>? available,
+                                             Gee.BidirSortedSet<Folder>? unavailable) {
         folders_available_unavailable(available, unavailable);
     }
 
     /** Fires a {@link folders_created} signal. */
-    protected virtual void notify_folders_created(Gee.List<Geary.Folder> created) {
+    protected virtual void notify_folders_created(Gee.BidirSortedSet<Geary.Folder> created) {
         folders_created(created);
     }
 
     /** Fires a {@link folders_deleted} signal. */
-    protected virtual void notify_folders_deleted(Gee.List<Geary.Folder> deleted) {
+    protected virtual void notify_folders_deleted(Gee.BidirSortedSet<Geary.Folder> deleted) {
         folders_deleted(deleted);
     }
 
diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala 
b/src/engine/imap-engine/imap-engine-generic-account.vala
index 0956856b..83369212 100644
--- a/src/engine/imap-engine/imap-engine-generic-account.vala
+++ b/src/engine/imap-engine/imap-engine-generic-account.vala
@@ -198,8 +198,8 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
 
         // Close folders and ensure they do in fact close
 
-        Gee.List<Geary.Folder> locals = sort_by_path(this.local_only.values);
-        Gee.List<Geary.Folder> remotes = sort_by_path(this.folder_map.values);
+        Gee.BidirSortedSet<Folder> locals = sort_by_path(this.local_only.values);
+        Gee.BidirSortedSet<Folder> remotes = sort_by_path(this.folder_map.values);
 
         this.local_only.clear();
         this.folder_map.clear();
@@ -583,9 +583,11 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
      * seen before and the {@link Geary.Account.folders_created} signal is
      * not fired.
      */
-    internal Gee.List<Geary.Folder> add_folders(Gee.Collection<ImapDB.Folder> db_folders,
+    internal Gee.Collection<Folder> add_folders(Gee.Collection<ImapDB.Folder> db_folders,
                                                 bool are_existing) {
-        Gee.List<Geary.Folder> built_folders = new Gee.ArrayList<Geary.Folder>();
+        Gee.TreeSet<MinimalFolder> built_folders = new Gee.TreeSet<MinimalFolder>(
+            Account.folder_path_comparator
+        );
         foreach(ImapDB.Folder db_folder in db_folders) {
             if (!this.folder_map.has_key(db_folder.get_path())) {
                 MinimalFolder folder = new_folder(db_folder);
@@ -595,8 +597,7 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
             }
         }
 
-        if (built_folders.size > 0) {
-            built_folders = sort_by_path(built_folders);
+        if (!built_folders.is_empty) {
             notify_folders_available_unavailable(built_folders, null);
             if (!are_existing) {
                 notify_folders_created(built_folders);
@@ -673,8 +674,11 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
      *
      * A collection of folders that was actually removed is returned.
      */
-    internal Gee.List<MinimalFolder> remove_folders(Gee.Collection<Geary.Folder> folders) {
-        Gee.List<MinimalFolder> removed = new Gee.ArrayList<MinimalFolder>();
+    internal Gee.BidirSortedSet<MinimalFolder>
+        remove_folders(Gee.Collection<Folder> folders) {
+        Gee.TreeSet<MinimalFolder> removed = new Gee.TreeSet<MinimalFolder>(
+            Account.folder_path_comparator
+        );
         foreach(Geary.Folder folder in folders) {
             MinimalFolder? impl = this.folder_map.get(folder.path);
             if (impl != null) {
@@ -684,7 +688,6 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
         }
 
         if (!removed.is_empty) {
-            removed = (Gee.List<MinimalFolder>) sort_by_path(removed);
             notify_folders_available_unavailable(null, removed);
             notify_folders_deleted(removed);
         }
@@ -809,8 +812,9 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
     }
 
     /** {@inheritDoc} */
-    protected override void notify_folders_available_unavailable(Gee.List<Geary.Folder>? available,
-        Gee.List<Geary.Folder>? unavailable) {
+    protected override void
+        notify_folders_available_unavailable(Gee.BidirSortedSet<Folder>? available,
+                                             Gee.BidirSortedSet<Folder>? unavailable) {
         base.notify_folders_available_unavailable(available, unavailable);
         if (available != null) {
             foreach (Geary.Folder folder in available) {
@@ -1313,14 +1317,16 @@ internal class Geary.ImapEngine.UpdateRemoteFolders : AccountOperation {
         if (remote_folders_suspect) {
             debug("Skipping removing folders due to prior errors");
         } else {
-            Gee.List<MinimalFolder> removed =
+            Gee.BidirSortedSet<MinimalFolder> removed =
                 this.generic_account.remove_folders(to_remove);
 
-            // Sort by path length descending, so we always remove children first.
-            removed.sort(
-                (a, b) => b.path.as_array().length - a.path.as_array().length
-            );
-            foreach (Geary.Folder folder in removed) {
+            Gee.BidirIterator<MinimalFolder> removed_iterator =
+                removed.bidir_iterator();
+            removed_iterator.last();
+            while (removed_iterator.valid) {
+                MinimalFolder folder = removed_iterator.get();
+                removed_iterator.previous();
+
                 try {
                     debug("Locally deleting removed folder %s", folder.to_string());
                     yield local.delete_folder_async(folder.path, cancellable);
diff --git a/src/engine/imap-engine/imap-engine-revokable-move.vala 
b/src/engine/imap-engine/imap-engine-revokable-move.vala
index 36fd358e..25f18aa3 100644
--- a/src/engine/imap-engine/imap-engine-revokable-move.vala
+++ b/src/engine/imap-engine/imap-engine-revokable-move.vala
@@ -90,20 +90,21 @@ private class Geary.ImapEngine.RevokableMove : Revokable {
             set_invalid();
         }
     }
-    
-    private void on_folders_available_unavailable(Gee.List<Folder>? available, Gee.List<Folder>? 
unavailable) {
+
+    private void on_folders_available_unavailable(Gee.Collection<Folder>? available,
+                                                  Gee.Collection<Folder>? unavailable) {
         // look for either of the folders going away
         if (unavailable != null) {
             foreach (Folder folder in unavailable) {
-                if (folder.path.equal_to(source.path) || folder.path.equal_to(destination.path)) {
+                if (folder.path.equal_to(source.path) ||
+                    folder.path.equal_to(destination.path)) {
                     set_invalid();
-                    
                     break;
                 }
             }
         }
     }
-    
+
     private void on_source_email_removed(Gee.Collection<EmailIdentifier> ids) {
         // one-way switch
         if (!valid)